An ExternalTool for downloading the grpc_python_plugin. #10927
Conversation
The default versions mentioned in this change have been uploaded to the relevant location, along with a README.md explaining how to build and upload other versions. [ci skip-rust] [ci skip-build-wheels]
| "1.32.0|darwin|b2db586656463841aa2fd4aab34fb6bd3ef887b522d80e4f2f292146c357f533|6215304", | ||
| "1.32.0|linux |1af99df9bf733c17a75cbe379f3f9d9ff1627d8a8035ea057c3c78575afe1687|4965728", | ||
| ] | ||
|
|
There was a problem hiding this comment.
You should also implement generate_exe. We stopped using a default implementation under the reasoning that it's more clear to be explicit at each subclass.
There was a problem hiding this comment.
This just downloads the file you need. generate_exe is for when the download is an archive, and you need to locate the binary within that archive.
There was a problem hiding this comment.
Do we expect the user to call downloaded_tool.exe? If so, we need to implement generate_exe, its @abstractmethod otherwise.
E.g. Pex:
pants/src/python/pants/backend/python/util_rules/pex_cli.py
Lines 54 to 55 in d526d4e
There was a problem hiding this comment.
Hmm, I think the semantics have changed since I wrote ExternalTool. In principle if you are downloading the file itself, not an archive, then you don't need exe, but it looks like now you have to implement it. So the docstring on generate_exe is wrong.
There was a problem hiding this comment.
Looking at the history here, I think the default was fine, why was it changed? If downloading an executable file directly, not in an archive, we already know its name, so why force people to add boilerplate?
There was a problem hiding this comment.
To be explicit that that's what's going on. I found it a bit magical before, that you had to look at the implementation of ExternalTool to be confident exe was right. Stu acked the change.
Feel free to change it back, this probably isn't a big deal either way.
There was a problem hiding this comment.
I mean, it's always right if the download is the executable itself. Maybe the confusion is that the old code was on the caller side, and really this should have been a default implementation of generate_exe(). I think that would make it more obvious.
[ci skip-rust] [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Internal only changes left off from the changelog: * Use cpython types in Rust functions that manipulate python objects (#10942) `PR #10942 <https://github.com/pantsbuild/pants/pull/10942>`_ * update libz-sys version to fix macOS compile error (#10941) `PR #10941 <https://github.com/pantsbuild/pants/pull/10941>`_ * Upgrade to Rust stable 1.47.0. (#10933) `PR #10933 <https://github.com/pantsbuild/pants/pull/10933>`_ * Finish CreateDigest Directory cleanup. (#10935) `PR #10935 <https://github.com/pantsbuild/pants/pull/10935>`_ * Hotfix broken import from merge conflict (#10934) `PR #10934 <https://github.com/pantsbuild/pants/pull/10934>`_ * Revert "Port nailgun client to rust (#10865)" (#10929) `PR #10929 <https://github.com/pantsbuild/pants/pull/10929>`_ * An ExternalTool for downloading the grpc_python_plugin. (#10927) `PR #10927 <https://github.com/pantsbuild/pants/pull/10927>`_ * Port nailgun client to rust (#10865) `PR #10865 <https://github.com/pantsbuild/pants/pull/10865>`_ * print stacktraces during import errors (#10906) `PR #10906 <https://github.com/pantsbuild/pants/pull/10906>`_ * fs.Digest is declared in Rust (#10905) `PR #10905 <https://github.com/pantsbuild/pants/pull/10905>`_ * add requests_ca_bundle to settable_env_vars (#10909) `PR #10909 <https://github.com/pantsbuild/pants/pull/10909>`_ [ci skip-rust]
The default versions mentioned in this change have been
uploaded to the relevant location, along with a README.md
explaining how to build and upload other versions.
[ci skip-rust]
[ci skip-build-wheels]