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

common JDKs don't support params files #3069

Closed
chaoren opened this issue May 28, 2017 · 10 comments
Closed

common JDKs don't support params files #3069

chaoren opened this issue May 28, 2017 · 10 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) type: bug

Comments

@chaoren
Copy link
Contributor

chaoren commented May 28, 2017

Please provide the following information. The more we know about your system and use case, the more easily and likely we can help.

Description of the problem / feature request / question:

If asking a question or requesting a feature, also tell us about the underlying problem you're trying to solve.

The scala rules is borrowing the java_stub_template from the java rules, and it seems like this line doesn't actually work.

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

$ ./bazel-bin/foo.runfiles/local_jdk/bin/java -classpath ./bazel-bin/foo.runfiles/__main__/foo.jar Foo
Hello world!
$ ./bazel-bin/foo.runfiles/local_jdk/bin/java -classpath @<(echo ./bazel-bin/foo.runfiles/__main__/foo.jar) Foo
Error: Could not find or load main class Foo
@laszlocsomor
Copy link
Contributor

@chaoren : What platform did you try on?

@laszlocsomor laszlocsomor self-assigned this May 29, 2017
@ittaiz
Copy link
Member

ittaiz commented May 29, 2017 via email

@laszlocsomor
Copy link
Contributor

I managed to repro this. Apparently neither OpenJDK for MacOS nor Zulu JRE (that we bundle Bazel 0.5.0 with) support parameter file syntax (i.e. java -cp @paramfile classname).

Repro:

cd /tmp

echo > Foo.java <<EOF
public class Foo { public static void main(String[] args) { System.out.println("Hello world!"); } }
EOF

javac Foo.java

Then this works:

$ java -cp /tmp Foo
Hello world!

But this doesn't:

$ echo /tmp > /tmp/cp.txt && java -cp @/tmp/cp.txt Foo
Error: Could not find or load main class Foo

Here's my Java:

$ java -version
java version "1.8.0_102"
Java(TM) SE Runtime Environment (build 1.8.0_102-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.102-b14, mixed mode)

@laszlocsomor laszlocsomor changed the title -classpath @<(echo $CLASSPATH) doesn't actually work. OpenJDK and ZuluJRE don't support params files May 29, 2017
@laszlocsomor laszlocsomor added type: bug P2 We'll consider working on this in future. (Assignee optional) and removed under investigation labels May 29, 2017
@laszlocsomor
Copy link
Contributor

This only affects users with very long classpaths (>120K) so I'm prioritizing it as P2.
We already work around this issue on Windows, so I'll just use that code path.

@laszlocsomor
Copy link
Contributor

laszlocsomor commented May 29, 2017

Just to clarify: process substitution (the @<(echo $CLASSPATH) thingy) works fine on MacOS (it creates a pipe as /dev/fd/63).

@ittaiz
Copy link
Member

ittaiz commented May 29, 2017 via email

@laszlocsomor
Copy link
Contributor

@ittaiz : It doesn't, I just tried.

@laszlocsomor laszlocsomor changed the title OpenJDK and ZuluJRE don't support params files common JDKs don't support params files May 30, 2017
@laszlocsomor
Copy link
Contributor

laszlocsomor commented May 30, 2017

Unfortunately Bazel doesn't pick up the fixed java_stub_template from the workspace. The template is compiled into Bazel instead. So to see the effects of the bugfix, you must rebuild Bazel from head and use that binary, or wait for the next release.

@chaoren
Copy link
Contributor Author

chaoren commented May 31, 2017

Won't this line cause a problem outside of cygwin?

MANIFEST_JAR_FILE=$(cygpath --windows "$MANIFEST_JAR_FILE")

Can we please have a test for this?

@laszlocsomor
Copy link
Contributor

Oops, you're right, good catch! I forgot to remove that line. Yes, this needs a test, I'll add one.

bazel-io pushed a commit that referenced this issue Jun 6, 2017
Remove the cygpath call on non-Windows platforms,
that was recently added by 102ce6d

Also add a test for Bazel's Java launcher.

Also update the testenv.sh:cleanup method to wait
for Bazel to shut down, don't give up immediately
if it could not clean up the inner Bazel's temp
dir.

Fixes #3092
See #3069

Change-Id: I82b1026a60056f340caa53a59b6f2ec8a1397ef3
PiperOrigin-RevId: 158139846
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) type: bug
Projects
None yet
Development

No branches or pull requests

5 participants