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

Java binary with long classpath fails on read-only file system #6289

Closed
brown opened this issue Oct 2, 2018 · 8 comments
Closed

Java binary with long classpath fails on read-only file system #6289

brown opened this issue Oct 2, 2018 · 8 comments
Assignees
Labels
team-Rules-Java Issues for Java rules untriaged

Comments

@brown
Copy link
Contributor

brown commented Oct 2, 2018

Description of the problem / feature request:

When a Java binary's classpath is too long, the wrapper script creates manifest and jar
files containing the classpath. The wrapper writes these files to the directory containing
the wrapper. This fails when the binary is run from a read-only file system.

Feature requests: what underlying problem are you trying to solve with this feature?

Allowing a Bazel compiled Java binary or test to run from a read-only file system or from
a directory that's not writable.

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

Run a Java binary or test with a classpath that exceeds the limit (roughly 120K) on a read-only
file system.

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

development version

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

bazel build src:bazel-bin
The version with the bug is 0.15.0

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

Have you found anything relevant by searching the web?

Any other information, logs, or outputs that you want to share?

The following patch to the wrapper should fix the problem.

***************
*** 303,313 ****
    done
    unset IFS

-   RAND_ID=$(cat /dev/urandom | head -c 128 | md5func)
    # Create manifest file
!   MANIFEST_FILE="${self}-${RAND_ID}.jar_manifest"
! 
!   echo "Manifest-Version: 1.0" >$MANIFEST_FILE
    CLASSPATH_LINE="Class-Path:$MANIFEST_CLASSPATH"
    # No line in the MANIFEST.MF file may be longer than 72 bytes.
    # A space prefix indicates the line is still the content of the last attribute.
--- 303,311 ----
    done
    unset IFS

    # Create manifest file
!   MANIFEST_FILE=$(mktemp)
!   echo "Manifest-Version: 1.0" > "$MANIFEST_FILE"
    CLASSPATH_LINE="Class-Path:$MANIFEST_CLASSPATH"
    # No line in the MANIFEST.MF file may be longer than 72 bytes.
    # A space prefix indicates the line is still the content of the last attribute.
***************
*** 316,327 ****
      if (($i == 0)); then
        PREFIX=""
      fi
!     echo "$PREFIX${CLASSPATH_LINE:$i:71}" >>$MANIFEST_FILE
    done
!   echo "Created-By: Bazel" >>$MANIFEST_FILE

    # Create classpath JAR file
!   MANIFEST_JAR_FILE="${self}-${RAND_ID}-classpath.jar"
    if is_windows; then
      MANIFEST_JAR_FILE="$(cygpath --windows "$MANIFEST_JAR_FILE")"
      MANIFEST_FILE="$(cygpath --windows "$MANIFEST_FILE")"
--- 314,325 ----
      if (($i == 0)); then
        PREFIX=""
      fi
!     echo "$PREFIX${CLASSPATH_LINE:$i:71}" >> "$MANIFEST_FILE"
    done
!   echo "Created-By: Bazel" >> "$MANIFEST_FILE"

    # Create classpath JAR file
!   MANIFEST_JAR_FILE=$(mktemp)
    if is_windows; then
      MANIFEST_JAR_FILE="$(cygpath --windows "$MANIFEST_JAR_FILE")"
      MANIFEST_FILE="$(cygpath --windows "$MANIFEST_FILE")"
@cushon
Copy link
Contributor

cushon commented Oct 15, 2018

We could fix this by creating the manifest jar as a separate action instead of in the launcher script.

One challenge with that is that we'd like to avoid creating it unless it's actually necessary, and checking how long the classpath it at analysis time is expensive (requires flattening a NestedSet).

This is analogous to the problem we have with params files, which get special handling. Ideally we could treat these arguments as an actual manifest file instead of a special .jar file, but that would require something like #6354.

Maybe @lberki or @iirina have better ideas?

@cushon
Copy link
Contributor

cushon commented Oct 17, 2018

@meteorcloudy can you take a look, as the author of 752b61e?

@brown's solution LGTM but you have more context.

@meteorcloudy
Copy link
Member

Sorry, I don't quite understand the patch. Does it just avoid creating the classpath jar? Then how could the correct classpath be set?

@cushon
Copy link
Contributor

cushon commented Oct 18, 2018

I think the crux of it is using MANIFEST_FILE=$(mktemp) instead of MANIFEST_FILE="${self}-${RAND_ID}.jar_manifest", since the latter assumes the output tree is writeable.

The patch doesn't apply cleanly at head, I had to sync back to 6ee36ef. Was it developed against a fairly old version of Bazel?

Here's a unified diff version in case that's easier to read:

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
index dbe84b4fae..0bc3ef2261 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
@@ -300,11 +300,9 @@ function create_and_run_classpath_jar() {
   done
   unset IFS
 
-  RAND_ID=$(cat /dev/urandom | head -c 128 | md5func)
   # Create manifest file
-  MANIFEST_FILE="${self}-${RAND_ID}.jar_manifest"
-
-  echo "Manifest-Version: 1.0" >$MANIFEST_FILE
+  MANIFEST_FILE=$(mktemp)
+  echo "Manifest-Version: 1.0" > "$MANIFEST_FILE"
   CLASSPATH_LINE="Class-Path:$MANIFEST_CLASSPATH"
   # No line in the MANIFEST.MF file may be longer than 72 bytes.
   # A space prefix indicates the line is still the content of the last attribute.
@@ -313,12 +311,12 @@ function create_and_run_classpath_jar() {
     if (($i == 0)); then
       PREFIX=""
     fi
-    echo "$PREFIX${CLASSPATH_LINE:$i:71}" >>$MANIFEST_FILE
+    echo "$PREFIX${CLASSPATH_LINE:$i:71}" >> "$MANIFEST_FILE"
   done
-  echo "Created-By: Bazel" >>$MANIFEST_FILE
+  echo "Created-By: Bazel" >> "$MANIFEST_FILE"
 
   # Create classpath JAR file
-  MANIFEST_JAR_FILE="${self}-${RAND_ID}-classpath.jar"
+  MANIFEST_JAR_FILE=$(mktemp)
   if is_windows; then
     MANIFEST_JAR_FILE="$(cygpath --windows "$MANIFEST_JAR_FILE")"
     MANIFEST_FILE="$(cygpath --windows "$MANIFEST_FILE")"

@meteorcloudy
Copy link
Member

@cushon Thanks for the explanation! Now it makes sense.
Can you send this as a PR or CL? Happy to review it!

brown added a commit to brown/bazel that referenced this issue Oct 23, 2018
Before this change, the Java binary shell stub created temporary files in the
directory containing the binary, but that fails when the binary is stored in a
read-only file system or the directory is write protected.

Fixes issue bazelbuild#6289
@brown
Copy link
Contributor Author

brown commented Oct 23, 2018

I created a pull request: #6471

bazel-io pushed a commit that referenced this issue Oct 23, 2018
Before this change, the Java binary shell stub created temporary files in the
directory containing the binary, but that fails when the binary is stored in a
read-only file system or the directory is write protected.

Fixes issue #6289

Closes #6471.

PiperOrigin-RevId: 218314425
@laszlocsomor
Copy link
Contributor

IIUC #6471 should have closed this bug, correct?

@meteorcloudy
Copy link
Member

Yes, #6471 should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules untriaged
Projects
None yet
Development

No branches or pull requests

5 participants