Use the new singlejar on Windows

[cushon]

The Java implementation of singlejar has been replaced by a faster c++ implementation everywhere except Bazel on Windows.

We should use the c++ implementation on Windows, and remove support for the Java singlejar:

• bazel/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java

  Line 269 in df03696

  if (singlejar.getFilename().endsWith(".jar")) {

• bazel/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java

  Line 65 in df03696

  if (singleJar.getFilename().endsWith(".jar")) {

[xingao267]

The cc implementation of singlejar actually hasn't been enabled yet. I'm enabling it except on Windows in https://bazel-review.googlesource.com/#/c/11530.

I tried to enable it on Windows in patch set 3 https://bazel-review.googlesource.com/#/c/11530/3. The CI failed on Windows, and here is the error message:

ERROR: C:/jenkins/workspace/gerrit-bazel-tests/bazel_version/latest/platform_name/windows-x86_64/src/tools/singlejar/BUILD:263:1: C++ compilation of rule '//src/tools/singlejar:input_jar' failed: msvc_cl.bat failed: error executing command
cd C:/bazel_ci/temp/_bazel_system/rvhmt5ue/execroot/windows-x86_64
SET INCLUDE=C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE;C:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um;C:\Program Files (x86)\Windows Kits\8.1\include\shared;C:\Program Files (x86)\Windows Kits\8.1\include\um;C:\Program Files (x86)\Windows Kits\8.1\include\winrt;C:/python_27_amd64/files/include
SET LIB=C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\LIB\amd64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\LIB\amd64;C:\Program Files (x86)\Windows Kits\10\lib\10.0.10240.0\ucrt\x64;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\lib\um\x64;C:\Program Files (x86)\Windows Kits\8.1\lib\winv6.3\um\x64;C:/python_27_amd64/files/libs
SET PATH=C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files (x86)\MSBuild\14.0\bin\amd64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64;C:\windows\Microsoft.NET\Framework64\v4.0.30319;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\VCPackages;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools;C:\Program Files (x86)\HTML Help Workshop;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Team Tools\Performance Tools\x64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Team Tools\Performance Tools;C:\Program Files (x86)\Windows Kits\8.1\bin\x64;C:\Program Files (x86)\Windows Kits\8.1\bin\x86;C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.6.1 Tools\x64;c:\tools\msys64\usr\bin;c:\windows\system32;c:\windows;c:\windows\system32\wbem;c:\windows\system32\windowspowershell\v1.0;c:\program files (x86)\google\cloud sdk\google-cloud-sdk\bin;c:\program files\google\compute engine\sysprep;c:\program files\google\compute engine\metadata_scripts;c:\program files (x86)\windows kits\8.1\windows performance toolkit;c:\programdata\chocolatey\bin;c:\programdata\chocolatey\bin;c:\python_27_amd64\files;c:\program files\java\jdk1.8.0_121\bin;C:\windows\system32
SET PWD=/proc/self/cwd
SET TMP=c:\windows\temp
SET TMPDIR=c:\bazel_ci\temp
external/local_config_cc/wrapper/bin/msvc_cl.bat /DOS_WINDOWS=OS_WINDOWS /DCOMPILER_MSVC /DNOGDI /DNOMINMAX /DPRAGMA_SUPPORTED /D_WIN32_WINNT=0x0600 /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS /D_SILENCE_STDEXT_HASH_DEPRECATION_WARNINGS /D_USE_MATH_DEFINES /bigobj /Zm500 /J /Gy /GF /W3 /EHsc /wd4351 /wd4291 /wd4250 /wd4996 -Xcompilation-mode=fastbuild -w /nologo /I. /Ibazel-out/msvc_x64-fastbuild/genfiles /Iexternal/bazel_tools /Ibazel-out/msvc_x64-fastbuild/genfiles/external/bazel_tools /Iexternal/bazel_tools/tools/cpp/gcc3 /showIncludes /MT /Od /Z7 /c src/tools/singlejar/input_jar.cc /Fobazel-out/msvc_x64-fastbuild/bin/src/tools/singlejar/_objs/input_jar/src/tools/singlejar/input_jar.o: com.google.devtools.build.lib.shell.BadExitStatusException: Process exited with status 2.
C:\bazel_ci\temp_bazel_system\rvhmt5ue\execroot\windows-x86_64\src/tools/singlejar/diag.h(29): fatal error C1189: #error: Unknown platformC:\bazel_ci\temp_bazel_system\rvhmt5ue\execroot\windows-x86_64\src/tools/singlejar/diag.h(29): fatal error C1189: #error: Unknown platform
Target //src:bazel failed to build

It complains that windows is an unknown platform at this line. https://github.com/bazelbuild/bazel/blob/master/src/tools/singlejar/diag.h#L29

[laszlocsomor]

For the record, 3 reasons why SingleJar doesn't work on Windows:

• bazel/src/tools/singlejar/mapped_file.h

  Line 18 in c3cf7d9

  #include <fcntl.h>

• bazel/src/tools/singlejar/mapped_file.h

  Line 21 in c3cf7d9

  #include <unistd.h>

• bazel/src/tools/singlejar/output_jar.cc

  Line 26 in c3cf7d9

  #include <unistd.h>

[ddwolf]

is there updates about this?

[laszlocsomor]

Hey, no updates.

@dslomov: we need to set a priority for this bug and put it on our OKRs. Without having looked at the actual state of the code and just speaking from experience, I guess the migration would take me one or two weeks.

@cushon: is there any deadline for Bazel-on-Windows to move away from the Java implementation?

[cushon]

@laszlocsomor from my perspective it would be nice to have a single implementation when we're doing maintenance or adding new features, but I don't know of a particular deadline.

[laszlocsomor]

I found out today that the Android rules in Bazel use a new feature of the C++ SingleJar that the Java SingleJar doesn't support. We'll need to reconsider the priority of this issue. /cc @dslomov

[aj-michael]

Regarding the Android feature, we could add that to the java singlejar (and in fact I believe there is an internal pending CL to do so). However, that feature is only enabled internally for now for some reasons dealing with remote execution.

So I don’t think that should impact the priority of this work

[xingao267]

Yea the feature is only enabled internally because it breaks remote execution. It won't be added to Bazel until recursive workspaces become supported.

[laszlocsomor]

Status update: I'm not actively working on this, even though it's blocking #4148. I'm currently focused on #4460, #4292, and #4319, which all seem to be more important. I'm waiting for replies to #4148 (comment) in determining whether to try and focus on this issue again.

[rongjiecomputer]

@laszlocsomor I currently have some patches locally that allows singlejar.exe to build, I cannot verify if it works at all yet since I don't know what it does, so I will be trying to port the tests too after I upstreamed some patches.

Porting the tests seems more challenging than porting singlejar itself. The tests are sprinkled with some bash commands inside C++ code (like running unzip). If the tests are to work on Windows, C++ code needs to know where bash is located to wrap those commands. I am not sure if borrowing code from //src/tools/launcher just to figure out where is bash is the right thing to do. Is there any other option?

Is singlejar only used for Android development?

[laszlocsomor]

@laszlocsomor I currently have some patches locally that allows singlejar.exe to build, I cannot verify if it works at all yet since I don't know what it does, so I will be trying to port the tests too after I upstreamed some patches.

Thanks!

Porting the tests seems more challenging than porting singlejar itself. The tests are sprinkled with some bash commands inside C++ code (like running unzip). If the tests are to work on Windows, C++ code needs to know where bash is located to wrap those commands. I am not sure if borrowing code from //src/tools/launcher just to figure out where is bash is the right thing to do. Is there any other option?

Yes, there's a better option:

• wrap unzip in a sh_binary:

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

  unzip.sh:

  unzip $*
  

• make that sh_binary a data-dependency of the C++ test

• use the C++ runfiles library to look up this sh_binary's path at runtime, run that instead of naked unzip (which just hopes "unzip.exe" is on the path)

Is singlejar only used for Android development?

Hm, I don't know. Why do you ask?

[rongjiecomputer]

Thanks for solution about C++ runfiles library!

Is singlejar only used for Android development?

Hm, I don't know. Why do you ask?

I was thinking about testing singlejar.exe directly. But then again, I don't normally code in Java, so this question does not matter anymore.

[laszlocsomor]

Thanks for solution about C++ runfiles library!

You're welcome! Keep in mind that this isn't a perfect solution, it just encapsulates the problem.
sh_binary is non-hermetic as it assumes unzip being on the PATH, but we are at least aware of it (#5265) and with this encapsulation you can make progress on porting the test.

[laszlocsomor]

@rongjiecomputer : thanks for you awesome contributions! What's the status of the porting? IIUC SingleJar itself is fully ported, and most of the tests too, right? Btw, I just found #6512, and just sent out #6513

[rongjiecomputer]

SingleJar itself is indeed fully ported. desugar_checking (depended by singlejar_local) can only be built with Bazel that includes 914b4ce (see #6292).

I will need to rebase #6251 and fix a few stuffs before saying that all singlejar tests pass (since my previous PR was changed quite a bit). Should I wait for #6513 to be imported first?

[laszlocsomor]

Thanks for the update! If you can wait for #6513, that won't hurt, though you can also do it without I guess. But it'll be merged in 1-2 hours anyway.

[rongjiecomputer]

Native SingleJar is enabled for Windows in master branch. For people who are currently being blocked by this issue, please build Bazel at HEAD and test things out!

[laszlocsomor]

This is fantastic news. Thank you for all your efforts @rongjiecomputer , I'm really pleased to see this getting resolved! By using the native SingleJar on Windows we can finally drop the old Java implementation and bring the Android rule support on Windows up to par with Linux and macOS.

Thank you so much!

[laszlocsomor]

I think c557399 resolves this issue.