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.

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, Java: non-fatal Java header compilation error while building Bazel #4096

Closed
laszlocsomor opened this issue Nov 15, 2017 · 11 comments
Closed
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: windows team-Rules-Java Issues for Java rules type: bug

Comments

@laszlocsomor
Copy link
Contributor

Description of the problem / feature request / question:

Java worker fails with "The filename or extension is too long." error, though this doesn't fail the whole build. The path is actually well below 259 characters, so the error message seems to be misguided.

INFO: From Compiling Java headers src/main/java/com/google/devtools/build/lib/libbuild-base-hjar.jar (441 files):
Action failed to execute: java.io.IOException: ERROR: src/main/native/windows/processes-jni.cc(410): nativeCreateProcess(D:\bazel_build\execroot\io_bazel\external\local_jdk\bin\java.exe): The filename or extension is too long.

If possible, provide a minimal example to reproduce the problem:

subst d: c:\d_drive

mkdir d:\os

d:

cd d:\os

git clone https://github.com/bazelbuild/bazel.git

git checkout 2686e4e301dec8d30b901e8fcdce70dd83cec69c

c:\work\bazel-0.7.0\bazel.exe --output_base=d:\bazel_build build --stamp --action_env=USE_DYNAMIC_CRT=1 --action_env=NO_MSVC_WRAPPER=1 --compilation_mode opt --verbose_failures src:bazel.exe

D:/bazel_build/execroot/io_bazel/bazel-out/msvc_x64-opt/bin/src/bazel.exe --output_base=d:\bazel_build build --stamp --action_env=USE_DYNAMIC_CRT=1 --action_env=NO_MSVC_WRAPPER=1 --compilation_mode opt --verbose_failures src:bazel.exe

Environment info

  • Operating System: Windows 10

  • Bazel version (output of bazel info release): 0.7.0, then a dev version

Have you found anything relevant by searching the web?

Found this while attempting to repro #4035

bazel-io pushed a commit that referenced this issue Nov 16, 2017
Error out if the command we try to pass to
CreateProcess is longer than the limit.

Doing so results in a nicer error message than
"The parameter is incorrect" which is confusing.

In this commit I also improve the error reporting
of CreateProcessWithExplicitHandles.

See #4083
See #4096

Change-Id: I00ec52238706fd8140483eddb488c3069eaa7814
PiperOrigin-RevId: 175969789
@laszlocsomor
Copy link
Contributor Author

This is still not fixed, see first message on #4924.

I found the culprit:

Solution:

  • JavaHeaderCompileAction must compute the length of directCommandLine:
  • JavaHeaderCompileAction must compare that length to the platform command line length limit, and create a parameter file if needed -- /cc @cushon @iirina
  • to compute the length of the CustomCommandLine, we have multiple options:
    • naive: flatten it and observe the length
    • smart: implement a CountingCustomCommandLine that computes the length in its Builder -- /cc @tomlu: WDYT of this?

@laszlocsomor laszlocsomor changed the title Windows, non-fatal Java worker error: "The filename or extension is too long." Windows, Java: non-fatal Java header compilation error while building Bazel Mar 28, 2018
@laszlocsomor
Copy link
Contributor Author

This problem isn't specific to Windows, Bazel might trigger it on other platforms too, if the two-tiered spawn's first command is longer than the platform's limit but the second one isn't.

@laszlocsomor
Copy link
Contributor Author

The failing command is in turbine-failure.txt.

Using a parameter file works:

C:\tmp3\o6Hbs7N0\execroot\io_bazel>type c:\tmp\javah-params
--output
c:/tmp/javah-libbuild-base-hjar.jar
--output_deps
c:/tmp/javah-libbuild-base-hjar.jdeps
--temp_dir
c:/tmp/javahlibbuild-base-hjar_temp
--bootclasspath
bazel-out/host/genfiles/external/bazel_tools/tools/jdk/platformclasspath.jar
--sources
src/main/java/com/google/devtools/build/lib/analysis/ActionsProvider.java
src/main/java/com/google/devtools/build/lib/analysis/AliasProvider.java
src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java
...
bazel-out/x64_windows-fastbuild/genfiles/third_party/_ijar/guava/third_party/guava/guava-24.1-jre-ijar.jar
bazel-out/x64_windows-fastbuild/genfiles/third_party/_ijar/jsr305/third_party/jsr305/jsr-305-ijar.jar

C:\tmp3\o6Hbs7N0\execroot\io_bazel>"C:\tmp3\o6hbs7n0\execroot\io_bazel\external\local_jdk\bin\java.exe" -Xverify:none -Xbootclasspath/p:external/bazel_tools/third_party/java/jdk/langtools/javac-9+181-r4173-1.jar -jar external/bazel_tools/tools/jdk/turbine_deploy.jar @c:/tmp/javah-params

@laszlocsomor
Copy link
Contributor Author

I talked with @aehlig and @dslomov about this problem. Here's what we thought out:

  • Evaluating the CustomCommandLine to count its length beats the purpose of using one, and a CountingCustomCommandLine (or its Builder) would still need to flatten NestedSets. However that is exactly what SpawnAction.Builder.buildCommandLine() does:
    totalLen += getCommandLineSize(commandLineAndParamFileInfo.commandLine);
  • The ...SpawnRunner objects could provide ground truth of the platform limitations such as command length limits, but they lack the ability to turn a specific long command line into a short parameter-file using one, nor do they know whether the executable supports params files anyway. Only the ConfiguredTarget creating the Action knows all this.
  • Therefore: if the rule anticipates its action exceeding the command length limit, then let the rule create the action with a fallback command line that's guaranteed to be short. We already have a similar machinery in SpawnAction.Builder (see .buildCommandLine() earlier), but we need a generic, Spawn-level solution. The SpawnRunner would evaluate the main command line, compare to the limit, and respectively use the main command line or create a parameter file and use the alternative command line.
  • Keeping two command lines in sync is error-prone, let's maybe add a state-changing method to CustomCommandLine.Builder that says "all following arguments may go into a params file, and here's the argument to pass this params file".

@tomlu , WDYT?

@tomlu
Copy link
Contributor

tomlu commented Mar 28, 2018 via email

@laszlocsomor
Copy link
Contributor Author

Sorry, I can't look at the doc, just don't have the mental bandwidth for it.

@cushon
Copy link
Contributor

cushon commented Mar 28, 2018

@tomlu's doc looks great to me. What is the priority of this bug? Can we wait for the principled solution?

Conditionally creating a params file the way that is currently supported has unacceptable performance overhead. Unconditionally creating a params file would address this bug, but also has performance overhead.

@laszlocsomor
Copy link
Contributor Author

@tomlu's doc looks great to me. What is the priority of this bug? Can we wait for the principled solution?

I don't think it's urgent, because Bazel still builds, but it displays this error message. I guess we can wait

@cushon
Copy link
Contributor

cushon commented Nov 17, 2018

Deferred params files happened, so this should be easy to fix if it's still happening. We aren't doing the two-tiered spawn thing for header compilation anymore, though, so maybe it's no longer a problem?

@cushon cushon assigned laszlocsomor and unassigned cushon Nov 17, 2018
@lberki lberki added P2 We'll consider working on this in future. (Assignee optional) and removed category: rules > java labels Dec 3, 2018
@laszlocsomor
Copy link
Contributor Author

I haven't seen this issue in a long time, maybe it was accidentally fixed. Setting P3.

@laszlocsomor laszlocsomor added the P3 We're not considering working on this, but happy to review a PR. (No assignee) label Feb 28, 2019
@laszlocsomor laszlocsomor added team-Rules-Java Issues for Java rules and removed P2 We'll consider working on this in future. (Assignee optional) labels Feb 28, 2019
@laszlocsomor laszlocsomor removed their assignment Feb 28, 2019
@lberki lberki removed the untriaged label Mar 14, 2019
@meisterT
Copy link
Member

Closing the issue since it seems to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: windows team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

No branches or pull requests

8 participants