rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

Son of "Bad Path Elements" (the next generation)

Open virusdave opened this issue 3 years ago • 8 comments
trafficstars

Issue #1257 is happening still with modern bazel and rules_scala.

$ bazel build  @io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac:scalac.jar   2>&1 | $PAGER

──────────────────────────────────────────────────────────────────────────
       │ STDIN
──────────────────────────────────────────────────────────────────────────
   1   │ INFO: Invocation ID: de2a58ee-4a26-428c-b775-8990ef9c6017
   2   │ Loading:
   3   │ Loading: 0 packages loaded
   4   │ Analyzing: target @io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac:scalac.jar (1 packages loaded, 0 targets configured)
   5   │ INFO: Analyzed target @io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac:scalac.jar (1 packages loaded, 7 targets configured).
   6   │ INFO: Found 1 target...
   7   │ [0 / 2] [Prepa] BazelWorkspaceStatusAction stable-status.txt
   8   │ INFO: From Building external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac.jar (4 source files):
   9   │ warning: [path] bad path element "/private/var/tmp/_bazel_dave/f3114e5a271e277eec60625145f8bcd1/execroot/rh/bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_scala_scala_compiler/io_bazel_rules_scala_scala_compiler.stamp/scala-
       │ reflect.jar": no such file or directory
  10   │ warning: [path] bad path element "/private/var/tmp/_bazel_dave/f3114e5a271e277eec60625145f8bcd1/execroot/rh/bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_scala_scala_compiler/io_bazel_rules_scala_scala_compiler.stamp/scala-
       │ library.jar": no such file or directory
  11   │ Target @io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac:scalac.jar up-to-date:
  12   │   bazel-bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac.jar
  13   │ INFO: Elapsed time: 0.292s, Critical Path: 0.01s
  14   │ INFO: 2 processes: 1 disk cache hit, 1 internal.
  15   │ INFO: Build completed successfully, 2 total actions
  16   │ INFO: Build completed successfully, 2 total actions

It was discussed in this upstream bazel thread rules_jvm_external solves this problem by stripping the classpath elements from the manifest of jars it imports.

It looks like rules_scala uses its own custom rule implementation to import its dependencies (such as scala-compiler-2.12.14.jar, which has this manifest entry set). We probably need to do something similar right here, either using rules_jvm_external's tool directly (if allowed), or having a similar bespoke implementation which does this if a dependency on rules_jvm_external from these rules_scala is not allowed.

In the meantime, i'm going to try to solve this locally by replacing the rules dependencies with jars which have already passed through the rules_jvm_external manifest rewriting, since we already do use that ruleset. Will report back if this hack works to prevent logspam.

virusdave avatar Mar 29 '22 01:03 virusdave

This seems to be a problem within ijar. There was a fix #1258 by @simuons, maybe it can be upstreamed to Bazel?

I assume, that users of Rules Scala use custom loaders like rules_jvm_external, which can solve this problem, so this is mostly a problem within rules_scala repo, no?

liucijus avatar Mar 29 '22 06:03 liucijus

I've only so far seen this warning output on the jars imported directly in rules_scala via its internal import mechanism. rules_jvm_external (modern versions) seem to handle this problem just fine in the "general jar" case (aside from signed jars, i imagine)

virusdave avatar Mar 29 '22 16:03 virusdave

I'm curious why you say

This seems to be a problem within ijar.

though. I didn't see anywhere in rules_scala where there manually-imported (ie, scala_imported) jars are being passed through ijar.

Until recently, rules_scala used a custom ijar to do manifest stamping, but now it just uses the standard java tools to do that. #1258 updated the custom tool (which was previously used for this) to also remove the classpath elements, but neither of those functionalities (stamping, manifest update) are really the point of ijar, right?

virusdave avatar Mar 29 '22 16:03 virusdave

java_common.stamp_jar uses ijar (I believe it was cc binary on the default JavaToolchain last time I checked) to do its job here.

liucijus avatar Mar 29 '22 18:03 liucijus

I looked into as well and I seem to think that this chunder should be fixed in Bazel by https://github.com/bazelbuild/bazel/commit/5ea498c449709f999ecfe60c697ed2c8c8e1cb4a, which hasn't been released yet. Maybe in Bazel 6?

See also:

  • https://github.com/bazelbuild/bazel/issues/12968
  • https://github.com/bazelbuild/rules_jvm_external/pull/518

long-stripe avatar Mar 29 '22 18:03 long-stripe

I looked into as well and I seem to think that this chunder should be fixed in Bazel by https://github.com/bazelbuild/bazel/commit/5ea498c449709f999ecfe60c697ed2c8c8e1cb4a, which hasn't been released yet. Maybe in Bazel 6?

I'm using a pre-release build of bazel 6 which includes this commit, and it does not solve the problem.

In the meantime, i'm going to try to solve this locally by replacing the rules dependencies with jars which have already passed through the rules_jvm_external manifest rewriting, since we already do use that ruleset. Will report back if this hack works to prevent logspam.

Indeed, running the set of repository jars through rules_jvm_external's jvm_import(), then using those filtered output jars// in custom classpath providers in our custom toolchain rule does indeed solve the bad path elements problem as expected.

This works fine because we already use rules_jvm_external in our repository, so it's readily available to us. IF we'd be willing to add that as a required dependency to rules_scala, then i'd be happy to upstream the patch to make use of it to solve the issue globally. If we're not willing to add that dependency, then we'd need something similar to what I added

I'll move on to the scalapb deprecations build spam next.

virusdave avatar Mar 29 '22 21:03 virusdave

just want to chime in: I (and others) still use https://github.com/johnynek/bazel-deps/ which does not depend on rules_jvm_external (yet?) so I kind of hope we don't require that to set up rules_scala.

One hallmark of monorepos and bazel installs is that they are generally highly customized. Putting minimal dependencies on folks is a big plus.

johnynek avatar Mar 30 '22 00:03 johnynek

just want to chime in: I (and others) still use https://github.com/johnynek/bazel-deps/ which does not depend on rules_jvm_external (yet?) so I kind of hope we don't require that to set up rules_scala.

One hallmark of monorepos and bazel installs is that they are generally highly customized. Putting minimal dependencies on folks is a big plus.

Yup, makes sense. If we don't want that dependency edge, then i think we need a custom implementation in these rules to do manifest classpath scrubbing like rules_jvm_external does (at least until this is solved upstream in bazel and exposed via a java_common rule/macro) :(

virusdave avatar Mar 30 '22 00:03 virusdave