Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

incompatible_windows_bashless_run_command: enables Bash-less "bazel run" on Windows #8240

Closed
laszlocsomor opened this issue May 6, 2019 · 10 comments
Closed

incompatible_windows_bashless_run_command: enables Bash-less "bazel run" on Windows #8240

laszlocsomor opened this issue May 6, 2019 · 10 comments
Assignees
@laszlocsomor
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) type: bug

Comments

@laszlocsomor
Copy link
Contributor

laszlocsomor commented May 6, 2019 •
edited
Loading

Description

Related bug: #8229

The option --incompatible_windows_bashless_run_command enables Bash-less "bazel run" on Windows. This flag has NO effect on other platforms.

When disabled, bazel run //:foo_bin runs the binary through Bash, and fails with --shell_executable="". Bazel will:

  • escape and join the arguments with Bash escaping semantics
  • run the binary through Bash with bash -c <joined_args>

When enabled, bazel run //:foo_bin does not require Bash and works with --shell_executable="". Bazel runs the binary directly (instead of through bash -c), and escapes the arguments Windows-style (the same as enabled by #7454). But Bash is still required when using --run_under=foo or --script_path=<path>, even when --incompatible_windows_bashless_run_command is enabled.

Migration recipe

None, as of 2019-05-06.

We don't expect any breakages when this flag is enabled. However if it breaks your build, please let us know so we can help fixing it and provide a migration recipe.

Rollout plan

  • Bazel 0.26.0 will not support this flag.
  • Bazel 0.27.0 is expected to support this flag, with default value being false.
  • Bazel 0.28.0 is expected to support this flag, with default value being true.
  • Bazel 0.29.0 is expected to not support this flag.
@laszlocsomor laszlocsomor self-assigned this May 6, 2019
@laszlocsomor laszlocsomor added bazel 1.0 incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) type: bug labels May 6, 2019
@laszlocsomor laszlocsomor mentioned this issue May 6, 2019
Closed
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 6, 2019
@laszlocsomor
Windows: add --incompatible_bashless_run_command
af4aa0d 
This flag enables Bash-less "bazel run" for simple
cases (no `--run_under`, no `--script_path`).

When enabled, "bazel run //foo:bin" no longer
requires Bash, and works with
`--shell_executable=""`.

Bash is still required with --run_under="foo" and
with --script_path=<path>.

Fixes bazelbuild#8229
See bazelbuild#8240
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 6, 2019
@laszlocsomor
Windows: add --incompatible_bashless_run_command
b769f31 
This flag enables Bash-less "bazel run" for simple
cases (no `--run_under`, no `--script_path`).

When enabled, "bazel run //foo:bin" no longer
requires Bash, and works with
`--shell_executable=""`.

Bash is still required with --run_under="foo" and
with --script_path=<path>.

Fixes bazelbuild#8229
See bazelbuild#8240

Change-Id: I36b9951674bf8651216178d6b34f28a496e724b9
@laszlocsomor laszlocsomor changed the title incompatible_bashless_run_command: enables Bash-less "bazel run" on Windows incompatible_windows_bashless_run_command: enables Bash-less "bazel run" on Windows May 8, 2019
bazel-io pushed a commit that referenced this issue May 8, 2019
@laszlocsomor @copybara-github
bazel run: no longer requires Bash for simple case
e8af81c 
Windows: add --incompatible_windows_bashless_run_command

This flag enables Bash-less "bazel run" for simple
cases (no `--run_under`, no `--script_path`).

When enabled, "bazel run //foo:bin" no longer
requires Bash, and works with
`--shell_executable=""`.

Bash is still required with `--run_under="foo"`
and with `--script_path=<path>`.

Fixes #8229
See #8240

Closes #8241.

PiperOrigin-RevId: 247183428
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 10, 2019
@laszlocsomor
Windows, bazel run: fix a bug in PR bazelbuild#8241
7b09310 
To "bazel run" a binary, the RunCommand creates an
ExecRequest and sends it to the Bazel client,
which then fulfills the request.

In PR bazelbuild#8241, RunCommand merged all command line
arguments and added that to the request as a
single argv entry. The client uses the first argv
element as the executable, but if all arguments
are joined into one string then argv0 contains the
whole argument vector as a single string.

Unfortunately the test didn't catch this because
it didn't attempt passing any arguments.

Fixes bazelbuild#8229
See bazelbuild#8240
@laszlocsomor laszlocsomor mentioned this issue May 10, 2019
Closed
bazel-io pushed a commit that referenced this issue May 10, 2019
@laszlocsomor @copybara-github
Windows, bazel run: don't join all args into argv0
cf7d02a 
To "bazel run" a binary, the RunCommand creates an
ExecRequest and sends it to the Bazel client,
which then runs the binary as the client's
subprocess.

In PR #8241, RunCommand merged all command line
arguments and added that to the request as a
single argv entry. The client uses the first argv
element as the executable, but if all arguments
are joined into one string then argv0 contains the
whole argument vector as a single string.

Unfortunately the test didn't catch this because
it didn't attempt passing any arguments.

Fixes #8229
See #8240

Closes #8287.

PiperOrigin-RevId: 247590904
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
@laszlocsomor @irengrig
Windows, bazel run: don't join all args into argv0
277c98d 
To "bazel run" a binary, the RunCommand creates an
ExecRequest and sends it to the Bazel client,
which then runs the binary as the client's
subprocess.

In PR bazelbuild#8241, RunCommand merged all command line
arguments and added that to the request as a
single argv entry. The client uses the first argv
element as the executable, but if all arguments
are joined into one string then argv0 contains the
whole argument vector as a single string.

Unfortunately the test didn't catch this because
it didn't attempt passing any arguments.

Fixes bazelbuild#8229
See bazelbuild#8240

Closes bazelbuild#8287.

PiperOrigin-RevId: 247590904
@keith
Copy link
Member

keith commented Jul 3, 2019

I believe the 0.27 tag should be removed here because this flag doesn't seem to exist in that version // @laurentlb

@laszlocsomor
Copy link
Contributor Author

@keith : I'm curious, why does it seem so? The flag exists in 0.27.0 -- search for incompatible_windows_bashless_run_command in src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java inside of https://github.com/bazelbuild/bazel/releases/download/0.27.1/bazel-0.27.1-dist.zip.

@siddharthab
Copy link
Contributor

This flag is only available with run and not with build, but bazelisk is not aware of the distinction between the different bazel commands, and will test all migration flags.

@laszlocsomor
Copy link
Contributor Author

Aah, thanks!

@keith
Copy link
Member

keith commented Jul 3, 2019 via email

@laszlocsomor
Copy link
Contributor Author

Current status: every Bazel CI downstream project except rules_cc pass with this flag flipped.
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1077#cd67234a-7298-4cb8-96d0-9f06c8ab412a. The other failing project (buildfarm) seems to be unrelated.

Using the same binary the CI used (https://buildkite.com/organizations/bazel/pipelines/bazel-at-head-plus-downstream/builds/1077/jobs/b8e6756c-fad0-4ef6-955c-6bca217137f7/artifacts/1d1a1a17-0818-43f8-818d-b5c7a39bac8b), testing rules_cc works on my machine.

@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@laszlocsomor
Copy link
Contributor Author

laszlocsomor commented Aug 6, 2019 •
edited
Loading

Blockers:

@laszlocsomor
Copy link
Contributor Author

Update: the test breakage is legitimate. It indicates an escaping bug in Bazel. I have a bugfix in https://github.com/laszlocsomor/bazel/tree/fix-GOOD-bashless-run-caller-escapes that I need to clean up before sending out.

This was referenced Aug 7, 2019
Closed
Closed
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Aug 8, 2019
@laszlocsomor
Windows: fix "bazel run" argument quoting
b2d34d6 
There are two axes of variables:
- server mode vs. batch mode (--[no]batch)
- Windows only: Bash-less vs. Bash-ful bazel run
  (--[no]incompatible_windows_bashless_run_command)

To "bazel run" a target, Bazel first builds the
target then creates a run request. The request is
a protobuf that contains the command line (argv)
and environment (envvars, cwd).

In server mode (--nobatch), the Bazel server sends
the run request to the client, and the client
executes the program (with CreateProcessW on
Windows / execv on Unixes).  In batch mode
(--batch), the Bazel server itself executes the
program using ProcessBuilder.

In Bash-ful bazel run mode the run request is for
"bash -c <program> <args>...", while in Bash-less
more it is for "<program> <args>...". The argument
escaping must be different in both cases, because
bash.exe uses Bash-style quoting/escaping while
most native Windows programs use the MSVC style.

Fixes the issue mentioned in bazelbuild#8240 (comment)
@laszlocsomor laszlocsomor mentioned this issue Aug 8, 2019
Closed
@drigz
Copy link
Contributor

drigz commented Sep 23, 2019

Does windows.md need updating? It says:

With Bazel 0.25.0 you still need Bash (MSYS2) to bazel run //foo:bin anything. [...] Follow issue #8240 for updates.

@laszlocsomor
Copy link
Contributor Author

Only after Bazel 1.0.0 came out -- it will flip --incompatible_windows_bashless_run_command to true by default.

bazel-io pushed a commit that referenced this issue Jan 15, 2020
@laszlocsomor @copybara-github
incompatible_windows_bashless_run_command: remove
be0e36f 
Remove this flag and every occurrence of it.
It was flipped in Bazel 1.0, so nobody should be
using it in production.

Fixes #8240

RELNOTES[INC]: The --[no]incompatible_windows_bashless_run_command flag is no longer supported. It was flipped in Bazel 1.0

PiperOrigin-RevId: 289835848
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
Flip --incompatible_windows_bashless_run_command
39d8a70 
    Fixes bazelbuild/bazel#8240

    RELNOTES[INC]: Windows: --incompatible_windows_bashless_run_command is now true by default, meaning "bazel run //foo:bin" will run the binary as a subprocess of the Bazel client. (When the flag is false, the binary is executed as a subprocess of Bash.)

    PiperOrigin-RevId: 266097847
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laszlocsomor laszlocsomor

Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@keith @katre @drigz @siddharthab @dslomov @laszlocsomor