Skip to content

Comments

export: fix directory dependencies#3120

Closed
mrcljx wants to merge 2 commits intopython-poetry:masterfrom
mrcljx:patch-2
Closed

export: fix directory dependencies#3120
mrcljx wants to merge 2 commits intopython-poetry:masterfrom
mrcljx:patch-2

Conversation

@mrcljx
Copy link

@mrcljx mrcljx commented Oct 7, 2020

Pull Request Check List

pip doesn't support foo @ some/path/foo directory dependencies.

Depends on #3118 (doesn't really depend on it, but will be in conflict), which is why this PR is in draft mode for now (and also includes the same changes).

  • Added tests for changed code.
  • Updated documentation for changed code.

markers = requirement.split(";", 1)[1].strip()
if markers:
line += "; {}".format(markers)
elif dependency.is_directory(): # pip doesn't support ` @ ` for directories
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think this should be done, because directory dependencies can be PEP 517 dependencies, which means -e will fail.

Copy link
Author

@mrcljx mrcljx Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow. Having a line foo @ some/path in requirements.txt will break pip install with ERROR: Invalid requirement: 'foo @ some/path'? Should the exporter accept a new PEP508 format option besides requirements.txt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what is being discussed in #3121 is a more complete solution. But yes, the comments in #3118 are more current.

Should this be closed in favour of that?

@github-actions
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants