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.

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

Windows,Bash detection: do not detect Git Bash #5751

Closed
laszlocsomor opened this Issue Aug 3, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@laszlocsomor
Contributor

laszlocsomor commented Aug 3, 2018

Description of the problem / feature request:

When BAZEL_SH envvar is not defined and Bazel searches for a suitable bash.exe, it should not look for Git Bash nor should it recommend installing it as a Bash implementation.

Git Bash does not support the full set of bintools that MSYS2 Bash does, nor does Git Bash support Pacman to install those tools. Git Bash is therefore unsuitable as a general-purpose Bash for Windows.

Even worse, if you have both Git Bash and MSYS Bash installed, they may collide if you run one's binaries from the other (see repro below).

Bazel should only look for MSYS2 Bash and recommend only that.

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

  1. Have both Git Bash and MSYS2 Bash installed, have paths to them that have no space characters:

    C:\src\tmp>mklink /j C:\git "C:\Program Files\Git"
    Junction created for C:\git <<===>> C:\Program Files\Git
    
  2. In MSYS2 Bash install "zip" by running pacman -Syu zip.

  3. Set "BAZEL_SH" envvar to point to Git Bash, put Git then MSYS on the PATH:

    BAZEL_SH=C:\git\usr\bin\bash.exe
    PATH=C:\git\usr\bin;C:\msys64\usr\bin
    
  4. Build a genrule that runs zip.

    C:\src\tmp>type BUILD
    genrule(
        name = "foo",
        outs = ["foo.txt"],
        cmd = ("echo \"DEBUG: $$(cygpath -w $$(which bash))\" ; " +
               "echo \"DEBUG: $$(cygpath -w $$(which zip))\" ; " +
               "zip > $@")
    )
    
    C:\src\tmp>c:\src\bazel-releases\0.16.0\bazel.exe build --verbose_failures //:foo.txt
    INFO: Analysed target //:foo.txt (0 packages loaded).
    INFO: Found 1 target...
    SUBCOMMAND: # //:foo [action 'Executing genrule //:foo']
    cd C:/users/laszlocsomor/_bazel_laszlocsomor/zovul4l7/execroot/__main__
      SET PATH=C:\git\usr\bin;C:\git\bin;C:\git\usr\bin;C:\msys64\usr\bin
      C:/git/usr/bin/bash.exe -c source external/bazel_tools/tools/genrule/genrule-setup.sh; echo "DEBUG: $(cygpath -w $(which bash))" ; echo "DEBUG: $(cygpath -w $(which zip))" ; zip > bazel-out/x64_windows-fastbuild/genfiles/foo.txt
    ERROR: C:/src/tmp/BUILD:107:1: Executing genrule //:foo failed (Exit 127): bash.exe failed: error executing command
      cd C:/users/laszlocsomor/_bazel_laszlocsomor/zovul4l7/execroot/__main__
      SET PATH=C:\git\usr\bin;C:\git\bin;C:\git\usr\bin;C:\msys64\usr\bin
      C:/git/usr/bin/bash.exe -c source external/bazel_tools/tools/genrule/genrule-setup.sh; echo "DEBUG: $(cygpath -w $(which bash))" ; echo "DEBUG: $(cygpath -w $(which zip))" ; zip > bazel-out/x64_windows-fastbuild/genfiles/foo.txt
    DEBUG: C:\git\usr\bin\bash.exe
    DEBUG: C:\msys64\usr\bin\zip.exe
          1 [main] zip (6740) C:\msys64\usr\bin\zip.exe: *** fatal error - cygheap base mismatch detected - 0x180318410/0x1802FF410.
    This problem is probably due to using incompatible versions of the cygwin DLL.
    Search for cygwin1.dll using the Windows Start->Find/Search facility
    and delete all but the most recent version.  The most recent version *should*
    reside in x:\cygwin\bin, where 'x' is the drive on which you have
    installed the cygwin distribution.  Rebooting is also suggested if you
    are unable to find another cygwin DLL.
    Target //:foo.txt failed to build
    INFO: Elapsed time: 1.160s, Critical Path: 0.70s
    INFO: 0 processes.
    FAILED: Build did NOT complete successfully
    

What operating system are you running Bazel on?

Windows 10

What's the output of bazel info release?

C:\src\tmp>c:\src\bazel-releases\0.16.0\bazel.exe info release
release 0.16.0

@laszlocsomor laszlocsomor self-assigned this Aug 3, 2018

laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue Aug 3, 2018

Windows: do not look for Git Bash
Fixes bazelbuild#5751

RELNOTES[INC]: Windows: when BAZEL_SH envvar is not defined and Bazel searches for a suitable bash.exe, Bazel will no longer look for Git Bash and no longer recommend installing it as a Bash implementation. See issue bazelbuild#5751.

Change-Id: I7350b1dd5c0a3777525956da6d620174fc6935ee

@bazel-io bazel-io closed this in f2732eb Aug 8, 2018

iirina added a commit that referenced this issue Aug 9, 2018

Windows: do not look for Git Bash
Fixes #5751

RELNOTES[INC]: Windows: when BAZEL_SH envvar is not defined and Bazel searches for a suitable bash.exe, Bazel will no longer look for Git Bash and no longer recommend installing it as a Bash implementation. See issue #5751.

Change-Id: I7350b1dd5c0a3777525956da6d620174fc6935ee

Closes #5752.

Change-Id: I7350b1dd5c0a3777525956da6d620174fc6935ee
PiperOrigin-RevId: 207891008

bazel-io pushed a commit that referenced this issue Sep 14, 2018

Release 0.17.1 (2018-09-14)
Baseline: aa118ca

Cherry picks:
   + 0e04625:
     Update checker framework dataflow and javacutil versions
   + 3987300:
     Stop using --release in versioned java_toolchains
   + 438b277:
     make_deb: Add new empty line in the end of conffiles file
   + 5044017:
     Properly mark configuration files in the Debian package.
   + 9ed9d8a:
     Add flag
     --incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfil
     es_tree.
   + 22d761a:
     Update protobuf to 3.6.1 -- add new files
   + 27303d7:
     Update protobuf to 3.6.1 -- update references
   + ddc97ed:
     Update protobuf to 3.6.1 -- remove 3.6.0 sources
   + ead1002:
     Fix protobuf in the WORKSPACE
   + 12dcd35:
     Revert "Update to JDK 10 javac"
   + 7eb9ea1:
     Automated rollback of
     808ec9f
     a106bc5249cacc8c54 and
     4c9149d
     b697f5852bc5742a36 and some manual merging.
   + 4566a42:
     Fix tests on JDK 9 and 10
   + 1e9f0aa:
     Fix more tests on JDK 9 and 10
   + a572c1c:
     Add ubuntu1804_nojava, ubuntu1804_java9, ubuntu1804_java10 to
     postsubmit.
   + 29f1de0:
     Disable Android shell tests on the "nojava" platform.
   + b495eaf:
     Update bazel_toolchains to latest release.
   + 9323c57:
     Windows: fix writing java.log
   + 1aba9ac:
     Automated rollback of commit
     de22ab0.
   + 2579b79:
     Fix toolchain_java9 on --host_javabase=<jdk9> after
     7eb9ea1
   + 2834613:
     Include also ext jars in the bootclasspath jar.

Incompatible changes:

  - Loading @bazel_tools//tools/build_defs/repo:git_repositories.bzl
    no longer works. Load @bazel_tools//tools/build_defs/repo:git.bzl
    instead.
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - Labels in C++ rules' linkopts attribute are not expanded anymore
    unless they are wrapped, e.g: $(location //foo:bar)
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - New bazelrc file list.
  - Windows: when BAZEL_SH envvar is not defined and Bazel searches
    for a suitable bash.exe, Bazel will no longer look for Git Bash
    and no longer recommend installing it as a Bash implementation.
    See issue #5751.
  - New bazelrc file list.

New features:

  - The aquery command now supports --output=text.
  - Java, runfiles: the Java runfiles library is now in
    @bazel_tools//tools/java/runfiles. The old target
    (@bazel_tools//tools/runfiles:java-runfiles) is deprecated and
    will be removed in Bazel 0.18.0.
  - Java, runfiles: the Java runfiles library is now in
    @bazel_tools//tools/java/runfiles. The old target
    (@bazel_tools//tools/runfiles:java-runfiles) is deprecated and
    will be removed in Bazel 0.19.0 (not 0.18.0, as stated earlier).

Important changes:

  - Allow @ in package names.
  - Remove support for java_runtime_suite; use alias() together with
    select() instead.
  - Python wrapper scripts for MSVC are removed.
  - [JavaInfo] Outputs are merged in java_common.merge().
  - Faster analysis by improved parallelization.
  - --experimental_shortened_obj_file_path is removed.
  - Introduce the --remote_cache_proxy flag,
    which allows for remote http caching to connect
    via a unix domain socket.
  - No longer define G3_VERSION_INFO for c++ linkstamp compiles, as
    it was a duplicate of G3_TARGET_NAME.
  - Added support for Android NDK r17. The default STL is now
    `libc++`, and support for targeting `mips`, `mips64` and `ARMv5`
    (`armeabi`) has been removed.
  - Add aquery command to get analysis time information about the
    action graph.
  - Fixed compatibility with aar_import when using aapt2.  AAPT2 is
    now supported for Android app builds without resource shrinking.
    To use it, pass the `--android_aapt=aapt2` flag or define
    android_binary.aapt_version=aapt2.
  - Code coverage is collected for Java binaries invoked from sh_test.
  - java_common.compile creates the native headers jar accesible via
    JavaInfo.outputs.native_headers.
  - Deleting deprecated no-op flag --show_package_location
  - The JDK shipped with Bazel was updated to JDK10.
  - Rename the startup flag --host_javabase to --server_javabase to
    avoid confusion with the build flag --host_javabase
  - newly added options --experimental_repository_hash_file and
      --experimental_verify_repository_rules allow to verify for
    repositories
      the directory generated against pre-recorded hashes. See
    documentation
      for those options.
  - Removed the gen_jars output group
  - --subcommands can now take a "pretty_print" value
    ("--subcommands=pretty_print") to print the
    arguments of subcommands as a list for easier reading.
  - follow-up to
    1ac3597
    645e3142f3c44b9e8
  - A rule error is now thrown if a Skylark rule implementation
    function returns multiple providers of the same type.
  - When using Bazel's remote execution feature and Bazel has to
    fallback to local execution for an action, Bazel used
    non-sandboxed
    local execution until now. From this release on, you can use the
    new
    flag --remote_local_fallback_strategy=<strategy> to tell Bazel
    which
    strategy to use in that case.
  - Execution Log Parser can now, when printing it out, filter the
    log by runner type
  - A rule error is now thrown if a Skylark rule implementation
    function returns multiple providers of the same type.
  - Removed the gen_jars output group
  - Removed the gen_jars output group
  - Set --defer_param_files to default to true.
  - Sort attribute lists in proto-form query output to fix
    non-deterministic genquery output.
  - Replace 0/1 with False/True for testonly attribute
  - bazel now supports a .bazelignore file specifying
      directories to be ignored; however, these directories still
      have to be well founded and, in particular, may not contain
      symlink cycles.
  - Add more detailed reporting of the differences between startup
    options.
  - update data binding to 3.2.0
  - For Android incremental dexing actions, Bazel now persists its
    DexBuilder process across individual actions. From our
    benchmarks, this results in a 1.2x speedup for clean local builds.
  - The standard `xcode_VERSION` feature now always uses exactly two
    components in the version, even if you specify `--xcode_version`
    with
    more or fewer than two.
  - A rule error will be thrown if a Skylark rule implementation
    function returns multiple providers of the same type. Try the
    `--incompatible_disallow_conflicting_providers` flag to ensure
    your code is forward-compatible.
  - Removed notion of FULLY_STATIC linking mode from C++ rules.
  - In documentation, we've renamed Skylark into Starlark.
  - Execution Log Parser can now, when printing it out, reorder the
    actions for easier text diffs
  - Linkstamps are no longer recompiled after server restart.
  - Use VanillaJavaBuilder and disable header compilation in
    toolchain_hostjdk8. The default toolchain will soon drop
    compatibility with JDK 8. Using a JDK 8 host_javabase
    will only be supported when using 'VanillaJavaBuilder' (which
    does not support Error Prone,
    Strict Java Deps, or reduced classpaths) and with header
    compilation disabled.
  - In the future, Bazel will expand tree artifacts in runfiles, too,
    which causes the sandbox to link each file individually into the
    sandbox directory, instead of symlinking the entire directory. In
    this release, the behavior is not enabled by default yet. Please
    try it out via
    --incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfile
    s_tree and let us know if it causes issues. If everything looks
    good, this behavior will become the default in a following
    release.

laurentlb added a commit that referenced this issue Sep 19, 2018

Release 0.17.1 (2018-09-14)
Baseline: aa118ca

Cherry picks:
   + 0e04625:
     Update checker framework dataflow and javacutil versions
   + 3987300:
     Stop using --release in versioned java_toolchains
   + 438b277:
     make_deb: Add new empty line in the end of conffiles file
   + 5044017:
     Properly mark configuration files in the Debian package.
   + 9ed9d8a:
     Add flag
     --incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfil
     es_tree.
   + 22d761a:
     Update protobuf to 3.6.1 -- add new files
   + 27303d7:
     Update protobuf to 3.6.1 -- update references
   + ddc97ed:
     Update protobuf to 3.6.1 -- remove 3.6.0 sources
   + ead1002:
     Fix protobuf in the WORKSPACE
   + 12dcd35:
     Revert "Update to JDK 10 javac"
   + 7eb9ea1:
     Automated rollback of
     808ec9f
     a106bc5249cacc8c54 and
     4c9149d
     b697f5852bc5742a36 and some manual merging.
   + 4566a42:
     Fix tests on JDK 9 and 10
   + 1e9f0aa:
     Fix more tests on JDK 9 and 10
   + a572c1c:
     Add ubuntu1804_nojava, ubuntu1804_java9, ubuntu1804_java10 to
     postsubmit.
   + 29f1de0:
     Disable Android shell tests on the "nojava" platform.
   + b495eaf:
     Update bazel_toolchains to latest release.
   + 9323c57:
     Windows: fix writing java.log
   + 1aba9ac:
     Automated rollback of commit
     de22ab0.
   + 2579b79:
     Fix toolchain_java9 on --host_javabase=<jdk9> after
     7eb9ea1
   + 2834613:
     Include also ext jars in the bootclasspath jar.

Incompatible changes:

  - Loading @bazel_tools//tools/build_defs/repo:git_repositories.bzl
    no longer works. Load @bazel_tools//tools/build_defs/repo:git.bzl
    instead.
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - Labels in C++ rules' linkopts attribute are not expanded anymore
    unless they are wrapped, e.g: $(location //foo:bar)
  - If the same artifact is generated by two distinct but identical
    actions, and a downstream action has both those actions' outputs
    in its inputs, the artifact will now appear twice in the
    downstream action's inputs. If this causes problems in Skylark
    actions, you can use the uniquify=True argument in Args.add_args.
  - New bazelrc file list.
  - Windows: when BAZEL_SH envvar is not defined and Bazel searches
    for a suitable bash.exe, Bazel will no longer look for Git Bash
    and no longer recommend installing it as a Bash implementation.
    See issue #5751.
  - New bazelrc file list.

New features:

  - The aquery command now supports --output=text.
  - Java, runfiles: the Java runfiles library is now in
    @bazel_tools//tools/java/runfiles. The old target
    (@bazel_tools//tools/runfiles:java-runfiles) is deprecated and
    will be removed in Bazel 0.18.0.
  - Java, runfiles: the Java runfiles library is now in
    @bazel_tools//tools/java/runfiles. The old target
    (@bazel_tools//tools/runfiles:java-runfiles) is deprecated and
    will be removed in Bazel 0.19.0 (not 0.18.0, as stated earlier).

Important changes:

  - Allow @ in package names.
  - Remove support for java_runtime_suite; use alias() together with
    select() instead.
  - Python wrapper scripts for MSVC are removed.
  - [JavaInfo] Outputs are merged in java_common.merge().
  - Faster analysis by improved parallelization.
  - --experimental_shortened_obj_file_path is removed.
  - Introduce the --remote_cache_proxy flag,
    which allows for remote http caching to connect
    via a unix domain socket.
  - No longer define G3_VERSION_INFO for c++ linkstamp compiles, as
    it was a duplicate of G3_TARGET_NAME.
  - Added support for Android NDK r17. The default STL is now
    `libc++`, and support for targeting `mips`, `mips64` and `ARMv5`
    (`armeabi`) has been removed.
  - Add aquery command to get analysis time information about the
    action graph.
  - Fixed compatibility with aar_import when using aapt2.  AAPT2 is
    now supported for Android app builds without resource shrinking.
    To use it, pass the `--android_aapt=aapt2` flag or define
    android_binary.aapt_version=aapt2.
  - Code coverage is collected for Java binaries invoked from sh_test.
  - java_common.compile creates the native headers jar accesible via
    JavaInfo.outputs.native_headers.
  - Deleting deprecated no-op flag --show_package_location
  - The JDK shipped with Bazel was updated to JDK10.
  - Rename the startup flag --host_javabase to --server_javabase to
    avoid confusion with the build flag --host_javabase
  - newly added options --experimental_repository_hash_file and
      --experimental_verify_repository_rules allow to verify for
    repositories
      the directory generated against pre-recorded hashes. See
    documentation
      for those options.
  - Removed the gen_jars output group
  - --subcommands can now take a "pretty_print" value
    ("--subcommands=pretty_print") to print the
    arguments of subcommands as a list for easier reading.
  - follow-up to
    1ac3597
    645e3142f3c44b9e8
  - A rule error is now thrown if a Skylark rule implementation
    function returns multiple providers of the same type.
  - When using Bazel's remote execution feature and Bazel has to
    fallback to local execution for an action, Bazel used
    non-sandboxed
    local execution until now. From this release on, you can use the
    new
    flag --remote_local_fallback_strategy=<strategy> to tell Bazel
    which
    strategy to use in that case.
  - Execution Log Parser can now, when printing it out, filter the
    log by runner type
  - A rule error is now thrown if a Skylark rule implementation
    function returns multiple providers of the same type.
  - Removed the gen_jars output group
  - Removed the gen_jars output group
  - Set --defer_param_files to default to true.
  - Sort attribute lists in proto-form query output to fix
    non-deterministic genquery output.
  - Replace 0/1 with False/True for testonly attribute
  - bazel now supports a .bazelignore file specifying
      directories to be ignored; however, these directories still
      have to be well founded and, in particular, may not contain
      symlink cycles.
  - Add more detailed reporting of the differences between startup
    options.
  - update data binding to 3.2.0
  - For Android incremental dexing actions, Bazel now persists its
    DexBuilder process across individual actions. From our
    benchmarks, this results in a 1.2x speedup for clean local builds.
  - The standard `xcode_VERSION` feature now always uses exactly two
    components in the version, even if you specify `--xcode_version`
    with
    more or fewer than two.
  - A rule error will be thrown if a Skylark rule implementation
    function returns multiple providers of the same type. Try the
    `--incompatible_disallow_conflicting_providers` flag to ensure
    your code is forward-compatible.
  - Removed notion of FULLY_STATIC linking mode from C++ rules.
  - In documentation, we've renamed Skylark into Starlark.
  - Execution Log Parser can now, when printing it out, reorder the
    actions for easier text diffs
  - Linkstamps are no longer recompiled after server restart.
  - Use VanillaJavaBuilder and disable header compilation in
    toolchain_hostjdk8. The default toolchain will soon drop
    compatibility with JDK 8. Using a JDK 8 host_javabase
    will only be supported when using 'VanillaJavaBuilder' (which
    does not support Error Prone,
    Strict Java Deps, or reduced classpaths) and with header
    compilation disabled.
  - In the future, Bazel will expand tree artifacts in runfiles, too,
    which causes the sandbox to link each file individually into the
    sandbox directory, instead of symlinking the entire directory. In
    this release, the behavior is not enabled by default yet. Please
    try it out via
    --incompatible_symlinked_sandbox_expands_tree_artifacts_in_runfile
    s_tree and let us know if it causes issues. If everything looks
    good, this behavior will become the default in a following
    release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment