Bazel: 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:

• https://github.com/bazelbuild/bazel/blob/df036962c94eeac0ccb6d1359d6f9681195b45ef/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java#L269
• https://github.com/bazelbuild/bazel/blob/df036962c94eeac0ccb6d1359d6f9681195b45ef/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java#L65

[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:

• https://github.com/bazelbuild/bazel/blob/c3cf7d917afd02d71de3800cd46ad8d14f1ddf55/src/tools/singlejar/mapped_file.h#L18
• https://github.com/bazelbuild/bazel/blob/c3cf7d917afd02d71de3800cd46ad8d14f1ddf55/src/tools/singlejar/mapped_file.h#L21
• https://github.com/bazelbuild/bazel/blob/c3cf7d917afd02d71de3800cd46ad8d14f1ddf55/src/tools/singlejar/output_jar.cc#L26

[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 https://github.com/bazelbuild/bazel/issues/4148#issuecomment-389442245 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 (https://github.com/bazelbuild/bazel/issues/5265) and with this encapsulation you can make progress on porting the test.

[hsyed]

Trying to add windows support to the Kotlin rules ... An executable is not being selected below, instead the deploy jar is primed. What do I need to do ?

single jar is configured as follows:

    "_singlejar": attr.label(
        executable = True,
        cfg = "host",
        default = Label("@bazel_tools//tools/jdk:singlejar"),
        allow_files = True,
    ),

It's used in a rule like this :

def _maybe_make_srcsjar_action(ctx):
    if len(ctx.files.srcs) > 0:
        output_srcjar = ctx.actions.declare_file(ctx.label.name + "-sources.jar")
        args = ["--output", output_srcjar.path]
        for i in ctx.files.srcs:
            args += ["--resources", i.path]

        ctx.action(
            mnemonic = "KotlinPackageSources",
            inputs = ctx.files.srcs,
            outputs = [output_srcjar],
            executable = ctx.executable._singlejar,
            arguments = args,
            progress_message="Creating Kotlin srcjar from %d srcs" % len(ctx.files.srcs),
        )
        return [output_srcjar]
    else:
        return []

I get

ERROR: C:/users/user/rules_kotlin/tests/integrationtests/jvm/basic/BUILD:47:1: Merging Kotlin output jar tests/integrationtests/jvm/basic/test_merge_resourcesjar.jar failed (Exit -1). Note: Remote connection/protocol failed with: execution failed: bazel-singlejar_deploy.jar failed: error executing command
  cd C:/users/user/_bazel_user/r6f3sd7x/execroot/io_bazel_rules_kotlin
external/bazel_tools/tools/jdk/singlejar/bazel-singlejar_deploy.jar --output bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/test_merge_resourcesjar.jar --sources bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/test_merge_resourcesjar-ktclass.jar --sources bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/libresourcejar.jar
Action failed to execute: java.io.IOException: ERROR: src/main/native/windows/processes-jni.cc(239): CreateProcessW("C:\users\user\_bazel_user\r6f3sd7x\execroot\io_bazel_rules_kotlin\external\bazel_tools\tools\jdk\singlejar\bazel-singlejar_deploy.jar" --output bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/test_merge_resourcesjar.jar --sources bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/test_merge_resourcesjar-ktclass.jar --sources bazel-out/x64_windows-fastbuild/bin/tests/integrationtests/jvm/basic/libresourcejar.jar): %1 is not a valid Win32 application.

[laszlocsomor]

Looking at @bazel_tools//tools/jdk:singlejar on Windows reveals that the filegroup selects a deploy jar:

  > bazel query @bazel_tools//tools/jdk:singlejar --output=build
# C:/_bazel/o2bbs3hu/external/bazel_tools/tools/jdk/BUILD:112:1
filegroup(
  name = "singlejar",
  srcs = select({"@bazel_tools//src/conditions:remote": ["@bazel_tools//src/tools/singlejar:singlejar"], "//conditions:default": ["@bazel_tools//tools/jdk:singlejar/bazel-singlejar_deploy.jar"]}),
)

On Linux the target is an executable file:

  $ bazel query @bazel_tools//tools/jdk:singlejar --output=build | buildifier
# /usr/local/google/home/laszlocsomor/.cache/bazel/_bazel_laszlocsomor/51c9aa8f783c7ef42fa88749bcc55eec/external/bazel_tools/tools/jdk/BUILD:112:1
filegroup(
    name = "singlejar",
    srcs = select({
        "@bazel_tools//src/conditions:remote": ["@bazel_tools//src/tools/singlejar:singlejar"],
        "//conditions:default": ["@bazel_tools//tools/jdk:singlejar/singlejar"],
    }),
)

  $ bazel query @bazel_tools//tools/jdk:singlejar/singlejar --output=label_kind
source file @bazel_tools//tools/jdk:singlejar/singlejar

  $ file $(bazel info output_base)/external/bazel_tools/tools/jdk/singlejar/singlejar
/(...)/external/bazel_tools/tools/jdk/singlejar/singlejar: ELF 64-bit LSB executable, x86-64 (...)

The explanation is in //tools/jdk:BUILD -- on Windows, Bazel uses the Java implementation of SingleJar (because of https://github.com/bazelbuild/bazel/issues/2241), while on other platforms it uses the native implementation.

What you could until https://github.com/bazelbuild/bazel/issues/2241 is fixed is:

1. create a java_import wrapping @bazel_tools//tools/jdk:singlejar
2. create a java_binary depending on this java_import and specifying the correct main_class (look at singlejar's BUILD file)
3. create a filegroup that selects on the current platform (@bazel_tools//src/conditions:windows vs. //conditions:default) and uses the java_binary from the previous step or singlejar in @bazel_tools
4. now you have a platform-independent filegroup that should behave as you expected of @bazel_tools//tools/jdk:singlejar

[laszlocsomor]

@rongjiecomputer : you worked a lot on this issue -- how far did you get?
@jasharpe : FYI

[rongjiecomputer]

@laszlocsomor Sorry for late reply.

There is just one more patch (*) to produce a usable singlejar.exe (can handle some test input files and produce correct output, but fails for > 4GB files).

I have also ported some of the C++ tests using Bazel C++ Runfiles library, but it is pretty messy. I can push them to a branch for you to take a look.

(*) My new semester have just started, so my schedule is a bit chaotic right now. I will try to make a PR for that as soon as I have some free time. Do you plan to raise the priority of this issue?

[laszlocsomor]

@rongjiecomputer : thanks for the update! No worries, and no rush. I'll also be on vacation next week. What is missing for large file support, how hard is it to fix? Yes, please show me the branch where you use the C++ runfiles library.

[rongjiecomputer]

What is missing for large file support, how hard is it to fix?

I am not sure if it is a Windows-specific bug. It looks like it is due to zlib not being able to handle large file directly and singlejar isn't prepared to workaround that.

The large file tests that fail happen to be the same tests that are marked manual (https://github.com/bazelbuild/bazel/commit/57a965b748975c79c744cd2c983802cad037e7d0). I wonder if CI tests include these tests?

[laszlocsomor]

Thanks for the explanation! I doubt CI runs manual tests. Do you think the Linux version can handle large files? The test may be marked manual because it's failing, or because it's slow. (We can't ask; the commit author no longer works on Bazel.)

[rongjiecomputer]

Do you think the Linux version can handle large files?

The simplest way to find out is to run the test //src/tools/singlejar:transient_bytes_test on Linux x64.

I am expecting the test to fail with timeout and have the following line in test log: ERROR: ./src/tools/singlejar/transient_bytes.h:137: Internal error inflating huge: inflater wrote 1 bytes , but the uncompressed entry should be 4294967297bytes long.

Note: 4294967297 B = 4GB+1B.

[laszlocsomor]

  $ uname
Linux

  $ git rev-parse HEAD
a1f1761a3e765acdd609e4867511b16ea9d9d093

  $ bazel test //src/tools/singlejar:transient_bytes_test
INFO: Build options have changed, discarding analysis cache.
INFO: Analysed target //src/tools/singlejar:transient_bytes_test (11 packages loaded).
INFO: Found 1 test target...
Target //src/tools/singlejar:transient_bytes_test up-to-date:
  bazel-bin/src/tools/singlejar/transient_bytes_test
INFO: Elapsed time: 136.283s, Critical Path: 135.45s
INFO: 2 processes: 2 linux-sandbox.
INFO: Build completed successfully, 3 total actions
//src/tools/singlejar:transient_bytes_test                               PASSED in 135.3s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 3 total actions

[laszlocsomor]

Ah, of course -- the CI machines 2 years ago probably didn't have as much RAM as my workstation does today, so a test allocating 4GB was probably too much to bear so it was marked manual.

[rongjiecomputer]

Oh, I forgot the obvious thing: my machine only has 4GB RAM (and less than that for the test itself, since OS, Bazel and other stuffs also consume RAM).

I will push my local change to a branch after my next PR so that you can test on a beefy Windows workstation.

[laszlocsomor]

@rongjiecomputer : Thank you for your continued efforts on closing this issue! Could you summarize the current state of Singlejar on Windows?

[laszlocsomor]

/cc @lberki : FYI

[meteorcloudy]

@rongjiecomputer Thanks again for working on singlejar for Windows! Looks like there are a few issues are related to it. (#4148 #6211 #6187) Is there anything we can do to help make progress on this?

[rongjiecomputer]

@meteorcloudy I am going to split #6031 into smaller PRs and send them out for review, then list down which tests pass and which tests fail so that we can try to coordinate who wants to work on which.

[meteorcloudy]

@rongjiecomputer Sounds good to me! Just ping us when you are ready. Thank you!

[rongjiecomputer]

A bit of a status update:

After #6248, #6251 and #6304 are merged, these tests will pass:

//src/tools/singlejar:input_jar_empty_jar_test                           PASSED in 2.3s
//src/tools/singlejar:input_jar_preambled_test                           PASSED in 579.9s
//src/tools/singlejar:input_jar_scan_jartool_test                        PASSED in 853.3s
//src/tools/singlejar:input_jar_scan_ziptool_test                        PASSED in 767.7s
//src/tools/singlejar:input_jar_bad_jar_test                             PASSED in 1.0s
//src/tools/singlejar:options_test                                       PASSED in 0.9s
//src/tools/singlejar:output_jar_bash_test                               PASSED in 7.8s
//src/tools/singlejar:output_jar_simple_test                             PASSED in 6.8s
//src/tools/singlejar:token_stream_test                                  PASSED in 4.6s
//src/tools/singlejar:transient_bytes_test                               PASSED in 750.9s
//src/tools/singlejar:zip_headers_test                                   PASSED in 0.8s
//src/tools/singlejar:zlib_interface_test                                PASSED in 0.7s
//src/tools/singlejar:zip64_test                                         PASSED in 109.8s

If you have > 4.65GB of RAM, this test will also pass:

//src/tools/singlejar:combiners_test                                     PASSED in 36.1s

EDIT: 2018/10/9

With https://github.com/bazelbuild/bazel/commit/914b4ce14624171a97ff8b41f9202058f10d15b2, //src/tools/singlejar:desugar_checking_test also passes too. This also means that all public singlejar tests are running on Windows!

[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 https://github.com/bazelbuild/bazel/issues/6512, and just sent out https://github.com/bazelbuild/bazel/pull/6513

[rongjiecomputer]

SingleJar itself is indeed fully ported. desugar_checking (depended by singlejar_local) can only be built with Bazel that includes https://github.com/bazelbuild/bazel/commit/914b4ce14624171a97ff8b41f9202058f10d15b2 (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 https://github.com/bazelbuild/bazel/commit/c557399cf6499918d6e4784b69e6bfa2dedd189f resolves this issue.