https://pantsbuild.org/ logo
#development
Title
# development
s

sparse-lifeguard-95737

05/04/2022, 2:38 PM
👋 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
PATH
to the
.shims
dir in the
DockerBinary
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
$(pwd)
or something, but looks like the values we set in the process env get escaped before being written to
__run.sh
Within the
export
logic there’s support for a magic
{digest_root}
placeholder in
argv
- 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
.shims
is just a symlink to another tmp dir - could we drop the symlink entirely, and instead set
PATH=<path-to-tmpdir>
?
👍 1
h

hundreds-father-404

05/04/2022, 4:46 PM
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
s

sparse-lifeguard-95737

05/04/2022, 4:50 PM
🤔 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
Digest
from the
BinaryShims
instance and somehow get the underlying absolute path of the directory containing the digest’s contents, I could plug that into the `DockerBinary`’s
extra_env
@hundreds-father-404 what do you think of this idea: when docker tools are configured, we write an extra shim for
docker
into the temp working dir, and call it instead of directly invoking the
docker
binary. the shim will
export PATH=$(pwd)/.shims/bin:${PATH}
and then exec to the
docker
binary
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?
c

curved-television-6568

05/05/2022, 9:49 AM
I’m looking into this. Rather than requiring an extra dedicated shim here, I’ll propose to support a
{root}
placeholder value in the
PATH
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