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

allow to deploy_manifest_lines in scala_binary and scala_test #166

Open
or-shachar opened this issue Mar 14, 2017 · 7 comments
Open

allow to deploy_manifest_lines in scala_binary and scala_test #166

or-shachar opened this issue Mar 14, 2017 · 7 comments

Comments

@or-shachar
Copy link
Contributor

Hi,

Java rules has this useful ability to add manifest lines: https://bazel.build/versions/master/docs/be/java.html#java_binary.deploy_manifest_lines

Our rules (scala_binary and scala_test) lack this property. We need to allow this behavior to happen in our rules as well.

@johnynek
Copy link
Member

I'm all for adding support for this (and generally anything in the java rules that we don't have now to maximize compatibility). It shouldn't be too hard to add. Would you like to take a stab?

@or-shachar
Copy link
Contributor Author

Sure thing!

@or-shachar
Copy link
Contributor Author

or-shachar commented Apr 6, 2017

BTW- why did bazel choose to limit this property only to "deploy_manifest"?
What if we want to modify the manifest of the clean lib jar?
I found this issue on bazel:
bazelbuild/bazel#2009

@johnynek
Copy link
Member

johnynek commented Apr 7, 2017

@or-shachar I don't know the answer.

@sdtwigg
Copy link
Contributor

sdtwigg commented Apr 7, 2017

java_library does not have a deploy jar.

Now, this does expose a different but related question of "why doesn't java_library have a deploy jar?". I don't know. I can think of a couple of candidates:

  1. As defined in java_binary, deploy jars are executable (both intrinsically via some 'cleverness' and via java -jar). A java_library deploy jar isn't really executable.

  2. There are potentially some build performance concerns with each library building a deploy jar when you have a large chain of libraries. Making the deploy jar requires linearizing the transitive runtime dependencies at each step. So, a chain of N library where next depends on previous will take O(N^2) time. (See motivation behind depsets for why you don't want to do this too much.) We might be fortunate that, since we never store that linearization in a provider, it doesn't have the O(N^2) memory use. However, we know that bazel won't build the deploy jar unless it is needed (since it isn't a default output to the rule). Thus, would have the O(N^2) memory problem if it is not discarding the action that would build the deploy jar.

I have been a little concerned about the performance of having deploy_jar for every scala_library as a scala repo grows. Ignoring java interop (which will be solved separately), where else have the library deploy_jars been helpful?

@paulocoutinhox
Copy link

It was solved?

I have the same problem:

ERROR: /Users/paulo/Developer/workspaces/cpp/djinni/src/BUILD:4:13: //src:djinni: no such attribute 'deploy_manifest_lines' in 'scala_binary' rule
ERROR: /Users/paulo/Developer/workspaces/cpp/djinni/generator/BUILD:1:8: Target '//src:djinni_deploy.jar' contains an error and its package is in error and referenced by '//generator:generator'
ERROR: Analysis of target '//generator:generator' failed; build aborted: Analysis failed

BUILD file:

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_binary")
load("//bzl:implementation_version.bzl", "IMPLEMENTATION_VERSION")

scala_binary(
    name = "djinni",
    srcs = glob(["source/**/*.scala"]),
    main_class = "djinni.Main",
    deps = [
        "@maven_djinni//:com_github_scopt_scopt_2_11",
        "@maven_djinni//:org_scala_lang_modules_scala_parser_combinators_2_11",
        "@maven_djinni//:org_yaml_snakeyaml",
    ],
    deploy_manifest_lines = [
        "Implementation-Version: " + IMPLEMENTATION_VERSION,
    ],
    visibility = ["//visibility:public"],
)

@mgosk
Copy link

mgosk commented May 24, 2024

Target solution should be configurable, but as workaround you can patch rules_scala.

image

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

No branches or pull requests

5 participants