Open cushon opened 6 years ago
johnynek
commented
6 years ago Yep. That’s an issue. I think bazel May be able to handle this by possibly hitching to its handling of annotation processors, but maybe there are some speed bumps there.
In the worst case we can write some rule that re-exports the runtime path as a compile time for use in macros.
johnynek
commented
6 years ago As described in the linked bazel issue above this can be improved even more with a scalainfo provider. We can’t totally migrate to new style providers yet because intellij does not support JavaInfo fully yet as far as I know.
I’d rather wait to solve this until we can do it in a migration to only new style providers.
johnynek
commented
6 years ago Note: the fix of repairing the classpath issue can be done now, at the cost of java dependencies seeing the full classpath. We could do that and then narrow the classpath when we move to only using providers.
sdtwigg
commented
6 years ago Yeah, this is a tad unfortunate. I ran into a variant of this issue when trying to get scalac plugins to work on a really old variant of the rules.
My main regret is that the macro implementations should generally only be using a small subset of the total dependencies (as they are really only fiddling with ASTs) and so exposing all dependencies is a bit pessimistic.
I suppose people could mitigate the impact of this by only having their scala_macro_library rules contain the macro implementations. (I can test this myself later but I don't think the macro invocations like def blah = macro MacroImpl need to be a scala_macro_library?)
On Sat, Apr 21, 2018, 4:48 PM P. Oscar Boykin notifications@github.com wrote:
Note: the fix of repairing the classpath issue can be done now, at the cost of java dependencies seeing the full classpath. We could do that and then narrow the classpath when we move to only using providers.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/445#issuecomment-383341856, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9_-FNjAoiMWfha0Kt_S20DADROK9HGks5tq8VMgaJpZM4Sw10C .
cushon
commented
6 years ago Note: the fix of repairing the classpath issue can be done now, at the cost of java dependencies seeing the full classpath.
Is the idea here to have scala_macro_library add its runtime dependencies to the compile-time classpath for downstream targets? I don't think that solves the problem, because there's a dependency ordering issue.
If a some library depends on, say, apache commons directly and via a scala_macro_library, the scala_macro_library can't ensure that the runtime jar appears before the ijar on the compile-time classpath of the downstream library. ScalaInfo.macro_classpath addresses that by keeping the macro's dependencies separate, and making sure they appears before non-macro deps on the downstream library's compilation classpath.
abhandaru
commented
5 years ago Also having this issue when trying to use with TableQueryMacroImpl in Slick.
Artifact in Maven2: com.typesafe.slick:slick_2.12:3.2.3
The workaround suggested by the original poster does work for me, just the --nouse_ijars was sufficient. I am very new to Bazel so not quite sure what all the negative repercussions are for using this hack.
Hoping to see an update!
ittaiz
commented
5 years ago @cushon @johnynek I've read the thread but got lost a bit.
Is there something we can do other than --nouse_ijar? I can see if we can prioritize some time for this if I have an idea on what needs to be done
johnynek
commented
5 years ago Yes, we can do something better.
I think the right solution is to rexport the full transitive runtime classpath as the compile classpath for scala_macro_libraries and to educate users that scala jars need to be imported with scala_import.
https://github.com/johnynek/bazel-deps
does this, for instance.
One problem with bazel is that there are many ways you could consume external maven dependencies. It is hard to give someone feedback when they just describe something not working without knowing how they are depending on the code.
We should add a local test showing two issues:
ioleo
commented
5 years ago It seems I'm having exacly this issue when depending on com.sksamuel.avro4s:avro4s-macros_2.11:1.7.0. The --nouse_ijar workaround works, but I would like to disable ijars only for this artifact (not the whole build).
I've tried using scala_import and filegroup as suggested here, but it seems I'm still missing something, I can't get it working. Is there anyone who can share a working example disabling ijar generation for just one artifact?
johnynek
commented
5 years ago @ioleo can you post the stack trace you get and the scala_import you used?
ioleo
commented
5 years ago @johnynek So, I've reverted to my original code already:
load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library", "scala_test")
load("//bzl-includes:iqvia_scala_opts.bzl", "SCALAC_JVM_FLAGS", "SCALAC_OPTS", "SCALAC_PLUGINS")
_3RDPARTY_DEPS = [
"//3rdparty/jvm/ca/mrvisser:sealerate",
"//3rdparty/jvm/com/chuusai:shapeless",
"//3rdparty/jvm/com/fasterxml/jackson/core:jackson_core",
"//3rdparty/jvm/com/fasterxml/jackson/core:jackson_databind",
"//3rdparty/jvm/com/sksamuel/avro4s:avro4s_core",
"//3rdparty/jvm/com/sksamuel/avro4s:avro4s_macros",
"//3rdparty/jvm/com/typesafe/play:play_functional",
"//3rdparty/jvm/com/typesafe/play:play_json",
"//3rdparty/jvm/joda_time:joda_time",
"//3rdparty/jvm/org/apache/avro:avro",
]
_INTERNAL_DEPS = []
_ALL_DEPS = _3RDPARTY_DEPS + _INTERNAL_DEPS
scala_library(
name = "acme_common_api_lib",
srcs = glob(["src/main/scala/**/*.scala"]),
resources = glob(["src/main/resources/**/*"]),
scalacopts = SCALAC_OPTS,
plugins = SCALAC_PLUGINS,
scalac_jvm_flags = SCALAC_JVM_FLAGS,
visibility = [
"//visibility:public"
],
deps = _ALL_DEPS,
)which
bazel build //acme/common/api:acme_common_api_lib --nouse_ijars builds successfullybazel build //acme/common/api:acme_common_api_lib throws:
Exception in thread "main" java.lang.ClassFormatError: Absent Code attribute in method that is not native or abstract in class file org/apache/avro/Schema
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
at java.net.URLClassLoader.defineClass(URLClassLoader.java:468)
(...)As you can see I'm importing from 3rdparty directory, which is generated by your bazel-deps tool (kudos for that! <3), the relevant part of jvm-dependencies.yml being:
dependencies:
# ...
com.sksamuel.avro4s:
avro4s:
lang: scala
modules: [ "core", "macros" ]
version: "1.7.0"
# ... As for the scala_import I've tried replacing the auto-generated scala import with my own:
load("@io_bazel_rules_scala//scala:scala_import.bzl", "scala_import")
// ...
_3RDPARTY_DEPS = [
"//3rdparty/jvm/ca/mrvisser:sealerate",
"//3rdparty/jvm/com/chuusai:shapeless",
"//3rdparty/jvm/com/fasterxml/jackson/core:jackson_core",
"//3rdparty/jvm/com/fasterxml/jackson/core:jackson_databind",
"//3rdparty/jvm/com/sksamuel/avro4s:avro4s_core",
"//3rdparty/jvm/com/typesafe/play:play_functional",
"//3rdparty/jvm/com/typesafe/play:play_json",
"//3rdparty/jvm/joda_time:joda_time",
"//3rdparty/jvm/org/apache/avro:avro",
":avro4s_macros",
]
// ...
scala_import(
name = "avro4s_macros",
jars = [
"//external:jar/com/sksamuel/avro4s/avro4s_macros_2_11"
],
visibility = [
"//visibility:public"
]
)
// ...The only diffrence beuing replacing in _3RDPARTY_DEPS line:
"//3rdparty/jvm/com/sksamuel/avro4s:avro4s_macros",
with ":avro4s_macros", pointing to scala import below.
However this leads to the same build error.
About the filegroup suggested solution as far as I understand it's when I incorporate the jar in my own repo (which I don't want here --> I want to reference maven artifact).
Please forgive any misconceptions, I'm just getting started with bazel.
johnynek
commented
5 years ago The class that is missing is: org/apache/avro/Schema
What jar is that in? It probably isn’t in a scala jar and that’s the issue. Macros can call non macro code even java code. In such cases now we have an issue because we need to remove the ijar and add the runtime jar.
A work around if you are using my bazel-deps tool is to say Avro is a scala/unmangled for the language. This will use scala import for avro which makes it not use ijar.
You really don’t need ijar for external dependencies since they don’t change often and when they do even the ijar likely changes. We should probably update bazel deps to not use ijar even for java dependencies. That would reduce these issues but still not totally solve them since we still can have this kind of problem for macro code defined inside the repo.
ioleo
commented
5 years ago @johnynek It's used by the avro4s-macro library. The dependency is added via sbt plugin.
I agree I don't need the IJAR for the external depepency. As far as I understand, using --nouse_ijars option disables IJARS completely for this build (so, also for my own code ~> forcing to rebuild downstream dependencies even if the API has not changed).
A work around if you are using my bazel-deps tool is to say Avro is a scala/unmangled for the language.
How do I do that?
johnynek
commented
5 years ago If you add the avro jar to your dependencies and give it a lang: scala/unmangled it should do it. That means we don't apply the _2.11 or _2.12 suffix, but use scala_import. That should fix it.
ioleo
commented
5 years ago I've tried both:
com.sksamuel.avro4s:
avro4s:
lang: scala/unmangled
modules: [ "core", "macros" ]
version: "1.7.0"and
com.sksamuel.avro4s:
avro4s-core:
lang: scala
version: "1.7.0"
avro4s-macros:
lang: scala/unmangled
version: "1.7.0"and got this error on running bazel-deps:
INFO: Analyzed target //:parse (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //src/scala/com/github/johnynek/bazel_deps:parseproject up-to-date:
bazel-bin/src/scala/com/github/johnynek/bazel_deps/parseproject
bazel-bin/src/scala/com/github/johnynek/bazel_deps/parseproject.jar
INFO: Elapsed time: 0.180s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
[ForkJoinPool-1-worker-1] ERROR bazel_deps.CoursierResolver - not found: /home/user/.ivy2/local/com.sksamuel.avro4s/avro4s-macros/1.7.0/ivys/ivy.xml
(...)
^^^ here the "not found" error repeats for every repository ^^^
(...)
[main] ERROR MakeDeps - resolution and sha collection failed
java.lang.RuntimeException: Failed to resolve dependencies
at com.github.johnynek.bazel_deps.CoursierResolver$$anonfun$buildGraph$1.apply(CoursierResolver.scala:256)
at com.github.johnynek.bazel_deps.CoursierResolver$$anonfun$buildGraph$1.apply(CoursierResolver.scala:251)
(...)The problem is obviously missing scala version. com.sksamuel.avro4s:avro4s-macros:1.7.0 does not exist
[user@work-IQV iqvia]$ coursier resolve com.sksamuel.avro4s:avro4s-macros:1.7.0
mirrorConfFiles=List(MirrorConfFile(/home/user/.config/coursier/mirror.properties, true))
allMirrors0=List()
Resolution error: Error downloading com.sksamuel.avro4s:avro4s-macros:1.7.0
not found: /home/user/.ivy2/local/com.sksamuel.avro4s/avro4s-macros/1.7.0/ivys/ivy.xml
not found: https://repo1.maven.org/maven2/com/sksamuel/avro4s/avro4s-macros/1.7.0/avro4s-macros-1.7.0.pomYou need to put avro itself. Maybe .org.apache:avro? I don’t really know. You need to put the artifact with the missing class into scala_import.
ioleo
commented
5 years ago Hmm.. actually I already have the "//3rdparty/jvm/org/apache/avro:avro", dependency, which refers to jvm-dependencies.yaml line:
org.apache.avro:
avro:
lang: java
version: "1.9.0"And that version does contain the org/apache/avro/Schema class.
ioleo
commented
5 years ago @johnynek Alright, actually changing the above to:
org.apache.avro:
avro:
lang: scala/unmangled
version: "1.9.0"fixed the problem! Thanks for help <3! Now I can build without the --nouse_ijars argument.
Perhaps this case should get a paragraph in the readme/docs? It's not obvious whats going on here.
johnynek
commented
5 years ago What's going on is that this is forcing this jar to be seen as a scala_import instead of java_import.
It's not clear where to document it. This is a hack work around. I think it would be better to fix the scala_macro_library rule so we don't need the hack and let this issue be the documentation for the rare cases of people that hit the issue (which isn't very common, most users don't write macros, and those that do tend to depend on scala libraries).
gleyba
commented
4 years ago @johnynek
I think it would be better to fix the scala_macro_library rule so we don't need the hack and let this issue be the documentation for the rare cases of people that hit the issue
We are using rules_jvm_external https://github.com/bazelbuild/rules_jvm_external and we faced the same problem, --nouse_ijars did not help.
May be I can try to implement a fix for the scala_macro_library rule if you can give me a few tips on where to start, or put me in touch with someone who can.
Thanks!
johnynek
commented
4 years ago here is the code for scala_library:
note the parameter non_macro_lib. This if False if we are dealing with a macro, and True for a regular library.
This line here: https://github.com/bazelbuild/rules_scala/blob/26cf9b74fc46f1e9a970c97837447549ed7257b6/scala/private/rules/scala_library.bzl#L112
if we are a macro should use transitive_runtime_jars (I think that's the field name) for all downstream nodes that depend on this library. This is because compiletime for a macro can use the runtime classpath.
Now, there is a complication that is significant here: if you depend on A and B and both depend on C. And A is not a macro but B is, you will get both the ijar for C and the full jar for C on the classpath if we are not careful. scalac/javac won't error in the case that there are duplicate classnames on the classpath, but it will take the first one it finds, so you can't count on getting the right jar. Since not everything needs the compile jar, this will manifest as sometimes working and sometimes not.
The correct solution would be some pass that removes ijars that need to be removed. A way to do that would be to propagate all the ijars to delete in the scala provider. So, before you compile, you iterate all the compile jars, but remove any that are in the deleted set (which have been replaced by full jars).
I think that will cover it. I'm happy to help you work through this problem. I think a good first step is writing test cases that show the bug, then implement the simple strategy that can duplicate the jars, then write a test that fails that shows the above bug about duplicate classes, then finally fix that with the solution that won't include duplicate jars.
ittaiz
commented
4 years ago Just wanted to add that we have a pending PR which refractors this repo significantly and that until I merge it in (couple of weeks) we probably won’t be merging other PRs to ease its merge.
If you’re interested it’s probably still a good idea to iterate on this since redoing it over the merged PR shouldn’t be that hard.
Finally I’m a bit concerned from the performance impact of the solution Oscar outlined but I’m not sure there is an alternative. We’ll need to discuss this over concrete code.
On Mon, 18 Nov 2019 at 21:46 P. Oscar Boykin notifications@github.com wrote:
here is the code for scala_library:
note the parameter non_macro_lib. This if False if we are dealing with a macro, and True for a regular library.
This line here:
if we are a macro should use transitive_runtime_jars (I think that's the field name) for all downstream nodes that depend on this library. This is because compiletime for a macro can use the runtime classpath.
Now, there is a complication that is significant here: if you depend on A and B and both depend on C. And A is not a macro but B is, you will get both the ijar for C and the full jar for C on the classpath if we are not careful. scalac/javac won't error in the case that there are duplicate classnames on the classpath, but it will take the first one it finds, so you can't count on getting the right jar. Since not everything needs the compile jar, this will manifest as sometimes working and sometimes not.
The correct solution would be some pass that removes ijars that need to be removed. A way to do that would be to propagate all the ijars to delete in the scala provider. So, before you compile, you iterate all the compile jars, but remove any that are in the deleted set (which have been replaced by full jars).
I think that will cover it. I'm happy to help you work through this problem. I think a good first step is writing test cases that show the bug, then implement the simple strategy that can duplicate the jars, then write a test that fails that shows the above bug about duplicate classes, then finally fix that with the solution that won't include duplicate jars.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/445?email_source=notifications&email_token=AAKQQFY4XO2QLJWIOP5YI7DQULWJLA5CNFSM4EWDLUBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEELVWKI#issuecomment-555178793, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF546Y3AC6MMX2SLCWTQULWJLANCNFSM4EWDLUBA .
johnynek
commented
4 years ago related somewhat to #366
The following example shows that a macro that calls a Java library doesn't work. This is related to bazelbuild/bazel#632. The library jar for
:JavaLibon compile-time classpath only contains the compile-time API of that library, but the macro needs the complete implementation to be available at compile-time.Demo: https://gist.github.com/cushon/a97470ed69f61d966c94bcde1ed018f0
Setup:
Building
:HelloLibfails:Disabling ijar and header compilation works around the bug: