Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all autotools usage #10132

Merged

Conversation

mkruskal-google
Copy link
Member

@mkruskal-google mkruskal-google commented Jun 12, 2022

This is the first step for "bazelifying" our codebase that removes Autotools and our custom test runner and replaces them with Bazel. The basic strategy here is a top-down one. A lot of the builds that are difficult to get properly modeled in Bazel have been wrapped in shell and genrules. By migrating the high level build/test/release system to be managed solely by Bazel though, we can parallelize any effort to clean up these more isolated cases. The ideal future will have Bazel as the source of truth, with generated configurations for other supported build systems (e.g. CMake, Rake, Composer, etc.).

This PR has been manually tested on Linux, Windows, and Mac in addition to all the Kokoro changes.

A high level breakdown of what this PR includes is:

  • Conformance tests migrated to build/run with Bazel
  • Benchmark tests migrated to build/run with Bazel
  • All protobuf runtimes and protoc support Bazel builds, which wrap (rather than replace) the current build systems.
  • tests.sh replaced by Bazel test rules
  • Autotools config files deleted
  • Kokoro test environment replaced with new Bazel-based images hosted on GCR

Copy link
Member

@deannagarcia deannagarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the overall strategy here is great! Just a few comments

php/README.md Show resolved Hide resolved
python/BUILD.bazel Show resolved Hide resolved
ruby/ext/google/protobuf_c/extconf.rb Show resolved Hide resolved
src/README.md Show resolved Hide resolved
src/README.md Show resolved Hide resolved
src/README.md Show resolved Hide resolved
src/README.md Show resolved Hide resolved
src/google/protobuf/BUILD.bazel Outdated Show resolved Hide resolved
tests.sh Outdated Show resolved Hide resolved
protobuf.bzl Show resolved Hide resolved
@mkruskal-google mkruskal-google force-pushed the goodbye-autotools branch 2 times, most recently from a96f4b7 to e8e262a Compare June 18, 2022 05:17
@mkruskal-google mkruskal-google marked this pull request as ready for review June 18, 2022 05:19
@mkruskal-google mkruskal-google changed the title [DO NOT SUBMIT] Removing autotools usage [WIP] Removing autotools usage Jun 18, 2022
@thomasvl
Copy link
Contributor

Looks like the README.md in conformance didn't get updated, what's the right way to build/run those these days (looking up update SwiftProtobuf)

@mkruskal-google
Copy link
Member Author

mkruskal-google commented Aug 15, 2022

Ah thanks for catching that Thomas! I'll send out a PR updating that one, but the TL;DR is that Bazel is the way to run these: e.g. bazel test //python:conformance_test.

Note: CMake can also be used to build the conformance test infrastructure and run the C++ conformance tests by setting the protobuf_BUILD_CONFORMANCE flag

@thomasvl
Copy link
Contributor

So for SwiftProtobuf, I need the runner and then how to run the runner since the conformance binary I'm testing lives in another repo.

@thomasvl
Copy link
Contributor

btw - you might also want to tweak linux install directions, bazel doesn't seem to support Focal (20.*) yet, which is tripping my updates for SwiftProtobuf builds.

@mkruskal-google
Copy link
Member Author

conformance_test_runner --help will show you the arguments you need to pass it. The last one is just an executable conformance test program + arguments (which you would provide from the swift repo).

What's the issue in Focal? Bazel claims it should be compatible with later releases, although they don't test against them

@thomasvl
Copy link
Contributor

conformance_test_runner --help will show you the arguments you need to pass it. The last one is just an executable conformance test program + arguments (which you would provide from the swift repo).

Ok, I'll just build it and run it. I was trying to use the runner.sh, but that always errored out on me:

❯ bazelisk build //conformance:conformance_test_runner       
...
❯ conformance/conformance_test_runner.sh --help
+ echo --help
--help
+ set -euo pipefail
+ [[ ! -d /dev/null ]]
+ [[ ! -f /dev/null ]]
+ [[ -f conformance/conformance_test_runner.sh.runfiles_manifest ]]
+ [[ -f conformance/conformance_test_runner.sh.runfiles/MANIFEST ]]
+ [[ -f conformance/conformance_test_runner.sh.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash ]]
+ [[ -f /dev/null/bazel_tools/tools/bash/runfiles/runfiles.bash ]]
+ [[ -f /dev/null ]]
+ echo 'ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash'
ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash
+ exit 1

What's the issue in Focal? Bazel claims it should be compatible with later releases, although they don't test against them

apt-get install bazel or apt-get install bazelisk fails not being able to find the package to install.

I'm having to make the SwiftProtobuf github steps install Go, so I can then go install bazelisk and then use that to build protoc/conformance-test.

@mkruskal-google
Copy link
Member Author

Whoops, I'll add a comment that that bash script is only for use within Bazel!

You should be able to build and run the test runner binary with CMake and then pass it whatever executable you want though. I noticed that right now arguments can't be forwarded properly, but I'll include that fix in my PR

@thomasvl
Copy link
Contributor

Whoops, I'll add a comment that that bash script is only for use within Bazel!

I would have thought bazelisk would work just fine then? That was building/running locally off my Mac with bazelisk in the root of a protobuf checkout.

@mkruskal-google
Copy link
Member Author

Sorry, I was referring to your comment about trying to run conformance_test_runner.sh. That script is only for internal use via Bazel and won't work elsewhere. Bazelisk should work just fine

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 25, 2022
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
halstead pushed a commit to openembedded/meta-openembedded that referenced this pull request Aug 26, 2022
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 26, 2022
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
halstead pushed a commit to openembedded/meta-openembedded that referenced this pull request Aug 27, 2022
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 27, 2022
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
@veblush
Copy link
Contributor

veblush commented Aug 30, 2022

gRPC protobuf-at-head test (building it with the HEAD version of protobuf) started failing with this PR since it removed py_proto_library. gRPC doesn't use that rule directly but one of its dependencies, com_github_cncf_udpa does. And error message is

ERROR: Traceback (most recent call last):
        File "/usr/local/google/home/veblush/.cache/bazel/_bazel_veblush/c4652c20fd8d5880d194bf82693e4fee/external/com_github_cncf_udpa/bazel/api_build_system.bzl", line 2, column 66, in <toplevel>
                load("@com_google_protobuf//:protobuf.bzl", _py_proto_library = "py_proto_library")
Error: file '@com_google_protobuf//:protobuf.bzl' does not contain symbol 'py_proto_library'
ERROR: /usr/local/google/home/veblush/git/grpc/BUILD:7688:34: error loading package '@com_github_cncf_udpa//xds/type/v3': Extension file 'bazel/api_build_system.bzl' has errors and referenced by '//:xds_type_upbdefs'

What should we do to resolve this issue?

@perezd
Copy link
Contributor

perezd commented Aug 30, 2022

I believe we explicitly wrote to not rely on this because it wasn't public/guaranteed to be stable:
https://github.com/protocolbuffers/protobuf/blob/3.20.x/protobuf.bzl#L502-L504

@perezd
Copy link
Contributor

perezd commented Aug 30, 2022

They could just simply define this locally for themselves if they like as a workaround.

Comment on lines +153 to +162
conformance_test(
name = "conformance_test_jruby",
failure_list = "//conformance:failure_list_jruby.txt",
testee = "//conformance:conformance_ruby",
text_format_failure_list = "//conformance:text_format_failure_list_jruby.txt",
target_compatible_with = select({
":java_ruby": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I run bazel test --cache_test_results=no //ruby:conformance_test_jruby locally now? It gives:

WARNING: /Users/chenzili/Brittani/protobuf/ruby/BUILD.bazel:99:28: in _proto_gen rule //ruby:test_ruby_protos_deps_genproto: target '//ruby:test_ruby_protos_deps_genproto' depends on deprecated target '//:well_known_protos': Prefer :well_known_type_protos instead.
ERROR: Target //ruby:conformance_test_jruby is incompatible and cannot be built, but was explicitly requested.
Dependency chain:
    //ruby:conformance_test_jruby   <-- target platform (@local_config_platform//:host) didn't satisfy constraint @platforms//:incompatible
INFO: Elapsed time: 0.115s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded, 69 targets configured)
FAILED: Build did NOT complete successfully (1 packages loaded, 69 targets configured)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby is in an awkward state right now, but I'm currently working on cleaning it up (#10525). Until then, this command should allow you to run the jruby conformance tests:
bazel test //ruby:conformance_test_jruby --define=ruby_platform=java --action_env=PATH --action_env=GEM_PATH --action_env=GEM_HOME

After my PR this should be as simple as running bazel test //ruby/... on a system where jruby is currently installed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issues here are that:

  1. Bazel can't currently determine if it should run the jruby or ruby conformance tests
  2. Ruby depends on some environment variables that Bazel strips by default

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out.txt

Interesting. Here is the output when I was debugging #10454. I ran:

bazel test //ruby:conformance_test_jruby --define=ruby_platform=java --action_env=PATH --action_env=GEM_PATH --action_env=GEM_HOME

... and it seems that still c ext is executed:

Starting local Bazel server and connecting to it...
WARNING: /Users/chenzili/Brittani/protobuf/ruby/BUILD.bazel:99:28: in _proto_gen rule //ruby:test_ruby_protos_deps_genproto: target '//ruby:test_ruby_protos_deps_genproto' depends on deprecated target '//:well_known_protos': Prefer :well_known_type_protos instead.
INFO: Analyzed target //ruby:conformance_test_jruby (102 packages loaded, 2180 targets configured).
INFO: Found 1 test target...
ERROR: /Users/chenzili/Brittani/protobuf/ruby/BUILD.bazel:59:24: Executing genrule //ruby:protobuf_java failed: (Exit 1): bash failed: error executing command /bin/bash -c ... (remaining 1 argument skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
rm -f 
rake aborted!
Don't know how to build task 'ext/google/protobuf_c/extconf.rb' (See the list of available tasks with `rake --tasks`)

Tasks: TOP => default => build => compile => compile:arm64-darwin21 => compile:protobuf_c:arm64-darwin21 => copy:protobuf_c:arm64-darwin21:3.1.2 => tmp/arm64-darwin21/protobuf_c/3.1.2/protobuf_c.bundle => tmp/arm64-darwin21/protobuf_c/3.1.2/Makefile
(See full trace by running task with --trace)
/private/var/tmp/_bazel_chenzili/56fb92eeedb5ac2159dafd3ab4543886/sandbox/darwin-sandbox/1/execroot/com_google_protobuf/ruby /private/var/tmp/_bazel_chenzili/56fb92eeedb5ac2159dafd3ab4543886/sandbox/darwin-sandbox/1/execroot/com_google_protobuf
mkdir -p tmp/arm64-darwin21/protobuf_c/3.1.2
Target //ruby:conformance_test_jruby failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 3.985s, Critical Path: 0.41s
INFO: 4 processes: 4 internal.
FAILED: Build did NOT complete successfully
//ruby:conformance_test_jruby                                   FAILED TO BUILD

FAILED: Build did NOT complete successfully

.. so I hard code RUBY_PLATFORM = "java" in ruby/Rakefile and rerun, then I got the output that logics in c ext "Error occurred during parsing" is printed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What shows up if you run ruby --version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ ruby --version
jruby 9.3.7.0 (2.6.8) 2022-08-16 c79ef237e0 OpenJDK 64-Bit Server VM 17.0.3+7 on 17.0.3+7 +jit [arm64-darwin]

I use rbenv to manage the version and run rbenv local jruby-9.3.7.0 at the root path of protobuf.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm it's possible Bazel is picking up a different version in its sandbox. Could you try rbenv global or rvm use? Note: these problems should totally disappear with my upcoming PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. rbenv global works. Thank you!

It will be helpful if you can give a review to #10454 :)

daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Autotools support has been removed and replaced completely by CMake.

See related issues for details:
protocolbuffers/protobuf#7911
protocolbuffers/protobuf#10132

Drop autotools patches too.

Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.