Skip to content

Upgrade external tools #22065

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

Merged
merged 13 commits into from
Mar 17, 2025
Merged

Upgrade external tools #22065

merged 13 commits into from
Mar 17, 2025

Conversation

grihabor
Copy link
Contributor

@grihabor grihabor commented Mar 9, 2025

The pr was generated with this command:

pants run build-support/bin:external-tool-upgrade -- -v src/python/pants/backend/

(Notes are not generated yet)

The tool code is here #22064

@grihabor grihabor changed the title External tool upgrade script preview Upgrade external tools Mar 9, 2025
@grihabor grihabor force-pushed the external-tool-upgrade branch 2 times, most recently from c3b8cb9 to 5c591da Compare March 9, 2025 20:22
@grihabor grihabor force-pushed the external-tool-upgrade branch from 5c591da to 1ffb917 Compare March 9, 2025 20:29
@grihabor grihabor marked this pull request as ready for review March 12, 2025 15:20
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Thank you!

@grihabor
Copy link
Contributor Author

@huonw Thank you for review! Ready for merge

  • fixed merge conflict in pex-cli
  • upgraded pex-cli to 2.33.4
  • formatted the table in notes
  • added a note about version format breaking change

@grihabor
Copy link
Contributor Author

native_engine.IntrinsicError: Could not identify a process to backtrack to for: Missing digest: Was not present in the local store: Digest { hash: Fingerprint, size_bytes: 1294 }, with workunit: Workunit { name: "process", level: Debug, span_id: SpanId(3050899624272423132), parent_ids: [SpanId(10285682066423053561)], state: Started { start_time: SystemTime { tv_sec: 1741864346, tv_nsec: 218582000 }, blocked: false }, metadata: Some(WorkunitMetadata { desc: Some("Scheduling: Resolving plugins: hdrhistogram"), message: None, stdout: None, stderr: None, artifacts: [], user_metadata: [] }) }

I believe it's the flaky test again

Comment on lines 44 to 45
default_version = "2.33.4"
default_url_template = "https://github.com/pex-tool/pex/releases/download/v{version}/pex"
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for thinking about aligning this to other tools... but I think this breaking change isn't appropriate in this form.

Can we (manually) switch back to the old style for this PR?

We can potentially make the breaking change as a separate piece of work, but we should do a deprecation for a while (i.e. support both old and new). In particular, I think it's relatively common for people to pin to a PEX version (particularly a newer one than the one in their current version of Pants) to pick up new features/bugfixes.

This change is instantly breaking anyone who does that, without any "live" explanation (i.e. when they try the upgrade, they'll just get download error explosions, rather than useful guidance about how to fix) and thus making their upgrade cycle harder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure 28dd523

Moved it to a separate pr #22090

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Looks great now, except for some extra release notes. Thanks!

Co-authored-by: Huon Wilson <wilson.huon@gmail.com>
@grihabor
Copy link
Contributor Author

@huonw Done

@huonw
Copy link
Contributor

huonw commented Mar 17, 2025

Thank you!

@huonw huonw merged commit 8f29720 into pantsbuild:main Mar 17, 2025
24 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.

2 participants