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

Experiments in removing setup.py #33

Merged
merged 15 commits into from
Feb 23, 2022
Merged

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented Nov 2, 2021

This is a very quick hack at #32.

The main issue here is that get_compiler() is making a dummy Distribution object to give it a hook to get the compiler options, which is calling the entry point function which is calling get_compiler in an infinite loop.

We might want to consider pausing this work until we resolve the future of this package without distutils.

@Cadair Cadair marked this pull request as draft November 2, 2021 17:23
@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #33 (b3273e0) into master (46f6c46) will decrease coverage by 1.90%.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
- Coverage   81.04%   79.13%   -1.91%     
==========================================
  Files           4        4              
  Lines         269      278       +9     
==========================================
+ Hits          218      220       +2     
- Misses         51       58       +7     
Impacted Files Coverage Δ
extension_helpers/__init__.py 46.15% <22.22%> (-53.85%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46f6c46...b3273e0. Read the comment docs.

@astrofrog
Copy link
Member

I think the downstream failures are actually unrelated and due to astropy/astropy#12425

@astrofrog
Copy link
Member

@olebole - just out of curiosity, does/can Debian packaging deal with packages that have no setup.py file but just the declarative setup configuration in setup.cfg? This is something setuptools supports but just curious if this would cause issues with Debian packaging if we started to roll this out across the astropy project.

@olebole
Copy link
Member

olebole commented Feb 8, 2022

@astrofrog not (yet) out of the box. I can, however, run whatever I want for the build, so it would be not a big problem. And I would guess that our Python "magic" will support it sooner or later. I could open a bug to trigger this extension if you like.

For reference: What I just tried was to remove from my latest new package, cmyt the trivial setup.py. The current Python debhelper 5.20220119 fails with

dh binary --with python3 --buildsystem=pybuild
   dh_update_autotools_config -O--buildsystem=pybuild
   dh_autoreconf -O--buildsystem=pybuild
   dh_auto_configure -O--buildsystem=pybuild
I: pybuild base:237: python3.10 setup.py config 
python3.10: can't open file '/build/cmyt-1.0.4/setup.py': [Errno 2] No such file or directory
E: pybuild pybuild:367: configure: plugin distutils failed with: exit code=2: python3.10 setup.py config 
dh_auto_configure: error: pybuild --configure -i python{version} -p "3.10 3.9" returned exit code 13
make: *** [debian/rules:6: binary] Error 25
dpkg-buildpackage: error: debian/rules binary subprocess returned exit status 2
I: copying local configuration
E: Failed autobuilding of package

so one sees that it just tries to run setup.py.

Update This feature is planned, see Debian#1001459 (comment)

cfg.read(config_files[0])
if (cfg.has_option("extension_helpers", "use_extension_helpers") and
cfg.get("extension_helpers", "use_extension_helpers")):
extension_helpers_cfg = cfg["extension_helpers"]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't actually using this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes guess we can remove this line


def _finalize_distribution_hook(distribution):
"""
Something something setuptools entrypoint
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

@astrofrog
Copy link
Member

@olebole - thanks for the update!

@Cadair
Copy link
Member Author

Cadair commented Feb 23, 2022

I think this looks good, I am happy with it if you are @astrofrog (I can't approve my own PR!)

@astrofrog astrofrog merged commit 55b7859 into astropy:master Feb 23, 2022
@Cadair Cadair deleted the no_more_setup_py branch February 23, 2022 10:46
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.

3 participants