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

-r in constraint files yields constraints #8821

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Aug 30, 2020

Fixes #8416

  • test
  • documentation

@uranusjr
Copy link
Member

I’m still not convinced this is the right thing to do, but the implementation should work if we decide to do this.

@pfmoore
Copy link
Member

pfmoore commented Aug 31, 2020

Agreed. I think people should be using -c in constraints files to include extra files, and -r in constraints files should be disallowed. The issue that triggered this (#8416) uses fairly muddled terminology ("-r constraints" and "-c constraints") which suggests that the OP doesn't actually have a very clear understanding of the difference between a requirement and a constraint (which is not at all unreasonable, as prior to the new resolver, the difference was very badly documented, and the behaviour was frankly a mess).

As part of cleaning up the model of what constraints are with the new resolver, I think disallowing -r in constraints files is far more reasonable. (And if we do want to keep -r in constraints files, I'd expect the reasoning to be documented clearly).

@sbidoul
Copy link
Member Author

sbidoul commented Sep 4, 2020

I don't know. To me the use case in #8416 sounds reasonable. And documenting -r as including requirements in the current context, and -c as including recursively as constraints seems quite natural to me.

@pfmoore
Copy link
Member

pfmoore commented Sep 4, 2020

I see no reason why #8416 can't be fixed by simply using -c instead of -r in the included file. But it's hard to tell because the use case uses "made up" names, so it's hard to be clear what the actual issue is.

I'm very keen on keeping a clear distinction between constraints files and requirements files, which is why I'm against this change.

@sbidoul
Copy link
Member Author

sbidoul commented Sep 4, 2020

Maybe. I don't have the use case myself, so that needs clarification for sure.

I'm very keen on keeping a clear distinction between constraints files and requirements files, which is why I'm against this change.

So that means rejecting -c in -r files and -r in -c files?

@pfmoore
Copy link
Member

pfmoore commented Sep 4, 2020

So that means rejecting -c in -r files and -r in -c files?

Yep, I'd assume so.

@xavfernandez
Copy link
Member

xavfernandez commented Sep 8, 2020

So that means rejecting -c in -r files and -r in -c files?

Since requirements files already allow a lot of pip command line options, I'd find it strange to disallow -c.

However I'd be totally on board to disallow -r (and other pip options) in -c files.

@pfmoore
Copy link
Member

pfmoore commented Sep 8, 2020

Thinking it through, some more, I agree. Requirements files are "a bunch of stuff you can supply on the command line collected together", so allowing -c in requirements files to add constraints makes sense. But not the other way round, constraints files should only contain constraints, and -c in constraints files is just an inclusion mechanism.

@HALtheWise
Copy link

Sorry for the delayed response, the reason why we end up with -r and -c nested is because we are using the same files as requirements for some build configurations and constraints for others. One example would be having a "dev" and "prod" environment, where the things that are required in dev (including transitively from other files) act as version constraints for prod, because we never want to deploy a different version of a package than we developed with, but we are ok with not deploying some package that was available in dev. Our actual usecase involves more complexity, with a large number of different "dev" and "prod"-like configurations, and we would ideally like to describe the relationship between them in the .txt files themselves.

As you point out, if we're willing to write a script that understands this tree structure outside of pip, we could put no -r or -c flags into any of the files, and have an external wrapper script that generates the appropriate -r/-c flags for each configuration we wish to invoke pip with, but that is less preferred because it obfuscates what exactly gets included with a given deploy configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected behavior with -r references inside -c references
5 participants