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

Expose full compile time jars for Java targets in Skylark #3528

Closed
iirina opened this issue Aug 9, 2017 · 62 comments
Closed

Expose full compile time jars for Java targets in Skylark #3528

iirina opened this issue Aug 9, 2017 · 62 comments
Assignees

Comments

@iirina
Copy link
Contributor

iirina commented Aug 9, 2017

Right now there is no way to retrieve the full jars (only ijars/hjars) used at compile time for a Java target in Skylark. The best way would be to expose it on java_common.provider (JavaInfo).

@ittaiz
Copy link
Member

ittaiz commented Aug 9, 2017

Thanks!
The use-case this surfaced from is enabling to toggle on/off what a specific java_import target propagates out. How will java_common.provider help there?

@dslomov
Copy link
Contributor

dslomov commented Aug 9, 2017

java_import provides a java_common.provider, so if the latter gives you full jars you will be good to go (in Scala rules, use full jars instead of ijars)

@iirina
Copy link
Contributor Author

iirina commented Aug 9, 2017

Can you give a bit more details? Why do you need control over what java_import propagates out and how do you intend to use this?

@ittaiz
Copy link
Member

ittaiz commented Aug 9, 2017

@dslomov thing is that we want to use ijars in rules_scala but some (rough swag I'd say 30%) have macros which mean ijar doesn't work for them. I'd like to be able to say that for a specific Foo artifat ijar should be turned of. This is what we enable currently for source targets but we don't have it for external targets.

@iirina the need is to have the ability to declare for a specific java_import_external) target that ijar shouldn't be created or exposed. Then I'll use that to mark external dependencies which have macros.

The feature you list above sounds interesting for rules_scala's own usage for source targets but that is currently solved internally

@iirina
Copy link
Contributor Author

iirina commented Aug 9, 2017

I'd like to be able to say that for a specific Foo artifat ijar should be turned of.

Is Foo a java_import target or a scala target?

@iirina the need is to have the ability to declare for a specific java_import_external) target that ijar shouldn't be created or exposed. Then I'll use that to mark external dependencies which have macros.

So you would want something like the --use_ijars flag, but at target level? You don't want to create the ijars at all, not only not pass them along?

@dslomov
Copy link
Contributor

dslomov commented Aug 9, 2017

@dslomov thing is that we want to use ijars in rules_scala but some (rough swag I'd say 30%) have macros which mean ijar doesn't work for them. I'd like to be able to say that for a specific Foo artifat ijar should be turned of. This is what we enable currently for source targets but we don't have it for external targets.

I am not sure what you mean by "for a specific Foo artifact ijar should be turned off".
java_import will give you (through the java_common.provider) both ijar and a full jar. What you do with those in Scala rules is up to you. You can then pass the full jar, not the ijar, as input to the scalac.
If a particular ijar is never used, it will not be built.
Am I missing something?

@ittaiz
Copy link
Member

ittaiz commented Aug 9, 2017

@iirina Yes, Foo is a maven import target. Use ijars at the target level sounds exactly what I need. I'd rather they not be created to save the time but I don't really care.

@dslomov the problem in your scenario is that the using target, which is where rules scala code runs, doesn't know if the originating dependency should be consumed as an ijar or regular jar. That is something the origin declares but in your suggestion I don't see any way for me to hint from java_import_external to rules_scala about it

@dslomov
Copy link
Contributor

dslomov commented Aug 9, 2017

@dslomov the problem in your scenario is that the using target, which is where rules scala code runs, doesn't know if the originating dependency should be consumed as an ijar or regular jar. That is something the origin declares but in your suggestion I don't see any way for me to hint from java_import_external to rules_scala about it

Full design of this is up to @iirina, but I expect there to be two fields on java_provider.common: one gives you ijars, the other gives you full jars. You use whichever you need.

@ittaiz
Copy link
Member

ittaiz commented Aug 9, 2017

But the information is in the declaration site not in the call site.
Another solution (which I don't particularly like) could involve the two fields you mention plus a third one saying something like targetAdvisedAgainstUseOfIjar

@dslomov
Copy link
Contributor

dslomov commented Aug 9, 2017

But the information is in the declaration site not in the call site.

I am sorry, I do not understand. Can you elaborate? Maybe an example will help.
Clearly every Java rule that produces compilation results has both the full jar and the ijar. So "declaration site" has both and can give the "call site" both - there is no information loss.

@ittaiz
Copy link
Member

ittaiz commented Aug 9, 2017

The information is "does the target have macros" which means "don't use my ijar since you won't compile". That information is known to the origin not to the destination. Is it clear? If not I'll try to come up with an example

@dslomov
Copy link
Contributor

dslomov commented Aug 9, 2017

Hmm. Java does not know what macros are, so that information does not belong in java_common.provider. java_import does not know Scala either, so it cannot provide that information.

Actually how Scala rules know that the source has macros?

@ittaiz
Copy link
Member

ittaiz commented Aug 10, 2017

The user knows. My suggestion is to expose a Boolean property saying something like "turn_ijars_off". The docs will say something like: "some jvm languages cannot use ijars since it breaks compilation".

The correct solution would be to fix ijar (#632) but that can take months and I don't see this being a priority in the next year or so.

@dslomov
Copy link
Contributor

dslomov commented Aug 10, 2017

The user knows. My suggestion is to expose a Boolean property saying something like "turn_ijars_off". The docs will say something like: "some jvm languages cannot use ijars since it breaks compilation".

A boolean property on what? On java_import?
Assuming you mean "add a boolean attribute 'turn_ijars_off on java_import and other java_* targets".
Well there are 2 cases:

  • whoever writes the corresponding java_* target knows that it will be used from scala. In that case, they can write a corresponding scala_* target instead (or smth like scala_import alongside the java_* target)
  • it is not known that java_* target will be used from scala. Then of course the author of that java_* target has no idea whether ijars should be on or off. E.g. Kotlin needs ijars turned off in different circumstances.

As a specific example, let's say maven_jar generates java_imports. Where would maven_jar get information about whether a specific jar from maven has scala macros?

Basically what you are asking for is a property 'i_have_scala_macros' on 'java_*' targets. This is not a good design. Next JVM language comes along and we add 'i_have_frotlin_flux_capacitors' :)

I see several solutions, all not great:

  • give up on ijars completely for scala rules
  • provide a way to annotate a set of deps as "macro-free" on scala level. You can have a rule scala_import_from_java(name = ..., deps = ....) that says: none of deps have any Scala macros. If a scala_* rule depends on this, it will use ijars for all deps
  • vice versa, add scala_java_macros_library rule similar to java_plugin. If scala_rule wants to depend on a java target and use macros defined in them, it should wrap it in scala_java_macros_library

@dslomov
Copy link
Contributor

dslomov commented Aug 10, 2017

(and I see that in #632 this has been discussed as well; and you are actually adding scala_import; I think we have too many threads on the same topic that are talking past each other; I think we first need a discussion on design of scala rules (w.r.t. ijar handling) and then after we converge on a solution we can talk about features from Bazel that scala rules need. Apparently the specific feature requested in the present feature request will still be needed no matter what we decide though)

@ittaiz
Copy link
Member

ittaiz commented Aug 10, 2017

I built a basic scala_import which utilizes maven_jar but then I had many integration issues with intellij. Add to that my need for source downloading, licences, sha256 and we'll get to me duplicating all of @jart's work in java_import_external and all of the features in java_import itself.

This sounds like a lot of duplication and fragmentation over ijars which obviously have their limits and I'd guess that most JVM languages (other than java) will have caveats with it since they try to piggyback on the bytecode.

If the composition story of rules and workspace rules would have been more advanced then maybe I could have built on top of these rules and just propagated my own boolean property but this just isn't doable in the current setting.

wdyt about exposing java_import under an additional name as jvm_import and have there the boolean property? I think that if you look at java_import as a way to import jvm jars then the above boolean makes sense. I think it's reasonable to add it to java_import but if you think it's too misleading maybe adding jvm_import (which reuses the implementation of java_import) would be better

@dslomov
Copy link
Contributor

dslomov commented Aug 10, 2017

wdyt about exposing java_import under an additional name as jvm_import and have there the boolean property? I think that if you look at java_import as a way to import jvm jars then the above boolean makes sense. I think it's reasonable to add it to java_import but if you think it's too misleading maybe adding jvm_import (which reuses the implementation of java_import) would be better

I think you want to cut your cake and eat it too.
jvm_import has the same problem: what about frotlin and its flux capacitors? In other words, whether ijars are consumable downstream or not depends on the consumer (Scala in one case, Kotlin in another). If ijars as a concept are problematic for Scala, principled solution is to not use them!

Let's talk about java_import_external though. We do not have any compositional framework for repository rules and rules, but it does not mean we cannot create a ad-hoc one. Here is a wild idea:
what if we have jvm_import_external that would be a "rule factory" that will be parametrized with a template for a rule? Then

   java_import_external = jvm_import_external("java_import(name = '%{name}', ...)", extra_attrs = {})
   scala_import_external =  jvm_import_external("scala_import(name = '%{name}', ...)")
   scala_pure_java_import_external =  jvm_import_external("scala_import(name = '%s', macros = False...)")
   frotlin_import_external = <you get the idea>

@ittaiz
Copy link
Member

ittaiz commented Aug 14, 2017

Sorry for the delay, am on vacation and with limited access.
I also thought about this but dismissed it since re-implementing java_import seemed to be a lot of work as well. What I now thought about is to actually make a scala_importy which will call native.java_import and will take its java provider and replace it with a new one using the information Irina wants to expose with this issue.
I think this with the above solution for the repository rule can give me the most code reuse I can get for now. WDYT?

@ittaiz
Copy link
Member

ittaiz commented Aug 20, 2017

@iirina @dslomov wdyt about the composition using native.java_import?

@iirina
Copy link
Contributor Author

iirina commented Aug 21, 2017

I wrote a little pseudocode to describe what you said, is this what you were thinking about?

java_import(name = "real_import",...)
java_import_wrapper(name = "wrapper_import", deps = [":real_import"], ...)
def _java_import_wrapper_impl(ctx):
  deps_providers = [dep[java_common.provider] for dep in ctx.attr.deps]
  deps_providers_without_ijars = [remove_ijar(dep_provider) for dep_provider in deps_providers]
  return java_common.merge(deps_providers_without_ijars)

def remove_ijar(provider):
  return java_common.create_provider(compile_time_jars=provider.compile_time_full_jars, ....)

@ittaiz
Copy link
Member

ittaiz commented Aug 22, 2017

Not really since this strips out all ijars. I want only for specific deps.

scala_import = rule(
  implementation=_scala_import_impl,
  attrs= java_import_attrs + {
      "use_ijar": attr.bool(default=True)
      },
)

def _scala_import_impl(ctx):
  native_import = native.java_import(use attrs from ctx)
  if (ctx.attr.use_ijar):
      provider = native_import[java_common.provider]
  else:
      provider = remove_ijar(native_import[java_common.provider])
  deps_providers = [dep[java_common.provider] for dep in ctx.attr.deps] + [provider]
  return java_common.merge(deps_providers)

def remove_ijar(provider):
  return java_common.create_provider(compile_time_jars=provider.compile_time_full_jars, ....)

And then for dependency X which has scala macros I'll define use_ijar as False.
Is this clearer?

@iirina
Copy link
Contributor Author

iirina commented Aug 22, 2017

Yes, only you cannot use native.java_import in a rule implementation (in analysis phase). You can only use it in macros (loading phase). Unfortunately the only solution I see here is to declare both a java_import and a scala_import, where the latter depends somehow on the former and retrieves whatever it needs from it:

def _scala_import_impl(ctx):
  native_import = ctx.attr.native_import
  if (ctx.attr.use_ijars):
      provider = native_import[java_common.provider]
  else:
      provider = remove_ijar(native_import[java_common.provider])
  deps_providers = [dep[java_common.provider] for dep in ctx.attr.deps] + [provider]
  return java_common.merge(deps_providers)

scala_import = rule(
  implementation = _scala_import_impl,
  attrs = {
      "use_ijars": attr.bool(default = True),
      "native_import": attr.label(),
      "deps": attr.label_list()
  }
)

And the BUILD file would look like this:

java_import(
  name = "whatever_import",
  jars = ["a.jar"]
)

scala_import(
  name = "whatever",
  native_import = ":whatever_import",
  use_ijars = 1
)

@ittaiz
Copy link
Member

ittaiz commented Aug 22, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Aug 22, 2017

Yup, that sounds like a plan!

@ittaiz
Copy link
Member

ittaiz commented Aug 22, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Aug 22, 2017

To expose the full jars?

@ittaiz
Copy link
Member

ittaiz commented Aug 22, 2017

If I understood you correctly compile_time_full_jars isn't exposed right? That's the only missing piece for the above puzzle right? (other than changing java_import_external to allow customization of the library rule)

@ittaiz
Copy link
Member

ittaiz commented Sep 6, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Sep 8, 2017

Next week should be at HEAD.

@ittaiz
Copy link
Member

ittaiz commented Sep 8, 2017

awesome, thanks! Any chance you know when is 0.6 targeted for?

@iirina
Copy link
Contributor Author

iirina commented Sep 8, 2017

No, probably when we get Bazel CI green again :) You can watch the release issue and ping the assignee for more details.

@ittaiz
Copy link
Member

ittaiz commented Sep 8, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Sep 8, 2017

I'm having a bit of a problem deciding how to create the API and its meaning.

Exposing full compile time jars is easy, straight-forward: target[JavaInfo].full_compile_time_jars. Done.

But for this to be hermetic, the user also should have the possibility of specifying full compile time jars when creating a provider. Here I have a problem.

Let's say we pass the compile time jars like this: java_common.create_provider(compile_time_jars = [...], full_compile_time_jars = [...], ...)
This looks good at first sight, but has some problems. compile_time_jars could store until now both ijars and full jars, it didn't matter as long as it was meant for compilation; I would like to keep it that way. However, it now raises multiple questions: what is the difference between compile_time_jars and full_compile_time_jars? can there be the same values in one and the other or are they mutually exclusive?

@ittaiz Does the following documentation make sense? Does it meet your needs? Do you thing it's easy to understand? Do you have any better proposal?

java_common.create_provider#full_compile_time_jars: The full (normal) jars that are meant to be on the compilation classpath. It should contain the full built jars of the corresponding ijars passed through java_common.create_provider#compile_time_jars (if the case) and any other full jars passed to the same field. If only full jars are specified through java_common.create_provider#compile_time_jars, then the same jars should be specified through this field as well.
It is not used by java_common.compile (i.e. they will not get added on the compilation classpath when java_common.compile is called). For that purpose, make sure to set java_common.create_provider#compile_time_jars accordingly. This gives the user the power to choose how and when to use ijars.

One drawback I see is that it puts more burden on the user to also add the full time jars as compile_time_jars, but only in some cases. Another is that It is error-prone as the user could easily put both the ijar and the full jar on the compilation classpath.

What is your user opinion about this? Thanks!

@ittaiz
Copy link
Member

ittaiz commented Sep 11, 2017

I agree it's a tricky one.
It feels to me like we should have a builder here. I don't know if it's pythonic /skylark idiomatic but it sounds like we want to keep the current method simple (maybe without full_compile_time_jars) and add a builder which has richer semantics.
This way we have a few design options:

  1. Expose with_compile_time_jar(ijar,full_jar) alongisde with_compile_time_jar(full_jar) where the latter means the full_jar is used for compilation and exposed in the provider.
  2. Expose with_compile_time_jar(jar,should_call_ijar_on_it=true) where you call ijar for the user on the needed jars.

WDYT?

BTW It's important IMO to notice that the need to expose the full compile time jars is not only the aspect I mentioned above but also for IDEs. Intellij currently has an agreement with each of the jvm rules about which attribute it should expose this from but using JavaInfo should be standard

@iirina
Copy link
Contributor Author

iirina commented Sep 11, 2017

Yes, I've also thought of a builder, but I don't think it aligns with the Skylark way. At the same time I want this object to be sealed in as fast as possible, and just be used for passing information. And keep everything simple.

Expose with_compile_time_jar(ijar,full_jar) alongisde with_compile_time_jar(full_jar) where the latter means the full_jar is used for compilation and exposed in the provider.

This make sense, but I think it's very Java-like. Besides, the main problem I see here is that you need another data structure to encapsulate what a compile time jar is (with or without full jar/ijar), which brings up more problems and complexity.

Expose with_compile_time_jar(jar,should_call_ijar_on_it=true) where you call ijar for the user on the needed jars.

For this extent we already exposed ijar on java_common, so the user could manipulate however they desire their jars.

This is what I came up with after some thinking:

java_common.create_provider(
  compile_jars = [...],
  runtime_jars = [...],
  source_jars = [...],
  deps = [...], # this should be a list/set of JavaInfo, to encapsulate the transitive information
  declare_full_jars = True/False,  # by default False
  full_compile_jars = [...], # only set if declare_full_jars is True
)

This presents 2 benefits:

  • doesn't repeat each field for transitive jars as well, but the information is retrieved from the given dependencies and java_common.create_provider will treat it as transitive; so if you want to create a provider with compile_jars A.jar, runtime jars B.jar and transitive runtime jars C.jar, then you create the following providers:
dep_provider = java_common.create_provider(runtime_jars = [C.jar])
provider = java_common.create_provider(
    compile_jars = [A.jar],
    runtime_jars = [B.jar], 
    deps = [dep_provider]
)

This is more readable IMO, and I think it would reduce the burden of understanding transitive jars.

  • the user has to specifically set declare_full_jars to True and only then can set the full compile time jars. I think this brings the attention to what full jars are and the cases where they should be used.

BTW It's important IMO to notice that the need to expose the full compile time jars is not only the aspect I mentioned above but also for IDEs. Intellij currently has an agreement with each of the jvm rules about which attribute it should expose this from but using JavaInfo should be standard

Thanks for bringing up the importance of full jars for IDEs. I'm not familiar with how they work with jvm rules. What kind of agreement do they have? In terms of who else needs the compile time full jars, I am aware of Kotlin also needing them as depoendencies.

@natansil
Copy link
Contributor

doesn't repeat each field for transitive jars as well, but the information is retrieved from the given dependencies and java_common.create_provider will treat it as transitive

sounds like a good idea to avoid repeating each field for transitive jars as well.

the user has to specifically set declare_full_jars to True and only then can set the full compile time jars. I think this brings the attention to what full jars are and the cases where they should be used.

It's still unclear to me (and so maybe for other users of the api), what happens if the user sets this field (full_compile_jars ) or not.

Just to be sure I understand, compile_jars , can contain either ijars or full jars that may or may not be part of a dependent's compilation classpath
full_compile_jars must NOT contain ijars and may or may not be part of any dependent's compilation classpath.
The same full jars may appear in both fields.

e.g. when depending on java_import in scala, full_compile_jars are needed, and compile_jars are not.
But usually, for faster compilation times, compile_jars will be used.

Am I correct?

If usage of full_compile_jars will be the exception of the rule, than the proposed api seems appropriate....

@ittaiz
Copy link
Member

ittaiz commented Sep 12, 2017

First I'd like to say I like the move to deps as it's another step on aligning jvm rules on this provider.
I think that we need to recap what are the problems we're trying to solve.
As far as I understand the problems are:

  1. Have the ability to depend on external jars without ijar. This is doable today and is even easier thanks to your addition of java_common.ijar since one can write a rule that gets the "raw" jars from the repository rule and adds them to the compile_jars or only adds their ijar to the compile_jars.
  2. Have the ability to get the full outputs of a target whether or not others depend on your ijar. Use-cases I know of are:
    1. Maven integration- like I said we have an aspect which collects the outputs so that it can upload them to maven.
    2. IDEs- @chaoren does intellij (as an example) have use-cases where it needs the full jars and not only the ijars? I think so but not sure what are they.

The problem with the API as I see it is that it's not clear to the user why should they declare full jars?
The advantage with the additional data-structure is that the semantics are similar (i.e. there's just one place to give compile jars and some of them are just not ijarred) while still giving Bazel the knowledge of the full jars (assuming that create_provider calls the ijar itself).

@iirina
Copy link
Contributor Author

iirina commented Sep 13, 2017

After a meeting with @ittaiz and @lberki yesterday, we agreed on the following API:

java_common.create_provider(
   compile_jars = [...],
   use_ijar = True/False, # defaults to False
   runtime_jars = [...],
   source_jars = [...], 
   deps = [...]  # A list of JavaInfo to encapsulate the transitive information
)

which is great. We now distinguish 2 cases:

  • use_ijar is True: ijar will be invoked on all compile_jars (which have to be normal jars). The resulting ijars will be add as compile time jars and the regular jars will be stored in the provider as full compile time jars.
  • use_ijar is False: compile_jars will be added as compile time jars and the returned provider will have an empty full_compile_jars.

The only inconvenience is that to use ijar, the method also needs access to the rule context (to register the ijar actions) and to the java toolchain (to retrieve the ijar tool). We end up with this API:

java_common.create_provider(
   compile_jars = [...],
   use_ijar = True/False, # defaults to False
   ctx = ctx, # only required when use_ijar is True
   java_toolchain = ctx.attr._java_toolchain, # only required when use_ijar is True
   runtime_jars = [...],
   source_jars = [...], 
   deps = [...]  # A list of JavaInfo to encapsulate the transitive information
)

I am happy with this approach. It's consistent with java_common.compile. I find it straight forward and easy to understand. If there are no strong objections, I'll submit this in the next couple of days.

@dslomov
Copy link
Contributor

dslomov commented Sep 13, 2017

Can you replace ctx with ctx.actions? (https://docs.bazel.build/versions/master/skylark/lib/actions.html)

@iirina
Copy link
Contributor Author

iirina commented Sep 13, 2017

@dslomov Yes, I managed to replace ctx with ctx.actions.

@ittaiz
Copy link
Member

ittaiz commented Sep 13, 2017 via email

@chaoren
Copy link
Contributor

chaoren commented Sep 13, 2017

IDEs- @chaoren does intellij (as an example) have use-cases where it needs the full jars and not only the ijars? I think so but not sure what are they.

Couple of use cases:

  1. If the sources are not available, attaching the decompiled full jar would be more helpful than the ijar.
  2. For Android, we sometimes want the resource ID values from the R.class.

@ittaiz
Copy link
Member

ittaiz commented Sep 13, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Sep 14, 2017

(sorry I forgot to press comment when I first wrote this -_-)

Thanks @chaoren for the use cases.

@ittaiz my answers are below:

  1. Maybe useIjar should default to True? That is the normal use case.

I chose the default value as False since in that case you don't need to set anything else up. When it's True, you additionally have to set ctx/actions and the toolchain.

  1. Can we have full_compile_jars always non empty? When I use the target I don't know if the original target used ijar or not

OK. Let's assume that compile_jars are full jars when use_ijar is False.

  1. If I pass a dep with neverlink this should be fine right? If I understand it correctly it should contribute to the compile deps transitively (strict deps story) and won't contribute to the runtime deps

For native rules yes. Unfortunately java_common.compile doesn't consider neverlink right now :(

  1. Is the reference to ctx.actions mean that I'll need to pass ctx.actions
    in? I think the symmetry with java_common.compile is valuable and I
    wouldn't want to lose it.

Dmitry is actually working on making it possible to pass ctx.actions to java_common.compile as well, so they will be consistent.

@ittaiz
Copy link
Member

ittaiz commented Sep 14, 2017

I chose the default value as False since in that case you don't need to set anything else up. When it's True, you additionally have to set ctx/actions and the toolchain.

Ok, aren't you concerned this will push people to not use ijars? this is exactly what @lberki was afraid of

OK. Let's assume that compile_jars are full jars when use_ijar is False.

Awesome, thanks.

For native rules yes. Unfortunately java_common.compile doesn't consider neverlink right now :(

ok, I think we'll need that fixes. Shall I open a new issue for this?

Dmitry is actually working on making it possible to pass ctx.actions to java_common.compile as well, so they will be consistent.

sounds great

@iirina
Copy link
Contributor Author

iirina commented Sep 14, 2017

Ok, aren't you concerned this will push people to not use ijars? this is exactly what @lberki was afraid of

Yeah, that's also true. I'll consider this.

ok, I think we'll need that fixes. Shall I open a new issue for this?

Yes, thanks!

Thanks everyone for participating, it was very helpful!

@ittaiz
Copy link
Member

ittaiz commented Sep 18, 2017 via email

@iirina
Copy link
Contributor Author

iirina commented Sep 18, 2017

I'll ask tomorrow if we can cherry pick this. I'm waiting for the change to be submitted internally, should be on GH tomorrow.

@ittaiz
Copy link
Member

ittaiz commented Sep 18, 2017 via email

bazel-io pushed a commit that referenced this issue May 29, 2018
This PR copies "upstream" a change made to java_import_external in`rules_scala` (see [PR](bazelbuild/rules_scala#473))

This change was originally proposed by @dslomov [here](#3528) (search for 'jvm_import_external')

java_import_external was changed to `jvm_import_external` 'template like' rule + `java_import_external` macro in order to allow for other jvm languages (e.g. scala and kotlin) to utilize the 'import_external' functionality without copying the boiler plate again and again.

This has already been used in `rules_scala` with the introduction of `scala_import_external` and `scala_maven_import_external`

In addition to the `import rule name`, `jvm_import_external` can also be called with custom attributes needed by the underlying import rules, as well as a custom load statement.

`java_import_external` is used as a macro in rules_scala to make sure it's still functioning properly after the change.

`jvm_maven_import_external` exposes maven artifact terminology.
This will also allow to create a `maven_import_external` macro that will delegate to `jvm_maven_import_external` with a `maven_import` rule which will have `MavenCoordinates` Provider as discussed [here](#4654)

Closes #5068.

PiperOrigin-RevId: 198398621
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants