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

Avoid implicitly compiling sources from the classpath #149

Open
cpovirk opened this issue Jun 1, 2018 · 4 comments
Open

Avoid implicitly compiling sources from the classpath #149

cpovirk opened this issue Jun 1, 2018 · 4 comments
Labels

Comments

@cpovirk
Copy link
Member

cpovirk commented Jun 1, 2018

You can see some internal discussion on CL 198652081, but in short:

  • Truth depends on guava-gwt.
  • The guava-gwt jar contains .java files.
  • Truth has a test that uses Compile-Testing, and it compiles not just the file we give it but also (automatically and undesired by us) those .java files.
  • This used to work, but because Truth uses guava-android but guava-gwt uses guava-jre, Maven puts guava-android on the classpath when guava-gwt needs guava-jre and its dependencies. So now the compilation fails.

We're going to work around this by:

  • Making Truth pass -sourcepath '' to Compile-Testing.
  • Adding an explicit dependency from guava-gwt to some dependencies that it uses directly but currently gets transitively through guava-jre.

But maybe Compile-Testing should be setting -sourcepath '' internally automatically? (Or maybe there's something else similar it should be doing; I've heard that all this is surprisingly complex.)

On the other hand, probably few people will be affected by this: They won't have .java files on their classpath, and even if they do, the files will compile fine because the dependencies will be there. Still, I wonder if they will see unexpected warnings or other output in some cases. (Maybe I've seen some reports of weird Compile-Testing behavior before in AutoValue or something? I'll see if I can dig anything up.) And presumably it's at least a little wasteful to compile files that we don't need.

ronshapiro pushed a commit to google/truth that referenced this issue Jun 1, 2018
Includes a workaround for google/compile-testing#149

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198895795
ronshapiro pushed a commit to google/truth that referenced this issue Jun 1, 2018
Includes a workaround for google/compile-testing#149

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198895795
ronshapiro pushed a commit to google/guava that referenced this issue Jun 1, 2018
The GWT sources (specifically, GwtSerializationDependencies) use @nullable directly, so we shouldn't rely on relying on it indirectly through guava-jre.

However, what actually prompted this is a strange behavior in Compile-Testing, which Truth uses. This CL should help, though it might not be a fully solution.
google/compile-testing#149

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198882337
ronshapiro pushed a commit to google/guava that referenced this issue Jun 4, 2018
The GWT sources (specifically, GwtSerializationDependencies) use @nullable directly, so we shouldn't rely on relying on it indirectly through guava-jre.

However, what actually prompted this is a strange behavior in Compile-Testing, which Truth uses. This CL should help, though it might not be a fully solution.
google/compile-testing#149

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198882337
@cpovirk
Copy link
Member Author

cpovirk commented Jul 24, 2018

Setting -sourcepath '' automatically internally breaks at least some Google-internal tests. Some look like they could be fixed by passing in all source files, rather than passing one and relying on javac to find the rest. The others were less clear to me. I believe they involved codegen, and I didn't dig further.

@tbroyer
Copy link

tbroyer commented Jul 25, 2018

I don't think this is the problem you're hitting, but just in case, and to raise awareness of this weirdness, an empty source path apparently doesn't play well with module (module-info.java I presume) compilation with JDK9+. Gradle works around it by using a custom StandardJavaFileManager that doesn't expose any source file:

https://github.com/gradle/gradle/blob/v4.9.0/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java#L61-L66

https://github.com/gradle/gradle/blob/v4.9.0/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/reflect/SourcepathIgnoringInvocationHandler.java

@cpovirk
Copy link
Member Author

cpovirk commented Jul 25, 2018

Weird! I went looking and found gradle/gradle#2356 (and slightly related gradle/gradle#2330), albeit without a clear indication of whether this is an intentional javac behavior or a bug.

I agree that that's not likely to be the problem I'm hitting here, but it sounds likely to bite in the future, so thanks.

@cpovirk
Copy link
Member Author

cpovirk commented Jul 25, 2018

Error from a module not on the sourcepath is...

file should be on source path, or on patch path for module

...which I guess sounds intentional, if sad, given that I've read your https://blog.ltgt.net/most-build-tools-misuse-javac/

Searching for that leads to gradle/gradle#2302 (edit: also gradle/gradle#3035)

@raghsriniv raghsriniv added the P3 label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants