Ignore pre-existing ERFA config.h when installing - #306
Merged
Conversation
eerovaher
force-pushed
the
config.h-generation
branch
3 times, most recently
from
July 31, 2026 16:22
a67c585 to
c1f4764
Compare
Previously `setup.py` did not generate ERFA `config.h` if the file already existed, but this also meant that when the `liberfa` submodule was updated then `config.h` needed to be manually removed to prevent `pyerfa` from using the outdated file. Now building `pyerfa` always generates a new `config.h`. This is mostly an improvement for `pyrfa` developement installations because an sdist tarball did not include `config.h`, so it had to be generated anyways. There is a small related change to generating the ERFA `configure` script, which can be used to generate `config.h`. Previously `configure` was generated if both itself and `config.h` did not exist (unless system ERFA was used), but now development installations attempt to generate `configure` (unless system ERFA is being used) and sdist installations never do. This helps avoid generating `config.h` with an outdated `configure` in development installs while keeping `setup.py` reasonably simple and without affecting sdist installations in practice because `configure` should be included in the tarball.
eerovaher
force-pushed
the
config.h-generation
branch
from
July 31, 2026 16:28
c1f4764 to
c13a333
Compare
avalentino
approved these changes
Jul 31, 2026
config.hconfig.h when installing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On current
mainthere is quite complicated code insetup.pyto decide if and how to generate ERFA'sconfig.h, but I don't think that code is behaving as it should. One problem is thatconfig.his not regenerated if it is already present, so if theliberfasubmodule is ever updated thenpyerfawill keep using the outdatedconfig.h. Another problem is thatconfig.hcan be generated in two different ways. The preferred way is to run ERFA'sconfigurescript, but that has to be generated by running ERFA'sbootstrap.sh, which requires additional C tooling (autoconf,automake,libtool). Because that tooling might not be available thensetup.pycan also directly generate a minimalconfig.hitself. Once the minimal file has been generatedpyerfawill keep using it even if the C tooling is installed and generating the full file becomes possible. Both problems are further worsened becauseconfig.his in theliberfasubmodule.gitignore, sogit statusdoes not reveal its presence.In this PR a pre-existing
config.his always ignored. Development installs try to generate the ERFAconfigurescript by running ERFA'sbootstrap.sh, but becauseconfigureshould be included in an sdist tarball then installing from an sdist tries to use the existingconfigureas is. If anything goes wrong thensetup.pystill directly generates a minimalconfig.h.One thing that worries me is that
setup.pyquite closely follows the intended ERFA build procedure from before it switched to using meson. I don't understand whypyerfadid not switch to meson when ERFA did.