Parsing vcs urls in requirements and zope-event fix #200
Conversation
|
Thanks for your contribution. I'll try to get this reviewed over the next days. |
There was a problem hiding this comment.
Thanks for this contribution. This already looks very good. As indicated by the inline-comment, my main concern is why the pip-requirements-parser was pinned to a specific version. If this was only done to allow stable test execution, you should leave this open as we anyhow compile a requirements.txt for the tests.
To get the test passing, you'll have to recompile the requirements.txt file. As I anyhow needed to do this to check some other assumptions, I have already pushed a commit which does this to the branch vcs-urls-suggestion. Feel free to cherry pick this.
In addition, you seem to have unintentionally changed the identifier generation for the JUnit report feature. The new identifier generation is actually better than the old, but we'll need to adjust the tests so they expect the right thing. For this, too, you can pick a commit from the branch vcs-urls-suggestions.
| @@ -64,6 +68,7 @@ def _find_wheel(self, name, version): | |||
| Find a wheel with the given name and version | |||
| """ | |||
| candidates = [WheelFile(filename) for filename in glob.iglob(path.join(self.wheelhouse, '*.whl'))] | |||
| name = self._standardize_package_name(name) | |||
There was a problem hiding this comment.
I am wondering whether this won't break cases like devpi-common that rely on the standard escaping to devpi_common. 🤔
A quick local test shows that this does not seem to be the case. 😀 Still, I find it sufficiently non-obvious that it would make sense to explicitly include one such package in the matrix for the test_build test. You can pick the corresponding commit from my vcs-urls-suggestions branch if this looks reasonable to you.
There was a problem hiding this comment.
I cherry-picked your commits. Thank you 😉
core-requirements.txt
Outdated
| @@ -5,3 +5,4 @@ wheel-filename | |||
| wheel-inspect>=1.6.0 | |||
| pip>=1.5.3 | |||
| junit-xml | |||
| pip-requirements-parser==32.0.1 | |||
There was a problem hiding this comment.
Is there a particular reason to pin this dependency down to the patch version? Are we expecting incompatibilities? I'd assume requiring a minimum version should be sufficient.
There was a problem hiding this comment.
It is just a habbit from some bigger project. I am used to pin exact version of dependencies in order to have more control and some kind of prevention from unexpected bugs on production.
I have just changed it to minimum version 😀
Bumps [pyramid](https://github.com/Pylons/pyramid) from 2.0 to 2.0.2. - [Changelog](https://github.com/Pylons/pyramid/blob/2.0.2/CHANGES.rst) - [Commits](Pylons/pyramid@2.0...2.0.2) --- updated-dependencies: - dependency-name: pyramid dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
From now it is possible to parse vcs urls in requirements file. Fixes blue-yonder#99
There was a problem with zope-event package. It is possible to `pip wheel zope-event` or `pip wheel zope.event` but each command produces wheel file with `.` in its name, because project_name in package metadata is `zope.event`. Builder._find_wheel method produced BuildError because there was no whl file containing `zope-event`.
…-parser dependency
…reak packages that don't normalise dashes to dots.
…st switch to pip-requirements-parser For the purpose of a junit file we don't actually care about how these names are normalised. It is only important that every entry in the requirements file gets a corresponding entry.
damianwasik98
force-pushed
the
vcs-urls
branch
from
557c123
to
e4cf2df
Compare
|
Hi @matthias-bach-by 😉 Are you planning to release a new version of package to pypi with this feature? |
|
The new version is published at https://pypi.org/project/devpi-builder/6.1.0/. Thanks for your contribution! |
Description in commit msgs☺️