Fix pip-compile handling of -r paths which are not subpaths of the cwd#2260
Fix pip-compile handling of -r paths which are not subpaths of the cwd#2260webknjaz merged 4 commits intojazzband:mainfrom
-r paths which are not subpaths of the cwd#2260Conversation
changelog.d/2231.bugfix.md
Outdated
There was a problem hiding this comment.
I need to break this habit of writing changelogs like commit messages. I even updated docs at work to remind people to use past tense for changelogs, and then I go around forgetting it myself! 😆
Thanks for the save.
There was a problem hiding this comment.
I wonder if we can get an LLM help out with this. I've been looking into configuring some LLM behaviors (CLAUDE.md / AGENTS.md) recently and this should be a func exercise.. Although, we could probably also try Vale.
There was a problem hiding this comment.
I'm interested in Vale, I think it looks very cool.
Less interested in using an LLM for this, personally. I'm waiting for the hype to die down, and plan to be a late adopter of the tech (after some not-great experiences trying it so far) assuming I can square it with my ethics.
There was a problem hiding this comment.
I think it's important to have these files in the repo because people are going to use agentic help regardless of what we tell them. And if there's some guidelines, the chance of them producing rubbish would be decreased. With that, they wouldn't waste as much compute retrying or generating nonsense. So in my book helping such tools be efficient is definitely positive if we're talking about the environmental impact.
We just got such configuration in ansible/ansible#85841 just now and I'm interested in exploring making such configs for projects better.
There was a problem hiding this comment.
@sirosen re:vale — I have this old issue where I collect various prose checkers @ aio-libs/multidict#276. If we succeed in integating any of them, I'd like to scale that to other projects.
There was a problem hiding this comment.
I think it's important to have these files in the repo because people are going to use agentic help regardless of what we tell them. And if there's some guidelines, the chance of them producing rubbish would be decreased. With that, they wouldn't waste as much compute retrying or generating nonsense. So in my book helping such tools be efficient is definitely positive if we're talking about the environmental impact.
I opened a discussion for this @ #2278
piptools/_compat/pip_compat.py
Outdated
There was a problem hiding this comment.
Perhaps this should go into a separate pathlib_compat.py?
There was a problem hiding this comment.
My only concern would be that it might only ever have this one function.
What about a path_compat.py so that it's not only scoped to pathlib?
I wouldn't move other things over right away, but any helpers for handling drive letters and such could go there.
There was a problem hiding this comment.
I snooped around and immediately found a second helper to put in here!
I still prefer path_compat.py, so I'll go with that for the moment, but I'm happy to rename as well.
There was a problem hiding this comment.
I unfortunately ended up with a little bit of extra change needed ( 3df3efa ) because my desire to put two related helpers together created an import cycle. I can revert if this feels like too much extraneous change.
There was a problem hiding this comment.
There was a problem hiding this comment.
I think you're right, especially coming back to this after a couple of days.
I'm going to partially revert, cutting it back to just this one helper in a path_compat.py module, as part of my rebase right now.
4ef3b4c to
21e5b23
Compare
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <578543+webknjaz@users.noreply.github.com>
3e781db to
95a824d
Compare
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <578543+webknjaz@users.noreply.github.com>
3df3efa to
bc8d8c8
Compare
piptools/_compat/path_compat.py
Outdated
There was a problem hiding this comment.
The literal can also be linked.
| """Return True if ``path1`` is relative to ``path2``.""" | |
| """Return :data:`True` if ``path1`` is relative to ``path2``.""" |
But as I said elsewhere, let's not include this helper.
There was a problem hiding this comment.
Since I reverted any move of this helper, I'm not changing the docstring, but 👍 to marking literals like this when possible.
piptools/_compat/path_compat.py
Outdated
There was a problem hiding this comment.
Both 3.8 and 3.9 are both EOL already: https://endoflife.date/python. And testing against pip is becoming difficult in older runtimes.
I think we could plan dropping Python 3.8 once #2257 is implemented and released. With that, there would be no need to even think about hosting this function in compat modules.
piptools/_compat/path_compat.py
Outdated
There was a problem hiding this comment.
| Compatibility helpers for working with paths and ``pathlib`` across platforms | |
| Compatibility helpers for working with paths and :mod:`pathlib` across platforms |
When `-r` is used to pass a path, we are normalizing with `pathlib.Path.relative_to`. This fails when the input is not a subpath of the current working directory. In Python 3.12+ pathlib supports this usage (`walk_up=True`), but on older Pythons we need to fallback to using `os.path.relpath`. In order to support this usage and give a clear indication of how we should upgrade when older versions are dropped, a small compat wrapper is here defined, `_relative_to_walk_up` which uses `relative_to(..., walk_up=True)` when it is available, and only uses `os.path.relpath` when necessary. After we drop Python 3.11 in a few years, the helper can be removed. A new regression test reproduces jazzband#2231 and is fixed with this patch. As part of writing that test, I wanted to leverage the `TestFilesCollection` helper, so this was extended in a small way to support usage without an explicit name.
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <578543+webknjaz@users.noreply.github.com>
`piptools._compat.path_compat` is a new module for holding any helpers dedicated to path inspection and manipulations.
bc8d8c8 to
7c7fff5
Compare
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <578543+webknjaz@users.noreply.github.com>
7c7fff5 to
d7da20b
Compare
resolves #2231
When
-ris used to pass a path, we are normalizing withpathlib.Path.relative_to. This fails when the input is not a subpath of the current working directory.In Python 3.12+ pathlib supports this usage (
walk_up=True), but on older Pythons we need to fallback to usingos.path.relpath.In order to support this usage and give a clear indication of how we should upgrade when older versions are dropped, a small compat wrapper is here defined,
_relative_to_walk_upwhich usesrelative_to(..., walk_up=True)when it is available, and only usesos.path.relpathwhen necessary. After we drop Python 3.11 in a few years, the helper can be removed.A new regression test reproduces #2231 and is fixed with this patch.
As part of writing that test, I wanted to leverage the
TestFilesCollectionhelper, so this was extended in a small way to support usage without an explicit name.Contributor checklist
changelog.d/(seechangelog.d/README.mdfor instructions) or the PR text says "no changelog needed".Maintainer checklist
skip-changeloglabel.