Support -sources.jar source jars and sourcejars with MANIFEST.MF files in them #48
Conversation
jjudd
force-pushed
the
handle-source-jars-better
branch
from
0115f0b to
cb7a448
Compare
|
I also tried re-enabling ijars for Scala 3 and it looks like that is working now after all the recent work, so I bolted that commit onto this PR. |
| src_jars = [ | ||
| file | ||
| for file in ctx.files.srcs | ||
| if file.path.endswith("srcjar") or file.path.endswith("-sources.jar") or |
There was a problem hiding this comment.
is the .lower() not necessary for the file.path like it is for the file.extension?
There was a problem hiding this comment.
Good catch. I'll fix and see if I can add a test for it.
There was a problem hiding this comment.
I've updated these to be consistent with the others and have lower()
That said, looks like allow_files is not case insensitive, so I can't add a test. Which makes me question whether we want the lower here, but I suppose it won't hurt.
rules/scala/private/import.bzl
Outdated
| @@ -24,7 +24,11 @@ def scala_import_implementation(ctx): | |||
| _jar = [] | |||
| _src_jar = [] | |||
| for jar in ctx.files.jars: | |||
| if jar.basename.endswith("sources.jar") or jar.basename.endswith("src.jar"): | |||
| if ( | |||
| jar.basename.endswith("sources.jar") or | |||
There was a problem hiding this comment.
In the other places we check for -sources.jar and -src.jar (with the -), should that be consistent here?
| # classfile format changed somehow for Scala 3 and Bazel does not yet | ||
| # handle that. | ||
| # | ||
| # In the meantime, we've added a use_ijar Scala configuration value and |
There was a problem hiding this comment.
Did you consider removing the use_ijar configuration altogether?
There was a problem hiding this comment.
I have, but I want to get this merged and try it out in our main repo before I remove it. The rest I wrote here no longer fails, but I'll need more evidence to feel more confident.
…s in them We only handled .srcjar before, but many source jars are -sources.jar Also many source jars have MANIFEST.MF files in them. We were treating those as Scala sources before this fix, which is not correct.
jjudd
force-pushed
the
handle-source-jars-better
branch
from
cb7a448 to
42c014e
Compare
We only handled .srcjar before, but many source jars are -sources.jar
Also many source jars have MANIFEST.MF files in them. We were treating those as Scala sources before this fix, which is not correct.