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

Use recursive include for requirements globbing #15

Closed
wants to merge 3 commits into from

Conversation

paulbovbel
Copy link
Contributor

@paulbovbel paulbovbel commented May 1, 2018

Allows using the full range of requirements constraints supported by pip, and gets rid of a bunch of code from catkin_virtualenv. Win-win.

@wkentaro
Copy link

wkentaro commented May 2, 2018

Actually, multiple requirements.txt cannot be passed to pip:

% ls req*
req1.txt  req2.txt

% cat req*
PyYAML>=3
PyYAML<=3.1

% pip install -r req1.txt req2.txt
Collecting req2.txt
  Could not find a version that satisfies the requirement req2.txt (from versions: )
No matching distribution found for req2.txt

and combining multiple requirements.txt files is not so easy, I think.
And in most case, it is used with ls req* | xargs -n1 pip install -r, and the order of passed filenames is crucial.

@paulbovbel
Copy link
Contributor Author

paulbovbel commented May 2, 2018

Multiple requirements files can be nominally passed in via multiple -r flags. However, it seems that pip is unable to resolve incompatible (or even intersecting) requirements, although an issue to do proper dependency resolution in pip is in progress (pypa/pip#988 (comment)).

So at the end of the day, this ends up enforcing a system where no requirements are 'merged', and a child package can only inherit it's parent's exact dependencies.

I think that 'merging' requirements intelligently (beyond what I had implemented) is extremely out of scope for this package, and I recognize that it's disingenuous to do it half-assed (hence purging it).

However I would be amenable to an approach that replaces parent dependencies with child dependencies when there's a repeated package key.

@@ -23,4 +23,5 @@
class TestVirtualenv(unittest.TestCase):

def test_import_wrapt(self):

Choose a reason for hiding this comment

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

No longer imports wrapt!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sharp eye!


class TestVirtualenv(unittest.TestCase):

def test_import_wrapt(self):

Choose a reason for hiding this comment

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

ditto.

@@ -23,4 +23,5 @@
class TestVirtualenv(unittest.TestCase):

def test_import_wrapt(self):

Choose a reason for hiding this comment

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

Hat trick.

@paulbovbel
Copy link
Contributor Author

I don't think this strategy is viable unless pip can resolve multiple requirements as ticketed. A smarter 'requirement merging' strategy for catkin_virtualenv might be feasible, handling multiple inline constraints.

@paulbovbel paulbovbel closed this Jun 4, 2018
@paulbovbel paulbovbel deleted the recursive-include branch March 18, 2019 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants