:wave: I was hopeful that it’d be easy to implemen...
# development
👋 I was hopeful that it’d be easy to implement the fix for https://github.com/pantsbuild/pants/issues/15322, but after digging around the code for a bit I might be stumped. we’re setting the
to the
dir in the
setup here - it’s not obvious to me if there’s a way to inject the absolute path of the final work-dir into that value. I thought about hacking in a
or something, but looks like the values we set in the process env get escaped before being written to
Within the
logic there’s support for a magic
placeholder in
- is there anything similar for process envs that I could use here?
cc @curved-television-6568 since I think you’re most familiar with this area
it looks like
is just a symlink to another tmp dir - could we drop the symlink entirely, and instead set
👍 1
That's definitely worth trying!
We have some test for BinaryShims in src/python/pants/core/util_rules/system_binaries_test.py which you can use when making edits to verify things still work edit: actually might need to adjust a bit since API contract is changing
Ah, src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py uses BinaryShims so that's a good integration test
🤔 I was hoping that I would only need to change the logic in the docker subsystem, not the binary-shims code itself
but still investigating to see if that’s feasible
i.e. if I can take the
from the
instance and somehow get the underlying absolute path of the directory containing the digest’s contents, I could plug that into the `DockerBinary`’s
@hundreds-father-404 what do you think of this idea: when docker tools are configured, we write an extra shim for
into the temp working dir, and call it instead of directly invoking the
binary. the shim will
export PATH=$(pwd)/.shims/bin:${PATH}
and then exec to the
it’s ugly, but (I think) any other approach would require some more major refactoring to components outside of the docker subsystem
https://github.com/pantsbuild/pants/pull/15330 should fix https://github.com/pantsbuild/pants/issues/15322 but the approach is a bit hacky, so I’ve opened for early review (still needs tests). can people take a look at let me know if it’s worth finishing out?
I’m looking into this. Rather than requiring an extra dedicated shim here, I’ll propose to support a
placeholder value in the
env, so we can inject the sandbox path into the env var. cc @witty-crayon-22786 wdyt about that approach?
I've pushed a couple PRs for review on this.
🙌 2
👀 1