diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl index cfb7aee697..43872cf3c6 100644 --- a/go/private/actions/compilepkg.bzl +++ b/go/private/actions/compilepkg.bzl @@ -207,12 +207,13 @@ def emit_compilepkg( _run_nogo( go, sources = sources, + cgo_go_srcs = cgo_go_srcs_for_nogo, importpath = importpath, importmap = importmap, archives = archives + ([cover_archive] if cover_archive else []), recompile_internal_deps = recompile_internal_deps, cover_mode = cover_mode, - cgo_go_srcs = cgo_go_srcs_for_nogo, + testfilter = testfilter, out_facts = out_facts, out_log = out_nogo_log, out_validation = out_nogo_validation, @@ -223,12 +224,13 @@ def _run_nogo( go, *, sources, + cgo_go_srcs, importpath, importmap, archives, recompile_internal_deps, cover_mode, - cgo_go_srcs, + testfilter, out_facts, out_log, out_validation, @@ -256,7 +258,10 @@ def _run_nogo( args.add("-importpath", go.label.name) if importmap: args.add("-p", importmap) + args.add("-package_list", go.package_list) + if testfilter: + args.add("-testfilter", testfilter) args.add_all(archives, before_each = "-facts", map_each = _facts) args.add("-out_facts", out_facts) diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 9e941c77b8..354f86a5d6 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -98,29 +98,9 @@ func compilePkg(args []string) error { return err } - // TODO(jayconrod): remove -testfilter flag. The test action should compile - // the main, internal, and external packages by calling compileArchive - // with the correct sources for each. - switch testFilter { - case "off": - case "only": - testSrcs := make([]fileInfo, 0, len(srcs.goSrcs)) - for _, f := range srcs.goSrcs { - if strings.HasSuffix(f.pkg, "_test") { - testSrcs = append(testSrcs, f) - } - } - srcs.goSrcs = testSrcs - case "exclude": - libSrcs := make([]fileInfo, 0, len(srcs.goSrcs)) - for _, f := range srcs.goSrcs { - if !strings.HasSuffix(f.pkg, "_test") { - libSrcs = append(libSrcs, f) - } - } - srcs.goSrcs = libSrcs - default: - return fmt.Errorf("invalid test filter %q", testFilter) + err = applyTestFilter(testFilter, &srcs) + if err != nil { + return err } return compileArchive( @@ -351,44 +331,10 @@ func compileArchive( gcFlags = append(gcFlags, createTrimPath(gcFlags, ".")) } - // Check that the filtered sources don't import anything outside of - // the standard library and the direct dependencies. - imports, err := checkImports(srcs.goSrcs, deps, packageListPath, importPath, recompileInternalDeps) - if err != nil { - return err - } - if compilingWithCgo { - // cgo generated code imports some extra packages. - imports["runtime/cgo"] = nil - imports["syscall"] = nil - imports["unsafe"] = nil - } - if coverMode != "" { - if coverMode == "atomic" { - imports["sync/atomic"] = nil - } - const coverdataPath = "github.com/bazelbuild/rules_go/go/tools/coverdata" - var coverdata *archive - for i := range deps { - if deps[i].importPath == coverdataPath { - coverdata = &deps[i] - break - } - } - if coverdata == nil { - return errors.New("coverage requested but coverdata dependency not provided") - } - imports[coverdataPath] = coverdata - } - - // Build an importcfg file for the compiler. - importcfgPath, err := buildImportcfgFileForCompile(imports, goenv.installSuffix, filepath.Dir(outLinkObj)) + importcfgPath, err := checkImportsAndBuildCfg(goenv, importPath, srcs, deps, packageListPath, recompileInternalDeps, compilingWithCgo, coverMode, workDir) if err != nil { return err } - if !goenv.shouldPreserveWorkDir { - defer os.Remove(importcfgPath) - } // Build an embedcfg file mapping embed patterns to filenames. // Embed patterns are relative to any one of a list of root directories @@ -490,6 +436,45 @@ func compileArchive( return nil } +func checkImportsAndBuildCfg(goenv *env, importPath string, srcs archiveSrcs, deps []archive, packageListPath string, recompileInternalDeps []string, compilingWithCgo bool, coverMode string, workDir string) (string, error) { + // Check that the filtered sources don't import anything outside of + // the standard library and the direct dependencies. + imports, err := checkImports(srcs.goSrcs, deps, packageListPath, importPath, recompileInternalDeps) + if err != nil { + return "", err + } + if compilingWithCgo { + // cgo generated code imports some extra packages. + imports["runtime/cgo"] = nil + imports["syscall"] = nil + imports["unsafe"] = nil + } + if coverMode != "" { + if coverMode == "atomic" { + imports["sync/atomic"] = nil + } + const coverdataPath = "github.com/bazelbuild/rules_go/go/tools/coverdata" + var coverdata *archive + for i := range deps { + if deps[i].importPath == coverdataPath { + coverdata = &deps[i] + break + } + } + if coverdata == nil { + return "", errors.New("coverage requested but coverdata dependency not provided") + } + imports[coverdataPath] = coverdata + } + + // Build an importcfg file for the compiler. + importcfgPath, err := buildImportcfgFileForCompile(imports, goenv.installSuffix, workDir) + if err != nil { + return "", err + } + return importcfgPath, nil +} + func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, pgoprofile, outLinkobjPath, outInterfacePath string) error { args := goenv.goTool("compile") args = append(args, "-p", packagePath, "-importcfg", importcfgPath, "-pack") diff --git a/go/tools/builders/filter.go b/go/tools/builders/filter.go index be4c8accb3..36cff41428 100644 --- a/go/tools/builders/filter.go +++ b/go/tools/builders/filter.go @@ -112,6 +112,36 @@ func filterAndSplitFiles(fileNames []string) (archiveSrcs, error) { return res, nil } +// applyTestFilter filters out test files from the list of sources in place +// according to the filter. +func applyTestFilter(testFilter string, srcs *archiveSrcs) error { + // TODO(jayconrod): remove -testfilter flag. The test action should compile + // the main, internal, and external packages by calling compileArchive + // with the correct sources for each. + switch testFilter { + case "off": + case "only": + testSrcs := make([]fileInfo, 0, len(srcs.goSrcs)) + for _, f := range srcs.goSrcs { + if strings.HasSuffix(f.pkg, "_test") { + testSrcs = append(testSrcs, f) + } + } + srcs.goSrcs = testSrcs + case "exclude": + libSrcs := make([]fileInfo, 0, len(srcs.goSrcs)) + for _, f := range srcs.goSrcs { + if !strings.HasSuffix(f.pkg, "_test") { + libSrcs = append(libSrcs, f) + } + } + srcs.goSrcs = libSrcs + default: + return fmt.Errorf("invalid test filter %q", testFilter) + } + return nil +} + // readFileInfo applies build constraints to an input file and returns whether // it should be compiled. func readFileInfo(bctx build.Context, input string) (fileInfo, error) { diff --git a/go/tools/builders/nogo.go b/go/tools/builders/nogo.go index c49e8bb125..9747942f89 100644 --- a/go/tools/builders/nogo.go +++ b/go/tools/builders/nogo.go @@ -23,6 +23,7 @@ func nogo(args []string) error { var unfilteredSrcs, recompileInternalDeps multiFlag var deps, facts archiveMultiFlag var importPath, packagePath, nogoPath, packageListPath string + var testFilter string var outFactsPath, outLogPath string var coverMode string fs.Var(&unfilteredSrcs, "src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and compiled") @@ -33,6 +34,7 @@ func nogo(args []string) error { fs.StringVar(&packageListPath, "package_list", "", "The file containing the list of standard library packages") fs.Var(&recompileInternalDeps, "recompile_internal_deps", "The import path of the direct dependencies that needs to be recompiled.") fs.StringVar(&coverMode, "cover_mode", "", "The coverage mode to use. Empty if coverage instrumentation should not be added.") + fs.StringVar(&testFilter, "testfilter", "off", "Controls test package filtering") fs.StringVar(&nogoPath, "nogo", "", "The nogo binary") fs.StringVar(&outFactsPath, "out_facts", "", "The file to emit serialized nogo facts to") fs.StringVar(&outLogPath, "out_log", "", "The file to emit nogo logs into") @@ -52,6 +54,11 @@ func nogo(args []string) error { return err } + err = applyTestFilter(testFilter, &srcs) + if err != nil { + return err + } + var goSrcs []string haveCgo := false for _, src := range srcs.goSrcs { @@ -68,42 +75,11 @@ func nogo(args []string) error { } defer cleanup() - imports, err := checkImports(srcs.goSrcs, deps, packageListPath, importPath, recompileInternalDeps) + compilingWithCgo := os.Getenv("CGO_ENABLED") == "1" && haveCgo + importcfgPath, err := checkImportsAndBuildCfg(goenv, importPath, srcs, deps, packageListPath, recompileInternalDeps, compilingWithCgo, coverMode, workDir) if err != nil { return err } - cgoEnabled := os.Getenv("CGO_ENABLED") == "1" - if haveCgo && cgoEnabled { - // cgo generated code imports some extra packages. - imports["runtime/cgo"] = nil - imports["syscall"] = nil - imports["unsafe"] = nil - } - if coverMode != "" { - if coverMode == "atomic" { - imports["sync/atomic"] = nil - } - const coverdataPath = "github.com/bazelbuild/rules_go/go/tools/coverdata" - var coverdata *archive - for i := range deps { - if deps[i].importPath == coverdataPath { - coverdata = &deps[i] - break - } - } - if coverdata == nil { - return errors.New("coverage requested but coverdata dependency not provided") - } - imports[coverdataPath] = coverdata - } - - importcfgPath, err := buildImportcfgFileForCompile(imports, goenv.installSuffix, filepath.Dir(outFactsPath)) - if err != nil { - return err - } - if !goenv.shouldPreserveWorkDir { - defer os.Remove(importcfgPath) - } return runNogo(workDir, nogoPath, goSrcs, facts, importPath, importcfgPath, outFactsPath, outLogPath) } @@ -111,7 +87,16 @@ func nogo(args []string) error { func runNogo(workDir string, nogoPath string, srcs []string, facts []archive, packagePath, importcfgPath, outFactsPath string, outLogPath string) error { if len(srcs) == 0 { // emit_compilepkg expects a nogo facts file, even if it's empty. - return os.WriteFile(outFactsPath, nil, 0o666) + // We also need to write the validation output log. + err := os.WriteFile(outFactsPath, nil, 0o666) + if err != nil { + return fmt.Errorf("error writing empty nogo facts file: %v", err) + } + err = os.WriteFile(outLogPath, nil, 0o666) + if err != nil { + return fmt.Errorf("error writing empty nogo log file: %v", err) + } + return nil } args := []string{nogoPath} args = append(args, "-p", packagePath) diff --git a/tests/core/nogo/BUILD.bazel b/tests/core/nogo/BUILD.bazel deleted file mode 100644 index 46c7ee89e2..0000000000 --- a/tests/core/nogo/BUILD.bazel +++ /dev/null @@ -1,5 +0,0 @@ -filegroup( - name = "rules_go_deps", - srcs = [":common.bzl"], - visibility = ["//visibility:public"], -) diff --git a/tests/core/nogo/common.bzl b/tests/core/nogo/common.bzl deleted file mode 100644 index 14089ce2fe..0000000000 --- a/tests/core/nogo/common.bzl +++ /dev/null @@ -1,38 +0,0 @@ -# Macros used by all nogo integration tests. - -BUILD_FAILED_TMPL = """ -if [[ result -eq 0 ]]; then - echo "TEST FAILED: expected build error" >&2 - result=1 -else - result=0 - {check_err} -fi -""" - -BUILD_PASSED_TMPL = """ -if [[ result -ne 0 ]]; then - echo "TEST FAILED: unexpected build error" >&2 - result=1 -else - {check_err} -fi -""" - -CONTAINS_ERR_TMPL = """ - lines=$(grep '{err}' bazel-output.txt | wc -l) - if [ $lines -eq 0 ]; then - echo "TEST FAILED: expected error message containing: '{err}'" >&2 - result=1 - elif [ $lines -ne 1 ]; then - echo "TEST FAILED: expected error message '{err}' appears more than once" >&2 - result=1 - fi -""" - -DOES_NOT_CONTAIN_ERR_TMPL = """ - if grep -q '{err}' bazel-output.txt; then - echo "TEST FAILED: received error message containing: '{err}'" >&2 - result=1 - fi -""" diff --git a/tests/core/nogo/tests/BUILD.bazel b/tests/core/nogo/tests/BUILD.bazel new file mode 100644 index 0000000000..b96835c488 --- /dev/null +++ b/tests/core/nogo/tests/BUILD.bazel @@ -0,0 +1,6 @@ +load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test") + +go_bazel_test( + name = "tests_test", + srcs = ["tests_test.go"], +) diff --git a/tests/core/nogo/tests/README.rst b/tests/core/nogo/tests/README.rst new file mode 100644 index 0000000000..c2e43550fb --- /dev/null +++ b/tests/core/nogo/tests/README.rst @@ -0,0 +1,13 @@ +nogo for go_test +============================= + +.. _nogo: /go/nogo.rst +.. _go_test: /docs/go/core/rules.md#_go_test + +Verifies interactions between `nogo`_ and `go_test`_. + + +tests_test +============================= + +Tests that `nogo`_ can handle various edge cases of external tests. diff --git a/tests/core/nogo/tests/tests_test.go b/tests/core/nogo/tests/tests_test.go new file mode 100644 index 0000000000..ed19dd41e2 --- /dev/null +++ b/tests/core/nogo/tests/tests_test.go @@ -0,0 +1,95 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package importpath_test + +import ( + "testing" + + "github.com/bazelbuild/rules_go/go/tools/bazel_testing" +) + +func TestMain(m *testing.M) { + bazel_testing.TestMain(m, bazel_testing.Args{ + Main: ` +-- BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "nogo") + +go_library( + name = "simple_lib", + srcs = ["simple_lib.go"], + importpath = "example.com/simple", +) + +go_test( + name = "simple_test", + size = "small", + srcs = ["simple_test.go"], + embed = [":simple_lib"], +) + +go_test( + name = "super_simple_test", + size = "small", + srcs = ["super_simple_test.go"], +) + +nogo( + name = "nogo", + vet = True, + visibility = ["//visibility:public"], +) +-- simple_lib.go -- +package simple + +func Foo() {} +-- simple_test.go -- +package simple_test + +import ( + "testing" + + "example.com/simple" +) + +func TestFoo(t *testing.T) { + simple.Foo() +} +-- super_simple_test.go -- +package super_simple_test + +import ( + "testing" +) + +func TestFoo(t *testing.T) { +} +`, + Nogo: `@//:nogo`, + }) +} + +func TestExternalTestWithFullImportpath(t *testing.T) { + if out, err := bazel_testing.BazelOutput("test", "//:all"); err != nil { + println(string(out)) + t.Fatal(err) + } +} + +func TestEmptyExternalTest(t *testing.T) { + if out, err := bazel_testing.BazelOutput("test", "//:all"); err != nil { + println(string(out)) + t.Fatal(err) + } +}