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

external/local_jdk/bin/java: No such file or directory #1116

Closed
schroederc opened this issue Apr 4, 2016 · 25 comments
Closed

external/local_jdk/bin/java: No such file or directory #1116

schroederc opened this issue Apr 4, 2016 · 25 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request

Comments

@schroederc
Copy link
Contributor

Commit 3f29d42 seems to break one of Kythe's custom Skylark rules. The rule tries to run a Bazel-built Java executable but cannot find the java binary in its runfiles. The error looks similiar to the snippet below.

Skylark rule impl: https://github.com/google/kythe/blob/1a8f55a12d6/tools/build_rules/kythe.bzl#L59
Test target: https://github.com/google/kythe/blob/1a8f55a12d6/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/BUILD#L73
Bazel command: bazel test //kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg:files_tests

ERROR: <redacted>/kythe/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/BUILD:73:1: error executing shell command: 'set -e
export KYTHE_ROOT_DIRECTORY="$PWD"
export KYTHE_OUTPUT_DIRECTORY="$(dirname bazel-out/local_linux-fastbuild/genfiles/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/fil...' failed: namespace-sandbox failed: error executing command <redacted>/.cache/bazel/_bazel_schroederc/ddace01a1ad38df359eb57115be59fad/kythe/_bin/namespace-sandbox ... (remaining 5 argument(s) skipped).
bazel-out/local_linux-fastbuild/bin/kythe/java/com/google/devtools/kythe/extractors/java/standalone/javac_extractor: line 194: <redacted>/.cache/bazel/_bazel_schroederc/ddace01a1ad38df359eb57115be59fad/kythe/bazel-out/local_linux-fastbuild/bin/kythe/java/com/google/devtools/kythe/extractors/java/standalone/javac_extractor.runfiles/io_kythe/external/local_jdk/bin/java: No such file or directory

@kchodorow
Copy link
Contributor

I'm not sure how this worked before, as it looks like it needs java as a runfile but kythe.bzl only declares javac and jar as attributes. I'm working on reproducing.

@kchodorow kchodorow added under investigation P2 We'll consider working on this in future. (Assignee optional) labels Apr 4, 2016
@schroederc
Copy link
Contributor Author

I believe the runfiles for the executable should contain the java binary and the runfiles should be included in the tool_inputs from the resolve_command method.

@damienmg
Copy link
Contributor

damienmg commented Apr 4, 2016

The problem is the path in the runfiles. Are you switching between Bazel
version? I reproduced the bug by testing with bazel 0.2.1 then testing
again with bazel @Head

Doing a bazel clean remove that error. Howevevr now a I get a
java.lang.NoSuchFieldError:
RELEASE_9

On Mon, Apr 4, 2016 at 11:06 PM Cody Schroeder notifications@github.com
wrote:

I believe the runfiles for the executable should contain the java binary
and the runfiles should be included in the tool_inputs from the
resolve_command method.


You are receiving this because you are subscribed to this thread.

Reply to this email directly or view it on GitHub
#1116 (comment)

@schroederc
Copy link
Contributor Author

The last good version that I was using is 0.2.0-2016-03-30 (@787abf9). After updating to HEAD, I found this breakage.

I can confirm that after doing a bazel clean, the RELEASE_9 error starts. Has there been a change to the bundled javac_tools.jar recently?

@schroederc
Copy link
Contributor Author

Ah, looks like the -Xbootclasspath/p:$$JAVA_RUNFILES/external/bazel_tools/third_party/java/jdk/langtools/javac.jar argument now has the wrong path. It's missing the io_kythe component before external/bazel_tools.

Should that be part of the $JAVA_RUNFILES variable?

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

I don't see the Xbootclasspath in the command line and I can't find it in the source code.

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

Oops missing the end: Where do you find it?

@schroederc
Copy link
Contributor Author

@shahms
Copy link

shahms commented Apr 5, 2016

It also fails if I change it to use "$(location //third_party/java/jdk/langtools:javac_jar)" rather than the hard-coded path as location fails to include the repository name prefix.

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

oh I see, no the workspace name should not be part of the JAVA_RUNFILES
environment variable (think on how it works in google) so indeed that is
the problem. Adding the path should work with head.

We need to think of a way to fix that correctly (so it works with bazel
@Head and bazel @0.2.1) unless you don't care about supporting latest
release.

On Tue, Apr 5, 2016 at 5:40 PM Cody Schroeder notifications@github.com
wrote:

It's part of the jvm_flags of the Java executable:
https://github.com/google/kythe/blob/master/kythe/java/com/google/devtools/kythe/extractors/java/standalone/BUILD#L17


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1116 (comment)

@shahms
Copy link

shahms commented Apr 5, 2016

I wasn't necessarily advocating for it to be part of $JAVA_RUNFILES, but it would be nice to be able to easily form workspace/repository name agnostic paths to targets that worked reliably. I'd say it works 95% of the time now, which is occasionally frustrating.

Ideally, we'd be able to support both @Head and the latest release, but as that doesn't seem feasible for the near future I would like to be able to build Kythe using the latest release, barring requirements for bugfixes, etc. that necessitate using a development version of Bazel.

I would strongly like Kythe releases to be buildable with most recent Bazel release at the time the Kythe release is cut.

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

I understand. Well the location way should work since a long time so for this case we should not have any problem. The correct way should be $(location @bazel_tools//third_party/java/jdk/langtools:javac_jar). Tell me if that fix.

On ci.bazel.io we test for Bazel HEAD and latest release (but not earlier release, when we reach 1.0, we might start doing more). However that's another discussion, we should continue it on bazelbuild/continuous-integration#4.

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

what about $(location @bazel_tools//third_party/java/jdk/langtools:javac_jar)?

On Tue, Apr 5, 2016 at 10:31 PM Shahms King notifications@github.com
wrote:

It also fails if I change it to use "$(location
//third_party/java/jdk/langtools:javac_jar)" rather than the hard-coded
path as location fails to include the repository name prefix.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1116 (comment)

@shahms
Copy link

shahms commented Apr 5, 2016

That, too, requires including the main workspace name before the path and after JAVA_RUNFILES and that only fixes the issue for the one script.

Given how many scripts we have which assume either an empty workspace or that the workspace name is omitted from the path, the only viable fix for now is to simply remove the name :-(

As a separate issue, we need to clean up how we're referencing the javac.jar generally.

@damienmg
Copy link
Contributor

damienmg commented Apr 5, 2016

$(location @bazel_tools//third_party/java/jdk/langtools:javac_jar) should definitely contains the workspace name, if not then it's a bazel bug.

We should also provide a way to build the runfiles path correctly I agree.

@shahms
Copy link

shahms commented Apr 5, 2016

I just double checked and it does not contain the correct path, e.g.
$ bazel info release
release 0.2.1-2016-03-31 (@950c1ab)

WORKSPACE: workspace(name="io_kythe")
kythe/java/com/google/devtools/kythe/extractors/java/standalone/BUILD:

....
jvm_flags = [
    "-Xbootclasspath/p:$$JAVA_RUNFILES/$(location @bazel_tools//third_party/java/jdk/langtools:javac_jar)",
],
....

bazel-out/local_linux-fastbuild/bin/kythe/java/com/google/devtools/kythe/extractors/java/standalone/javac_extractor:

ARGS=(
  ${JVM_DEBUG_FLAGS}
  ${JVM_FLAGS}
  -Xbootclasspath/p:$JAVA_RUNFILES/external/bazel_tools/third_party/java/jdk/langtools/javac.jar
  "${JVM_FLAGS_CMDLINE[@]}"
  ${MAIN_ADVICE}
  com.google.devtools.kythe.extractors.java.standalone.Javac8Wrapper
  "${ARGS[@]}"

and the test Cody mentioned earlier fails with the RELEASE_9 error.

Happy to file a separate bug, if you'd like.

@damienmg
Copy link
Contributor

damienmg commented Apr 6, 2016

After thinking a bit it's logical the execroot should be inside the workspace name, Can you try to replace the jvm_flags with "-Xbootclasspath/p:$$PWD/$(location @bazel_tools//third_party/java/jdk/langtools:javac_jar)" instead?

@shahms
Copy link

shahms commented Apr 6, 2016

That also fails.

@damienmg
Copy link
Contributor

damienmg commented Apr 6, 2016

@kchodorow do you have anymore idea?

@shahms
Copy link

shahms commented Apr 6, 2016

Looking at the rule which defines the test, it does a (likely ill-advised) cd before invoking the dependency in question, which breaks using PWD. Aside from that, the wrapper script does a bit of work to make sure JAVA_RUNFILES points at the correct location. I suspect we'll have to include the workspace name in the path for now.

@damienmg damienmg added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Apr 12, 2016
@damienmg
Copy link
Contributor

We want to baseline a new release candidate ASAP, this is the only block that I know of for it. We should not breakage Kythe with the next release.

@damienmg
Copy link
Contributor

@shahms is it still an issue or do you have a workaround?

@shahms
Copy link

shahms commented Apr 12, 2016

I have a worked around this by including the workspace name in the path. While a more general workspace-agnostic runfiles path solution would be nice, I don't think it's a blocker.

@damienmg damienmg added P3 We're not considering working on this, but happy to review a PR. (No assignee) category: External repositories and removed under investigation P1 I'll work on this now. (Assignee required) Release blocker labels Apr 12, 2016
@shahms
Copy link

shahms commented Apr 14, 2016

Not sure if this should be a separate bug or not, but in attempting to re-set the workspace name I discovered that on OS X runfiles does not include the workspace name, but on Linux it does. For now, I can add both paths to the bootclasspath but that's a dirty hack. I'm happy to file a separate bug for this, if so desired.

bazel-io pushed a commit that referenced this issue Apr 20, 2016
…directory

This also sets the Bazel workspace name to io_bazel_source.

Fixes #848.

Relevant to #1116, #1124,

RELNOTES[INC]: All repositories are now directly under the x.runfiles directory in the runfiles tree (previously, external repositories were at x.runfiles/main-repo/external/other-repo. This simplifies handling remote repository runfiles considerably, but will break existing references to external repository runfiles.
---
Furthermore, if a Bazel project does not provide a workspace name in the WORKSPACE file, Bazel will now default to using __main__ as the workspace name (instead of "", as previously). The repository's runfiles will appear under x.runfiles/__main__/.

--
MOS_MIGRATED_REVID=120224534
bazel-io pushed a commit that referenced this issue Apr 22, 2016
*** Reason for rollback ***

Broke non-Bazel projects on ci.bazel.io

Fixes #1168

*** Original change description ***

Move the runfiles for external repositories to under the x.runfiles/ directory

This also sets the Bazel workspace name to io_bazel_source.

Fixes #848.

Relevant to #1116, #1124,

RELNOTES[INC]: All repositories are now directly under the x.runfiles directory in the runfiles tree (previously, external repositories were at x.runfiles/main-repo/external/other-repo. This simplifies handling remote repository runfiles considerably, but will break existing references to external repository runfiles....

***

--
MOS_MIGRATED_REVID=120535721
@kchodorow
Copy link
Contributor

This should work in 0.3.0.

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) type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants