From c4c258816f04e9a6148e07567d7f6a607a2e86fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Tue, 2 Feb 2021 16:58:31 +0100 Subject: [PATCH 1/4] MultiBaseLocalDiskRepositoryManagerTest: fix for running on Mac from Eclipse The alternateRepositoryLocation test was failing when running from Eclipse on Mac because the created repository location would be: /private/var/rest/of/path while the alternateBasePath is: /var/rest/of/path This difference comes from the fact that on Mac the /var is a symlink to /private/var and the creation of the repository would resolve the location using getCanonicalFile(). Use alternateBasePath.toRealPath() to resolve any symlinks in the alternateBasePath. NOTE: this issue doesn't show up when running tests from bazel as bazel's sandboxing uses different path locations. Change-Id: I2cb1adc48e16edc5adffd7b7f8eea2b5ea1a60f3 --- .../server/git/MultiBaseLocalDiskRepositoryManagerTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java b/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java index 29f520b0b7..4902830b86 100644 --- a/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java +++ b/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java @@ -93,12 +93,14 @@ public class MultiBaseLocalDiskRepositoryManagerTest { Repository repo = repoManager.createRepository(someProjectKey); assertThat(repo.getDirectory()).isNotNull(); assertThat(repo.getDirectory().exists()).isTrue(); - assertThat(repo.getDirectory().getParent()).isEqualTo(alternateBasePath.toString()); + assertThat(repo.getDirectory().getParent()) + .isEqualTo(alternateBasePath.toRealPath().toString()); repo = repoManager.openRepository(someProjectKey); assertThat(repo.getDirectory()).isNotNull(); assertThat(repo.getDirectory().exists()).isTrue(); - assertThat(repo.getDirectory().getParent()).isEqualTo(alternateBasePath.toString()); + assertThat(repo.getDirectory().getParent()) + .isEqualTo(alternateBasePath.toRealPath().toString()); assertThat(repoManager.getBasePath(someProjectKey).toAbsolutePath().toString()) .isEqualTo(alternateBasePath.toString()); From 4332dc53ec3be27bb9f556f20b3fe29a5b50c0e5 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Wed, 3 Feb 2021 16:04:49 +0100 Subject: [PATCH 2/4] Bump Bazel version to 4.0.0 Change-Id: I06072449cf14e80f7c5e585ee97a02f86cb75bd8 --- .bazelversion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bazelversion b/.bazelversion index 7c69a55dbb..fcdb2e109f 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -3.7.0 +4.0.0 From 8731af3ae785efe9ecff7f3d04302b6b01c4fc0b Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Tue, 17 Nov 2020 11:09:05 +0100 Subject: [PATCH 3/4] ChangeInfo: Serve reviewers when LABELS are requested With the launch of attention set, the UI started requesting DETAILED_LABELS to populate reviewers on dashboards. Unfortunately, this also computes which reviewer is allowed to vote on any label. This check requires getting the group memberships for each reviewer. This operation is expensive when external group backends are used. In this change, we expose reviewers also when LABELS are requested. This will enable the UI to stop requesting DETAILED_LABELS and request only the cheaper LABELS instead. Bug: Issue 13899 Change-Id: I2fb5da919da28c99766109b263124f2cdac5e585 (cherry picked from commit ae92ad8a9989bed688ecdc55c00c12f60a8ffbe1) --- Documentation/rest-api-changes.txt | 18 ++++++++++++------ .../gerrit/server/change/ChangeJson.java | 2 ++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 84e9c7b924..12f616aebc 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -222,15 +222,18 @@ default. Optional fields are: [[labels]] -- * `LABELS`: a summary of each label required for submit, and - approvers that have granted (or rejected) with that label. + approvers that have granted (or rejected) with that label + as well as all reviewers by state, and reviewers that may + be removed by the current user. -- [[detailed-labels]] -- * `DETAILED_LABELS`: detailed label information, including numeric values of all existing approvals, recognized label values, values - permitted to be set by the current user, all reviewers by state, and - reviewers that may be removed by the current user. + permitted to be set by any reviewer and the change owner, all + reviewers by state, and reviewers that may be removed by the + current user. -- [[current-revision]] @@ -6414,7 +6417,8 @@ Only set if link:#detailed-labels[detailed labels] are requested. |`removable_reviewers`|optional| The reviewers that can be removed by the calling user as a list of link:rest-api-accounts.html#account-info[AccountInfo] entities. + -Only set if link:#detailed-labels[detailed labels] are requested. +Only set if link:#labels[labels] or +link:#detailed-labels[detailed labels] are requested. |`reviewers` |optional| The reviewers as a map that maps a reviewer state to a list of link:rest-api-accounts.html#account-info[AccountInfo] entities. @@ -6423,13 +6427,15 @@ Possible reviewer states are `REVIEWER`, `CC` and `REMOVED`. + `CC`: Users that were added to the change, but have not voted. + `REMOVED`: Users that were previously reviewers on the change, but have been removed. + -Only set if link:#detailed-labels[detailed labels] are requested. +Only set if link:#labels[labels] or +link:#detailed-labels[detailed labels] are requested. |`pending_reviewers` |optional| Updates to `reviewers` that have been made while the change was in the WIP state. Only present on WIP changes and only if there are pending reviewer updates to report. These are reviewers who have not yet been notified about being added to or removed from the change. + -Only set if link:#detailed-labels[detailed labels] are requested. +Only set if link:#labels[labels] or +link:#detailed-labels[detailed labels] are requested. |`reviewer_updates`|optional| Updates to reviewers set for the change as link:#review-update-info[ReviewerUpdateInfo] entities. diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index 8323cfd10b..7b2663a9f2 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -602,7 +602,9 @@ public class ChangeJson { ? labelsJson.permittedLabels(user.getAccountId(), cd) : ImmutableMap.of(); } + } + if (has(LABELS) || has(DETAILED_LABELS)) { out.reviewers = reviewerMap(cd.reviewers(), cd.reviewersByEmail(), false); out.pendingReviewers = reviewerMap(cd.pendingReviewers(), cd.pendingReviewersByEmail(), true); out.removableReviewers = removableReviewers(cd, out); From b5867f452be52dbd6e7bd26baaa24b885a4b9899 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 7 Feb 2021 15:56:55 +0100 Subject: [PATCH 4/4] Patch rules_nodejs library to fix wrong regex string escape On stable-3.2 branch outdated version of rules_nodejs version is used: 1.5, that suffers from usage of wrong regular expression escapes. More recent Bazel versions added a check for that regex string escape pattern and flagging the external rules_nodejs repository as invalid. Given that upgrade path to recent rules_nodejs version is not a trivial one: 1.5 => 1.7 => 2.0, and given that also plugins would need to be updated, we prefer to keep that outdated rules_nodejs version as-is and patch the wrong line in rules_nodejs distribution instead. That would allow us to upgrade to recent bazel versions on stable-3.2 branch. We prefer to keep the same Bazel version on all supported stable branches to reduce/avoid rebuilds when switching between branches. Note, that this workaround should be reverted when this change is merged up to master branch. Change-Id: Ia99ea8398ea87847c27d75f8e47099b2c04c9186 --- WORKSPACE | 2 ++ rules_nodejs-1.5.patch | 12 ++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 rules_nodejs-1.5.patch diff --git a/WORKSPACE b/WORKSPACE index d968710cae..d2aba31a0a 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -60,6 +60,8 @@ http_archive( http_archive( name = "build_bazel_rules_nodejs", + patch_args = ["-p1"], + patches = ["//:rules_nodejs-1.5.patch"], sha256 = "d0c4bb8b902c1658f42eb5563809c70a06e46015d64057d25560b0eb4bdc9007", urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/1.5.0/rules_nodejs-1.5.0.tar.gz"], ) diff --git a/rules_nodejs-1.5.patch b/rules_nodejs-1.5.patch new file mode 100644 index 0000000000..baa566b3ac --- /dev/null +++ b/rules_nodejs-1.5.patch @@ -0,0 +1,12 @@ +--- build_bazel_rules_nodejs/internal/node/node.bzl 2021-02-07 16:09:38.099484740 +0100 ++++ build_bazel_rules_nodejs/internal/node/node_fixed.bzl 2021-02-07 16:10:33.583847582 +0100 +@@ -87,6 +87,6 @@ + for d in ctx.attr.data: + if hasattr(d, "runfiles_module_mappings"): + for [mn, mr] in d.runfiles_module_mappings.items(): +- escaped = mn.replace("/", "\/").replace(".", "\.") ++ escaped = mn.replace("/", "\\/").replace(".", "\\.") + mapping = "{module_name: /^%s\\b/, module_root: '%s'}" % (escaped, mr) + module_mappings.append(mapping) + +