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

fix: Don't override PIP_CONSTRAINT if set by the user #1675

Merged
merged 6 commits into from
Mar 9, 2024

Conversation

joerick
Copy link
Contributor

@joerick joerick commented Nov 24, 2023

Following on from #1667, change the logic so that we retain user-set PIP_CONSTRAINT options, by concatting them with our own constraints

docs/options.md Outdated Show resolved Hide resolved
cibuildwheel/macos.py Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member

@joerick it just occurred to me — what if cibuildwheel would take the user-provided $PIP_CONSTRAINT, then create the following file:

-c user-provided-constraints.txt
-c cibuildwheel-provided-constraints.txt

perhaps, removing entries from cibuildwheel-provided-constraints.txt that are defined in the user-provided-constraints.

As an alternative, cibuildwheel could write (of symlink?) its selected constraints file for the current session to a predictable location on each invocation, letting the end-users include it into their constraints via -c.

Would that be usable or would it make things more problematic?

@joerick
Copy link
Contributor Author

joerick commented Feb 27, 2024

what if cibuildwheel would take the user-provided $PIP_CONSTRAINT, then create the following file:

That would have the same behaviour as this PR. In this PR, the users' constraints are concat'd with our constraints.

perhaps, removing entries from cibuildwheel-provided-constraints.txt that are defined in the user-provided-constraints.

Since #1725, I hope there isn't a big problem with conflicts. I'd rather see how this plays out before adding more complexity to how we handle this particular problem.

It's worth noting that the 'removing duplicate entries' idea is good but isn't foolproof, because each of the packages also have their own requirements specifiers that could conflict with what's set in the constraints. I'd rather just tell the minority of users that have conflicts to use DEPENDENCY_VERSIONS=latest and they can pin pip, build, delocate, auditwheel themselves, if it's important to them.

@joerick
Copy link
Contributor Author

joerick commented Feb 27, 2024

@henryiii or @mayeut, a review here would be great. I'd like to get some of these PRs merged if we can, they've been hanging around for a while.

cibuildwheel/windows.py Outdated Show resolved Hide resolved
cibuildwheel/macos.py Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member

It's worth noting that the 'removing duplicate entries' idea is good but isn't foolproof, because each of the packages also have their own requirements specifiers that could conflict with what's set in the constraints. I'd rather just tell the minority of users that have conflicts to use DEPENDENCY_VERSIONS=latest and they can pin pip, build, delocate, auditwheel themselves, if it's important to them.

True, I also realized this later on, after posting that comment ;)

@joerick
Copy link
Contributor Author

joerick commented Mar 2, 2024

ready for another review henryiii

@joerick joerick merged commit f7de18e into main Mar 9, 2024
20 checks passed
@joerick joerick deleted the pip-constraints-fix branch March 9, 2024 13:53
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.

3 participants