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

Separate java_tools into platform independent and prebuilt part. #12546

Closed
wants to merge 13 commits into from

Conversation

comius
Copy link
Contributor

@comius comius commented Nov 24, 2020

java_tools is repository package containing tools needed during Java compilation: JavaBuilder, patches for Java compiler, ijar, singlejar, ...
Most of the files are jars with Java classes. java_tools are released for three platforms: linux, windows and darwin, however the only difference is in two binaries: ijar and singlejar.

This is part one of splitting java_tools and releasing split version (following PR makes use of released split version in Bazel)

Java_tools used to be released for multiple Java versions, but all the releases were the same except a some string substitutions in BUILD file. I changed to build only a single version, since it already supports Java from 8 to 14.

Changes:

  • BUILD.java_tools is split into BUILD.java_tools_prebuilt (where the second contains prebuilt binaries)
  • toolchain definitions are removed from BUILD.java_tools and will be added to tools/jdk/BUILD in the second part
  • java_toolchain_default.bzl.java_tools is removed (default_java_toolchain.bzl will be updated with its features in the second part).
  • src/BUILD: JAVA_VERSION is removed, targets used to build java_tools.zip are duplicated to build java_tools_prebuilt.zip (done some cleanup as well)
  • upload_all_java_tools.sh and upload_java_tools.sh: used by Build kite, I removed java_version over the release, but kept it over tests (for different JDKs)
  • create_java_tools_release.sh: used by the user in the release process - added platform independent part
  • tests are updated to use platform independent and platform files, some tests had to be disabled and will be reenabled after the release

@google-cla google-cla bot added the cla: yes label Nov 24, 2020
@comius comius requested review from c-mita and philwo November 24, 2020 20:03
@comius comius marked this pull request as ready for review November 24, 2020 20:03
@comius comius requested a review from cushon November 24, 2020 20:11
@comius
Copy link
Contributor Author

comius commented Nov 24, 2020

After this PR is merged I can do java_tools release.
Second part is in #12552 (it will build without errors only after java_tools are released).

@comius comius self-assigned this Nov 24, 2020
@cushon
Copy link
Contributor

cushon commented Nov 24, 2020

Java_tools used to be released for multiple Java versions, but all the releases were the same except a some string substitutions in BUILD file. I changed to build only a single version, since it already supports Java from 8 to 14.

Is the goal to avoid separate releases of those tools for different Java versions in the future, or does this just reflect that we don't need separate versions for 8 through 14? i.e. do you want to commit to having JavaBuilder at head support whatever range of Java versions Bazel supports, or could we potentially start releasing different JavaBuilder versions corresponding to different Java versions again in the future if we needed to?

@comius
Copy link
Contributor Author

comius commented Nov 25, 2020

Is the goal to avoid separate releases of those tools for different Java versions in the future, or does this just reflect that we don't need separate versions for 8 through 14? i.e. do you want to commit to having JavaBuilder at head support whatever range of Java versions Bazel supports, or could we potentially start releasing different JavaBuilder versions corresponding to different Java versions again in the future if we needed to?

I think both. The goal should be to avoid separate releases, because this makes it easier for both the person making the release as well as for the user configuring dependencies and upgrading versions. It also reflects the current fact that we don't need separate version.

I would commit to having one JavaBuilder (and other tools, ijar, singlejar). In case this is not possible and it needs to be forked, there are two better ways than the current one:

  1. Add both versions of JavaBuilder into the release
  2. Add single version, but increase major version of the release (I would slightly prefer this one).

Notice also java_toolchain targets are also moved into @bazeltools/tools/jdk/BUILD. This allows us to change/extend java_toolchain rule definition easier (the modifications if conservative don't require re-releasing of java_tools). So in case JavaBuilder is forked, both options would still allow us to fix java_toolchain targets potentially to support both versions.

bazel-io pushed a commit that referenced this pull request Nov 25, 2020
This is a github only patch from PR #12546. Needs to go in first.

Closes #12556.

Signed-off-by: Philipp Wollermann <[email protected]>
Copy link
Member

@c-mita c-mita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

bazel-io pushed a commit that referenced this pull request Nov 25, 2020
This is a github only patch from PR #12546. Needs to go in first.

PiperOrigin-RevId: 344260438
@cushon
Copy link
Contributor

cushon commented Nov 25, 2020

In case this is not possible and it needs to be forked, there are two better ways than the current one...

Thanks--that all makes sense, I just wanted to confirm we weren't completely shutting the door on that. I was partly thinking about what will happen once we finish upgrading to Java 11 internally. If we're trying to use the same version of JavaBuilder for JDK 8, that would mean we don't have a path to using Java 11 features or APIs in JavaBuilder.

java_tools is package containing tools needed during Java compilation: JavaBuilder, patches for Java compiler, ijar, singlejar, ...
Most of the files are pure Java .jars. java_tools are released for three platforms: linux, windows and darwin, however the only diffrence is in two binaries: ijar and singlejar.

This is part one: splitting java_tools and releasing split version (follows the use of split version in Bazel)

Java_tools used to be released for multiple Java versions, but all the releases were the same except a some string substitutions in BUILD file. I changed to build only a single version, since it already supports Java from 8 to 14.

Changes:
- BUILD.java_tools is split into BUILD.java_tools_prebuilt (where the second contains prebuilt binaries)
- toolchain definitions are removed from BUILD.java_tools and will be added to tools/jdk/BUILD in the second part
- java_toolchain_default.bzl.java_tools is removed (default_java_toolchain.bzl will be updated with its features in the second part).
- src/BUILD: JAVA_VERSION is removed, targets used to build java_tools.zip are duplicated to build java_tools_prebuilt.zip (done some cleanup as well)
- upload_all_java_tools.sh and upload_java_tools.sh: used by Build kite, I removed java_version over the release, but kept it over tests (for different JDKs)
- create_java_tools_release.sh: used by the user in the release process - added platform independent part
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants