Skip to content

requirements: Add version exclusion qualifier for Click on 8.2.2#2052

Merged
juergbi merged 1 commit intoapache:masterfrom
shymega-ct:domrodriguez/lock-click-dep
Sep 10, 2025
Merged

requirements: Add version exclusion qualifier for Click on 8.2.2#2052
juergbi merged 1 commit intoapache:masterfrom
shymega-ct:domrodriguez/lock-click-dep

Conversation

@shymega-ct
Copy link
Contributor

@shymega-ct shymega-ct commented Sep 2, 2025

Click ==8.2.2 causes a regression that affects Buildstream's argument parsing. Currently, one known case is where --no-strict is set when not explicitly configured to by the command-line.

This change to requirements.in adds a exclusion for Click 8.2.2, but allows any other version higher than Click 7.0.

@juergbi
Copy link
Contributor

juergbi commented Sep 2, 2025

click 8.2.2 got yanked from PyPI due to this unintended change in behavior, so this should no longer be needed: https://pypi.org/project/click/#history

@juergbi juergbi closed this Sep 2, 2025
@jjardon
Copy link
Contributor

jjardon commented Sep 2, 2025

@juergbi Can we please still merge this patch? Not everyone uses pip to install dependencies: we might make it more flexible to only restrict 8.2.2 being used? ie allow anything < 8.2.2 and > 8.2.2

@shymega
Copy link

shymega commented Sep 2, 2025

@jjardon I think we could achieve something like that with 'version specifiers', which is implemented by pip - specifically, a version exclusion clause.

I could edit this PR to implement something like:

Click >= 7.0; != 8.2.2, if that syntax is correct - off the top of my head.

@juergbi
Copy link
Contributor

juergbi commented Sep 3, 2025

Click >= 7.0; != 8.2.2, if that syntax is correct - off the top of my head.

I think multiple version specifiers need to be separated by a comma, not a semicolon. I'm happy to merge that if that helps integrations where the yanking is not sufficient. (It obviously still won't help when installing the already released BuildStream 2.5.0).

@abderrahim
Copy link
Contributor

Not everyone uses pip to install dependencies: we might make it more flexible to only restrict 8.2.2 being used? ie allow anything < 8.2.2 and > 8.2.2

I think that it won't help those who don't use pip to install dependencies. For instance, even if we had this in 2.5.0, it wouldn't have prevented freedesktop-sdk from picking up the update. I don't know how other distros work (would e.g. the Fedora packaging scripts pick up on incompatible versions requirements? I don't think they would unless they're calling to pip under the hood)

@jjardon
Copy link
Contributor

jjardon commented Sep 3, 2025

btw, I know is not going to help much with 2.5.0, I'm thinking more in 2.5.1 and future releases

@abderrahim abderrahim reopened this Sep 4, 2025
@shymega-ct shymega-ct force-pushed the domrodriguez/lock-click-dep branch from 08c9511 to e188034 Compare September 4, 2025 15:33
@shymega
Copy link

shymega commented Sep 4, 2025

@abderrahim @juergbi Could you approve the CI on this PR so we can check the change works?

Thanks.

@shymega-ct shymega-ct marked this pull request as ready for review September 5, 2025 00:28
@abderrahim
Copy link
Contributor

Please update the pull request title

@shymega-ct shymega-ct changed the title requirements: Restrict Click to 8.2.1 or below requirements: Exclude the v8.2.2 version of Click Sep 8, 2025
@shymega-ct shymega-ct changed the title requirements: Exclude the v8.2.2 version of Click requirements: Add version exclusion qualifier for Click on 8.2.2 Sep 8, 2025
Click ==8.2.2 causes a regression that affects Buildstream's argument
parsing. Currently, one known case is where `--no-strict` is set when
not explicitly configured to by the command-line.

This change to requirements.in adds a exclusion for Click 8.2.2, but
allows any other version higher than Click 7.0.
@shymega-ct shymega-ct force-pushed the domrodriguez/lock-click-dep branch from e188034 to 6d900f7 Compare September 8, 2025 13:58
@shymega-ct
Copy link
Contributor Author

@abderrahim That's now done, as well as typos fixed, and PR description updated in-line with commit message.

@juergbi juergbi merged commit d1191c6 into apache:master Sep 10, 2025
18 checks passed
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.

5 participants