Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT: add static configuration for setuptools-scm #166

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

neutrinoceros
Copy link
Contributor

Address #165

@neutrinoceros neutrinoceros force-pushed the bld/setuptools_scm_section branch from c5a9309 to 19af77e Compare January 25, 2025 07:53
@neutrinoceros
Copy link
Contributor Author

good news, it looks like we can actually reproduce the real error on this repo now: https://github.com/liberfa/pyerfa/actions/runs/12963097535/job/36160295587

@neutrinoceros
Copy link
Contributor Author

The build error on aarch64 is unrelated. See upstream issue at tonistiigi/binfmt#215

@pllim
Copy link
Contributor

pllim commented Jan 25, 2025

Is it also possible to clean up setup.py version stuff? Do we really need guess next version?

@neutrinoceros
Copy link
Contributor Author

Oh I missed that part. I was assuming that setuptools-scm was relying on some default behavior but it actually is all done dynamically... you're right, I do need to address this in the same PR !

Do we really need guess next version?

Indeed we shouldn't need this. I'm guessing it was done to support building without setuptools-scm available 4 years ago (65603bd)

@neutrinoceros neutrinoceros force-pushed the bld/setuptools_scm_section branch from 19af77e to 66f30fc Compare January 25, 2025 14:04
@neutrinoceros neutrinoceros changed the title MNT: add explicit configuration for setuptools-scm to pyproject.toml MNT: use static configuration for setuptools-scm Jan 25, 2025
@neutrinoceros neutrinoceros changed the title MNT: use static configuration for setuptools-scm MNT: use static configuration for setuptools-scm Jan 25, 2025
@pllim
Copy link
Contributor

pllim commented Jan 25, 2025

There's also a version.py but it's probably ok as is. Second pair of eyes won't hurt. Thanks!

@mhvk mhvk requested a review from avalentino January 26, 2025 14:47
@mhvk
Copy link
Contributor

mhvk commented Jan 26, 2025

This looks good but as I don't remember quite why we had the fairly complicated code in setup.py, I've requested a review from @avalentino.

@mhvk
Copy link
Contributor

mhvk commented Jan 26, 2025

p.s. Thanks for linking to the upstream issue with aarch64.

@avalentino
Copy link
Member

Dear @neutrinoceros , @pllim , @mhvk ,
I fear that not using a proper guess_next_dev function in setup.py would break the version scheme described in RELEASING.rst (that is also enforced in some of the tests).
Even if no tests fails right now, I fear that we would have failures at the next update of the C liberfa version.

According to my understanding you want to have

[tool.setuptools_scm]
version_file = "erfa/_version.py"

in the pyproject.toml file.
IMHO this is still possible without dropping also

use_scm_version = {
    'version_scheme': guess_next_dev,
}

from setup.py.
By the way, I would be in favour of a simplification of the current setuptools-scm machinery, and I would be open to discuss a change in the versioning scheme if you want to propose something,

@neutrinoceros neutrinoceros force-pushed the bld/setuptools_scm_section branch from 66f30fc to 5e25e47 Compare January 27, 2025 08:24
@neutrinoceros
Copy link
Contributor Author

Thanks @avalentino for your quick review !
I reverted most of the changes in setup.py, now this PR is only about moving the deprecated write_to option to pyproject.toml and replacing it with it's not-deprecated equivalent. I checked that setuptools-scm knows how to combine static and dynamic configuration options by setting version_file = "erfa/_version_test.py" and inspecting build artifacts.

@neutrinoceros neutrinoceros changed the title MNT: use static configuration for setuptools-scm MNT: add static configuration for setuptools-scm Jan 27, 2025
@avalentino
Copy link
Member

Thanks @neutrinoceros

@avalentino avalentino merged commit cceb1ab into liberfa:main Jan 27, 2025
23 of 24 checks passed
@neutrinoceros neutrinoceros deleted the bld/setuptools_scm_section branch January 27, 2025 08:51
@pllim
Copy link
Contributor

pllim commented Jan 27, 2025

Thanks all!

@pllim
Copy link
Contributor

pllim commented Jan 27, 2025

A release soon would be nice so the "exotic" job can pick it up. We don't run those jobs on dev pyerfa.

@pllim
Copy link
Contributor

pllim commented Jan 27, 2025

@mhvk
Copy link
Contributor

mhvk commented Jan 27, 2025

OK, I'm making a new release. I think the release gets into ubuntu-rolling via Debian, i.e., really via @olebole.

p.s. I guess the wheels built will fail for aarch64...

@mhvk
Copy link
Contributor

mhvk commented Jan 27, 2025

Hmm, the aarch64 failure also causes the wheel building generally to be aborted, so I cannot really release the wheels. I could do a github source release, but am not sure that is useful...

@pllim
Copy link
Contributor

pllim commented Jan 27, 2025

Hmm is it possible to opt out of aarch64 wheel for this release?

@avalentino
Copy link
Member

OK, I'm making a new release. I think the release gets into ubuntu-rolling via Debian, i.e., really via @olebole.

p.s. I guess the wheels built will fail for aarch64...

I can help for the debian part. Once the release is ready I can quickly push the new version in debian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants