Windows, Android: make the Hello World example build on Windows #3264

laszlocsomor · 2017-06-26T06:38:38Z

Goal:

bazel build //examples/android/java/bazel:hello_world

should work on Windows.

The text was updated successfully, but these errors were encountered:

The android_sdk_repository now uses the native Android build tools (e.g. aapt.exe, aidl.exe) when --host_cpu is one of x64_windows{,_msys,_msvc}. On Linux/MacOS we create a wrapper script for these tools and data-depend on SDK/build-tools/VERSION/lib/*, but on Windows we have no runfiles support and Bazel just creates a junction to SDK/build-tools, so all the actual tools are there where the binaries expect them. Also change the ":fail" target to select srcs on the host platform and use a .cmd file on Windows. See #3264 Change-Id: Ica586fa1fd4914f4795b4387b449f1c5562164e0 PiperOrigin-RevId: 160383278

Introduce host and target path separation in the Android incremental_install.py. This will allow running this script (and use bazel mobile-install) on platforms with non-POSIX path semantics (e.g. Windows). See #3264 Change-Id: If6ec09f100dd2e0be3389dce25cb1a13305226e9 PiperOrigin-RevId: 160531950

Move the Java JNI sources to a separate package: c.g.devtools.build.lib.windows.jni and c.g.devtools.build.lib.windows.runfiles. Make the native method declarations private, create public wrapper methods for them that ensure that the JNI library is loaded. Split the C++ JNI source processes.cc into two parts (processes-jni.cc and file-jni.cc), extract common functionality to jni-util.{h,cc}. This change preparse the code for Android rule support on Windows, specifically it lets the Android BusyBox use the file JNI library so it can create junctions on Windows to work around long path issues when calling external tools. See #3264 Change-Id: I7f1a746d73f822ae419d11b893a91f4eb45d64da PiperOrigin-RevId: 160643355

ScopedTemporaryDirectory now makes all files writable before attempting to delete them. This is important on Windows where readonly files cannot be deleted, the attempt resulting in an exception. See #3264 Change-Id: If79478a4b419c05d77ce89cc30cb701d42df1b75 PiperOrigin-RevId: 160644599

And so does its test. See #3264 Change-Id: I32dd906b94747d601254ab55e7962bf6cda058f4 PiperOrigin-RevId: 160953063

Prepare tests for an upcoming change where some Android actions will use a parameter file, because they expect command line flags with lists of files and the list separators don't work on Windows. See #3264 RELNOTES: none PiperOrigin-RevId: 161064058

Now all tests under //src/test/j/c/g/devtools/build/android/...:* pass on Windows, yay! Also adjust test sizes as advised by Bazel (using --test_verbose_timeout_warnings). See #3264 Change-Id: I3f1f4978306bdedaf805149295daa413d2248fbb PiperOrigin-RevId: 161373699

Introduce the JunctionCreator classes that the Android BusyBox can use to work around path length limitations on Windows. See #3264 Change-Id: Ia5ee39f0635dcc2690ffb1755dc56d21e7bc7536 PiperOrigin-RevId: 161378422

Use the recently added JunctionCreator class in the Android BusyBox. Query the current OS's name, where the BusyBox process runs (using System.getProperty("os.name")) and instantiate the corresponding JunctionCreator implementation. This allows the BusyBox to work around path length limitations on Windows; see WindowsJunctionCreator. See #3264 Change-Id: Ifc1a3c86971e64c2f42bcec2988b7e9239a1d29a PiperOrigin-RevId: 161494557

Make sure that ZipEntry paths always use forward slashes, even on Windows. Also add a test. See #3264 Change-Id: I4508e46dde49cd44c8e3792017d0d280a51dc565 PiperOrigin-RevId: 161500049

Apparently `rlocation` is not defined in the environment of regular sh_binary rules, only in tests. So this commit adds an implementation for it. See #3264 Change-Id: I70ecb91cef5e1b66679e6ba04823831183da5cd9 PiperOrigin-RevId: 161775438

This commit: - deprecates PathListConverter - removes ExistingPathListConverter because it was not used in production, only tests - deprecated List<Path> type flags that use PathListConverter - introduces new List<Path> type flags next to the deprecated ones that use @Options.allowMultiple and convert with PathConverter; the new and old lists are concatenated, yielding the flag value PathListConverter and all of its occurrences should be removed after 2018-01-31 (about 6 months from now, which is a safe enough timeframe for everyone to upgrade Bazel so it uses the new-style flags). Reason for deprecation is that colon-separated path lists don't work on Windows because paths have colons in them. Since the Android BusyBox is not intended to be executed by users but by Bazel only, there's no release notes necessary. See #3264 RELNOTES: none PiperOrigin-RevId: 162193998

Otherwise these envvars are not available to the Java program and it can't load its runfiles. I tested that this is necessary both on Linux and Windows. Related to #3264 Change-Id: I2bd8eee0793b26aeedeafc6900f7854c816b5b14 PiperOrigin-RevId: 163688341

Add a data-dependency on the windows_jni.dll from the BusyBox in BUILD.tools, so the BusyBox in @build_tools// can actually find it at runtime. Also update the script that builds the .dll so that it works if the source files have an "external/bazel_tools/" prefix. Related to #3264 Change-Id: I005e9d2c00253a59d2cd5cc9f3a93528dc4d2e9e PiperOrigin-RevId: 163691320

@ulfjack

SpawnActions that run the Android BusyBox now use the default shell environment. This has the following benefits: - Bazel propagates the PATH, TMPDIR envvars to the action - Bazel propagates the --action_env envvars to the action This allows the Bazel client to pass --action_env=TMP or --action_env=TEMP (whichever of the envvars is defined) to the server, so the BusyBox actions will have TMP/TEMP set (to the same value as the clientenv), so they can create temp directories using java.nio.file.Files.createTempDirectory. This method seems to be calling the GetTempPath WinAPI function, which needs the TMP or TEMP envvar, otherwise it falls back to returning c:\windows which is non-writable. There's one drawback of using the default shell environment, although @ulfjack is working on it: - PATH is now also part of the action's cache key. However in a single-machine environment (no remote execution) and assuming PATH isn't likely to change between builds, this probably doesn't poision the action cache in practice. This change is a short-term solution. Propagating the client env's TMP/TEMP means we make that part of the action's cache key. The ideal long-term solution will be to not propagate this envvar, and instead let the execution strategy set it to some client-env-independent value. See #3264 Change-Id: I756a4203b5d86c881bc36cc089e35cde0d419914 PiperOrigin-RevId: 164114502

Deprecate the --libraries flag of the GENERATE_BINARY_R tool in favour of --library. The new flag is multi-value and uses "," as the pair-separator instead of ":". The value converter still supports ":"-separated pairs as well, but looks for "," first. Old format: --libraries=key1:value1,key2:value2,... New format: --library=key1,value1 --library=key2,value2 Motivation: - the ":"-separator prevents using absolute paths on Windows The old flag is still supported, but will be removed after 2018-02-28 (about 6 months from now). Also in this commit: - add a new method to CustomCommandLine.Builder to lazily construct the command line for the --library flag See #3264 RELNOTES: none PiperOrigin-RevId: 164246506

The Adb command needs the SYSTEMROOT environment variable (as does Bazel), otherwise it produces weird errors. So add that to Adb's environment. See #3264 Change-Id: Ia393f32ef00e21c90e4fc6d4a3188b7987aa89b0 PiperOrigin-RevId: 164454924

On Windows, Bazel will always use params files for some BusyBox tools, because some flags of these tools expect values with special characters in them. We need this change so Bazel can safely pass such flags to the BusyBox on Windows. See #3264 RELNOTES: none PiperOrigin-RevId: 164582899

@ulfjack

*** Reason for rollback *** Updated to avoid #3501 This is a rollback of a rollback, with additional modifications to BazelConfiguration.java to fix #3501, the issue that was the reason we rolled back the original change. The additional updates serve to propagate the client's TMP and TEMP envvars to the action, which is a short-term solution to allow actions have a TMP/TEMP envvar on Windows. They need at least one of those to create temp directories. The long-term solution is to set a value for TMP or TEMP in the executor just before executing the actions, so the TMP/TEMP would not be part of the action key. All of this only affects Bazel on Windows. *** Original change description *** Automated rollback of commit 0abf5fa. *** Reason for rollback *** Breaks Bazel CI (#3501) *** Original change description *** Android BusyBox: actions use the default shell env SpawnActions that run the Android BusyBox now use the default shell environment. This has the following benefits: - Bazel propagates the PATH, TMPDIR envvars to the action - Bazel propagates the --action_env envvars to the action This allows the Bazel client to pass --action_env=TMP or --action_env=TEMP (whichever of the envvars is defined) to the server, so the BusyBox actions will have TMP/TEMP set (to the same value as the clientenv), so they can create temp directories using java.nio.file.Files.createTempDirectory. This method seems to be calling the GetTempPath WinAPI function, which needs the TMP or TEMP envvar, otherwise it falls back to returning c:\windows which is non-writable. There's one drawback of using the default shell environment, although @ulfjack is working on it: - PATH is now also part of the action's cache key. However in a single-machine environment (no remote execution) and assuming PATH isn't likely to change between builds, this probably doesn't poision the action cache in practice. This change is a short-term solution. Propagating the client env's TMP/TEMP means we make that part of the action's cache key. The ideal long-term solution will be to not propagate this envvar, and instead let the execution strategy set it to some client-env-independent value. See #3264 Change-Id: I756a4203b5d86c881bc36cc089e35cde0d419914 RELNOTES: none PiperOrigin-RevId: 164696600

One of the output files in the incremental apk (stub_application_data.txt) was using CRLF on Windows, so the mobile-install'ed app was crashing on startup. Fix is to open the output file in binary mode so line endings are not converted to the host-platform-native one. See #3264 Change-Id: Id7d4b5aa4362a21e699517b97dd24858c396eaa7 PiperOrigin-RevId: 164722314

Always open files in binary mode to avoid automatic conversion between LF and CRLF on Windows, which is particularly problematic when a file is written on Windows but consumed on Android or when a binary file is opened for reading in text mode and if it happens to have an LF byte it would be converted to CRLF. See #3264 Change-Id: I4d9d885a488b9693eeb3f6d929e3396ef8406d62 PiperOrigin-RevId: 164826587

laszlocsomor · 2017-08-10T12:38:12Z

This works at head:

C:\work\bazel>git rev-parse HEAD
fad339f333f48bf7b640606c518bb6937b8f351c

C:\work\bazel>c:\work\bazel-0.5.3\bazel.exe --output_user_root=c:\tmp1 build src:bazel.exe
(...)
Target //src:bazel.exe up-to-date:
  C:/tmp1/o6hbs7n0/execroot/io_bazel/bazel-out/msvc_x64-fastbuild/bin/src/bazel.exe
INFO: Elapsed time: 229.167s, Critical Path: 121.12s

C:\work\bazel>C:\tmp1\o6Hbs7N0\execroot\io_bazel\bazel-out\msvc_x64-fastbuild\bin\src\bazel.exe --output_user_root=c:\tmp2 mobile-install //examples/android/java/bazel:hello_world --python_path=c:\python27\python.exe
(...)
Target //examples/android/java/bazel:hello_world up-to-date:
  C:/tmp2/o6hbs7n0/execroot/io_bazel/bazel-out/msvc_x64-fastbuild/bin/examples/android/java/bazel/hello_world_files/full_deploy_marker
  C:/tmp2/o6hbs7n0/execroot/io_bazel/bazel-out/msvc_x64-fastbuild/bin/examples/android/java/bazel/hello_world_files/deploy_info_incremental.deployinfo.pb
INFO: Elapsed time: 52.827s, Critical Path: 12.99s

App is correctly deployed on my device and I can start it. \o/

aj-michael · 2017-08-10T14:34:11Z

This was fantastic work Laszlo! Thanks so much!

dslomov · 2017-08-10T14:35:54Z

woohoo! \o/

…

On Aug 10, 2017 4:34 PM, "Adam Michael" ***@***.***> wrote: This was fantastic work Laszlo! Thanks so much! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3264 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AExrX4XbcVFplbwcybbShtosyhL0f4MXks5sWxTvgaJpZM4OE7Ej> .

laszlocsomor · 2017-08-10T14:44:22Z

Thanks! :) Really looking forward to adoption.

This commit: - deprecates PathListConverter - removes ExistingPathListConverter because it was not used in production, only tests - deprecated List<Path> type flags that use PathListConverter - introduces new List<Path> type flags next to the deprecated ones that use @Options.allowMultiple and convert with PathConverter; the new and old lists are concatenated, yielding the flag value PathListConverter and all of its occurrences should be removed after 2018-01-31 (about 6 months from now, which is a safe enough timeframe for everyone to upgrade Bazel so it uses the new-style flags). Reason for deprecation is that colon-separated path lists don't work on Windows because paths have colons in them. Since the Android BusyBox is not intended to be executed by users but by Bazel only, there's no release notes necessary. See bazelbuild/bazel#3264 RELNOTES: none PiperOrigin-RevId: 162193998 GitOrigin-RevId: 5752463ece84ebb4fb074888cba57412ab8d86b3 Change-Id: I10dd387d28c5462c27f63e12d3a3a87a202270ff

Deprecate the --libraries flag of the GENERATE_BINARY_R tool in favour of --library. The new flag is multi-value and uses "," as the pair-separator instead of ":". The value converter still supports ":"-separated pairs as well, but looks for "," first. Old format: --libraries=key1:value1,key2:value2,... New format: --library=key1,value1 --library=key2,value2 Motivation: - the ":"-separator prevents using absolute paths on Windows The old flag is still supported, but will be removed after 2018-02-28 (about 6 months from now). Also in this commit: - add a new method to CustomCommandLine.Builder to lazily construct the command line for the --library flag See bazelbuild/bazel#3264 RELNOTES: none PiperOrigin-RevId: 164246506 GitOrigin-RevId: 025a7b0a33680c53d872d241fdb49f3ab578afd6 Change-Id: Ic070877f13ca525b5cbcb2e4bf0ca9dd28e70001

@ulfjack

*** Reason for rollback *** Updated to avoid bazelbuild/bazel#3501 This is a rollback of a rollback, with additional modifications to BazelConfiguration.java to fix bazelbuild/bazel#3501, the issue that was the reason we rolled back the original change. The additional updates serve to propagate the client's TMP and TEMP envvars to the action, which is a short-term solution to allow actions have a TMP/TEMP envvar on Windows. They need at least one of those to create temp directories. The long-term solution is to set a value for TMP or TEMP in the executor just before executing the actions, so the TMP/TEMP would not be part of the action key. All of this only affects Bazel on Windows. *** Original change description *** Automated rollback of commit 0abf5fa2d64c76def5a8fa0f960b73ce0566af4d. *** Reason for rollback *** Breaks Bazel CI (bazelbuild/bazel#3501) *** Original change description *** Android BusyBox: actions use the default shell env SpawnActions that run the Android BusyBox now use the default shell environment. This has the following benefits: - Bazel propagates the PATH, TMPDIR envvars to the action - Bazel propagates the --action_env envvars to the action This allows the Bazel client to pass --action_env=TMP or --action_env=TEMP (whichever of the envvars is defined) to the server, so the BusyBox actions will have TMP/TEMP set (to the same value as the clientenv), so they can create temp directories using java.nio.file.Files.createTempDirectory. This method seems to be calling the GetTempPath WinAPI function, which needs the TMP or TEMP envvar, otherwise it falls back to returning c:\windows which is non-writable. There's one drawback of using the default shell environment, although @ulfjack is working on it: - PATH is now also part of the action's cache key. However in a single-machine environment (no remote execution) and assuming PATH isn't likely to change between builds, this probably doesn't poision the action cache in practice. This change is a short-term solution. Propagating the client env's TMP/TEMP means we make that part of the action's cache key. The ideal long-term solution will be to not propagate this envvar, and instead let the execution strategy set it to some client-env-independent value. See bazelbuild/bazel#3264 Change-Id: I756a4203b5d86c881bc36cc089e35cde0d419914 RELNOTES: none PiperOrigin-RevId: 164696600

On Windows, Bazel will always use params files for some BusyBox tools, because some flags of these tools expect values with special characters in them. We need this change so Bazel can safely pass such flags to the BusyBox on Windows. See bazelbuild/bazel#3264 RELNOTES: none PiperOrigin-RevId: 164582899

Deprecate the --libraries flag of the GENERATE_BINARY_R tool in favour of --library. The new flag is multi-value and uses "," as the pair-separator instead of ":". The value converter still supports ":"-separated pairs as well, but looks for "," first. Old format: --libraries=key1:value1,key2:value2,... New format: --library=key1,value1 --library=key2,value2 Motivation: - the ":"-separator prevents using absolute paths on Windows The old flag is still supported, but will be removed after 2018-02-28 (about 6 months from now). Also in this commit: - add a new method to CustomCommandLine.Builder to lazily construct the command line for the --library flag See bazelbuild/bazel#3264 RELNOTES: none PiperOrigin-RevId: 164246506

Use the recently added JunctionCreator class in the Android BusyBox. Query the current OS's name, where the BusyBox process runs (using System.getProperty("os.name")) and instantiate the corresponding JunctionCreator implementation. This allows the BusyBox to work around path length limitations on Windows; see WindowsJunctionCreator. See bazelbuild/bazel#3264 Change-Id: Ifc1a3c86971e64c2f42bcec2988b7e9239a1d29a PiperOrigin-RevId: 161494557

Prepare tests for an upcoming change where some Android actions will use a parameter file, because they expect command line flags with lists of files and the list separators don't work on Windows. See bazelbuild/bazel#3264 RELNOTES: none PiperOrigin-RevId: 161064058

ScopedTemporaryDirectory now makes all files writable before attempting to delete them. This is important on Windows where readonly files cannot be deleted, the attempt resulting in an exception. See bazelbuild/bazel#3264 Change-Id: If79478a4b419c05d77ce89cc30cb701d42df1b75 PiperOrigin-RevId: 160644599

laszlocsomor added platform: windows category: rules > android P1 I'll work on this now. (Assignee required) type: feature request labels Jun 26, 2017

laszlocsomor self-assigned this Jun 26, 2017

bazel-io pushed a commit that referenced this issue Jul 5, 2017

Android: zip_manifest_creator now runs on Windows

e212c05

And so does its test. See #3264 Change-Id: I32dd906b94747d601254ab55e7962bf6cda058f4 PiperOrigin-RevId: 160953063

laszlocsomor added this to the 0.7 milestone Jul 20, 2017

This was referenced Jul 20, 2017

Android builds of cc_binary targets on Windows produce executable files with '.exe' extensions #3418

Closed

TMP: Tools used for Actions on Windows can't use tempdir() easily #2870

Closed

laszlocsomor closed this as completed Aug 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows, Android: make the Hello World example build on Windows #3264

Windows, Android: make the Hello World example build on Windows #3264

laszlocsomor commented Jun 26, 2017

laszlocsomor commented Aug 10, 2017

aj-michael commented Aug 10, 2017

dslomov commented Aug 10, 2017 via email

laszlocsomor commented Aug 10, 2017

Windows, Android: make the Hello World example build on Windows #3264

Windows, Android: make the Hello World example build on Windows #3264

Comments

laszlocsomor commented Jun 26, 2017

laszlocsomor commented Aug 10, 2017

aj-michael commented Aug 10, 2017

dslomov commented Aug 10, 2017 via email

laszlocsomor commented Aug 10, 2017