Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

sync with upstream #17

Open
wants to merge 478 commits into
base: master
Choose a base branch
from
Open

Conversation

dan-busch-gr
Copy link

Description

Motivation

Vaidas Pilkauskas and others added 30 commits December 9, 2020 14:19
scala_repositories should use the passed parameter value instead of providing _default_maven_server_urls as a value passed downstream.
* jacocorunner option on scala_toolchain

* Use jacocorunner from scala toolchain

* Remove jacocorunner option from scala_test

* Test scala_toolchain jacocorunner
* Script to generate custom jacocorunner

It can be used to generate custom jacocorunner, that then can be used to
e.g. fix Scala 2.12 code coverage for lambdas.

* Build jacocorunner in /tmp in a deterministic subdir

* Documentation fixes and better Mac OS support

* Manual test for built local jacocorunner

Expected coverage file manually compared with
git show 2aba2cd:test/coverage/expected-coverage.dat.
Missing function lines are shown again.
This can be further proved by running:

genhtml -o /tmp/coverage_results $(bazel info bazel-testlogs)/test/coverage/test-all/coverage.dat

* Documentation for custom jacocorunner usage
* Fix diagnostics tests to get binary folder by default

* Add tests for different reporters in different scala versions

* Move reporter tests to test_version package
* Fix error reporting on 2.11

* Fix typo on test_version.sh
* Refactor/cleanup

* Always add target label to scalac args

* Stamp jars produced by ScalacWorker

* Add "-t label" to usage string
* Add .bazelci config validation

* Add ubuntu2004 build

* Check if we can run shell cmdw as test

* Add Macos verisons test to .bazelci

* Add ci task names

* Remove version tests from Travis
* Fix scala_import file name collision in the same package

* Lint

* Add issue link
* Add examples tests to BK

* Remove examples tests from Travis

* Fix mistype

* Attempt with a different key

* Fix platform key
* Revert scala_import stamping

* Revert scala_import stamping tests

* Lint
* Fix action_should_fail_with_message sh helper

Add missing `exit 1` to make test actaully fail

Had to remove `jvm_opts = ["-Xbootclasspath/p:$(location
@bazel_tools//third_party/java/jdk/langtools:javac_jar)"],`
because test fails for wrong reason `-Xbootclasspath/p is no longer a
supported option.`

* Fix test_scalac_jvm_flags_are_expanded

Prefix flag with a name otherwise it looks it up as class name and fails
with

Caused by: java.lang.ClassNotFoundException:
test_expect_failure.compilers_jvm_flags.args.txt

* Rename test flag to clarify intent
* scala_proto: configure ScalaPbCodeGenerator instead of worker

* Add test. Need to clean up

* Simplify generator

* Make custom generator dumb as possible. Ignore inputs and generate fixed file

* cleanups

* rc: append single element with :+
The sha256 is set for scalap:2.11.12, but the version set was 2.11.10 which
caused a hash conflict.

$ sha256sum scalap-2.11.12.jar
a6dd7203ce4af9d6185023d5dba9993eb8e80584ff4b1f6dec574a2aba4cd2b7  scalap-2.11.12.jar
$ sha256sum scalap-2.11.10.jar
3f6f352fce91c33055398a7081e35e5091fd5e095130905633e72f52124c1d27  scalap-2.11.10.jar
gergelyfabian and others added 30 commits April 12, 2024 09:33
* JacocoRunner script: update for Jacoco 0.8.11 and Bazel 7.0.2

rules_scala has issues with coverage on JDK 21.

Bazel has solved this in bazelbuild/bazel#20845
by upgrading Jacoco and ASM.

Added option to build JacocoRunner for Bazel 7, the version with fix
will be used with Jacoco interface changes.
Jacoco upgraded to 0.8.11 (including the Jacoco/Bazel branches used for
that).

* Require using Java 17 for building Jacoco 0.8.11
* Move default toolchain dependencies logic to .bzl file

Purely refactoring change.
Encapsulates the logic of selecting dependencies into a function.
Should come in handy in future changes.

* Undo `setup_scala_toolchain_with_default_classpaths` macro
For multiple `SCALA_VERSIONS` we will need to download artifacts for each version.
Some naming collision avoidance is needed.

Co-authored-by: mkuta <mkuta@virtuslab.com>
* Start documenting "From config setting" section

* Provide `select_for_scala_version` utility macro
…ules (#1569)

Co-authored-by: mkuta <mkuta@virtuslab.com>
A small preparation for multiple `SCALA_VERSIONS`.

Co-authored-by: mkuta <mkuta@virtuslab.com>
…ration includes the dependency (#1568)

* Propagate maven_coordinates tag to generated scala_import so pom generation includes the dependency

* add a check for coordinates before adding tag
* Add backward-compatible repo alias for artifacts

* Use version-aware naming for artifact repositories

Co-authored-by: mkuta <mkuta@virtuslab.com>

* Migrate rules internally away from alias repos

---------

Co-authored-by: mkuta <mkuta@virtuslab.com>
Co-authored-by: mkuta <mkuta@virtuslab.com>
These should also point to Scala version-specific artifacts.
See previous PR: #1561
* Delete _semanticdb folder before compiling

* lint fix

* Consolidated some functions in Scalacworker.
Co-authored-by: mkuta <mkuta@virtuslab.com>
This prints stack traces with both cause and suppressions unlike
the naive implementation from JUnitXmlReporter. Without this
change, suppressions aren't printed and causes are overly long.
…1584)

* fix scaladoc rule to handle transient deps better
Also Refactors and tests

* lint fixes
* Set up example

* A few examples

---------

Co-authored-by: mkuta <mkuta@virtuslab.com>
* Set up test

* Tests with version-specific codes
* Move the cross-compilation doc to docs/

* Quick-start tutorial for cross-build users
* Extract common functions for testing scalafmt

* Add scalafmt test with multiple Scala versions

---------

Co-authored-by: mkuta <mkuta@virtuslab.com>
* bumped artifact versions with sha sums and scala versions

* reset cla

* removed library version update

* update sha sums

* update scala version in workspace

* update scala version in workspace and build

* update scala version in workspaces and build

* removed scalafmt updated versions

* removed docs update

* removed support for 3.4.2 due to failing scala3_4_example

* removed support for 3.4.2 due to failing scala3_4_example

* lint issue

* update
…cktraces (#1606)

* Improve handling of invalid settings passed to Scala compiler. Throw known exception instead of allowing to None.get (Scala3) or NullPointerException (Scala2)

* Prevent printing stack trace of ScalacInvoker on known compilation errors

* Run reporter.flush in Scala3

* Restore previous error messages

* Addi tests to ensure corect error message and no stack traces are shown

* Remove unused code from test_invalid_scalacopts.sh

* Print exception message only once. Previously printed in both e.getMessage and e.printStackTrace

* Ident with spaces instead of tabs
* scala 3.4.2 support

* 3.4.3 hotfix
* extra javac opts should override default ones

* add test
Related to #1482, #1618, and #1619. Results from the investigation
documented at:

- #1619 (comment)

Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle
differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources`
values between Bazel 6 and Bazel 7.

Without this change, `_import_paths()` emits incorrect values under
Bazel 7, causing targets containing generated `.proto` inputs to fail,
e.g.  `//test/proto3:test_generated_proto`.

See also:

- Fix paths for sibling repository setup and generated .proto files
  bazelbuild/bazel@6c6c196

- The docstring for `ProtoInfo.proto_source_root` in the Bazel sources:
  https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172

- Remove incompatible_generated_protos_in_virtual_imports
  bazelbuild/bazel@3efaa32

- Comment from: Generated Protos are no longer considered as
  virtual_imports in Bazel 7
  bazelbuild/bazel#21075 (comment)

---

I cherrypicked this commit into #1618. While it fixed the
`//test/proto3` build failure, it does _not_ fix the hanging
scalapb_workers from the ProtoScalaPBRule aspect.

I'll have to investiate further whether than hang is related to Bazel,
rules_proto, com_google_protobuf, or some mixture thereof. Still,
progress!
* Add toolchain_type to thrift_lib, scrooge aspects

This fixes the following error:

```txt
Error in create_header_compilation_action: Rule 'thrift_library' in
  'rules_scala/test/src/main/scala/scalarules/test/twitter_scrooge/thrift/
  thrift_with_compiler_args/BUILD:3:15'
must declare '@@bazel_tools//tools/jdk:toolchain_type' toolchain in
order to use java_common.

See bazelbuild/bazel#18970.
```

Interestingly, adding the toolchain type to `thrift_library` isn't
enough; I had to add it to the twitter_scrooge aspects as well. Until I
did, it produced the same error message pointing at `thrift_library`,
after first reporting:

```txt
ERROR: rules_scala/test/src/main/scala/scalarules/test/twitter_scrooge/
  thrift/thrift_with_compiler_args/BUILD:3:15:

in //twitter_scrooge:twitter_scrooge.bzl%scrooge_java_aspect aspect
  on thrift_library rule
  //test/src/main/scala/scalarules/test/twitter_scrooge/thrift/
    thrift_with_compiler_args:thrift5:

Traceback (most recent call last):
  File "rules_scala/twitter_scrooge/twitter_scrooge.bzl",
    line 420, column 49, in _scrooge_java_aspect_impl
  return _common_scrooge_aspect_implementation(target, ctx, "java",
    _compile_generated_java)
  [ ...snip... ]
```

* Fix proto_cross_repo_boundary build_file attribute

Bazel 7 enforces that this be a proper target label, not a relative
path.

```txt
ERROR: WORKSPACE:91:37: fetching new_local_repository rule
  //external:proto_cross_repo_boundary:
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

INFO: Repository remote_jdk8_macos instantiated at:
  WORKSPACE:175:18: in <toplevel>
  .../external/rules_java/java/repositories.bzl:120:10:
    in remote_jdk8_repos
  .../external/bazel_tools/tools/build_defs/repo/utils.bzl:240:18:
    in maybe
  .../external/rules_java/toolchains/remote_java_repository.bzl:48:17:
    in remote_java_repository

Repository rule http_archive defined at:
  .../external/bazel_tools/tools/build_defs/repo/http.bzl:384:31:
  in <toplevel>

ERROR: no such package '@@proto_cross_repo_boundary//':
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

ERROR: test/proto_cross_repo_boundary/BUILD:3:22:
  //test/proto_cross_repo_boundary:sample_scala_proto depends on
  @@proto_cross_repo_boundary//:sample_proto in repository
  @@proto_cross_repo_boundary which failed to fetch. no such package
  '@@proto_cross_repo_boundary//':
  .../external/test/proto_cross_repo_boundary/repo/BUILD.repo
  is not a regular file; if you're using a relative or absolute path for
  `build_file` in your `new_local_repository` rule, please switch to
  using a label instead

Use --verbose_failures to see the command lines of failed build steps.
ERROR: Analysis of target
  '//test/proto_cross_repo_boundary:sample_scala_proto' failed;
  build aborted: Analysis failed
```

* Add rules_python stanza to WORKSPACE files

Fixes the following breakages under Bazel 7:

```txt
$ bazel build //src/...

ERROR: error loading package '@@bazel_tools//src/main/protobuf':
  cannot load '@@rules_python//python:proto.bzl': no such file

ERROR:
  .../src/java/io/bazel/rulesscala/coverage/instrumenter/BUILD:3:12:
  error loading package '@@bazel_tools//src/main/protobuf':
  cannot load '@@rules_python//python:proto.bzl':
  no such file and referenced by
  '//src/java/io/bazel/rulesscala/coverage/instrumenter:instrumenter'
```

* Set --noenable_bzlmod to prevent MODULE.bazel

Makes sure Bazel doesn't autogenerate MODULE.bazel and MODULE.bazel.lock
files for now.

* Update to Bazel 6.5.0

This is the last release in the Bazel 6 line. With the changes up to
this point, the Bazel 7 build only fails on protobuf generation, e.g.
on `bazel build //test/proto3:all`, as seen below. Bumping to Bazel
6.5.0 on top of all the previous changes isolates the Bazel 7.0.0
changes causing the failure.

```txt
$ bazel build //test/proto3:all

ERROR: .../rules_scala/test/proto3/BUILD:14:14:
  ProtoScalaPBRule test/proto3/generated-proto-lib_jvm_extra_protobuf_generator_scalapb.srcjar
  failed: (Exit 1): scalapb_worker failed:
  error executing ProtoScalaPBRule command
  (from target //test/proto3:generated-proto-lib)
  bazel-out/darwin_arm64-opt-exec-ST-13d3ddad9198/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)

Could not find file in descriptor database:
  bazel-out/darwin_arm64-fastbuild/bin/test/proto3/generated.proto:
  No such file or directory

java.lang.RuntimeException: Exit with code 1
        at scala.sys.package$.error(package.scala:30)
        at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44)
        at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96)
        at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
        at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39)
        at scripts.ScalaPBWorker.main(ScalaPBWorker.scala)
Use --verbose_failures to see the command lines of failed build steps.

ERROR: .../rules_scala/test/proto3/BUILD:14:14
  scala @//test/proto3:generated-proto-lib
  failed: (Exit 1): scalapb_worker failed:
  error executing ProtoScalaPBRule command
  (from target //test/proto3:generated-proto-lib)
  bazel-out/darwin_arm64-opt-exec-ST-13d3ddad9198/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)
```

* Fix WORKSPACE lint errors

One of these days I'll remember to run `bazel run //tools:lint_fix`
before opening a pull request.

* Set --enable_workspace on last_green CI build

Learning more about BuildKite every day.

* Allow test_all.sh to pass under Bazel 7

`bash ./test_all.sh` passes after minor updates to the following test
cases to handle slightly different behavior under Bazel 7:

- `test_custom_reporter_class_has_been_set` makes the test output dir
  writable, since it no longer is by default.

- `test_scala_import_expect_failure_on_missing_direct_deps_warn_mode`
  removes preexisting output, since Bazel 7 won't emit warnings if it
  already exists.

---

FYI: Under Bazel 7.0 and Bazel 7.1, buildifier warnings for external
targets in the following test cases changed to begin with `@@repo_name`
instead of `@repo_name`:

- test/shell/test_scala_library.sh:
  `test_scala_library_expect_failure_on_missing_direct_external_deps_jar`
  `test_scala_library_expect_failure_on_missing_direct_external_deps_file_group`

- test/shell/test_strict_dependency.sh:
  `test_stamped_target_label_loading`
  `test_strict_deps_filter_included_target`

- test/shell/test_test_unused_dependency.sh:
  `test_unused_deps_filter_included_target`

However, as of Bazel 7.2, those warnings changed back to `@repo_name`,
so those changes aren't reflected in this commit.

* Bump to rules_go 0.39.1 for Bazel 7 compatibility

`bazel run //tools:lint_{check,fix}` required updating rules_go to
0.39.1 to work under Bazel 7.

The previous rules_go version 0.38.1 caused the following error:

```txt
$ ./test_lint.sh
ERROR: .../external/io_bazel_rules_go/BUILD.bazel:71:16:
  in (an implicit dependency) attribute of go_context_data rule
  @@io_bazel_rules_go//:go_context_data:
  in $whitelist_function_transition attribute of go_context_data rule
  @@io_bazel_rules_go//:go_context_data: package group
  '@@bazel_tools//tools/whitelists/function_transition_whitelist:function_transition_whitelist'
  is misplaced here (they are only allowed in the visibility attribute)

ERROR: .../external/io_bazel_rules_go/BUILD.bazel:71:16:
  Analysis of target '@@io_bazel_rules_go//:go_context_data' failed

ERROR: Analysis of target '//tools:lint_check' failed;
  build aborted: Analysis failed
```

* Bump to rules_java 7.9.0 for Bazel 7 compatibility

Updated rules_java to 7.9.0 to fix errors of the following type under
Bazel 7.2.1:

```txt
ERROR: test/BUILD:640:10:
  While resolving toolchains for target //test:jar_lister (096dcc8):
  invalid registered toolchain
  '@remote_jdk8_linux_aarch64_toolchain_config_repo//:bootstrap_runtime_toolchain':
  no such target
  '@@remote_jdk8_linux_aarch64_toolchain_config_repo//:bootstrap_runtime_toolchain':
  target 'bootstrap_runtime_toolchain' not declared in package '' defined by
  .../external/remote_jdk8_linux_aarch64_toolchain_config_repo/BUILD.bazel

ERROR: Analysis of target '//test:jar_lister' failed; build aborted
```

Also removed the seemingly unused `rules_java_extra` stanza from
`WORKSPACE`.

---

Though `rules_java` version 7.12.1 is available, and largely works with
this repo when using Bazel 7.3.2, it requires a few temporary
workarounds. (As I write this, 8.0.0 came out just a few hours ago and I
haven't tried it.) Rather than commit the workarounds, upgrading only to
7.9.0 now seems less crufty.

Though this commit only updates Bazel to 7.2.1, I suspect it's probably
the same basic problem at play.

What follows is a very detailed explanation of what happens with 7.12.1
with Bazel 7.3.2, just to have it on the record.

---

The workaround is to change a few toolchain and macro file targets from
`@bazel_tools//tools/jdk:` to `@rules_java//toolchains:`. This isn't a
terribly bad or invasive workaround, but `@bazel_tools//tools/jdk:` is
clearly the canonical path. Best to keep it that way, lest we build up
technical debt.

Without the workaround, these targets would fail:

- //test/src/main/resources/java_sources:CompiledWithJava11
- //test/src/main/resources/java_sources:CompiledWithJava8
- //test/toolchains:java21_toolchain
- //test:JunitRuntimePlatform
- //test:JunitRuntimePlatform_test_runner
- //test:scala_binary_jdk_11

with this error:

```txt
ERROR: .../external/rules_java_builtin/toolchains/BUILD:254:14:

While resolving toolchains for target
@@rules_java_builtin//toolchains:platformclasspath (096dcc8):

No matching toolchains found for types
@@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type.
```

This appears to be a consequence of both upgrading the Bazel version to
7.3.2 and updating `rules_java` to 7.12.1. The `rules_java_builtin` repo
is part of the `WORKSPACE` prefix that adds implicit dependencies:

- https://bazel.build/external/migration#builtin-default-deps

This repo was added to 7.0.0-pre.20231011.2 in the following change,
mapped to `@rules_java` within the scope of the `@bazel_tools` repo:

- bazelbuild/bazel: Add rules_java_builtin to the users of Java modules
  bazelbuild/bazel@ff1abb2

This change tried to ensure `rules_java` remained compatible with
earlier Bazel versions. However, it changed all instances of
`@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` to
`//toolchains:bootstrap_runtime_toolchain_type`:

- bazelbuild/rules_java: Make rules_java backwards compatible with Bazel
  6.3.0
  bazelbuild/rules_java@30ecf3f

Bazel has bumped `rules_java` in its `workspace_deps.bzl` from 7.9.0 to
7.11.0, but it's only available as of 8.0.0-pre.20240911.1.

- bazelbuild/bazel: Update rules_java 7.11.1 / java_tools 13.8
  bazelbuild/bazel#23571
  bazelbuild/bazel@f92124a

---

What I believe is happening is, under Bazel 7.3.2 and `rules_java`
7.12.1:

- Bazel creates `rules_java` 7.9.0 as `@rules_java_builtin` in the
  `WORKSPACE` prefix.

- `@bazel_tools` has `@rules_java` mapped to `@rules_java_builtin` when
  initialized during the `WORKSPACE` prefix, during which
  `@bazel_tools//tools/jdk` registers `alias()` targets to
  `@rules_java` toolchain targets. These aliased toolchains specify
  `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` in their
  `toolchains` attribute.

- `WORKSPACE` loads `@rules_java` 7.12.1 and registers all its
  toolchains with type
  `@rules_java//toolchains:bootstrap_runtime_toolchain_type`.

- Some `@rules_java` rules explicitly specifying toolchains from
  `@bazel_tools//tools/jdk` can't find them, because the
  `@bazel_tools//tools/jdk` toolchain aliases expect toolchains of type
  `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type`.

This has broken other projects in the same way:

- bazelbuild/bazel: [Bazel CI] Downstream project broken by rules_java
  upgrade #23619
  bazelbuild/bazel#23619

These problems don't appear under Bzlmod, whereby `@rules_java_builtin`
was never required. This is because `WORKSPACE` executes its statements
sequentially, while Bzlmod builds the module dependency graph _before_
instantiating repositories (within module extensions).

It seems a fix is on the way that removes `@rules_java_builtin` from the
`WORKSPACE` prefix, and adds `@rules_java` to the suffix. At this
moment, though, it's not even in a prerelease:

- bazelbuild/bazel: Remove rules_java_builtin in WORKSPACE prefix
  bazelbuild/bazel@7506690

---

Note that the error message matches that from the following resolved
issue, but that issue was for non-Bzlmod child repos when `WORKSPACE`
was disabled.

- bazelbuild/bazel: Undefined @@rules_java_builtin repository with
  --noenable_workspace option
  bazelbuild/bazel#22754
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.