When statically parsing pyproject.toml, convert relative paths to absolute for use as file URIs #2221
Conversation
| assert out.exit_code == 0 | ||
| assert out.stdout == dedent( | ||
| f"""\ | ||
| foo[ext1] @ {src_file.parent.absolute().as_uri()} |
There was a problem hiding this comment.
Is this really what we want? I don't think people would expect absolute paths from someone else's computer to end up in their lock files..
There was a problem hiding this comment.
I agree, and it's why I was pretty hesitant at first. But also, that's the behavior from 7.4.1 -- it's just not tested.
In theory, someone could be using this in some controlled context like an image build and wanting the concrete dependency path that this provides. So although I'm uncertain that it's right for the long term, I don't want to treat it as blocking the fix.
There was a problem hiding this comment.
Wait, did it always use absolute paths in 7.4.1? Not relative? I thought it'd conform to the shape of input that the 7.5.0 changelog promises.. Can we use file://./rel/path/?
There was a problem hiding this comment.
The path in this case is part of the InstallRequirement itself -- the "link" attribute is populated with this file URI.
The normalization in 7.5.0 only applies to the comes_from data ("via").
We could reach down into this as well and convert to file://./rel/path/ as you say, but I'm not sure if we should. It would be a separate bit of data transformation -- we could share code but it's a different attribute of the parsed requirements data that we'd be manipulating.
There was a problem hiding this comment.
I suppose, we could postpone that for now. There might be a use case coverage missing for stripping the @ http://..source in some contexts.
|
Oh, looks like Windows is unhappy about the delimiters again. |
sirosen
force-pushed
the
fix-relpath-static-pyproject-parse
branch
from
4507cdf to
fc92905
Compare
When statically parsing a pyproject.toml , we can encounter self-referential package usage. For example, an extra which names another extra on the same package. The source for these requirements is the current package, but as a concrete requirement, not an abstract one. To make it concrete, we use a URI dependency link pointed at the package directory as a file-URI. When the input path to `pyproject.toml` is relative, it needs to be made absolute for use as a URI. Failing to do so results in errors. Because the `pip-compile` output of an such an extra depends concretely on the specific version of the package found in-tree, and not abstractly on the package name (without the URI), tests confirm that we see the rendered package directory as a file URI in `pip-compile` output.
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
sirosen
force-pushed
the
fix-relpath-static-pyproject-parse
branch
from
fc92905 to
b05d736
Compare
|
The thing I broke in the tests is treatment of Line 91 in 6e0ef33 It's easy to "fix" by just adding an |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as off-topic.
This comment was marked as off-topic.
# Pull Request There is an issue with self-referential packages in a pyproject.toml with the latest pip-tools release. The self-referential [all] package, which is not written to the requirements.txt as it is not selected still throws a pip-compile error. More information for this can be found in the ticket: jazzband/pip-tools#2215 The pin should be removed when jazzband/pip-tools#2221 gets merged.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
I've just gotten back to this, taken a look at the open threads and marked the ones which were applied as suggestions as resolved. I looked at what happens in the case of dynamic metadata, and we have a similar bit of path formatting in there. I'd like to resolve this by adding calls to @webknjaz, are you okay with that increase in scope? I'll need to add some new tests to cover it and another changelog note. |
|
@sirosen agreed. Increasing the scope sounds justified. |
When statically parsing a
pyproject.toml, we can encounter self-referential package usage. For example, an extra which names another extra on the same package.The source for these requirements is the current package, but as a concrete requirement, not an abstract one. To make it concrete, we use a URI dependency link pointed at the package directory as a file-URI.
When the input path to
pyproject.tomlis relative, it needs to be made absolute for use as a URI. Failing to do so results in errors.Because the
pip-compileoutput of an such an extra depends concretely on the specific version of the package found in-tree, and not abstractly on the package name (without the URI), tests confirm that we see the rendered package directory as a file URI inpip-compileoutput.Contributor checklist
changelog.d/(seechangelog.d/README.mdfor instructions) or the PR text says "no changelog needed".Maintainer checklist
skip-changeloglabel.