8000 FIX: mixup between _egg_info and _sdist by DimitriPapadopoulos · Pull Request #342 · python-versioneer/python-versioneer · GitHub
[go: up one dir, main page]
Skip to content

FIX: mixup bet 8000 ween _egg_info and _sdist #342

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 19, 2022
Merged

FIX: mixup between _egg_info and _sdist #342

merged 2 commits into from
Oct 19, 2022

Conversation

DimitriPapadopoulos
Copy link
Contributor

This fixes the following error message that appeared when upgrading Numpy from Versioneer 0.19 to 0.26:

Traceback (most recent call last):
  File "setup.py", line 102, in <module>
    cmdclass = versioneer.get_cmdclass(numpy_cmdclass)
  File "/cygdrive/d/a/numpy/numpy/versioneer.py", line 1980, in get_cmdclass
    class cmd_egg_info(_egg_info):
UnboundLocalError: local variable '_egg_info' referenced before assignment

@DimitriPapadopoulos
FIX: mixup between _egg_info and _sdist
468ea21 
This fixes the following error message that appeared when upgrading Numpy
from Versioneer 0.19 to 0.26:
	Traceback (most recent call last):
	  File "setup.py", line 102, in <module>
	    cmdclass = versioneer.get_cmdclass(numpy_cmdclass)
	  File "/cygdrive/d/a/numpy/numpy/versioneer.py", line 1980, in get_cmdclass
	    class cmd_egg_info(_egg_info):
	UnboundLocalError: local variable '_egg_info' referenced before assignment
DimitriPapadopoulos added a commit to DimitriPapadopoulos/numpy that referenced this pull request Oct 15, 2022
@DimitriPapadopoulos
MAINT: Work around bug introduced by Versioneer 0.23
2172528 
python-versioneer/python-versioneer#342
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Oct 15, 2022
Merged
@effigies
Copy link
Contributor

Looks correct. I guess we should have a regression test where we prepopulate the cmdclass.

@effigies
Copy link
Contributor

@DimitriPapadopoulos Are you interested in adding a test? Would you like guidance?

@DimitriPapadopoulos
Copy link
Contributor Author

Yes, some guidance would be nice. I don't know what the modified code is for. I just fixed the obvious to be able to update Versioneer in another project, without really understanding the purpose of the modified code.

@effigies
8000 Copy link
Contributor

No problem. The idea is that we're subclassing existing cmdclasses so that setup() will insert versioneer code at the right place in its sequences of build steps. If you have your own cmdclass, then we will inject our code and call super() to make sure your code gets run. When you run versioneer.get_cmdclass() with no arguments, then we just subclass the setuptools defaults.

We have some unit tests, but mostly full integration tests with example projects: https://github.com/python-versioneer/python-versioneer/tree/master/test/

I think probably the easiest way to test this is just to make sure that calling get_cmdclass() on the results of get_cmdclass() is idempotent in one of our test projects. So maybe in https://github.com/python-versioneer/python-versioneer/blob/master/test/demoapp2-setuptools/setup.py:

 commands = versioneer.get_cmdclass().copy()
+commands = versioneer.get_cmdclass(commands).copy()

I think that should fail without your fix and pass with your fix.

@effigies
Copy link
Contributor

@DimitriPapadopoulos Went ahead and tested the above idea locally, and it does break things with the error you describe. Let's see if everything works with your fix, or if things are more complicated than I thought...

@effigies
Copy link
Contributor

Looks like it worked. Thanks!

@effigies effigies merged commit 2eb638c into python-versioneer:master Oct 19, 2022
@DimitriPapadopoulos DimitriPapadopoulos deleted the _egg_info branch October 19, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DimitriPapadopoulos @effigies
0