native.sh_binary converts binary arguments starting with /

[filipesilva]

Description of the problem / feature request:

When using Bazel under Windows and using native.sh_binary with a script that invokes a binary, arguments starting with / will be converted to paths according to Posix_path_conversion.

This becomes a problem when constructing scripts that need to take user input for which this format might be relevant, such as specifying a serving path for a http server (bazelbuild/rules_typescript#327 (comment)).

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

Have a script that invokes a binary, providing it with an argument starting with /.

echo /something
node -e "console.log(process.argv)" /something

native.sh_binary(
    name = "script",
    srcs = ["script.sh"],
)

I used node as the binary because I had it in my system and this didn't seem to manifest with built-ins like echo.

Below is a repository with this code.

git clone https://github.com/filipesilva/bazel-msys-script
cd bazel-msys-script
bazel run :script

You should see

INFO: Analysed target //:script (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:script up-to-date:
  C:/users/kamik/_bazel_kamik/f23qfmhz/execroot/__main__/bazel-out/x64_windows-fastbuild/bin/script
  C:/users/kamik/_bazel_kamik/f23qfmhz/execroot/__main__/bazel-out/x64_windows-fastbuild/bin/script.exe
INFO: Elapsed time: 0.498s, Critical Path: 0.01s, Remote (0.00% of the time): [queue: 0.00%, setup: 0.00%, process: 0.00%]
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Running command line: C:/users/kamik/_bazel_kamik/f23qfmhz/execroot/__main__/bazel-out/x64_windows-fastbuild/bin/sINFO: Build completed successfully, 1 total action
echo /something
[ 'C:\\Program Files\\nodejs\\node.exe', 'C:/msys64/something' ]

What operating system are you running Bazel on?

Windows 10 1803.

What's the output of bazel info release?

0.19.2.

Have you found anything relevant by searching the web?

No.

Any other information, logs, or outputs that you want to share?

Path conversion behaviour in MSYS can be disabled using MSYS2_ARG_CONV_EXCL="*" (msys2/MSYS2-packages#84 (comment)).

Related to #3001, #5966.

[laszlocsomor]

Thanks for the report! Do you see any problem with MSYS2_ARG_CONV_EXCL="*"? Looks like that would solve the problem, if Bazel always set it in sh_binary.

[filipesilva]

@laszlocsomor prepending MSYS2_ARG_CONV_EXCL to the binary call (MSYS2_ARG_CONV_EXCL="*" node -e "console.log(process.argv)" /something) will disable the path conversion.

But that puts the onus on Bazel developers to be aware of an implementation detail (usage of MSYS on Windows platforms) inside of Bazel. I'm not sure how many are aware of this caveat.

Are there any cases where the path conversion is desirable? Perhaps the flag could be set by Bazel when invoking MSYS.

Off the top of my head:

• inside shell scripts proper one is always using win32
• any path sent in by the user running Bazel on a MSYS shell will reach Bazel already converted

[laszlocsomor]

Sorry for the long delay.

This is a difficult question. Inside the shell script, it should be fine to use Unix paths like /bin/sh or /dev/null. Especially when these scripts must run on multiple platforms. The path conversion is exactly what allows using scripts both on Linux and Windows.

Disabling path conversion means those paths will be passed to binaries unchanged. That's often not what you want. Demo: https://github.com/laszlocsomor/projects/tree/d7f909c11c45b3d508cb117a4fde13bcbb6a0f6c/bazel/msys-path-conv
https://github.com/laszlocsomor/projects/tree/c3caa558c7fe59388df734b9f59baac92dae86fe/bazel/msys-path-conv

Even inside of a script and even within one command, you may want to mix paths (/foo/bar) with path-looking strings (/server/path/index.html), and enable path conversion for one but disable for the other. The only way to make sure everything is right is to disable path conversion and explicitly convert arguments with cygpath.

Bazel cannot make a right choice here. The onus is indeed on the user. (Is that what you meant? You wrote "on Bazel developers".)

WDYT?

[laszlocsomor]

Here's an even better demo: https://github.com/laszlocsomor/projects/tree/c3caa558c7fe59388df734b9f59baac92dae86fe/bazel/msys-path-conv (added to previous comment)

[filipesilva]

Well there are three types of developers, so it is worth to clarify:

• t1: Bazel developers (e.g. developers on https://github.com/bazelbuild/bazel)
• t2: Bazel rules developers (e.g. developers on https://github.com/bazelbuild/rules_nodejs)
• t3: Developers/users of Bazel (e.g. developers that use https://github.com/alexeagle/angular-bazel-example)

When I said "But that puts the onus on Bazel developers to be aware of an implementation detail" I should have clarified. I meant the t2 developers, that have to understand that Bazel uses MSYS2 on windows and safeguard their code against path conversion where applicable.

If in the future MSYS2 is no longer used, t2 developers would to remove MSYS2 logic from their rules. I won't do anything if it stays there but it's essentially dead code.

If t2 developers properly safeguard their rules, t3 developers won't have to care about MSYS2. But if they don't, path conversion can give rise to odd bugs.

I think it's accurate that t1 developers cannot make the choice to blanket disable path conversion logic, given the myriad use cases.

Perhaps it is a matter of documentation for t2 developers on how to properly support Windows.

I'm not aware of a definite resource for this topic right now, but if I had to choose a few topics they would be:

• wrapping executable calls in native.sh_binary scripts
• using the Bash runfiles library
• creating runfiles helpers for language rules
• dynamic resolution using runfiles
• guarding against MSYS2 path conversion

[laszlocsomor]

Thanks for the clarification!

I see no way to relieve t2 developers from having to know about Bazel's use of MSYS2 and about MSYS2's quirks in particular. I agree documentation could help explaining this so t2 developers don't have to reinvent the wheel and learn about these quirks every time.

Where do you think such documentation should live so it's easily discoverable? (Where would you look for it?)

Thanks for the topics! That's a great set of Qs for a FAQ.

[filipesilva]

I would look for it in https://bazel.build/ -> Documentation -> Rules -> Developing your own rules (a new section.

I'd expect to find a small tutorial on how to make my own language rule and how to ensure windows compatibility, and some resources for further reading.

[laszlocsomor]

Thanks! I think #3889 is the most relevant bug.

[meteorcloudy]

This issue looks resolved by #3889