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

Relax constraints on typing-extensions #12710

Closed
wants to merge 2 commits into from
Closed

Conversation

asherf
Copy link
Member

@asherf asherf commented Aug 31, 2021

new version of black has an issue w/ a busted versions of type-extension.
see: https://github.com/psf/black/blob/79575f3376f043186d8b8c4885ef51c6b3c36246/setup.py#L85
psf/black#2460

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

I'm confused, is something breaking with the status quo in pantsbuild/pants? Or this breaks a consumer like Toolchain?

@asherf
Copy link
Member Author

asherf commented Aug 31, 2021

this prevents upgrading to a new version of black in repos that consume pants, because the new version of black doesn't allow the version of type-extension pants requires (see links)

@Eric-Arellano
Copy link
Contributor

this prevents upgrading to a new version of black in repos that consume pants, because the new version of black doesn't allow the version of type-extension pants requires (see links)

Hm, using what Pants version? The version of typing-extensions used by pantsbuild.pants should have zero impact on what [black].version is allowed to be set to, even before Pants 2.7's tool lockfiles. In Pants 2.6 and earlier, your constraints.txt was being awkwardly applied to tool installations - that may be causing issues if that's pinned improperly? In Pants 2.7, constraints.txt no longer has any impact and tools have their own dedicated lockfiles.

@asherf
Copy link
Member Author

asherf commented Aug 31, 2021

@Eric-Arellano
Copy link
Contributor

I see typing-extensions==3.7.4.3 in Toolchain's constraints.txt. That is likely the cause of this. The fix is not with this PR, but updating Toolchain's constraints.

cc @stuhood does this imply we should cherry-pick not using constraints.txt with tool lockfiles to Pants 2.6? Hm. If we do that, we don't have tool lockfiles yet, so the only way to pin is via [black].extra_requirements.

@asherf asherf closed this Aug 31, 2021
@asherf
Copy link
Member Author

asherf commented Aug 31, 2021

I am pretty sure this will not work, since if I manually change constraints.txt to use the latest typing-extensions other stuff will complain since pants pins it to the older versions.

@asherf
Copy link
Member Author

asherf commented Aug 31, 2021

Screen Shot 2021-08-31 at 1 30 33 PM

@asherf asherf reopened this Aug 31, 2021
@Eric-Arellano
Copy link
Contributor

Okay, that makes sense. This issue specifically happens for people who depend on pantsbuild.pants as a 3rd party requirement for their own code (so, plugin authors). And it happens because constraints.txt is being improperly applied to lool PEXes until Pants 2.7+.

@stuhood
Copy link
Sponsor Member

stuhood commented Aug 31, 2021

cc @stuhood does this imply we should cherry-pick not using constraints.txt with tool lockfiles to Pants 2.6? Hm. If we do that, we don't have tool lockfiles yet, so the only way to pin is via [black].extra_requirements.

We could... but I don't think that this issue is likely to affect other users unless they:

  1. manually change their black version
  2. use a constraints file that includes an incompatible dep on typing-extensions (which @asherf's repo only has via depending on Pants itself?)
  3. use resolve-all-constraints

It doesn't seem like the best candidate for a cherry-pick... but a targeted fix only for black might be more palatable?

@asherf
Copy link
Member Author

asherf commented Aug 31, 2021

Okay, that makes sense. This issue specifically happens for people who depend on pantsbuild.pants as a 3rd party requirement for their own code (so, plugin authors). And it happens because constraints.txt is being improperly applied to lool PEXes until Pants 2.7+.

I think this might be a common scenario for advanced uses who will have custom rules and logic (plugins) in their repo to handle custom workflows....

@Eric-Arellano
Copy link
Contributor

but a targeted fix only for black might be more palatable?

Yeah, okay, I think you're right we should not cherry-pick, it's potentially too disruptive.

@asherf
Copy link
Member Author

asherf commented Sep 7, 2021

ping

@stuhood
Copy link
Sponsor Member

stuhood commented Sep 7, 2021

@asherf : It doesn't look like the title aligns with what this does anymore... it seems to just bump versions in the lockfiles, which is fine, but unrelated. 2.7.x and main no longer mix end-user constraints.txt files in with tool resolves, so it shouldn't be possible to encounter the issue you originally reported.

@asherf
Copy link
Member Author

asherf commented Sep 7, 2021

@asherf : It doesn't look like the title aligns with what this does anymore... it seems to just bump versions in the lockfiles, which is fine, but unrelated. 2.7.x and main no longer mix end-user constraints.txt files in with tool resolves, so it shouldn't be possible to encounter the issue you originally reported.

that is correct. closing.

@asherf asherf closed this Sep 7, 2021
@asherf asherf deleted the m2 branch September 7, 2021 23:19
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.

None yet

3 participants