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

Use tools provider #15

Merged
merged 3 commits into from Dec 17, 2015
Merged

Use tools provider #15

merged 3 commits into from Dec 17, 2015

Conversation

hinerm
Copy link
Member

@hinerm hinerm commented Dec 2, 2015

Update the JavaCompiler to first try the compiler returned by ToolsProvider - which avoids a hard dependency on javac.

@hinerm
Copy link
Member Author

hinerm commented Dec 2, 2015

ScriptEditor compilation behavior:

Java 7

  • minimaven.jar updated: compilation successful - with compiler warnings
  • tools.jar and javac.jar removed: compilation successful

Java 8

  • minimaven.jar updated: _compilation FAILED_ - complaining about Java version
  • tools.jar and javac.jar removed: compilation successful

@hinerm
Copy link
Member Author

hinerm commented Dec 2, 2015

Note that the failure comes because the java6 javac is picked up as the tools java compiler, even in Java8. So we need to kill our javac fork

When compiling, check the javax.tools.ToolsProvider compiler first.
Failing that we can look for com.sun.tools and then fall back to the
system javac.

This should allow - for example - Fiji to move away from shipping a
com.sun.tools jar as long as it's launched with a JDK.

Closes #9
@hinerm
Copy link
Member Author

hinerm commented Dec 3, 2015

I was hoping that we could arrange things so the javac fork's compiler is picked up after any system java. I was hoping this would be possible with service or tool priorities, but it looks like the compiler tool is the exact same class in each case. So whichever is loaded first will win, and it seems like preference is given to the /jars on the classpath over system jars.

So to preserve javac and avoid breaking existing Java 6 installations we would have to load system classes first.

@ctrueden
Copy link
Member

ctrueden commented Dec 3, 2015

So to preserve javac and avoid breaking existing Java 6 installations we would have to load system classes first.

Or shade the old java6 javac to a different package prefix.

But let's not. 😝

Add a call(String[], boolean, boolean) signature that can accept both
verbose and debug parameters.

These parameters are now used to determine if semi-error messages (e.g.
exception, but continuing execution) or debugging (e.g. location of
javac) information is printed.
@hinerm
Copy link
Member Author

hinerm commented Dec 17, 2015

Luckily, the BuildEnvironment already has verbose and debug flags

@ctrueden thanks for pointing this out. I added a signature and logic to use these variables more liberally.

ctrueden added a commit that referenced this pull request Dec 17, 2015
@ctrueden ctrueden merged commit 3d97f84 into master Dec 17, 2015
@ctrueden ctrueden deleted the use-tools-provider branch December 17, 2015 16:20
@ctrueden
Copy link
Member

Thanks @hinerm! 🐻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants