Skip to content
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.

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

Preserve relative paths for editable dependencies in output #2087

Merged
merged 11 commits into from
May 10, 2024

Conversation

macro1
Copy link
Contributor

@macro1 macro1 commented May 7, 2024

Accomplishes preservation of relative dependency paths by reverting back to input requirements definition for editable dependencies.

There are a couple prior attempts of this. The two I read through treated this like a formatting problem, I'm not sure it is. The issue, as has been pointed out in #204 is that paths are converted into urls in the form of absolute paths within pip.

If we maintain knowledge of the original relative paths within pip-tools, the values will be maintained more faithfully, and the complexity should be lower.

This PR accomplishes it by forcing the return value from parse_requirements() to give the values we need elsewhere. A better approach may be to return something different than InstallRequirement or preserve the relationship with the parsed requirement and provide both where it makes sense.

Contributor checklist
  • Included tests for the changes.
  • PR title is short, clear, and ready to be included in the user-facing changelog.
Maintainer checklist
  • Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@chrysle chrysle changed the title Preserve relative paths for editable dependancies Preserve relative paths for editable dependencies in output May 7, 2024
@chrysle chrysle added bug Something is not working pip Related to pip writer Related to results output writer component annotations Related to packages annotations labels May 7, 2024
@chrysle
Copy link
Contributor

chrysle commented May 7, 2024

The change looks good to me (we'll need a unit test, though). The CI failure is unrelated, I'll look into it.

A better approach may be to return something different than InstallRequirement

Do you have anything in mind?

@macro1
Copy link
Contributor Author

macro1 commented May 7, 2024

The change looks good to me (we'll need a unit test, though). The CI failure is unrelated, I'll look into it.

A better approach may be to return something different than InstallRequirement

Do you have anything in mind?

I was mainly thinking we could add a new structure containing the InstallRequirement and any other attributes pip-tools may need to track. However, I understand that it would impact many internal interfaces, which could pose some challenges.

@macro1 macro1 force-pushed the relative-paths branch 4 times, most recently from cfbc946 to d71761d Compare May 7, 2024 18:14
@chrysle
Copy link
Contributor

chrysle commented May 7, 2024

Ah yes, I love Windows, too. Will review soon.

tests/test_pip_compat.py Outdated Show resolved Hide resolved
tests/test_pip_compat.py Show resolved Hide resolved
piptools/_compat/pip_compat.py Outdated Show resolved Hide resolved
tests/test_pip_compat.py Outdated Show resolved Hide resolved
macro1 and others added 3 commits May 8, 2024 12:28
Co-authored-by: chrysle <96722107+chrysle@users.noreply.github.com>
Co-authored-by: chrysle <96722107+chrysle@users.noreply.github.com>
for more information, see https://pre-commit.ci
@chrysle chrysle linked an issue May 9, 2024 that may be closed by this pull request
@macro1 macro1 marked this pull request as ready for review May 9, 2024 20:35
@macro1 macro1 enabled auto-merge May 10, 2024 23:21
@macro1 macro1 added this pull request to the merge queue May 10, 2024
Merged via the queue into jazzband:main with commit fb7d14a May 10, 2024
34 of 36 checks passed
@chrysle
Copy link
Contributor

chrysle commented May 11, 2024

@macro1 Could you look into fixing the linting errors?

@macro1 macro1 mentioned this pull request May 11, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annotations Related to packages annotations bug Something is not working pip Related to pip writer Related to results output writer component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip-compile absolute links when given relative path
2 participants