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

Upgrade Pants's PEX to 2.1.137. #273

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Conversation

benjyw
Copy link
Contributor

@benjyw benjyw commented Sep 11, 2023

To match the one embedded in pbt.

To match the one embedded in pbt.
@benjyw benjyw requested review from thejcannon and huonw September 11, 2023 04:43
@benjyw
Copy link
Contributor Author

benjyw commented Sep 11, 2023

Before this change, cargo run -p package -- test failed with

>> Verifying the tools.pex built by the package crate matches the tools.pex built by Pants
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"8ee6822086594dd83394f921c8bac0a25a8d70fa530522d59064062f3ba2b63f"`,
 right: `"e7bf24a357bd0d959614df1d2516d088e8e44497a354c615d415104d6f830d14"`', package/src/test.rs:288:9

With this change, cargo run -p package -- test passes.

@huonw
Copy link
Contributor

huonw commented Sep 11, 2023

Do you have ideas about why CI didn't catch this?

(I guess this may've been caused by #259 ? although that was pinning PEX to 2.1.126, not .137 🤔 )

@benjyw
Copy link
Contributor Author

benjyw commented Sep 11, 2023

Do you have ideas about why CI didn't catch this?

I assume because of the --tools-pex-mismatch-warn flag (see #2)

(I guess this may've been caused by #259 ? although that was pinning PEX to 2.1.126, not .137 🤔 )

@benjyw benjyw merged commit 8cc5afa into pantsbuild:main Sep 11, 2023
@benjyw benjyw deleted the upgrade_pants_pex branch September 11, 2023 07:10
@benjyw
Copy link
Contributor Author

benjyw commented Sep 11, 2023

I spoke too soon, #274 is also required to get identical tools.pex.

@thejcannon
Copy link
Member

If anything, I think we should have pbts version track the pants one... it'd be less to maintain

@thejcannon
Copy link
Member

Also how does this relate to my original post in #development?
https://pantsbuild.slack.com/archives/C0D7TNJHL/p1694101689659639?thread_ts=1694101689.659639&cid=C0D7TNJHL

The metadata in pbt..toml doesn't use 137, it uses 134

@benjyw
Copy link
Contributor Author

benjyw commented Sep 11, 2023

Also how does this relate to my original post in #development?
https://pantsbuild.slack.com/archives/C0D7TNJHL/p1694101689659639?thread_ts=1694101689.659639&cid=C0D7TNJHL

The metadata in pbt..toml doesn't use 137, it uses 134

I don't think that is correct. See my post in that thread.

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