Skip to content

fix: add missing proto_library load to support Bazel 8#263

Merged
tetromino merged 10 commits intobazelbuild:masterfrom
rickeylev:fix.bazel8.proto_library
Nov 27, 2024
Merged

fix: add missing proto_library load to support Bazel 8#263
tetromino merged 10 commits intobazelbuild:masterfrom
rickeylev:fix.bazel8.proto_library

Conversation

@rickeylev
Copy link
Copy Markdown
Contributor

@rickeylev rickeylev commented Nov 26, 2024

The load for proto_library is missing, which breaks when used with Bazel 8.

Fixes #261

@rickeylev rickeylev force-pushed the fix.bazel8.proto_library branch from cb96cb3 to cf74bc7 Compare November 26, 2024 03:34
@rickeylev
Copy link
Copy Markdown
Contributor Author

rickeylev commented Nov 26, 2024

Well, bzlmod seems to be happy, but workspace seems to be unhappy.

external/com_google_protobuf/bazel/private/java_proto_support.bzl:9:6: 
cannot load '@@rules_java//java/private:proto_support.bzl':
no such file and referenced by '@@bazel_tools//tools/proto:java_toolchain'

Let's see if upgrading rules_java in workspace helps. bzlmod is using 7.12.2, so let's try that...ok, yeah that seems to have fixed that part

But now an issue in rules_cc referencing @protobuf

error loading package '@@rules_java//toolchains':
at .../external/rules_cc/cc/defs.bzl:16:6:
Unable to find package for @@protobuf//bazel:cc_proto_library.bzl:
The repository '@@protobuf' could not be resolved:
Repository '@@protobuf' is not defined.

Ivo mentioned that the name must be com_google_protobuf for workspace builds. Lets try upgrading rules_cc 0.1.0 ... ah nope:

external/remote_java_tools/BUILD", line 7, column 60, in <toplevel>
    load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_proto_library")
Error: file '@rules_cc//cc:defs.bzl' does not contain symbol 'cc_proto_library'

This sounds vaguely familiar. Lets try rules_cc 0.0.17 ... ah nope:

ERROR: .../external/com_google_protobuf/src/google/protobuf/BUILD.bazel:219:14:
in proto_library rule @@com_google_protobuf//src/google/protobuf:descriptor_proto:
Traceback (most recent call last):
	File ".../external/com_google_protobuf/bazel/private/bazel_proto_library_rule.bzl", line 67, column 39, in _proto_library_impl
		import_prefix = _get_import_prefix(ctx)
	File ".../external/com_google_protobuf/bazel/private/bazel_proto_library_rule.bzl", line 39, column 17, in _get_import_prefix
		if not paths.is_normalized(import_prefix):
Error: 'struct' value has no field or method 'is_normalized'

Not sure if this is a step forward or backwards. Well, i think the issue there is paths.is_normalized came about in skylib 1.7, so lets try bazel_skylib 1.7.1 ... almost there ... success! well, pretty close, anyways. Bazel@head fails compiling absl, which is probably related to protobuf. Everything else is happy though.

When I try this PR with rules_python, it gets past the error. (Eventually fails when compiling absl, but that is probably a separate issue specific to protobuf; maybe adding a --copt i saw elsewhere would fix that; wouldn't be surprised if the compiling failure in stardoc and rules_python originate from the same cause).

@meteorcloudy
Copy link
Copy Markdown
Member

meteorcloudy commented Nov 26, 2024

For the remaining failures, we might want to just upgrade the platforms tested here (ubuntu1804 has a very old gcc and is no longer supported):

stardoc/.bazelci/presubmit.yml

Lines 4 to 6 in 91e429d

- ubuntu1804
- ubuntu2004
- macos

Probably should be:

- ubuntu2004
- ubuntu2204
- macos_arm64

@meteorcloudy
Copy link
Copy Markdown
Member

meteorcloudy commented Nov 26, 2024

@comius Does it look correct that autoload didn't work for Stardoc?

@tetromino
Copy link
Copy Markdown
Collaborator

tetromino commented Nov 26, 2024

When I try to build this PR on my Mac, I get a failure (some sort of java version incompatibility caused by rules_jvm_external).

ERROR: /private/var/tmp/_bazel_arostovtsev/4db74ff3612d0bdc1259d9a6e5cd2136/external/protobuf+/java/core/BUILD.bazel:182:22: Building external/protobuf+/java/core/liblite_runtime_only.jar (93 source files) failed: (Exit 1): java failed: error executing Javac command (from target @@protobuf+//java/core:lite_runtime_only) external/rules_java++toolchains+remotejdk21_macos/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
error: cannot access module-info
  bad class file: /modules/java.base/module-info.class
    class file has wrong version 67.0, should be 65.0
    Please remove or make sure it appears in the correct subdirectory of the classpath.
error: [BazelJavaConfiguration] The Java 21 runtime used to run javac is not recent enough to compile for the Java 23 runtime in external/rules_java++toolchains+local_jdk. Either register a Java toolchain with a newer java_runtime or specify a lower --java_runtime_version.
Use --verbose_failures to see the command lines of failed build steps.

@rickeylev
Copy link
Copy Markdown
Contributor Author

rickeylev commented Nov 26, 2024

The Java 21 runtime used to run javac is not recent enough to compile for the Java 23 runtime in external/rules_java++toolchains+local_jdk. Either register a Java toolchain with a newer java_runtime or specify a lower --java_runtime_version.

Ohhhhh this looks so familiar. 🤔

HTH

@tetromino
Use a consistent set of deps, fix WORKSPACE build, fix build if jdk>2…
967b7b7 
…1 is installed

Use a consistent set of dependencies for both bzlmod and WORKSPACE, as
identified by bzlmod error messages. Remove various obsolete comments.

Fix WORKSPACE - we now explicitly need to call rules_java_dependencies() and
protobuf_deps(), thanks to recent java & proto changes.

Set --java_runtime_version in .bazelrc to unbreak the build if a newer jdk
(such as jdk-23) happens to be installed systemwide (this issue appears thanks
to the rules_java dependency version bump).
@tetromino
Copy link
Copy Markdown
Collaborator

tetromino commented Nov 26, 2024

@rickeylev - thanks for the tip, futzing with --java_runtime_version turned out to be the solution (the new rules_java picks up the systemwide installed jdk-23 on my machine and gets confused).

@tetromino
Copy link
Copy Markdown
Collaborator

tetromino commented Nov 26, 2024 •
edited
Loading

@rickeylev - please check if everything still works for you after my commits

Note the updated WORKSPACE blurb for copy-paste is now

load("@io_bazel_stardoc//:setup.bzl", "stardoc_repositories")

stardoc_repositories()

load("@rules_java//java:rules_java_deps.bzl", "rules_java_dependencies")

rules_java_dependencies()

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

load("@rules_jvm_external//:repositories.bzl", "rules_jvm_external_deps")

rules_jvm_external_deps()

load("@rules_jvm_external//:setup.bzl", "rules_jvm_external_setup")

rules_jvm_external_setup()

load("@io_bazel_stardoc//:deps.bzl", "stardoc_external_deps")

stardoc_external_deps()

load("@stardoc_maven//:defs.bzl", stardoc_pinned_maven_install = "pinned_maven_install")

stardoc_pinned_maven_install()

@rickeylev
Copy link
Copy Markdown
Contributor Author

rickeylev commented Nov 27, 2024

Yes, it works in rules_python

@tetromino tetromino merged commit aefab4c into bazelbuild:master Nov 27, 2024
This was referenced Nov 27, 2024
Closed
Closed
@rickeylev rickeylev deleted the fix.bazel8.proto_library branch November 27, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tetromino tetromino tetromino approved these changes

@brandjon brandjon Awaiting requested review from brandjon

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

proto_library is not defined error with bazel@head

3 participants

@rickeylev @meteorcloudy @tetromino