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

Deploy jars differ when compiled with the same toolchain on different platforms #17316

Closed
shs96c opened this issue Jan 25, 2023 · 3 comments
Closed
Labels

Comments

@shs96c
Copy link
Contributor

shs96c commented Jan 25, 2023

Description of the bug:

When building a deploy_jar target, Bazel will helpfully add a build-data.properties file. This contains some useful information, but part of the data is a build.target which contains a path to the output file and not (as the name would suggest) the target being built.

This means that jars built with the same version of the same java toolchain but on different platforms will differ, but only because of this file. This reduces our ability to have stable cross-platform builds.

I discovered this while attempting to add a test for rules_jvm_external to ensure that all prebuilt outputs are up-to-date. While the artifacts are most assuredly okay, the test failed since I work on macOS and the builds happen on Linux.

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

Create a simple java_binary target and compile it on Linux and macOS using the same version of the same java toolchain (eg. remote_jdk11). Compare the shasums. They should be identical. They are not.

Which operating system are you running Bazel on?

macOS and Linux

What is the output of bazel info release?

release 6.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@fmeum
Copy link
Collaborator

fmeum commented Feb 2, 2023

We have also been surprised to see this indirect leak of platform information into java_binary deploy JARs. It does hinder work on cross-platform caching for Java outputs. With approaches that remove or rewrite the config section of artifacts paths, the problem would go away, but the path left behind may not be resolvable afterwards.

@cushon Would you accept a PR adding a flag that makes it so that build.target contains the owner label of the output artifact rather than its exec path? And just because that would, at least to me, seem to be the more desirable default behavior: Do you expect google3 to be sensitive to the particular value of this build data field?

@cushon
Copy link
Contributor

cushon commented Feb 2, 2023

I found an internal bug from 2016 asking why build.target was the output path for _deploy.jars instead of the build target, and pointing out that it doesn't match go and c++ stamping (which use the build target):

// TODO(b/28294322): do we need to resolve the path to be absolute or
// canonical?
build_properties_.AddProperty("build.target", options_->output_jar.c_str());

I think we should try just changing the default behaviour to use the build target. Let me do some testing and see anything is relying on the current behaviour.

copybara-service bot pushed a commit that referenced this issue Feb 3, 2023
If set, and build stamping is enabled, it is used as the `build.target`
property instead of the output path.

#17316

PiperOrigin-RevId: 506932307
Change-Id: I06a710445ab79e98fc3794b7eb81a9abd61dce69
copybara-service bot pushed a commit that referenced this issue Feb 10, 2023
#17316

PiperOrigin-RevId: 508700297
Change-Id: Iced40505b32aa4e6aa32fa969ef0c75224f2e5bc
@cushon
Copy link
Contributor

cushon commented Feb 10, 2023

This is fixed by cc4af3e

@cushon cushon closed this as completed Feb 10, 2023
hvadehra pushed a commit that referenced this issue Feb 14, 2023
If set, and build stamping is enabled, it is used as the `build.target`
property instead of the output path.

#17316

PiperOrigin-RevId: 506932307
Change-Id: I06a710445ab79e98fc3794b7eb81a9abd61dce69
hvadehra pushed a commit that referenced this issue Feb 14, 2023
#17316

PiperOrigin-RevId: 508700297
Change-Id: Iced40505b32aa4e6aa32fa969ef0c75224f2e5bc
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

4 participants