pants icon indicating copy to clipboard operation
pants copied to clipboard

RFC: Make ExternalTool exportable

Open lilatomic opened this issue 1 year ago • 3 comments

This MR wires ExternalTools into the export machinery.

It exposes them under a separate cli arg --bin. Although it uses some of the same machinery as --resolve, there are several differences that I think support a separate flag (and some separate internal):

  1. ExternalTools don't have a resolve. They therefore must not show up for generating lockfiles. We have to implement this separation interally, so we should probably surface that.
  2. They would be invoked directly (instead of source dist/export/.../activate)
  3. We can put them all in a folder "dist/export/bin" so people can get them all

Closes #21251

Bikeshedding:

  • Do we like --bin
  • Is putting all the bins in the same dir what we want

TODO:

  • [x] tests
  • [ ] make all tools exportable
  • [ ] update examples with ExternalTool and subclasses to include exporting

lilatomic avatar Aug 04 '24 02:08 lilatomic

I wonder if we don't, due to two key downsides of putting them all in the same directory

Both of these are valid.

There's also a parameter "generate_exe" which points to the actual executable. We could export everything to separate dirs and then symlink the executable itself into a bin dir

lilatomic avatar Aug 07 '24 18:08 lilatomic

Looks like a very sensible solution to me, and --bin seems fine to me. I would put them in different dirs, but the symlinking is a good idea so that it's easy to access them via a single PATH entry.

benjyw avatar Aug 07 '24 20:08 benjyw

do we want to make all TemplatedExternalTools exportable? We could extend Subsystem.rules and add the UnionRule there

lilatomic avatar Aug 28 '24 20:08 lilatomic

We've just branched for 2.23, so merging this pull request now will come out in 2.24, please move the release notes updates to docs/notes/2.24.x.md. Thank you!

huonw avatar Sep 11 '24 21:09 huonw