Skip to content

Commit

Permalink
main.star: run longtest builders in presubmit if vendor is modified
Browse files Browse the repository at this point in the history 
This change enables longtest builders in presubmit if files related to
vendoring are modified. Having these enabled would've caught several
issues in vendored repositories and in the main Go repository in the
past.

While we're here, let's also ignore location filters for security
presubmit. Previously we applied them and this filtered down where the
staticlockranking builder would run, but if we apply them now then we'll
limit where longtest builders are run. However, we always want longtest
builders to run for security presubmit. Ignoring location filters
entirely is a reasonable decision here: we want to be as safe as
possible with security presubmit, so we want to test as many
configurations as possible. The extra builder runs are unlikely to cost
much and will help to ensure stability when landing security CLs
upstream.

For golang/go#42661.

Change-Id: Ic35216722ddca4c26cccbd96da045db5c100defb
Reviewed-on: https://go-review.googlesource.com/c/build/+/547596
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
  • Loading branch information
@mknyszek
mknyszek committed Dec 6, 2023
1 parent de06e77 commit 7344bd8
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 33 deletions.
79 changes: 60 additions & 19 deletions generated/commit-queue.cfg
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13136,11 +13136,6 @@ config_groups {
builders {
name: "golang/security-try/go1.20-linux-amd64-staticlockranking"
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-internal-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/runtime/.+"
}
}
builders {
name: "golang/security-try/go1.20-linux-arm64"
Expand Down Expand Up @@ -13521,11 +13516,6 @@ config_groups {
builders {
name: "golang/security-try/go1.21-linux-amd64-staticlockranking"
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-internal-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/runtime/.+"
}
}
builders {
name: "golang/security-try/go1.21-linux-arm64"
Expand Down Expand Up @@ -13918,11 +13908,6 @@ config_groups {
builders {
name: "golang/security-try/gotip-linux-amd64-staticlockranking"
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-internal-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/runtime/.+"
}
}
builders {
name: "golang/security-try/gotip-linux-arm64"
Expand Down Expand Up @@ -15367,8 +15352,22 @@ config_groups {
}
builders {
name: "golang/try/gotip-linux-386-longtest"
includable_only: true
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}go[.]{mod,sum}"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}vendor/.+"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/.+_bundle.go"
}
}
builders {
name: "golang/try/gotip-linux-amd64"
Expand All @@ -15380,13 +15379,41 @@ config_groups {
}
builders {
name: "golang/try/gotip-linux-amd64-longtest"
includable_only: true
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}go[.]{mod,sum}"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}vendor/.+"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/.+_bundle.go"
}
}
builders {
name: "golang/try/gotip-linux-amd64-longtest-race"
includable_only: true
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}go[.]{mod,sum}"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}vendor/.+"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/.+_bundle.go"
}
}
builders {
name: "golang/try/gotip-linux-amd64-misccompile"
Expand Down Expand Up @@ -15491,8 +15518,22 @@ config_groups {
}
builders {
name: "golang/try/gotip-windows-amd64-longtest"
includable_only: true
disable_reuse: true
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}go[.]{mod,sum}"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/{,cmd/}vendor/.+"
}
location_filters {
gerrit_host_regexp: "go-review.googlesource.com"
gerrit_project_regexp: "^go$"
path_regexp: "src/.+_bundle.go"
}
}
builders {
name: "golang/try/gotip-windows-amd64-race"
Expand Down
37 changes: 23 additions & 14 deletions main.star
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,12 +354,21 @@ def make_run_mod(add_props = {}, add_env = {}, enabled = None):
apply = apply_mod,
)

# enable only if project matches one of the provided projects, or
# for the release branches in the Go project.
# enable only if project matches one of the provided projects and certain source
# locations are modified by the CL, or always for the release branches of the go project.
# projects is a dict mapping a project name to filters.
def presubmit_only_for_projs_or_on_release_branches(projects):
def f(port, project, go_branch_short):
presubmit = project in projects or (project == "go" and go_branch_short != "gotip")
return (True, presubmit, True, [])
filters = []
if project == "go":
presubmit = project in projects or go_branch_short != "gotip"
if project in projects and go_branch_short == "gotip":
filters = projects[project]
else:
presubmit = project in projects
if presubmit:
filters = projects[project]
return (True, presubmit, True, filters)

return f

Expand Down Expand Up @@ -401,7 +410,15 @@ def define_for_go_postsubmit_or_presubmit_with_filters(filters):
# RUN_MODS is a list of valid run-time modifications to the way we
# build and test our various projects.
RUN_MODS = dict(
longtest = make_run_mod({"long_test": True}, {"GO_TEST_TIMEOUT_SCALE": "5"}, enabled = presubmit_only_for_projs_or_on_release_branches(["protobuf"])),
longtest = make_run_mod({"long_test": True}, {"GO_TEST_TIMEOUT_SCALE": "5"}, enabled = presubmit_only_for_projs_or_on_release_branches({
"protobuf": [],
"go": [
# Enable longtest builders on go against tip if files related to vendored code are modified.
"src/{,cmd/}go[.]{mod,sum}",
"src/{,cmd/}vendor/.+",
"src/.+_bundle.go",
],
})),
race = make_run_mod({"race_mode": True}, enabled = presubmit_only_for_ports_or_on_release_branches(["linux-amd64"])),
boringcrypto = make_run_mod(add_env = {"GOEXPERIMENT": "boringcrypto"}),
# The misccompile mod indicates that the builder should act as a "misc-compile" builder,
Expand Down Expand Up @@ -1364,7 +1381,7 @@ def _define_go_internal_ci():
)

for builder_type in BUILDER_TYPES:
exists, _, _, presubmit_filters = enabled(LOW_CAPACITY_HOSTS, "go", go_branch_short, builder_type)
exists, _, _, _ = enabled(LOW_CAPACITY_HOSTS, "go", go_branch_short, builder_type)

# The internal host only has access to some machines. As of
# writing, that means all the GCE-hosted (high-capacity) builders
Expand All @@ -1380,14 +1397,6 @@ def _define_go_internal_ci():
builder = name,
cq_group = cq_group_name,
disable_reuse = True,
location_filters = [
cq.location_filter(
gerrit_host_regexp = "go-internal-review.googlesource.com",
gerrit_project_regexp = "^go$",
path_regexp = filter,
)
for filter in presubmit_filters
],
)

_define_go_ci()
Expand Down

0 comments on commit 7344bd8

Please sign in to comment.