bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.19k stars 4.06k forks source link

java_binary: No way to provide dynamic manifest file content #2009

Open davido opened 8 years ago

davido commented 8 years ago

There seems to be no way to provide dynamic content, like build info into MANIFEST.MF file in java_binary rule. Only deploy_manifest_lines is supported, where only list of static strings can be provided.

In Buck there is manifest_file attribute in java_binary rule where a label can be provided.

What people normally want to achieve is to add build version into manifest file. This can be done (since Bazel 0.3.2) with --workspace_status_command=./tools/workspace-status.sh option in .bazelrc file and this genrule:

genrule(
    name = "gerrit_plugin_version",
    stamp = 1,
    cmd = ("grep STABLE_BUILD_GIT_LABEL < bazel-out/stable-status.txt | " +
        "cut -d ' ' -f 2 > $@"),
    outs = ['gerrit_plugin_version.txt'],
)

However, unless I'm missing something obvious, there is no way to merge this version into MANIFEST.MF in java_binary rule:

java_binary(
    name = "gerrit_foo_plugin",
    main_class = "Dummy",
    # TODO(davido): Move this part into manifest
    # when Bazel supports manifest file merging
    resources = [":gerrit_plugin_version.txt"],
    runtime_deps = ["@guava//jar"],
    # Only static key value pairs can be added here.
    deploy_manifest_lines = ["bar: baz"],
)

gerrit_plugin_version.txt is included in the root of the plugin JAR:

cat gerrit_plugin_version.txt
5a6f4fd-dirty

With the above rules we have this content of META-INF/MANIFEST.MF:

  $ bazel build gerrit_foo_plugin_deploy.jar
  INFO: Found 1 target...
Target //:gerrit_foo_plugin_deploy.jar up-to-date:
  bazel-bin/gerrit_foo_plugin_deploy.jar
INFO: Elapsed time: 0.551s, Critical Path: 0.46s

  $ jar -xf bazel-bin/gerrit_foo_plugin_deploy.jar META-INF/MANIFEST.MF
  $ cat META-INF/MANIFEST.MF
Manifest-Version: 1.0
bar: baz
Created-By: blaze-singlejar
Main-Class: Dummy

What I would like to be able to have instead, is:

$ cat META-INF/MANIFEST.MF
[...]
  Implementation-Version: 5a6f4fd-dirty
laszlocsomor commented 8 years ago

I also can't find a way to do this, so I declare this a missing feature.

@ulfjack : Can we add support for this feature? Summary: @davido wants to write workspace status data into the META-INF/MANIFEST.MF in a deploy jar. I think we could add a label attribute to java_binary where the user can specify a custom MANIFEST.MF to achieve this. Do you have anything against that?

ulfjack commented 8 years ago

Fewer features is better. I think there are two alternatives here:

1) Since the manifest is a file and building files is what the build system does, we could deprecate deploy_manifest_lines, and instead add an attribute deploy_manifest which takes a single file. However, we'd need to specify how Main-Class and Created-By get set (or not set), plu s any other values (e.g., for coverage). That'd allow the genrule with stamp=1 approach to work. I had a quick look through our internal code base, and people would like to get stamping information into their deploy manifests as well.

2) We could always write the stamping information into the deploy manifest. However, I'm not sure we can do that for non-deploy jars (I think that's the reason why stamping information is in a text file right now).

laszlocsomor commented 8 years ago

Fewer features is better. I think there are two alternatives here:

+1

1) Since the manifest is a file and building files is what the build system does, we could deprecate deploy_manifest_lines, and instead add an attribute deploy_manifest which takes a single file. However, we'd need to specify how Main-Class and Created-By get set (or not set), plu s any other values (e.g., for coverage). That'd allow the genrule with stamp=1 approach to work. I had a quick look through our internal code base, and people would like to get stamping information into their deploy manifests as well.

How about adding another attribute combine_manifest = {append|replace}?

2) We could always write the stamping information into the deploy manifest. However, I'm not sure we can do that for non-deploy jars (I think that's the reason why stamping information is in a text file right now).

I don't know why that matters, could you explain?

ulfjack commented 8 years ago

How about adding another attribute combine_manifest = {append|replace}?

I'm not sure what the semantics of those values are. I am not sure that having different modes is necessary, and we need to talk about how we'd add the aforementioned values to the manifest in the first place.

I don't know why that matters, could you explain?

The code in the deploy jar is the same as in the binary, just that the binary consists of many jar files. Ideally, we use the same mechanism in both cases, otherwise we'd need to detect if we're running from a deploy jar or not. I think it's more 'consistent' with how Java generally works to put the stamping information into the manifest instead of a separate .txt file, but it's not clear to me how that'd work for non-deploy jars.

laszlocsomor commented 8 years ago

I'm not sure what the semantics of those values are. I am not sure that having different modes is necessary, and we need to talk about how we'd add the aforementioned values to the manifest in the first place.

My proposal for the semantics is, if the value is append then bazel generates the default MANIFEST.MF with the usual Created-By and Main-Class fields, and appends the user-provided manifest to that, yielding the final manifest that we pack in the jar. If the value is replace then we don't generate a default manifest but use the user-provided manifest as-is (and maybe check that it contains these fields).

The code in the deploy jar is the same as in the binary, just that the binary consists of many jar files. Ideally, we use the same mechanism in both cases, otherwise we'd need to detect if we're running from a deploy jar or not. I think it's more 'consistent' with how Java generally works to put the stamping information into the manifest instead of a separate .txt file, but it's not clear to me how that'd work for non-deploy jars.

We could pack the same manifest file in both, no? I'm assuming we have control over what goes into the jar file.

ulfjack commented 7 years ago

It's not clear how the code finds the right manifest in the non-deploy case.

I think we'd need to look at an example or two to figure out what the right semantics should be here.

or-shachar commented 7 years ago

+1 to this issue. We actually more interested in controlling the content of the manifest in the non-deploy use case. I'm not sure whether I should open a new issue just on that or just to ask to change the title of this one to be more general to all java_rules.

GinFungYJF commented 6 years ago

Any update? Interested in controlling the content of the manifest in the non-deploy use case, too.

laszlocsomor commented 6 years ago

@iirina : What do you think of this feature request?

sgowroji commented 1 year ago

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

davido commented 1 year ago

@bazelbuild/triage not stale.