From 733b1cd0bb66641a3c763fbba216a91026b60182 Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Thu, 23 Feb 2023 18:04:53 -0800 Subject: [PATCH] Remove //tools/nogo:full flag. With the latest nogo change, facts are no longer in the build paths. Since full is only used to speed these paths (and is enabled for full analysis) the full flag is now redundant and can be removed safely. PiperOrigin-RevId: 511935530 --- .bazelrc | 4 ++++ .buildkite/pipeline.yaml | 10 +++++----- Makefile | 6 +++--- tools/nogo/BUILD | 8 +------- tools/nogo/cli/cli.go | 10 +--------- tools/nogo/defs.bzl | 14 -------------- 6 files changed, 14 insertions(+), 38 deletions(-) diff --git a/.bazelrc b/.bazelrc index 273f5b0df..4ca507937 100644 --- a/.bazelrc +++ b/.bazelrc @@ -27,6 +27,10 @@ test:race --@io_bazel_rules_go//go/config:race --@io_bazel_rules_go//go/config:p build --@io_bazel_rules_go//go/config:pure test --@io_bazel_rules_go//go/config:pure +# By default, exclude nogo targets from building. They will still be included +# by default for all tests. +build --build_tag_filters=-nogo + # Set flags for x86_64. build:x86_64 --crosstool_top=@crosstool//:toolchains build:x86_64 --cpu=k8 diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 28bd50977..2919ee004 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -93,8 +93,8 @@ steps: <<: *source_test_presubmit label: ":world_map: Build runsc and pkg (AMD64)" commands: - - "make build OPTIONS=--build_tag_filters=-nogo TARGETS=//pkg/..." - - "make build OPTIONS=--build_tag_filters=-nogo TARGETS=//runsc/..." + - "make build TARGETS=//pkg/..." + - "make build TARGETS=//runsc/..." agents: arch: "amd64" @@ -102,8 +102,8 @@ steps: <<: *source_test_presubmit label: ":world_map: Build runsc and pkg (ARM64)" commands: - - "make build OPTIONS=--build_tag_filters=-nogo TARGETS=//pkg/..." - - "make build OPTIONS=--build_tag_filters=-nogo TARGETS=//runsc/..." + - "make build TARGETS=//pkg/..." + - "make build TARGETS=//runsc/..." agents: arch: "arm64" @@ -112,7 +112,7 @@ steps: <<: *source_test_continuous label: ":world_map: Build everything" commands: - - "make build OPTIONS=--build_tag_filters=-nogo TARGETS=//..." + - "make build TARGETS=//..." # Check that the Go branch builds. This is not technically required, as this build is maintained # as a GitHub action in order to preserve this maintaince across forks. However, providing the diff --git a/Makefile b/Makefile index 01c321277..8308bc566 100644 --- a/Makefile +++ b/Makefile @@ -199,18 +199,18 @@ smoke-race-tests: ## Runs a smoke test after build building runsc in race config .PHONY: smoke-race-tests nogo-tests: - @$(call test,--build_tag_filters=nogo --test_tag_filters=nogo --//tools/nogo:full //:all pkg/... tools/...) + @$(call test,--test_tag_filters=nogo //:all pkg/... tools/...) .PHONY: nogo-tests # For unit tests, we take everything in the root, pkg/... and tools/..., and # pull in all directories in runsc except runsc/container. unit-tests: ## Local package unit tests in pkg/..., tools/.., etc. - @$(call test,--build_tag_filters=-nogo --test_tag_filters=-nogo --test_filter=-//runsc/container/... //:all pkg/... tools/... runsc/... vdso/... test/trace/...) + @$(call test,--test_tag_filters=-nogo --test_filter=-//runsc/container/... //:all pkg/... tools/... runsc/... vdso/... test/trace/...) .PHONY: unit-tests # See unit-tests: this includes runsc/container. container-tests: $(RUNTIME_BIN) ## Run all tests in runsc/container/... - @$(call test,--test_env=RUNTIME=$(RUNTIME_BIN) runsc/container/...) + @$(call test,--test_tag_filters=-nogo --test_env=RUNTIME=$(RUNTIME_BIN) runsc/container/...) .PHONY: container-tests tests: ## Runs all unit tests and syscall tests. diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index 844d68a04..47357329b 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -1,14 +1,8 @@ -load("//tools:defs.bzl", "bool_flag", "bzl_library", "go_binary", "select_goarch", "select_goos") +load("//tools:defs.bzl", "bzl_library", "go_binary", "select_goarch", "select_goos") load("//tools/nogo:defs.bzl", "nogo_stdlib", "nogo_target") package(licenses = ["notice"]) -bool_flag( - name = "full", - build_setting_default = False, - visibility = ["//visibility:public"], -) - nogo_target( name = "target", goarch = select_goarch(), diff --git a/tools/nogo/cli/cli.go b/tools/nogo/cli/cli.go index 0ade483f8..1298b2785 100644 --- a/tools/nogo/cli/cli.go +++ b/tools/nogo/cli/cli.go @@ -22,7 +22,6 @@ import ( "os" "path" "path/filepath" - "regexp" "github.com/google/subcommands" "golang.org/x/sys/unix" @@ -228,17 +227,12 @@ func (b *Bundle) SetFlags(fs *flag.FlagSet) { b.setFlags(fs, "bundle") fs.StringVar(&b.Root, "root", "", "root regular expression (for package discovery)") fs.StringVar(&b.Prefix, "prefix", "", "package prefix to apply (for complete names)") - fs.StringVar(&b.Filter, "filter", ".*", "Filter packages to analyze") } // Execute implements subcommands.Command.Execute. func (b *Bundle) Execute(ctx context.Context, fs *flag.FlagSet, args ...any) subcommands.ExitStatus { // Perform the analysis. if err := b.execute(func() (check.FindingSet, facts.Serializer, error) { - pathRegexp, err := regexp.Compile(b.Filter) - if err != nil { - return nil, nil, fmt.Errorf("invalid filter: %v", err) - } // Discover the correct common root. srcRootPrefix, err := check.FindRoot(fs.Args(), b.Root) if err != nil { @@ -251,9 +245,7 @@ func (b *Bundle) Execute(ctx context.Context, fs *flag.FlagSet, args ...any) sub if b.Prefix != "" { path = b.Prefix + "/" + path // Subpackage. } - if pathRegexp.MatchString(path) { - sources[path] = append(sources[path], srcs...) - } + sources[path] = append(sources[path], srcs...) } return check.Bundle(sources) }); err != nil { diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index 3a9d68f26..539f9280a 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -1,6 +1,5 @@ """Nogo rules.""" -load("//tools/bazeldefs:defs.bzl", "BuildSettingInfo") load("//tools/bazeldefs:go.bzl", "go_context", "go_embed_libraries", "go_importpath", "go_rule") load("//tools:arch.bzl", "arch_transition", "transition_allowlist") @@ -65,10 +64,6 @@ NogoStdlibInfo = provider( ) def _nogo_stdlib_impl(ctx): - package_filter = ".*" - if not ctx.attr._nogo_full[BuildSettingInfo].value: - package_filter = "^runtime$" - # Build the configuration for the stdlib. go_ctx, args, inputs, raw_findings = _nogo_config(ctx, deps = []) @@ -95,7 +90,6 @@ def _nogo_stdlib_impl(ctx): "-findings=%s" % findings_file.path, "-facts=%s" % facts_file.path, "-root=.*?/src/", - "-filter=%s" % package_filter, ] + [f.path for f in go_ctx.stdlib_srcs], ) @@ -122,10 +116,6 @@ nogo_stdlib = go_rule( default = "//tools/nogo:target", cfg = "target", ), - "_nogo_full": attr.label( - default = "//tools/nogo:full", - cfg = "exec", - ), }, ) @@ -425,10 +415,6 @@ nogo_test = rule( default = "//tools/nogo:target", cfg = arch_transition, ), - "_nogo_full": attr.label( - default = "//tools/nogo:full", - cfg = "exec", - ), "_allowlist_function_transition": attr.label( default = transition_allowlist, ),