I'm just not very experienced with setuptools, so I wanted to keep the changes as minimal as possible. But if it makes sense for dist-info as you suggest, I can also try adding that. A unit test should be doable, too, assuming there are existing examples I can work off of (haven't looked yet). I can probably look today.
Test added for egg-info and code made a bit more DRY. I didn't change the handling for dist-info because it looks like it might already do the right thing -- the dist-info tests seem to have version numbers in them already. But again I'm not too familiar with this stuff so I'm not sure.
I also noticed that there was this bit of code:
version = _version_from_file(self._get_metadata(self.PKG_INFO))
if version is None:
tmpl = "Missing 'Version:' header and/or %s file"
raise ValueError(tmpl % self.PKG_INFO, self)
So it looks like it should almost do what I need, but when I tried setting version = None for the .egg-info parsing -- which should have made the self.version property look at the .egg-info file later, I got some other errors in tests. So the current changeset seems like the minimal amount necessary to get things working, but I suspect there might be a cleaner solution for someone who understands the code better.
I've taken a look at this, and I have some reservations.
My first reservation is that the test is in setuptools, but the changes are in pkg_resources. I would expect since the tests are for functionality in pkg_resources, the tests should similarly be encapsulated in the pkg_resources tests.
My second reservation is that although 'distutils' is implicated in the test name, it's obvious that setuptools is being invoked (a pure distutils install won't generate egg-info unless patched by setuptools). I don't yet understand how the run_setup_py is somehow causing setuptools to be invoked in that clear distutils-only script. I'll have to investigate more.
On the bright side, I did rebase just the test without the fix, and the test does fail, which is good. The failure, however, doesn't match what I would expect.
Indeed, even if I run the tests in the submitted commits (at 2b21ff85422d), it fails with the same error, so it seems that (a) the test is failing with the patch and (b) the patch isn't fixing anything that's being tested.
Changing the assertion to use .startswith should fix it, I'll do that later
today or tomorrow. Regarding the appropriateness of the fix, I don't know
the "right" way to fix this problem. This very well might not be it, but
I'm afraid I can't fully grasp the setuptools flow to know better. I am
pretty sure this fixed the problem I was having, but it has been a while
since I worked on and thought about it. I should be able to double check
Hi Eric. I took a look at the latest test commits. I rebased them onto the parent commit you mentioned, but the tests do not fail for me (nor for Travis). You can see there that all tests pass, including the one you added, with only the two commits to the test suite.
So it seems that in a clean environment, the test isn't yet capturing what it is meant to capture. Can you have another look?
The linked comparison has changes not just to the test file but also a fix
in pkg_resources so I'm not surprised it passes. I guess my commit names
were not quite all encompassing :) I think to test what you meant to test
you'd need to manually create a clean branch and copy in my modified file.
Can you try locally and confirm the test fails for you?
Hmm. I did actually rebase the two test-related commits against 18.5 in the pr-144-tests branch. That (git) branch shouldn't have the pkg_resources commit, but Github does a poor job of rendering that history, showing the commits in chronological order rather than in order of ancestry.
The easy thing to do would be to take the modified test_eeg_info.py from this PR and copy it into a clean master branch. You should hopefully see it fail. Then with the changes applied to pkg_resources/__init__.py (i.e. all changes from this PR), see that it passes.
I've managed to separate the commits as just the test and just the changes in the pr-144-tests and pr-144 branches in the jaraco/setuptools git mirror.
In the tests branch, I've adapted the tests to more directly invoke distutils but still capture the failure. I now understand much better what's going on here.
The crux of the issue is that the packages are not using setuptools but are only using distutils, which has older versions of the version escaping. I'm slightly tempted to say that this use-case should not be supported and that the projects that are failing should use setuptools and not rely on distutils.
Next I'll look at the pr-144 branch and see if the changes there are acceptable in light of my new understanding.
At the same time, though, distutils is embedding the proper version number
somewhere, and setuptools currently isn't using that information. The PR
makes it look in the right place for it. So I consider it a bug fix for
where setuptools gets the version information.
And to put it differently, if package A uses distutils for installation,
package B using setuptools will experience an error during installation on
master, if I understand the problem correctly. So I don't think it's a
reasonable solution to require all possible package A's (i.e. all Python
packages) to use setuptools to avoid errors for package B's.