Skip to content

Conversation

@fmeum
Copy link
Contributor

@fmeum fmeum commented May 20, 2025

This avoids an unnecessary dependency on a C++ toolchain matching the target platform when not building for Windows.

Also fix the configuration of the launcher, which should be built for the target platform.

@fmeum fmeum force-pushed the cross-compile-to-unix branch from 7c1ce31 to 6777439 Compare May 20, 2025 19:51
This avoids an unnecessary dependency on a C++ toolchain matching the target platform when not building for Windows.
@fmeum fmeum force-pushed the cross-compile-to-unix branch 2 times, most recently from 2e93b45 to db60200 Compare May 20, 2025 20:11
@fmeum fmeum force-pushed the cross-compile-to-unix branch from db60200 to 5a8b32a Compare May 20, 2025 20:51
@fmeum
Copy link
Contributor Author

fmeum commented May 21, 2025

@hvadehra Adding a dep on bazel_features from rule logic would require a new setup step with WORKSPACE. Is that better or worse than replicating more features in compatibility_proxy?

@fmeum fmeum marked this pull request as ready for review May 21, 2025 08:21
@fmeum fmeum requested review from a team and comius as code owners May 21, 2025 08:21
@hvadehra
Copy link
Member

Why do we need a new step? Isn't it enough to add bazel_features to rules_java_dependencies?

@fmeum
Copy link
Contributor Author

fmeum commented May 21, 2025

Why do we need a new step? Isn't it enough to add bazel_features to rules_java_dependencies?

bazel_features also needs

load("@bazel_features//:deps.bzl", "bazel_features_deps")
bazel_features_deps()

similar to protobuf. Did protobuf just start to use a rebranded bazel_features and added that to its macros? Maybe those two can be combined.

@hvadehra
Copy link
Member

Ah, lets add bazel_features to rules_java_deps and that extra bit to the release notes WORKSPACE snippet in https://github.com/bazelbuild/rules_java/blob/master/distro/relnotes.bzl

Did protobuf just start to use a rebranded bazel_features and added that to its macros? Maybe those two can be combined

cc @comius

@fmeum fmeum force-pushed the cross-compile-to-unix branch from e60a0b5 to deb670e Compare May 21, 2025 12:34
@hvadehra
Copy link
Member

hvadehra commented May 21, 2025

I think we'll need bazelbuild/bazel#26119 for this change

We can try and get that into Bazel 8.3.0. And until then, Bazel 8 + WORKSPACE users will need to use the flag.

@fmeum fmeum force-pushed the cross-compile-to-unix branch from 7f2519c to 5df2cbe Compare May 21, 2025 13:41
@fmeum fmeum requested a review from hvadehra May 21, 2025 13:47
@hvadehra
Copy link
Member

The new test breaks internally (obviously), so I'm going to manually import and fix.

@pzembrod
Copy link
Contributor

Sorry, I'm late to the party.

mbland added a commit to mbland/rules_scala that referenced this pull request Jun 13, 2025
Fixes a few problems when building under `WORKSPACE` with Bazel 8.2.1
(7.6.1 doesn't require these changes). Adds to `.bazelrc` the
`--incompatible_autoload_externally=` flag as a common option for all
builds, and a (disabled) line of options for `WORKSPACE` builds.

Bumps these development dependency versions:

- `com_google_buildifier_buildtools`: 5.1.0 => 8.2.1
- `rules_shell`: 0.4.1 => 0.5.0

---

Though `WORKSPACE` is on the way out, we should ensure that
`rules_scala` remains as compatible as it can be until it's totally
gone. All of these errors happened when running `./test_all` with Bazel
8.2.1 and `WORKSPACE` enabled while working on bazel-contrib#1747.

The first error was the following "cycle". (I later realized it's
somehow due to bazelbuild/rules_java#294 from `rules_java` 8.12.0. See
the note at the very end below.)

```sh
$ bazel run //tools:lint_check

ERROR: Cycle caused by autoloads, failed to load .bzl file
  '@@bazel_features_version//:version.bzl'.
Add 'bazel_features_version' to --repositories_without_autoloads
  or disable autoloads by setting '--incompatible_autoload_externally='
More information on bazelbuild/bazel#23043.
```

`--incompatible_autoload_externally=` fixed this problem, but also
precipitated all the other errors below.

- bazelbuild/bazel#23043
- https://bazel.build/reference/command-line-reference#common_options-flag--incompatible_autoload_externally

Updating `com_github_bazelbuild_buildtools` to v8.2.1 fixes the next
error, whereby Bazel no longer autoloaded `sh_test`. v5.1.0 was missing
the required `load("@rules_shell//shell:sh_test.bzl", "sh_test")`
statement, added in v8.0.3 by bazelbuild/buildtools#1332:

```sh
$ bazel run //tools:lint_check

ERROR: .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel:60:1:
  name 'sh_test' is not defined (did you mean 'cc_test'?)

ERROR: .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel:
  no such target '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template':
  target 'runner.bash.template' not declared in package 'buildifier' defined by
  .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel;
  however, a source file of this name exists.
  (Perhaps add 'exports_files(["runner.bash.template"])' to buildifier/BUILD?)

ERROR: /Users/mbland/src/bazel-contrib/rules_scala/tools/BUILD:19:11:
  every rule of type _buildifier implicitly depends upon the target
  '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template',
  but this target could not be found because of: no such target
  '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template':
  target 'runner.bash.template' not declared in package 'buildifier' defined by
  .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel;
  however, a source file of this name exists.
  (Perhaps add 'exports_files(["runner.bash.template"])' to buildifier/BUILD?)

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

Upgrading to v8.2.1 and updating `//tools:lint_check` to become a
`buildifier_test` also finally got rid of this next warning. This
required exporting the `MODULE.bazel` file from the root package,
disabling one lint warning, and accepting a couple of new lint fixes:

```txt
DEBUG: .../external/+dev_deps+com_github_bazelbuild_buildtools/buildifier/internal/factory.bzl:17:10:
DEPRECATION NOTICE: value 'check' for attribute 'mode' will be removed
in the future. Migrate '@@//tools:lint_check' to buildifier_test.
```

Adding `rules_jvm_external` 6.7 to `//scala:latest_deps.bzl` fixes this
next error, described in detail in:

- protocolbuffers/protobuf#19129 (comment)

```sh
$ bazel build --test_output=errors src/... test/...

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'javacopts' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'resources' in 'java_library' rule
  (did you mean 'features'?)

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'runtime_deps' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'javacopts' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'resources' in 'java_library' rule (did you mean 'features'?)

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'runtime_deps' in 'java_library' rule

ERROR: .../external/com_google_protobuf/BUILD.bazel:475:6:
  Target '@@com_google_protobuf//java/core:core'
  contains an error and its package is in error
  and referenced by '@@com_google_protobuf//:protobuf_java'

ERROR: Analysis of target
  '//test/proto/custom_generator:failing_scala_proto_deps_toolchain_def'
  failed; build aborted: Analysis failed
```

All of the previous errors only affected `rules_scala`'s own development
builds and test runs, when it is the main repository/root module. This
final error is something users could possibly run into when using
`--incompatible_autoload_externally=` with `rules_scala`.

`jvm_import` statements in Maven dependency repos generated by
`jvm_import_external` from `//scala:scala_maven_import_external.bzl`
began to fail. Defining `_JAVA_IMPORT_RULE_LOAD` and using it as the
default value for the `rule_load` attribute of the
`_jvm_import_external` rule fixed this:

```sh
$ bazel test --test_output=errors third_party/...

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:7:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'jars' in 'java_import' rule

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:7:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'neverlink' in 'java_import' rule

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:14:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'jars' in 'java_import' rule

ERROR: .../third_party/dependency_analyzer/src/test/BUILD:4:6:
  Target '@@org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file'
  contains an error and its package is in error and referenced by
  '//third_party/dependency_analyzer/src/test:strict_deps_test'

ERROR: Analysis of target
  '//third_party/dependency_analyzer/src/test:strict_deps_test' failed;
  build aborted: Analysis failed
```

---

And just now, I'm noticing that I'd already added the following to
`.bazelrc`, where the `rules_java` release references
bazelbuild/bazel#26119:

```sh
  # Uncomment for WORKSPACE builds for Bazel [8.0.0, 8.3.0) per:
  # https://github.com/bazelbuild/rules_java/releases/tag/8.12.0
  #common --repositories_without_autoloads=bazel_features_version,bazel_features_globals
```

Oh well. But now we're future proof.
simuons pushed a commit to bazel-contrib/rules_scala that referenced this pull request Jul 23, 2025
Fixes a few problems when building under `WORKSPACE` with Bazel 8.2.1
(7.6.1 doesn't require these changes). Adds to `.bazelrc` the
`--incompatible_autoload_externally=` flag as a common option for all
builds, and a (disabled) line of options for `WORKSPACE` builds.

Bumps these development dependency versions:

- `com_google_buildifier_buildtools`: 5.1.0 => 8.2.1
- `rules_shell`: 0.4.1 => 0.5.0

---

Though `WORKSPACE` is on the way out, we should ensure that
`rules_scala` remains as compatible as it can be until it's totally
gone. All of these errors happened when running `./test_all` with Bazel
8.2.1 and `WORKSPACE` enabled while working on #1747.

The first error was the following "cycle". (I later realized it's
somehow due to bazelbuild/rules_java#294 from `rules_java` 8.12.0. See
the note at the very end below.)

```sh
$ bazel run //tools:lint_check

ERROR: Cycle caused by autoloads, failed to load .bzl file
  '@@bazel_features_version//:version.bzl'.
Add 'bazel_features_version' to --repositories_without_autoloads
  or disable autoloads by setting '--incompatible_autoload_externally='
More information on bazelbuild/bazel#23043.
```

`--incompatible_autoload_externally=` fixed this problem, but also
precipitated all the other errors below.

- bazelbuild/bazel#23043
- https://bazel.build/reference/command-line-reference#common_options-flag--incompatible_autoload_externally

Updating `com_github_bazelbuild_buildtools` to v8.2.1 fixes the next
error, whereby Bazel no longer autoloaded `sh_test`. v5.1.0 was missing
the required `load("@rules_shell//shell:sh_test.bzl", "sh_test")`
statement, added in v8.0.3 by bazelbuild/buildtools#1332:

```sh
$ bazel run //tools:lint_check

ERROR: .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel:60:1:
  name 'sh_test' is not defined (did you mean 'cc_test'?)

ERROR: .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel:
  no such target '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template':
  target 'runner.bash.template' not declared in package 'buildifier' defined by
  .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel;
  however, a source file of this name exists.
  (Perhaps add 'exports_files(["runner.bash.template"])' to buildifier/BUILD?)

ERROR: /Users/mbland/src/bazel-contrib/rules_scala/tools/BUILD:19:11:
  every rule of type _buildifier implicitly depends upon the target
  '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template',
  but this target could not be found because of: no such target
  '@@com_github_bazelbuild_buildtools//buildifier:runner.bash.template':
  target 'runner.bash.template' not declared in package 'buildifier' defined by
  .../external/com_github_bazelbuild_buildtools/buildifier/BUILD.bazel;
  however, a source file of this name exists.
  (Perhaps add 'exports_files(["runner.bash.template"])' to buildifier/BUILD?)

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

Upgrading to v8.2.1 and updating `//tools:lint_check` to become a
`buildifier_test` also finally got rid of this next warning. This
required exporting the `MODULE.bazel` file from the root package,
disabling one lint warning, and accepting a couple of new lint fixes:

```txt
DEBUG: .../external/+dev_deps+com_github_bazelbuild_buildtools/buildifier/internal/factory.bzl:17:10:
DEPRECATION NOTICE: value 'check' for attribute 'mode' will be removed
in the future. Migrate '@@//tools:lint_check' to buildifier_test.
```

Adding `rules_jvm_external` 6.7 to `//scala:latest_deps.bzl` fixes this
next error, described in detail in:

- protocolbuffers/protobuf#19129 (comment)

```sh
$ bazel build --test_output=errors src/... test/...

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'javacopts' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'resources' in 'java_library' rule
  (did you mean 'features'?)

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:169:21:
  @@com_google_protobuf//java/core:lite_mvn-lib:
  no such attribute 'runtime_deps' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'javacopts' in 'java_library' rule

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'resources' in 'java_library' rule (did you mean 'features'?)

ERROR: .../external/com_google_protobuf/java/core/BUILD.bazel:287:21:
  @@com_google_protobuf//java/core:core_mvn-lib:
  no such attribute 'runtime_deps' in 'java_library' rule

ERROR: .../external/com_google_protobuf/BUILD.bazel:475:6:
  Target '@@com_google_protobuf//java/core:core'
  contains an error and its package is in error
  and referenced by '@@com_google_protobuf//:protobuf_java'

ERROR: Analysis of target
  '//test/proto/custom_generator:failing_scala_proto_deps_toolchain_def'
  failed; build aborted: Analysis failed
```

All of the previous errors only affected `rules_scala`'s own development
builds and test runs, when it is the main repository/root module. This
final error is something users could possibly run into when using
`--incompatible_autoload_externally=` with `rules_scala`.

`jvm_import` statements in Maven dependency repos generated by
`jvm_import_external` from `//scala:scala_maven_import_external.bzl`
began to fail. Defining `_JAVA_IMPORT_RULE_LOAD` and using it as the
default value for the `rule_load` attribute of the
`_jvm_import_external` rule fixed this:

```sh
$ bazel test --test_output=errors third_party/...

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:7:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'jars' in 'java_import' rule

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:7:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'neverlink' in 'java_import' rule

ERROR: .../external/org_apache_commons_commons_lang_3_5_without_file/BUILD:14:12:
  @@org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file:
  no such attribute 'jars' in 'java_import' rule

ERROR: .../third_party/dependency_analyzer/src/test/BUILD:4:6:
  Target '@@org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file'
  contains an error and its package is in error and referenced by
  '//third_party/dependency_analyzer/src/test:strict_deps_test'

ERROR: Analysis of target
  '//third_party/dependency_analyzer/src/test:strict_deps_test' failed;
  build aborted: Analysis failed
```

---

And just now, I'm noticing that I'd already added the following to
`.bazelrc`, where the `rules_java` release references
bazelbuild/bazel#26119:

```sh
  # Uncomment for WORKSPACE builds for Bazel [8.0.0, 8.3.0) per:
  # https://github.com/bazelbuild/rules_java/releases/tag/8.12.0
  #common --repositories_without_autoloads=bazel_features_version,bazel_features_globals
```

Oh well. But now we're future proof.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants