Bazel distribtests by gnossen · Pull Request #25318 · grpc/grpc

gnossen · 2021-02-01T17:03:59Z

This PR adds Bazel Distribtests, officially codifying our supported range of Bazel versions. Two new CI jobs are added, whose triggers are TBD:

Basic Distribtests - Runs bazel build on all supported Bazel versions, based on a manually curated list.
Bazel Latest Version Distribtest - Pulls down and builds with the latest version of Bazel.

Several targets are currently on a blocklist due to preexisting build failures. A next step is to triage this build errors and have them either fixed or removed.

One snag -- upb doesn't currently support our range of builds. An older version of their repo supports the 2.X series, while the commit I've introduced in this PR supports 3+. I've added a check for the Bazel version and manually swap out the upb version based on the Bazel version. This pattern should be moved to the upb repo so it can support multiple Bazel versions and we can remove the check from our repo.

jtattermusch · 2021-02-01T17:57:54Z

FYI The toolchain problem is the same as in here: #25001

jtattermusch · 2021-02-02T14:01:48Z

test/distrib/bazel/run_bazel_distrib_test.sh

+
+set -ex
+
+shopt -s nullglob


nit: why do you need this? same for the other .sh files.

jtattermusch · 2021-02-02T14:07:16Z

third_party/toolchains/bazel_0.26.0_rbe_windows/BUILD

        "x64_windows_msys": ":cc-compiler-x64_windows_msys",
        "x64_windows": ":cc-compiler-x64_windows",
        "armeabi-v7a": ":cc-compiler-armeabi-v7a",
+        "k8": ":_dummy_toolchain",


why the changes? this file is autogenerated AFAIK so I don't feel comfortable updating it manually.
Would regenerating the BUILD help?
go/rbe-windows-user-guide

jtattermusch · 2021-02-02T14:07:51Z

tools/internal_ci/linux/grpc_build_submodule_at_head.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-# Copyright 2017 gRPC authors.
+# Copyright 2020 The gRPC authors.


seems unrelated to this PR? revert?

jtattermusch · 2021-02-02T14:08:45Z

tools/internal_ci/linux/grpc_bazel_distribtest_latest.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Build portability tests with an updated submodule


this comment is now out of date

jtattermusch · 2021-02-02T14:11:30Z

test/distrib/bazel/test_single_bazel_version.sh

+
+cd "$(dirname "$0")"/../../..
+
+EXCEPTIONS=(


nit: rename to EXCLUDED_TARGETS or EXCLUDED_PATTERNS (as you wish)?

jtattermusch · 2021-02-02T14:30:59Z

third_party/toolchains/backwards_compatibility.bzl

@@ -0,0 +1,45 @@
+# Copyright 2021 The gRPC authors.


since you're adding this file, I think we need to cherrypick this PR into an import and see if this works internally as well.

jtattermusch · 2021-02-02T14:33:35Z

tools/internal_ci/linux/grpc_bazel_distribtest.cfg

@@ -0,0 +1,26 @@
+# Copyright 2017 gRPC authors.


Generally I think we should avoid creating more kokoro jobs as these become hard to maintain, but for now I don't have a better alternative, so creating the 2 new kokoro jobs seems acceptable.

jtattermusch · 2021-02-02T14:36:39Z

third_party/toolchains/backwards_compatibility.bzl

+        )
+
+
+def bc_platform(**kwargs):


do you really need the bc_platform function?

In my PR this seemed to be the same case as create_rbe_exec_properties_dict
https://github.com/grpc/grpc/pull/25001/files#diff-e084337b59570d9fb8e3d40828b096dddf9056a9096eb32b44ceeb7fbb0383fcR33

jtattermusch · 2021-02-02T14:38:37Z

test/distrib/bazel/run_bazel_distrib_test.sh

+)
+
+for VERSION in "${SUPPORTED_VERSIONS[@]}"; do
+    ./test_single_bazel_version.sh "$VERSION"


nit: the output of 4 consecutive builds is going to be difficult to parse by humans.
Ideally these would be 4 independent "tasks".

Should we instead add 4-5 tasks into distribtest_targets.py and save on needing to define 2 new kokoro jobs?
That would also make the output of the separate builds separated.

jtattermusch · 2021-02-02T14:42:16Z

test/distrib/bazel/test_single_bazel_version.sh

+
+
+export OVERRIDE_BAZEL_VERSION="$VERSION"
+printf ' -%s' "${EXCEPTIONS[@]}" | xargs bazel build -- //...


this can be addressed later, but one thing this distribtest doesn't cover is depending upon grpc as a library in a simple "helloworld" project. Like this we will know that gRPC builds fine, but not whether it can be safely imported as a dependency.

stale · 2021-06-02T17:47:19Z

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

stale · 2021-09-19T19:39:56Z

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

jtattermusch · 2021-09-20T07:44:33Z

Is this still blocked on the bazel toolchain update (same as in #25001). It would be good to address this somehow and unblock this functionality. This PR has been open for >6months now.

jtattermusch · 2021-09-27T10:39:49Z

I upgraded the repo to bazel 4.2.1, which makes many changes in this PR unnecessary:
#27410

jtattermusch · 2021-09-28T11:07:01Z

This needs to be rebased / updated now that #27486 has been merged.

stale · 2022-01-03T20:39:06Z

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

jtattermusch · 2022-01-13T13:41:07Z

@gnossen let's revive this.

jtattermusch · 2022-02-22T10:25:30Z

This PR is very out of date and we might not need the compatibility layer for bazel 2.x anymore (since we support bazel 3.x,4.x and 5.x now), so I'm going to remove myself from the reviewer list.
Consider updating / closing the PR.

gnossen added 7 commits January 29, 2021 12:22

Make repo globally buildable

08e8c7d

Add initial distrib test script

ae93c51

Add test script

77727e8

Support multiple versions

6ebf2d5

Split out a separate latest version test

864110c

Integrate with kokoro

174e5c4

Merge remote-tracking branch 'origin/master' into bazel_distribtests

93c5c42

gnossen added area/test infra/Bazel lang/all wrapped languages release notes: no Indicates if PR should not be in release notes labels Feb 1, 2021

gnossen marked this pull request as ready for review February 1, 2021 17:20

gnossen requested review from jtattermusch, nicolasnoble and veblush as code owners February 1, 2021 17:20

gnossen assigned jtattermusch Feb 1, 2021

jtattermusch reviewed Feb 2, 2021

View reviewed changes

stale bot added the disposition/stale label Jun 2, 2021

stale bot closed this Jun 9, 2021

gnossen reopened this Jun 21, 2021

stale bot removed the disposition/stale label Jun 21, 2021

gnossen added the kokoro:force-run label Jun 21, 2021

grpc-kokoro removed the kokoro:force-run label Jun 21, 2021

stale bot added the disposition/stale label Sep 19, 2021

stale bot removed the disposition/stale label Sep 20, 2021

jtattermusch mentioned this pull request Sep 22, 2021

Upgrade bazel to 4.2.1 (LTS), upgrade bazel toolchain to 4.1.0 #27410

Merged

jtattermusch mentioned this pull request Sep 27, 2021

Add bazel distribtests (minimalist version that only tests with bazel 3.x and 4.x) #27486

Merged

stale bot added the disposition/stale label Jan 3, 2022

jtattermusch added the disposition/never stale label Jan 13, 2022

stale bot removed the disposition/stale label Jan 13, 2022

jtattermusch removed their assignment Feb 22, 2022



		export OVERRIDE_BAZEL_VERSION="$VERSION"
		printf ' -%s' "${EXCEPTIONS[@]}" \| xargs bazel build -- //...


		set -ex

		shopt -s nullglob

		)


		def bc_platform(**kwargs):

Conversation

gnossen commented Feb 1, 2021

Uh oh!

jtattermusch commented Feb 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Jun 2, 2021

Uh oh!

stale bot commented Sep 19, 2021

Uh oh!

jtattermusch commented Sep 20, 2021

Uh oh!

jtattermusch commented Sep 27, 2021

Uh oh!

jtattermusch commented Sep 28, 2021

Uh oh!

stale bot commented Jan 3, 2022

Uh oh!

jtattermusch commented Jan 13, 2022

Uh oh!

jtattermusch commented Feb 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants