https://pantsbuild.org/ logo
Join Slack
Powered by
Why does the `AdhocProcessRequest` construction sp...
# development
b
Why does the 
AdhocProcessRequest
construction specify the 
shell_command.address.spec
after the command? https://github.com/pantsbuild/pants/blob/d12d7e2026225159219b21aba159bd0b6c3f8d39/src/python/pants/backend/shell/util_rules/shell_command.py#L147 So it looks like:
Copy code
run_shell_command(
    name = "foo",
    command=f"echo hi",
    workdir="/",
)
turns into:
Copy code
/usr/bin/bash -c $'echo hi' $'//:foo'
CC @ancient-vegetable-10556
a
@bitter-ability-32190 because 
bash -c 'blah'
doesn’t inject a 
$0
b
So it isn't actually passing the arg to the command?
a
so the first param needs to be 
$0
, so that you can use other positional parameters. 
$0
is also what bash shows on errors, so it makes the error messages friendlier
The command does receive it as 
$0
the alternative would be to write out the 
command
into a shell script and executing that, but that would be similarly obtuse (and less efficient)
b
(I'm running 
jq
in a sandbox and trying to figure out how to make these not reference a dead path:
Copy code
jq - commandline JSON processor [version 1.6]

Usage:  /tmp/pants-sandbox-zLIAv5/3rdparty/tools/jq-linux64 [options] <jq filter> [file...]
        /tmp/pants-sandbox-zLIAv5/3rdparty/tools/jq-linux64 [options] --args <jq filter> [strings...]
        /tmp/pants-sandbox-zLIAv5/3rdparty/tools/jq-linux64 [options] --jsonargs <jq filter> [JSON_TEXTS...]

jq is a tool for processing JSON inputs, applying the given filter to
its JSON text inputs and producing the filter's results as JSON on
standard output.

The simplest filter is ., which copies jq's input to its output
unmodified (except for formatting, but note that IEEE754 is used
for number representation internally, with all that that implies).

For more advanced filters see the jq(1) manpage ("man jq")
and/or <https://stedolan.github.io/jq>

Example:

        $ echo '{"foo": 0}' | jq .
        {
                "foo": 0
        }

For a listing of options, use /tmp/pants-sandbox-zLIAv5/3rdparty/tools/jq-linux64 --help.
Also, assuming 
$0
worked here, should we switch it to putting 
f"{bin_name() run {spec}"
?
a
how are you invoking 
jq
, there’s not a world in which this should matter
(yes, 
{bin_name()} run {spec}
could be an improvement
👍 1
b
(OK filed that issue and self-assigned. back on topic)
a
so there’s likely something confusing going on with passing args into 
run_shell_command
that we haven’t explored fully yet. This is generally why I prefer 
adhoc_tool
and would prefer to make these runnable
b
Remind me the difference?
Oh CWD is the difference
a
adhoc_tool
directly invokes processes, which means you don’t have to compensate for bash being obscene
b
Is it runnable? Let me see
a
Not presently
b
Ah yeah so that's the razor
So, my invocation... Here's the sandbox and command:
Copy code
josh@cephandrius:/tmp/pants-sandbox-zLIAv5$ tree .
.
├── 3rdparty
│   └── tools
│       └── jq-linux64
└── __run.sh

2 directories, 2 files
Copy code
#!/bin/bash
# This command line should execute the same process as pants did internally.
export <a bunch of env vars>
cd /home/josh/work/<repo>
/usr/bin/bash -c $'/tmp/pants-sandbox-zLIAv5/3rdparty/tools/jq-linux64' $'3rdparty/tools:jq' --help
I'm guessing this works for 
$0
ina shell script, but not other programs?
Copy code
$ /usr/bin/bash -c $'echo $0' $'3rdparty/tools:jq'
3rdparty/tools:jq
a
I’ll look here shortly
So the short of it is that 
jq
shouldn’t be receiving 
argv[0]
from 
bash
. Bash should only be using the 
argv[0]
for its internal purposes
are you passing 
$@
into 
jq
or something like that?
b
You have the 
__run.sh
and the tree
I'm laid bare
a
reads
OK, I see the 
__run.sh
, but in this case, I’ll actually need to see the target definition from your 
BUILD
file, and possibly also your pants invocation
b
Macros 🙈
a
because the input from the target definition and the pants invocation are what determine the contents of the 
__run.sh
file
There’s no reason why $0 should be in 
__run.sh
unless you pass it in
b
Why not use 
exec -a "string" <command>
According to 
exec --help
Copy code
Options:
      -a name   pass NAME as the zeroth argument to COMMAND
      -c        execute COMMAND with an empty environment
      -l        place a dash in the zeroth argument to COMMAND
a
bash -c "exec -a \"shell name\" shlexed_command"
?
b
I think so? I'm gonna bang on it
a
if you can somehow figure out what the value of 
command
is and spit it out to the terminal, that will really help
b
Copy code
file(
        name=f"downloaded-{name}",
        source = http_source(
            len = len,
            sha256 = sha256,
            url = url,
        ),
    )
    filename = url.rsplit("/", 1)[-1]
    command  = f"chmod +x {filename}"
    if filename.endswith(".tar.gz"):
        command = f"tar -xf {filename} &&" + command
    shell_command(
        name = f"sandboxed-{name}",
        command=command,
        tools = ["tar", "chmod"],
        execution_dependencies=[f":downloaded-{name}"],
        output_directories = ["."],
        #root_output_directory = f"unpacked-{name}",
    )
    run_shell_command(
        name = name,
        command=f"{{chroot}}/{build_file_dir() / exe}",
        execution_dependencies=[f":sandboxed-{name}"],
        workdir="/",
    )
Let me try and 
peek
and see if that dumps it easily
Copy code
{
  "address": "3rdparty/tools:jq",
  "target_type": "run_shell_command",
  "command": "{chroot}/3rdparty/tools/jq-linux64",
  "dependencies": [
    "3rdparty/tools:sandboxed-jq"
  ],
  "description": null,
  "execution_dependencies": [
    ":sandboxed-jq"
  ],
  "runnable_dependencies": null,
  "tags": null,
  "workdir": "/"
}
a
no args. So you’re just invoking pants with 
--
?
b
With
Copy code
{
  "address": "3rdparty/tools:downloaded-jq",
  "target_type": "file",
  "dependencies": [],
  "dependencies_raw": null,
  "description": null,
  "source_raw": {
    "url": "<https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64>",
    "len": 3953824,
    "sha256": "af986793a515d500ab2d35f8d2aecd656e764504b789b66d7e1a0b727a124c44",
    "filename": "jq-linux64"
  },
  "sources": [],
  "sources_fingerprint": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
  "tags": null
}
and
Copy code
{
  "address": "3rdparty/tools:sandboxed-jq",
  "target_type": "shell_command",
  "_sources_raw": null,
  "command": "chmod +x jq-linux64",
  "dependencies": [
    "3rdparty/tools:downloaded-jq"
  ],
  "description": null,
  "environment": "__local__",
  "execution_dependencies": [
    ":downloaded-jq"
  ],
  "extra_env_vars": null,
  "log_output": false,
  "output_dependencies_raw": null,
  "output_directories": [
    "."
  ],
  "output_files": [],
  "outputs": null,
  "root_output_directory": "/",
  "runnable_dependencies": null,
  "sources": [],
  "sources_fingerprint": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
  "tags": null,
  "timeout": 30,
  "tools": [
    "tar",
    "chmod"
  ],
  "workdir": "."
}
Ah the pants command is 
pants run <thing> -- --help
. Sorry there a small bit of messaging of the files before I posted here
Let me get you a working 
BUILD
file, one moment
Copy code
file(
    name="downloaded-jq",
    source = http_source(
        len = 3953824,
        sha256 = "af986793a515d500ab2d35f8d2aecd656e764504b789b66d7e1a0b727a124c44",
        url = "<https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64>",
    ),
)
shell_command(
    name = "sandboxed-jq",
    command="chmod +x jq-linux64",
    tools = ["chmod"],
    execution_dependencies=[":downloaded-jq"],
    output_directories = ["."],
)
run_shell_command(
    name = "jq",
    command=f"{{chroot}}/{build_file_dir() / 'jq-linux64'}",
    execution_dependencies=[":sandboxed-jq"],
    workdir="/",
)
This did the right thing, although I suspect the escaping is going to be an issue:
Copy code
/usr/bin/bash -c "exec -a $'//:jq' /tmp/pants-sandbox-AqjSjG/jq-linux64 --help"
Copy code
Usage:  //:jq [options] <jq filter> [file...]
        //:jq [options] --args <jq filter> [strings...]
        //:jq [options] --jsonargs <jq filter> [JSON_TEXTS...]
a
I will take some time to figure this out. I think this is something that should work like being able to 
run
a 
system_binary
target (which I believe you can do).
b
Oh hey, TIL
a
The indirection of 
bash
is always going to mean there’s some fuckery going on that’s impossible to inspect (this is why I prefer people to not use 
shell_command
etc)
The thing about 
system_binary
is that it has to be on your system
but you could probably make a target that looks very very similar, and makes a 
file
dependency executable
run_system_binary
is very easy to understand, and I’d welcome a pull request 🙂
adhoc_tool
exists entirely to make it easier to reason about how a process is getting executed
b
system_binary
has no 
dependencies
field 😭
(makes sense, but damn)
a
I don’t think you should extend 
system_binary
, but I think some concept of a 
file_as_binary
would make sense; in case you have an arbitrary binary in your codebase, or need to download it
b
I had that in a PR once 😅
a
there’s a subsystem now, where that’s a coherent idea 🙂
b
a
I started working on 
adhoc_tool
the following month 🙃
b
Ideally we bugfix 
2.16.x
though.
a
Sure. Just pointing out that everything that messes around with 
bash
is going to continue having bugs, and directing people away from indirection through bash is the way of the future 😉
do not want buggy software before that happens though 🙂
b
I'm all for: • Less intermediate targets (see my 3 tagrets) • Less bash
a
Once I’m out of meetings I should be able to come up with an 
exec
-based alternative that fixes your issue
unless you get there first
b
🙂
a
but the requirement is that for 
command="blah"
, 
blah
is executed inside a bash shell
b
Carry that thought over into the tests executed in PR, because otherwise there's no way to be sure we got it all 🙂
a
well sure
b
The escaping is horrific 😱
Oh, the answer is staring us in the face...
a
Nice. That’ll do.
b
Yeah, so obsessed with Pants running 
exec -a
I realized I should be running it 🤦‍♂️
Open in Slack
PreviousNext
Powered by