args attribute expansion handles shell expansion prematurely

[laszlocsomor]

Description of the problem / feature request:

Bazel does not pass empty test arguments from *_test.args to the test, but it does pass them from --test_arg.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

BUILD file:

cc_test(
    name = "x",
    srcs = ["x.cc"],
    args = [
        "foo",
        "",
        "bar",
    ],
)

x.cc file:

#include <stdio.h>

int main(int argc, char** argv) {
  printf("TEST: argc=%d\n", argc);
  for (int i = 0; i < argc; ++i) {
    printf("TEST: argv[%d]=(%s)\n", i, argv[i]);
  }
  return 1;  // make sure the test fails, so --test_output=errors prints the output
}

Repro:

  $ bazel test //:x -s --test_output=errors --test_arg="baz" --test_arg="" --test_arg="qux"
(...)
  external/bazel_tools/tools/test/test-setup.sh ./x foo bar baz '' qux)
FAIL: //:x (see (...)/testlogs/x/test.log)
INFO: From Testing //:x:
==================== Test output for //:x:
TEST: argc=6
TEST: argv[0]=((...)/x.runfiles/__main__/x)
TEST: argv[1]=(foo)
TEST: argv[2]=(bar)
TEST: argv[3]=(baz)
TEST: argv[4]=()
TEST: argv[5]=(qux)
(...)

As you see the command line does not contain the "" argument from the BUILD file, but does from the --test_arg flag.

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

  $ bazel info release
release 0.17.2

[jmmv]

Oh noes. I think the chain of events that leads to this is:

1. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java;l=439;drc=ca48e9ac4ddae455e816fb0bd3895f572d149d56
2. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/Expander.java;l=208;drc=af685cf89e1351227c4b734a6ae3b7a8f23578d7
3. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/Expander.java;l=216;drc=af685cf89e1351227c4b734a6ae3b7a8f23578d7
4. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/Expander.java;l=182;drc=af685cf89e1351227c4b734a6ae3b7a8f23578d7
5. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/Expander.java;l=128;drc=af685cf89e1351227c4b734a6ae3b7a8f23578d7
6. https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java;l=140;drc=af685cf89e1351227c4b734a6ae3b7a8f23578d7

which results in attempting to perform shell expansion on an empty argument, and because an empty string has no meaning as a lone shell argument, it's not added to the tokenized list. This is just plain wrong, but it goes so deep (and is in a generic part of the code) that I'm afraid fixing this will make many things blow up...

[jmmv]

Given #6275, which reports similar problems with spaces in arguments... it seems that this code is going further and attempting to perform even more shell expansions on the arguments. I think the args list shouldn't be subject to any kind of shell expansion/handling and passed to the test program directly...

[jxramos]

I'm facing this with a py_test target that passes some marker expressions down to the underlying pytest invocation, eg something that would go like pytest -m "markerA and not markerB" is being placed in the py_test target like

py_test(
    name = "test_foo",
    args = [ "-m", "markerA and not markerB"],
    ....
)

In the pytest scenario it's taking the tokens individually and yielding

============================ no tests ran in 0.00s =============================
ERROR: file not found: and

Update
looks like I could resolve it by enclosing in single quotes

    args = [ "-m", "'markerA and not markerB'"],

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[fmeum]

@bazelbuild/triage Not stale