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.

Sign up for GitHub

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

Jump to bottom

bad path element warnings when compiling java with bazel 4 #12968

Closed
shs96c opened this issue Feb 5, 2021 · 13 comments
Closed

bad path element warnings when compiling java with bazel 4 #12968

shs96c opened this issue Feb 5, 2021 · 13 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: bug

Comments

@shs96c
Copy link
Contributor

shs96c commented Feb 5, 2021

Description of the problem / feature request:

When compiling java code with bazel 4, builds occasionally produce spurious bad path element warnings in an otherwise clean build. This behaviour wasn't seen in bazel 3.7.2.

The warnings is typical of a problem caused by javac attempting to interpret the Class-Path manifest attribute and being unable to locate jars mentioned there. This should be disabled by either compiling with -nowarn or by setting the -Xlint:-path flag (both on the javac invocation). Neither of these approaches prevents the warning being emitted by the bazel build.

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

I have attached a minimal sample project that demonstrates the problem. The readme of this example contains reproduction steps, but it is sufficient to do a clean build (without any cached state) of bazel build //... to see the problem occur.

bazel-bad-path-element.zip

What operating system are you running Bazel on?

macOS Big Sur

What's the output of bazel info release?

release 4.0.0

Have you found anything relevant by searching the web?

The flags can be found by using your favourite search engine to search for "java bad path element"
@illicitonion
Copy link
Contributor

/cc @cushon - This feels like your corner of the world?

@shs96c
Copy link
Contributor Author

shs96c commented Feb 5, 2021

Digging deeper, the problem appears to be this line in BlazeJavacMain where, no matter what the flags passed in -Xlint:path is forced to be enabled. This was part of fb0f51d and there's no readily apparent reason for this change being made.

@shs96c
Copy link
Contributor Author

shs96c commented Feb 5, 2021

Removing the line where lint checking of paths is enabled leads to incorrect warnings about strict java deps violations, so I think that there's more to the fix than simply deleting the extra option.

@kdehairy
Copy link

kdehairy commented Feb 5, 2021

I have same issue. And after some code navigation, I found that the only way around that is to pass the undocumented (to the extent of my knowledge) Werror:-path instead of the standard Xlint:-path and Werror. But I don't like it at all. It removes the distinction between both flags.

@cushon
Copy link
Contributor

cushon commented Feb 5, 2021

cc @comius

ijar normally sanitizes Class-Path manifest entries because they aren't useful with Bazel (e.g. because of sandboxing, and because they typically aren't using paths that match the layout of bazel-out/). The repro uses rules_jvm_external, which disables ijar (e.g. bazel-contrib/rules_jvm_external#263). However it does have a custom tool that's updating manifests to set Target-Label, so perhaps that tool could also sanitize Class-Path: https://github.com/bazelbuild/rules_jvm_external/blob/master/private/tools/java/rules/jvm/external/jar/AddJarManifestEntry.java

I think part of the issue with -Xlint:-path is a bug with the javac API where the FileManager only processes options once, so if you initialize the file manager separately from the JavacTask, the options passed in to the JavacTask don't update the file manager state.

If you're using -Werror, I recommend adding -Werror:-path as a workaround.

But I don't like it at all. It removes the distinction between both flags.

@kdehairy can you expand on that? I'm not sure which distinction that is.

@cushon
Copy link
Contributor

cushon commented Feb 5, 2021

Removing the line where lint checking of paths is enabled leads to incorrect warnings about strict java deps violations, so I think that there's more to the fix than simply deleting the extra option.

@shs96c I'm a little surprised by that and am having trouble repro'ing, do you have any more context on the strict deps errors you saw when you tried that?

@uri-canva
Copy link
Contributor

I can repro:

[11:40:16] uri@Uris-MacBook-Pro /Users/uri/Downloads/bazel-bad-path-element
> bazelisk build //...
2021/02/08 11:40:55 Downloading https://releases.bazel.build/4.0.0/release/bazel-4.0.0-darwin-x86_64...
Starting local Bazel server and connecting to it...
INFO: Analyzed 2 targets (72 packages loaded, 715 targets configured).
INFO: Found 2 targets...
INFO: From Building java/src/com/example/bazel/badpath/libbase.jar (1 source file):
warning: [path] bad path element "/private/var/tmp/_bazel_uri/567263f15eaedcb96d499a39a75494e9/execroot/bazel_classpath/bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/xalan/xalan/2.7.2/xercesImpl.jar": no such file or directory
warning: [path] bad path element "/private/var/tmp/_bazel_uri/567263f15eaedcb96d499a39a75494e9/execroot/bazel_classpath/bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/xalan/xalan/2.7.2/xml-apis.jar": no such file or directory
warning: [path] bad path element "/private/var/tmp/_bazel_uri/567263f15eaedcb96d499a39a75494e9/execroot/bazel_classpath/bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/xalan/xalan/2.7.2/serializer.jar": no such file or directory
warning: [path] bad path element "/private/var/tmp/_bazel_uri/567263f15eaedcb96d499a39a75494e9/execroot/bazel_classpath/bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/xalan/serializer/2.7.2/xml-apis.jar": no such file or directory
INFO: Elapsed time: 38.879s, Critical Path: 3.94s
INFO: 50 processes: 1 internal, 47 darwin-sandbox, 2 worker.
INFO: Build completed successfully, 50 total actions

@shs96c
Copy link
Contributor Author

shs96c commented Feb 8, 2021

@cushon I just rebuilt the java builder binary from source with the change to -Xlint:path backed out and reran the command manually.

I'm curious about why the flag is set: the commit doesn't really explain the reasoning, but it doesn't seem arbitrary.

@kdehairy
Copy link

kdehairy commented Feb 8, 2021

But I don't like it at all. It removes the distinction between both flags.

@kdehairy can you expand on that? I'm not sure which distinction that is.

@cushon What I mean is -Xlint flag is concerned with declaring "what warnings are you interested in", where -Werror flag is only concerned with "whether you want to fail the build for those or not".

With the -Xerror:, the concerns are overlapping and will get confusing. IMO.

@cushon
Copy link
Contributor

cushon commented Feb 10, 2021

TL;DR I agree this is something that should be improved, the following is mostly context about how we got here / some of the requirements that fb0f51d doesn't provide context on.

@shs96c do you remember if you built and ran it at head using the java_binary wrapper script, or did you build a _deploy.jar and drop it into the original command with the original JVM flags and everything? I'm asking because strict deps depends on the vendored 'error-prone-javac' version, which is set by passing JVM flags, and building it at head could break that? Otherwise I have no idea why strict deps would break, that could potentially be a separate bug I'd like to investigate.

re: the flag always being set, it's related to the filemanager options processing bug I mentioned in #12968 (comment) noticed this because there's a test to ensure -Xlint:path warnings work (that test is not yet open source :( ), and the choice seemed to be to enable the warnings always or never. With the exception of the manifest Class-Path entries Bazel is responsible for all of the paths on javac's classpath, so if one of those jars doesn't exist or something that would usually indicate a Bazel bug, so it's a useful safety check.

If the only situation where this diagnostic is occurring in bazel builds is with Class-Path manifest entries jars that aren't processed by ijar, one option might be to ensure that the affected jars' manifests are still normalized, e.g. by updating https://github.com/bazelbuild/rules_jvm_external/blob/master/private/tools/java/rules/jvm/external/jar/AddJarManifestEntry.java to do that normalization.

@kdehairy thanks for the context. One of the problems the -Werror: syntax is trying to solve is that not all diagnostics can be controlled by -Xlint:, there are some warnings that don't have a lint category. So with just -Werror you end up with errors for diagnostics that aren't suppressible. It also means you can't have both warnings and errors, either everything is a warning, or everything is an error. We've found it more useful to be able to enable (or disable) specific lint categories as errors, even though the flags are non-standard and aren't necessarily the most ergonomic choice.

shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 10, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
acdf97a 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
@shs96c shs96c mentioned this issue Feb 10, 2021
Merged
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 25, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
7d6fcae 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 25, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
be679c3 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 25, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
c74550f 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 27, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
816c78f 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Mar 1, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation
57b0a0d 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
shs96c added a commit to bazel-contrib/rules_jvm_external that referenced this issue Mar 1, 2021
@shs96c
Generate pseudo-compile jars for bazel java compilation (#518)
ec2c561 
Normally, bazel uses `ijar` to prepare a jar containing just
ABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"

Because of bazelbuild/bazel#4549
`rules_jvm_external` doesn't use `ijar`, and instead uses the
downloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets `-Xlint:path` for javac invocations, and this means
that if there's a `Class-Path` manifest entry in a compile jar, the
jars within _that_ will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968

This change generates a "pseudo-compile jar" for `jvm_import`
targets. All it does is repack the input jar, excluding the
`META-INF/MANIFEST.MF` file. This means that we should avoid
compilation problems whilst still working well with Kotlin projects.
@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 4, 2021
This was referenced Mar 30, 2021
Closed
Closed
@mtippmann
Copy link

mtippmann commented Apr 12, 2021 •
edited
Loading

is that really noise? I'm also seeing bad-path elements with 4.0.0 but the files are non-existent like in xalan example from @uri-canva - when running the build the jvm also throws classpath-exceptions. in maven this works fine, there is probably a bug somewhere else? Is this related to optional dependencies like for xalan: https://mvnrepository.com/artifact/xalan/xalan/2.7.2

fwiw in 3.7.2 there are no warnings but the jvm still doesn't find the jars in the classpath and throws classnotfound exceptions.

from my (very limited) perspective this a bug in resolving transitive dependencies - either those are there or not, but listing them in the classpath without jars looks sketchy?

@long-stripe long-stripe mentioned this issue Mar 29, 2022
Open
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label May 10, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@shs96c @mtippmann @cushon @kdehairy @illicitonion @comius @oquenchil @uri-canva