diff --git a/MODULE.bazel b/MODULE.bazel index c1af0602d3..bc7eb86bdd 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -1,6 +1,6 @@ module( name = "rules_go", - version = "0.49.0", + version = "0.50.1", compatibility_level = 0, repo_name = "io_bazel_rules_go", ) diff --git a/docs/go/extras/extras.md b/docs/go/extras/extras.md index b3eb5be1df..1c3a2007b7 100644 --- a/docs/go/extras/extras.md +++ b/docs/go/extras/extras.md @@ -34,7 +34,7 @@ This rule has moved. See [gazelle rule] in the Gazelle repository. 
 gomock(name, out, library, source_importpath, source, interfaces, package, self_package, aux_files,
-       mockgen_tool, imports, copyright_file, mock_names, kwargs)
+       mockgen_tool, mockgen_args, imports, copyright_file, mock_names, kwargs)
Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library. @@ -57,6 +57,7 @@ If `source` is given, the mocks are generated in source mode; otherwise in refle | self_package | the full package import path for the generated code. The purpose of this flag is to prevent import cycles in the generated code by trying to include its own package. See [mockgen's -self_package](https://github.com/golang/mock#flags) for more information. | "" | | aux_files | a map from source files to their package path. This only needed when source is provided. See [mockgen's -aux_files](https://github.com/golang/mock#flags) for more information. | {} | | mockgen_tool | the mockgen tool to run. | Label("//extras/gomock:mockgen") | +| mockgen_args | additional arguments to pass to the mockgen tool. | [] | | imports | dictionary of name-path pairs of explicit imports to use. See [mockgen's -imports](https://github.com/golang/mock#flags) for more information. | {} | | copyright_file | optional file containing copyright to prepend to the generated contents. See [mockgen's -copyright_file](https://github.com/golang/mock#flags) for more information. | None | | mock_names | dictionary of interface name to mock name pairs to change the output names of the mock objects. Mock names default to 'Mock' prepended to the name of the interface. See [mockgen's -mock_names](https://github.com/golang/mock#flags) for more information. | {} | diff --git a/extras/gomock.bzl b/extras/gomock.bzl index 20e06d2a4a..c6d034f225 100644 --- a/extras/gomock.bzl +++ b/extras/gomock.bzl @@ -160,6 +160,10 @@ _gomock_source = rule( cfg = "exec", mandatory = False, ), + "mockgen_args": attr.string_list( + doc = "Additional arguments to pass to the mockgen tool", + mandatory = False, + ), "_go_context_data": attr.label( default = "//:go_context_data", ), @@ -167,7 +171,7 @@ _gomock_source = rule( toolchains = [GO_TOOLCHAIN], ) -def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs): +def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, mockgen_args = [], imports = {}, copyright_file = None, mock_names = {}, **kwargs): """Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library. If `source` is given, the mocks are generated in source mode; otherwise in reflective mode. @@ -183,6 +187,7 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int self_package: the full package import path for the generated code. The purpose of this flag is to prevent import cycles in the generated code by trying to include its own package. See [mockgen's -self_package](https://github.com/golang/mock#flags) for more information. aux_files: a map from source files to their package path. This only needed when `source` is provided. See [mockgen's -aux_files](https://github.com/golang/mock#flags) for more information. mockgen_tool: the mockgen tool to run. + mockgen_args: additional arguments to pass to the mockgen tool. imports: dictionary of name-path pairs of explicit imports to use. See [mockgen's -imports](https://github.com/golang/mock#flags) for more information. copyright_file: optional file containing copyright to prepend to the generated contents. See [mockgen's -copyright_file](https://github.com/golang/mock#flags) for more information. mock_names: dictionary of interface name to mock name pairs to change the output names of the mock objects. Mock names default to 'Mock' prepended to the name of the interface. See [mockgen's -mock_names](https://github.com/golang/mock#flags) for more information. @@ -199,6 +204,7 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int self_package = self_package, aux_files = aux_files, mockgen_tool = mockgen_tool, + mockgen_args = mockgen_args, imports = imports, copyright_file = copyright_file, mock_names = mock_names, @@ -213,6 +219,7 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int package = package, self_package = self_package, mockgen_tool = mockgen_tool, + mockgen_args = mockgen_args, imports = imports, copyright_file = copyright_file, mock_names = mock_names, @@ -375,6 +382,11 @@ _gomock_prog_exec = rule( cfg = "exec", mandatory = False, ), + "mockgen_args": attr.string_list( + doc = "Additional arguments to pass to the mockgen tool", + mandatory = False, + default = [], + ), "_go_context_data": attr.label( default = "//:go_context_data", ), @@ -398,5 +410,7 @@ def _handle_shared_args(ctx, args): if len(ctx.attr.mock_names) > 0: mock_names = ",".join(["{0}={1}".format(name, pkg) for name, pkg in ctx.attr.mock_names.items()]) args += ["-mock_names", mock_names] + if ctx.attr.mockgen_args: + args += ctx.attr.mockgen_args return args, needed_files diff --git a/go/def.bzl b/go/def.bzl index b32c7e2248..a8908ec98d 100644 --- a/go/def.bzl +++ b/go/def.bzl @@ -121,7 +121,7 @@ TOOLS_NOGO = [str(Label(l)) for l in _TOOLS_NOGO] # Current version or next version to be tagged. Gazelle and other tools may # check this to determine compatibility. -RULES_GO_VERSION = "0.49.0" +RULES_GO_VERSION = "0.50.1" go_context = _go_context gomock = _gomock diff --git a/go/nogo.rst b/go/nogo.rst index c1c9b58e1e..c1a815ecd8 100644 --- a/go/nogo.rst +++ b/go/nogo.rst @@ -24,14 +24,9 @@ .. footer:: The ``nogo`` logo was derived from the Go gopher, which was designed by Renee French. (http://reneefrench.blogspot.com/) The design is licensed under the Creative Commons 3.0 Attributions license. Read this article for more details: http://blog.golang.org/gopher -**WARNING**: This functionality is experimental, so its API might change. -Please do not rely on it for production use, but feel free to use it and file -issues. - ``nogo`` is a tool that analyzes the source code of Go programs. It runs -alongside the Go compiler in the Bazel Go rules and rejects programs that -contain disallowed coding patterns. In addition, ``nogo`` may report -compiler-like errors. +in an action after the Go compiler in the Bazel Go rules and rejects sources that +contain disallowed coding patterns from the configured analyzers. ``nogo`` is a powerful tool for preventing bugs and code anti-patterns early in the development process. It may be used to run the same analyses as `vet`_, diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl index e2da3fbe2d..1bbd135d73 100644 --- a/go/private/actions/archive.bzl +++ b/go/private/actions/archive.bzl @@ -17,6 +17,10 @@ load( "as_tuple", "split_srcs", ) +load( + "//go/private:context.bzl", + "get_nogo", +) load( "//go/private:mode.bzl", "LINKMODE_C_ARCHIVE", @@ -61,14 +65,16 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d # store export information for compiling dependent packages separately out_export = go.declare_file(go, name = source.library.name, ext = pre_ext + ".x") out_cgo_export_h = None # set if cgo used in c-shared or c-archive mode - out_facts = None - out_nogo_log = None - out_nogo_validation = None - nogo = go.get_nogo(go) + + nogo = get_nogo(go) if nogo: out_facts = go.declare_file(go, name = source.library.name, ext = pre_ext + ".facts") out_nogo_log = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.log") out_nogo_validation = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo") + else: + out_facts = None + out_nogo_log = None + out_nogo_validation = None direct = [get_archive(dep) for dep in source.deps] runfiles = source.runfiles diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl index cfb7aee697..a7d26b616e 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, @@ -244,7 +246,7 @@ def _run_nogo( args.add_all(sources, before_each = "-src") if cgo_go_srcs: inputs.append(cgo_go_srcs) - args.add_all([cgo_go_srcs], before_each = "-src") + args.add_all([cgo_go_srcs], before_each = "-ignore_src") if cover_mode: args.add("-cover_mode", cover_mode) if recompile_internal_deps: @@ -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/private/context.bzl b/go/private/context.bzl index c2c02effff..1eb69a5ea6 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -435,9 +435,9 @@ def _matches_scopes(label, scopes): return True return False -def _get_nogo(go): +def get_nogo(go): """Returns the nogo file for this target, if enabled and in scope.""" - label = go._ctx.label + label = go.label if _matches_scopes(label, NOGO_INCLUDES) and not _matches_scopes(label, NOGO_EXCLUDES): return go.nogo else: @@ -503,6 +503,14 @@ def go_context(ctx, attr = None): # # See https://go.dev/doc/toolchain for more info. "GOTOOLCHAIN": "local", + + # NOTE(#4049): Since Go 1.23.0, os.Readlink (and consequently + # filepath.EvalSymlinks) stopped treating Windows mount points and + # reparse points (junctions) as symbolic links. Bazel uses junctions + # when constructing exec roots, and we use filepath.EvalSymlinks in + # GoStdlib, so this broke us. Setting GODEBUG=winsymlink=0 restores + # the old behavior. + "GODEBUG": "winsymlink=0", } # Path mapping can't map the values of environment variables, so we pass GOROOT to the action @@ -615,7 +623,6 @@ def go_context(ctx, attr = None): library_to_source = _library_to_source, declare_file = _declare_file, declare_directory = _declare_directory, - get_nogo = _get_nogo, # Private # TODO: All uses of this should be removed diff --git a/go/private/platforms.bzl b/go/private/platforms.bzl index ee6a1c767e..0a1e343f1d 100644 --- a/go/private/platforms.bzl +++ b/go/private/platforms.bzl @@ -22,6 +22,7 @@ BAZEL_GOOS_CONSTRAINTS = { "freebsd": "@platforms//os:freebsd", "ios": "@platforms//os:ios", "linux": "@platforms//os:linux", + "qnx": "@platforms//os:qnx", "windows": "@platforms//os:windows", } @@ -77,10 +78,19 @@ GOOS_GOARCH = ( ("openbsd", "amd64"), ("openbsd", "arm"), ("openbsd", "arm64"), + ("osx", "386"), + ("osx", "amd64"), + ("osx", "arm"), + ("osx", "arm64"), + ("qnx", "386"), + ("qnx", "amd64"), + ("qnx", "arm"), + ("qnx", "arm64"), ("plan9", "386"), ("plan9", "amd64"), ("plan9", "arm"), ("solaris", "amd64"), + ("wasip1", "wasm"), ("windows", "386"), ("windows", "amd64"), ("windows", "arm"), diff --git a/go/private/rules/nogo.bzl b/go/private/rules/nogo.bzl index 9d6b1f9806..7aa63cfa53 100644 --- a/go/private/rules/nogo.bzl +++ b/go/private/rules/nogo.bzl @@ -25,7 +25,6 @@ load( "EXPORT_PATH", "GoArchive", "GoLibrary", - "get_archive", ) load( "//go/private/rules:transition.bzl", @@ -47,7 +46,7 @@ def _nogo_impl(ctx): if ctx.attr.debug: nogo_args.add("-debug") nogo_inputs = [] - analyzer_archives = [get_archive(dep) for dep in ctx.attr.deps] + analyzer_archives = [dep[GoArchive] for dep in ctx.attr.deps] analyzer_importpaths = [archive.data.importpath for archive in analyzer_archives] nogo_args.add_all(analyzer_importpaths, before_each = "-analyzer_importpath") if ctx.file.config: diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index b5db446928..5e0fad37c3 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -60,10 +60,14 @@ def _go_test_impl(ctx): go = go_context(ctx) + validation_outputs = [] + # Compile the library to test with internal white box tests internal_library = go.new_library(go, testfilter = "exclude") internal_source = go.library_to_source(go, ctx.attr, internal_library, ctx.coverage_instrumented()) internal_archive = go.archive(go, internal_source) + if internal_archive.data._validation_output: + validation_outputs.append(internal_archive.data._validation_output) go_srcs = split_srcs(internal_source.srcs).go # Compile the library with the external black box tests @@ -82,6 +86,8 @@ def _go_test_impl(ctx): ), external_library, ctx.coverage_instrumented()) external_source, internal_archive = _recompile_external_deps(go, external_source, internal_archive, [t.label for t in ctx.attr.embed]) external_archive = go.archive(go, external_source, is_external_pkg = True) + if external_archive.data._validation_output: + validation_outputs.append(external_archive.data._validation_output) # now generate the main function repo_relative_rundir = ctx.attr.rundir or ctx.label.package or "." @@ -165,7 +171,6 @@ def _go_test_impl(ctx): version_file = ctx.version_file, info_file = ctx.info_file, ) - validation_output = test_archive.data._validation_output env = {} for k, v in ctx.attr.env.items(): @@ -187,7 +192,7 @@ def _go_test_impl(ctx): ), OutputGroupInfo( compilation_outputs = [internal_archive.data.file], - _validation = [validation_output] if validation_output else [], + _validation = validation_outputs, ), coverage_common.instrumented_files_info( ctx, diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 244513000a..631dac1590 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -63,6 +63,7 @@ filegroup( "ar.go", "asm.go", "builder.go", + "cc.go", "cgo2.go", "compilepkg.go", "constants.go", diff --git a/go/tools/builders/builder.go b/go/tools/builders/builder.go index f9b96b8b8e..fd0a5ba6af 100644 --- a/go/tools/builders/builder.go +++ b/go/tools/builders/builder.go @@ -54,6 +54,8 @@ func main() { action = stdlib case "stdliblist": action = stdliblist + case "cc": + action = cc default: log.Fatalf("unknown action: %s", verb) } diff --git a/go/tools/builders/cc.go b/go/tools/builders/cc.go new file mode 100644 index 0000000000..90f9258b9c --- /dev/null +++ b/go/tools/builders/cc.go @@ -0,0 +1,47 @@ +package main + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "syscall" +) + +func cc(args []string) error { + cc := os.Getenv("GO_CC") + if cc == "" { + errors.New("GO_CC environment variable not set") + } + ccroot := os.Getenv("GO_CC_ROOT") + if ccroot == "" { + errors.New("GO_CC_ROOT environment variable not set") + } + normalized := []string{cc} + normalized = append(normalized, args...) + transformArgs(normalized, cgoAbsEnvFlags, func(s string) string { + if strings.HasPrefix(s, cgoAbsPlaceholder) { + trimmed := strings.TrimPrefix(s, cgoAbsPlaceholder) + abspath := filepath.Join(ccroot, trimmed) + if _, err := os.Stat(abspath); err == nil { + // Only return the abspath if it exists, otherwise it + // means that either it won't have any effect or the original + // value was not a relpath (e.g. a path with a XCODE placehold from + // macos cc_wrapper) + return abspath + } + return trimmed + } + return s + }) + if runtime.GOOS == "windows" { + cmd := exec.Command(normalized[0], normalized[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() + } else { + return syscall.Exec(normalized[0], normalized, os.Environ()) + } +} 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/env.go b/go/tools/builders/env.go index cc8b37d721..bb01ac205f 100644 --- a/go/tools/builders/env.go +++ b/go/tools/builders/env.go @@ -35,6 +35,8 @@ var ( cgoEnvVars = []string{"CGO_CFLAGS", "CGO_CXXFLAGS", "CGO_CPPFLAGS", "CGO_LDFLAGS"} // cgoAbsEnvFlags are all the flags that need absolute path in cgoEnvVars cgoAbsEnvFlags = []string{"-I", "-L", "-isysroot", "-isystem", "-iquote", "-include", "-gcc-toolchain", "--sysroot", "-resource-dir", "-fsanitize-blacklist", "-fsanitize-ignorelist"} + // cgoAbsPlaceholder is placed in front of flag values that must be absolutized + cgoAbsPlaceholder = "__GO_BAZEL_CC_PLACEHOLDER__" ) // env holds a small amount of Go environment and toolchain information @@ -160,10 +162,33 @@ func (e *env) runCommandToFile(out, err io.Writer, args []string) error { return runAndLogCommand(cmd, e.verbose) } -func absEnv(envNameList []string, argList []string) error { +// absCCCompiler modifies CGO flags to workaround relative paths. +// Because go is having its own sandbox, all CGO flags should use +// absolute paths. However, CGO flags are embedded in the output +// so we cannot use absolute paths directly. Instead, use a placeholder +// for the absolute path and we replace CC with this builder so that +// we can expand the placeholder later. +func absCCCompiler(envNameList []string, argList []string) error { + err := os.Setenv("GO_CC", os.Getenv("CC")) + if err != nil { + return err + } + err = os.Setenv("GO_CC_ROOT", abs(".")) + if err != nil { + return err + } + err = os.Setenv("CC", abs(os.Args[0])+" cc") + if err != nil { + return err + } for _, envName := range envNameList { splitedEnv := strings.Fields(os.Getenv(envName)) - absArgs(splitedEnv, argList) + transformArgs(splitedEnv, argList, func(s string) string { + if filepath.IsAbs(s) { + return s + } + return cgoAbsPlaceholder + s + }) if err := os.Setenv(envName, strings.Join(splitedEnv, " ")); err != nil { return err } @@ -320,10 +345,16 @@ func abs(path string) string { // absArgs applies abs to strings that appear in args. Only paths that are // part of options named by flags are modified. func absArgs(args []string, flags []string) { + transformArgs(args, flags, abs) +} + +// transformArgs applies fn to strings that appear in args. Only paths that are +// part of options named by flags are modified. +func transformArgs(args []string, flags []string, fn func(string) string) { absNext := false for i := range args { if absNext { - args[i] = abs(args[i]) + args[i] = fn(args[i]) absNext = false continue } @@ -341,7 +372,7 @@ func absArgs(args []string, flags []string) { possibleValue = possibleValue[1:] separator = "=" } - args[i] = fmt.Sprintf("%s%s%s", f, separator, abs(possibleValue)) + args[i] = fmt.Sprintf("%s%s%s", f, separator, fn(possibleValue)) break } } 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..44222013a5 100644 --- a/go/tools/builders/nogo.go +++ b/go/tools/builders/nogo.go @@ -20,12 +20,14 @@ func nogo(args []string) error { fs := flag.NewFlagSet("GoNogo", flag.ExitOnError) goenv := envFlags(fs) - var unfilteredSrcs, recompileInternalDeps multiFlag + var unfilteredSrcs, ignoreSrcs, 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") + fs.Var(&unfilteredSrcs, "src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked") + fs.Var(&ignoreSrcs, "ignore_src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked, but with its diagnostics ignored") fs.Var(&deps, "arc", "Import path, package path, and file name of a direct dependency, separated by '='") fs.Var(&facts, "facts", "Import path, package path, and file name of a direct dependency's nogo facts file, separated by '='") fs.StringVar(&importPath, "importpath", "", "The import path of the package being compiled. Not passed to the compiler, but may be displayed in debug data.") @@ -33,6 +35,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") @@ -47,7 +50,12 @@ func nogo(args []string) error { } // Filter sources. - srcs, err := filterAndSplitFiles(unfilteredSrcs) + srcs, err := filterAndSplitFiles(append(unfilteredSrcs, ignoreSrcs...)) + if err != nil { + return err + } + + err = applyTestFilter(testFilter, &srcs) if err != nil { return err } @@ -68,50 +76,28 @@ func nogo(args []string) error { } defer cleanup() - imports, err := checkImports(srcs.goSrcs, deps, packageListPath, importPath, recompileInternalDeps) - 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)) + compilingWithCgo := os.Getenv("CGO_ENABLED") == "1" && haveCgo + importcfgPath, err := checkImportsAndBuildCfg(goenv, importPath, srcs, deps, packageListPath, recompileInternalDeps, compilingWithCgo, coverMode, workDir) if err != nil { return err } - if !goenv.shouldPreserveWorkDir { - defer os.Remove(importcfgPath) - } - return runNogo(workDir, nogoPath, goSrcs, facts, importPath, importcfgPath, outFactsPath, outLogPath) + return runNogo(workDir, nogoPath, goSrcs, ignoreSrcs, facts, importPath, importcfgPath, outFactsPath, outLogPath) } -func runNogo(workDir string, nogoPath string, srcs []string, facts []archive, packagePath, importcfgPath, outFactsPath string, outLogPath string) error { +func runNogo(workDir string, nogoPath string, srcs, ignores []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) @@ -120,6 +106,9 @@ func runNogo(workDir string, nogoPath string, srcs []string, facts []archive, pa args = append(args, "-fact", fmt.Sprintf("%s=%s", fact.importPath, fact.file)) } args = append(args, "-x", outFactsPath) + for _, ignore := range ignores { + args = append(args, "-ignore", ignore) + } args = append(args, srcs...) paramsFile := filepath.Join(workDir, "nogo.param") diff --git a/go/tools/builders/nogo_main.go b/go/tools/builders/nogo_main.go index 9cf558ba1d..8a3871d3f7 100644 --- a/go/tools/builders/nogo_main.go +++ b/go/tools/builders/nogo_main.go @@ -77,6 +77,8 @@ func run(args []string) (error, int) { importcfg := flags.String("importcfg", "", "The import configuration file") packagePath := flags.String("p", "", "The package path (importmap) of the package being compiled") xPath := flags.String("x", "", "The archive file where serialized facts should be written") + var ignores multiFlag + flags.Var(&ignores, "ignore", "Names of files to ignore") flags.Parse(args) srcs := flags.Args() @@ -85,7 +87,7 @@ func run(args []string) (error, int) { return fmt.Errorf("error parsing importcfg: %v", err), nogoError } - diagnostics, facts, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, srcs) + diagnostics, facts, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, srcs, ignores) if err != nil { return fmt.Errorf("error running analyzers: %v", err), nogoError } @@ -156,7 +158,7 @@ func readImportCfg(file string) (packageFile map[string]string, importMap map[st // It returns an empty string if no source code diagnostics need to be printed. // // This implementation was adapted from that of golang.org/x/tools/go/checker/internal/checker. -func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap map[string]string, factMap map[string]string, filenames []string) (string, []byte, error) { +func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap map[string]string, factMap map[string]string, filenames, ignoreFiles []string) (string, []byte, error) { // Register fact types and establish dependencies between analyzers. actions := make(map[*analysis.Analyzer]*action) var visit func(a *analysis.Analyzer) *action @@ -211,8 +213,22 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil act.pkg = pkg } + ignoreFilesSet := map[string]struct{}{} + for _, ignore := range ignoreFiles { + ignoreFilesSet[ignore] = struct{}{} + } // Process nolint directives similar to golangci-lint. + // Also skip over fully ignored files. for _, f := range pkg.syntax { + if _, ok := ignoreFilesSet[pkg.fset.Position(f.Pos()).Filename]; ok { + for _, act := range actions { + act.nolint = append(act.nolint, &Range{ + from: pkg.fset.Position(f.FileStart), + to: pkg.fset.Position(f.End()).Line, + }) + } + continue + } // CommentMap will correctly associate comments to the largest node group // applicable. This handles inline comments that might trail a large // assignment and will apply the comment to the entire assignment. @@ -356,9 +372,7 @@ func (act *action) execOnce() { act.pass = pass var err error - if act.pkg.illTyped && !pass.Analyzer.RunDespiteErrors { - err = fmt.Errorf("analysis skipped due to type-checking error: %v", act.pkg.typeCheckError) - } else { + if !act.pkg.illTyped || pass.Analyzer.RunDespiteErrors { act.result, err = pass.Analyzer.Run(pass) if err == nil { if got, want := reflect.TypeOf(act.result), pass.Analyzer.ResultType; got != want { @@ -455,7 +469,13 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string { if cwd == "" || err != nil { errs = append(errs, fmt.Errorf("nogo failed to get CWD: %w", err)) } + numSkipped := 0 for _, act := range actions { + if act.pkg.illTyped && !act.a.RunDespiteErrors { + // Don't report type-checking errors once per analyzer. + numSkipped++ + continue + } if act.err != nil { // Analyzer failed. errs = append(errs, fmt.Errorf("analyzer %q failed: %v", act.a.Name, act.err)) @@ -527,6 +547,9 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string { } } } + if numSkipped > 0 { + errs = append(errs, fmt.Errorf("%d analyzers skipped due to type-checking error: %v", numSkipped, pkg.typeCheckError)) + } if len(diagnostics) == 0 && len(errs) == 0 { return "" } diff --git a/go/tools/builders/stdlib.go b/go/tools/builders/stdlib.go index 15d52bb012..105ca5c635 100644 --- a/go/tools/builders/stdlib.go +++ b/go/tools/builders/stdlib.go @@ -158,9 +158,7 @@ You may need to use the flags --cpu=x64_windows --compiler=mingw-gcc.`) installArgs = append(installArgs, "-ldflags="+allSlug+strings.Join(ldflags, " ")) installArgs = append(installArgs, "-asmflags="+allSlug+strings.Join(asmflags, " ")) - // Modify CGO flags to use only absolute path - // because go is having its own sandbox, all CGO flags must use absolute path - if err := absEnv(cgoEnvVars, cgoAbsEnvFlags); err != nil { + if err := absCCCompiler(cgoEnvVars, cgoAbsEnvFlags); err != nil { return fmt.Errorf("error modifying cgo environment to absolute path: %v", err) } diff --git a/go/tools/builders/stdliblist.go b/go/tools/builders/stdliblist.go index 098be0408c..2648d2486d 100644 --- a/go/tools/builders/stdliblist.go +++ b/go/tools/builders/stdliblist.go @@ -257,9 +257,7 @@ func stdliblist(args []string) error { } os.Setenv("CC", quotePathIfNeeded(abs(ccEnv))) - // Modify CGO flags to use only absolute path - // because go is having its own sandbox, all CGO flags must use absolute path - if err := absEnv(cgoEnvVars, cgoAbsEnvFlags); err != nil { + if err := absCCCompiler(cgoEnvVars, cgoAbsEnvFlags); err != nil { return fmt.Errorf("error modifying cgo environment to absolute path: %v", err) } diff --git a/go/tools/gopackagesdriver/bazel.go b/go/tools/gopackagesdriver/bazel.go index 554c2af596..a88a60dcea 100644 --- a/go/tools/gopackagesdriver/bazel.go +++ b/go/tools/gopackagesdriver/bazel.go @@ -35,11 +35,12 @@ const ( ) type Bazel struct { - bazelBin string - workspaceRoot string - bazelStartupFlags []string - info map[string]string - version bazelVersion + bazelBin string + workspaceRoot string + buildWorkingDirectory string + bazelStartupFlags []string + info map[string]string + version bazelVersion } // Minimal BEP structs to access the build outputs @@ -52,11 +53,12 @@ type BEPNamedSet struct { } `json:"namedSetOfFiles"` } -func NewBazel(ctx context.Context, bazelBin, workspaceRoot string, bazelStartupFlags []string) (*Bazel, error) { +func NewBazel(ctx context.Context, bazelBin, workspaceRoot string, buildWorkingDirectory string, bazelStartupFlags []string) (*Bazel, error) { b := &Bazel{ - bazelBin: bazelBin, - workspaceRoot: workspaceRoot, - bazelStartupFlags: bazelStartupFlags, + bazelBin: bazelBin, + workspaceRoot: workspaceRoot, + buildWorkingDirectory: buildWorkingDirectory, + bazelStartupFlags: bazelStartupFlags, } if err := b.fillInfo(ctx); err != nil { return nil, fmt.Errorf("unable to query bazel info: %w", err) @@ -163,6 +165,10 @@ func (b *Bazel) WorkspaceRoot() string { return b.workspaceRoot } +func (b *Bazel) BuildWorkingDirectory() string { + return b.buildWorkingDirectory +} + func (b *Bazel) ExecutionRoot() string { return b.info["execution_root"] } diff --git a/go/tools/gopackagesdriver/bazel_json_builder.go b/go/tools/gopackagesdriver/bazel_json_builder.go index f36108ce4b..a86c396011 100644 --- a/go/tools/gopackagesdriver/bazel_json_builder.go +++ b/go/tools/gopackagesdriver/bazel_json_builder.go @@ -21,7 +21,6 @@ import ( "fmt" "io/ioutil" "os" - "path" "path/filepath" "regexp" "runtime" @@ -39,14 +38,9 @@ var _defaultKinds = []string{"go_library", "go_test", "go_binary"} var externalRe = regexp.MustCompile(".*\\/external\\/([^\\/]+)(\\/(.*))?\\/([^\\/]+.go)") -func (b *BazelJSONBuilder) fileQuery(filename string) string { - label := filename - - if filepath.IsAbs(filename) { - label, _ = filepath.Rel(b.bazel.WorkspaceRoot(), filename) - } else if strings.HasPrefix(filename, "./") { - label = strings.TrimPrefix(filename, "./") - } +func (b *BazelJSONBuilder) fileQuery(label string) string { + label = b.adjustToRelativePathIfPossible(label) + filename := filepath.FromSlash(label) if matches := externalRe.FindStringSubmatch(filename); len(matches) == 5 { // if filepath is for a third party lib, we need to know, what external @@ -56,7 +50,7 @@ func (b *BazelJSONBuilder) fileQuery(filename string) string { } relToBin, err := filepath.Rel(b.bazel.info["output_path"], filename) - if err == nil && !strings.HasPrefix(relToBin, "../") { + if err == nil && !strings.HasPrefix(relToBin, filepath.FromSlash("../")) { parts := strings.SplitN(relToBin, string(filepath.Separator), 3) relToBin = parts[2] // We've effectively converted filename from bazel-bin/some/path.go to some/path.go; @@ -93,12 +87,7 @@ func (b *BazelJSONBuilder) getKind() string { } func (b *BazelJSONBuilder) localQuery(request string) string { - request = path.Clean(request) - if filepath.IsAbs(request) { - if relPath, err := filepath.Rel(workspaceRoot, request); err == nil { - request = relPath - } - } + request = b.adjustToRelativePathIfPossible(request) if !strings.HasSuffix(request, "...") { request = fmt.Sprintf("%s:*", request) @@ -107,6 +96,22 @@ func (b *BazelJSONBuilder) localQuery(request string) string { return fmt.Sprintf(`kind("^(%s) rule$", %s)`, b.getKind(), request) } +func (b *BazelJSONBuilder) adjustToRelativePathIfPossible(request string) string { + // If request is a relative path and gopackagesdriver is ran within a subdirectory of the + // workspace, we must first resolve the absolute path for it. + // Note: Using FromSlash/ToSlash for handling windows + absRequest := filepath.FromSlash(request) + if !filepath.IsAbs(absRequest) { + absRequest = filepath.Join(b.bazel.BuildWorkingDirectory(), absRequest) + } + if relPath, err := filepath.Rel(workspaceRoot, absRequest); err == nil { + request = filepath.ToSlash(relPath) + } else { + fmt.Fprintf(os.Stderr, "error adjusting path to be relative to the workspace root from request %s: %v\n", request, err) + } + return request +} + func (b *BazelJSONBuilder) packageQuery(importPath string) string { if strings.HasSuffix(importPath, "/...") { importPath = fmt.Sprintf(`^%s(/.+)?$`, strings.TrimSuffix(importPath, "/...")) diff --git a/go/tools/gopackagesdriver/gopackagesdriver_test.go b/go/tools/gopackagesdriver/gopackagesdriver_test.go index 3d61ab96ae..47d2ce6ce7 100644 --- a/go/tools/gopackagesdriver/gopackagesdriver_test.go +++ b/go/tools/gopackagesdriver/gopackagesdriver_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "path" + "path/filepath" "strings" "testing" @@ -61,6 +62,25 @@ package hello_test import "testing" func TestHelloExternal(t *testing.T) {} + +-- subhello/BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "subhello", + srcs = ["subhello.go"], + importpath = "example.com/hello/subhello", + visibility = ["//visibility:public"], +) + +-- subhello/subhello.go -- +package subhello + +import "os" + +func main() { + fmt.Fprintln(os.Stderr, "Subdirectory Hello World!") +} `, }) } @@ -71,7 +91,7 @@ const ( ) func TestBaseFileLookup(t *testing.T) { - resp := runForTest(t, "file=hello.go") + resp := runForTest(t, ".", "file=hello.go") t.Run("roots", func(t *testing.T) { if len(resp.Roots) != 1 { @@ -140,8 +160,80 @@ func TestBaseFileLookup(t *testing.T) { }) } +func TestRelativeFileLookup(t *testing.T) { + resp := runForTest(t, "subhello", "file=./subhello.go") + + t.Run("roots", func(t *testing.T) { + if len(resp.Roots) != 1 { + t.Errorf("Expected 1 package root: %+v", resp.Roots) + return + } + + if !strings.HasSuffix(resp.Roots[0], "//subhello:subhello") { + t.Errorf("Unexpected package id: %q", resp.Roots[0]) + return + } + }) + + t.Run("package", func(t *testing.T) { + var pkg *FlatPackage + for _, p := range resp.Packages { + if p.ID == resp.Roots[0] { + pkg = p + } + } + + if pkg == nil { + t.Errorf("Expected to find %q in resp.Packages", resp.Roots[0]) + return + } + + if len(pkg.CompiledGoFiles) != 1 || len(pkg.GoFiles) != 1 || + path.Base(pkg.GoFiles[0]) != "subhello.go" || path.Base(pkg.CompiledGoFiles[0]) != "subhello.go" { + t.Errorf("Expected to find 1 file (subhello.go) in (Compiled)GoFiles:\n%+v", pkg) + return + } + }) +} + +func TestRelativePatternWildcardLookup(t *testing.T) { + resp := runForTest(t, "subhello", "./...") + + t.Run("roots", func(t *testing.T) { + if len(resp.Roots) != 1 { + t.Errorf("Expected 1 package root: %+v", resp.Roots) + return + } + + if !strings.HasSuffix(resp.Roots[0], "//subhello:subhello") { + t.Errorf("Unexpected package id: %q", resp.Roots[0]) + return + } + }) + + t.Run("package", func(t *testing.T) { + var pkg *FlatPackage + for _, p := range resp.Packages { + if p.ID == resp.Roots[0] { + pkg = p + } + } + + if pkg == nil { + t.Errorf("Expected to find %q in resp.Packages", resp.Roots[0]) + return + } + + if len(pkg.CompiledGoFiles) != 1 || len(pkg.GoFiles) != 1 || + path.Base(pkg.GoFiles[0]) != "subhello.go" || path.Base(pkg.CompiledGoFiles[0]) != "subhello.go" { + t.Errorf("Expected to find 1 file (subhello.go) in (Compiled)GoFiles:\n%+v", pkg) + return + } + }) +} + func TestExternalTests(t *testing.T) { - resp := runForTest(t, "file=hello_external_test.go") + resp := runForTest(t, ".", "file=hello_external_test.go") if len(resp.Roots) != 2 { t.Errorf("Expected exactly two roots for package: %+v", resp.Roots) } @@ -167,7 +259,7 @@ func TestExternalTests(t *testing.T) { } } -func runForTest(t *testing.T, args ...string) driverResponse { +func runForTest(t *testing.T, relativeWorkingDir string, args ...string) driverResponse { t.Helper() // Remove most environment variables, other than those on an allowlist. @@ -210,7 +302,7 @@ func runForTest(t *testing.T, args ...string) driverResponse { } }() - // Set workspaceRoot global variable. + // Set workspaceRoot and buildWorkingDirectory global variable. // It's initialized to the BUILD_WORKSPACE_DIRECTORY environment variable // before this point. wd, err := os.Getwd() @@ -218,8 +310,13 @@ func runForTest(t *testing.T, args ...string) driverResponse { t.Fatal(err) } oldWorkspaceRoot := workspaceRoot + oldBuildWorkingDirectory := buildWorkingDirectory workspaceRoot = wd - defer func() { workspaceRoot = oldWorkspaceRoot }() + buildWorkingDirectory = filepath.Join(wd, relativeWorkingDir) + defer func() { + workspaceRoot = oldWorkspaceRoot + buildWorkingDirectory = oldBuildWorkingDirectory + }() in := strings.NewReader("{}") out := &bytes.Buffer{} diff --git a/go/tools/gopackagesdriver/main.go b/go/tools/gopackagesdriver/main.go index 0613a3be79..ea591f6f7c 100644 --- a/go/tools/gopackagesdriver/main.go +++ b/go/tools/gopackagesdriver/main.go @@ -62,6 +62,7 @@ var ( bazelQueryScope = getenvDefault("GOPACKAGESDRIVER_BAZEL_QUERY_SCOPE", "") bazelBuildFlags = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_BUILD_FLAGS")) workspaceRoot = os.Getenv("BUILD_WORKSPACE_DIRECTORY") + buildWorkingDirectory = os.Getenv("BUILD_WORKING_DIRECTORY") additionalAspects = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_ADDTL_ASPECTS")) additionalKinds = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_KINDS")) emptyResponse = &driverResponse{ @@ -81,7 +82,7 @@ func run(ctx context.Context, in io.Reader, out io.Writer, args []string) error return fmt.Errorf("unable to read request: %w", err) } - bazel, err := NewBazel(ctx, bazelBin, workspaceRoot, bazelStartupFlags) + bazel, err := NewBazel(ctx, bazelBin, workspaceRoot, buildWorkingDirectory, bazelStartupFlags) if err != nil { return fmt.Errorf("unable to create bazel instance: %w", err) } diff --git a/go/tools/internal/stdlib_tags/stdlib_tags.go b/go/tools/internal/stdlib_tags/stdlib_tags.go index 8bd507af3a..dfb9745b75 100644 --- a/go/tools/internal/stdlib_tags/stdlib_tags.go +++ b/go/tools/internal/stdlib_tags/stdlib_tags.go @@ -125,7 +125,7 @@ func isConstraint(line string) bool { } // Taken from -// https://github.com/golang/go/blob/3d5391ed87d813110e10b954c62bf7ed578b591f/src/go/build/syslist.go +// https://github.com/golang/go/blob/2693f77b3583585172810427e12a634b28d34493/src/internal/syslist/syslist.go var knownOS = map[string]bool{ "aix": true, "android": true, @@ -142,6 +142,7 @@ var knownOS = map[string]bool{ "openbsd": true, "plan9": true, "solaris": true, + "wasip1": true, "windows": true, "zos": true, } 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/custom/custom_test.go b/tests/core/nogo/custom/custom_test.go index a7a6786b36..3eb220d1aa 100644 --- a/tests/core/nogo/custom/custom_test.go +++ b/tests/core/nogo/custom/custom_test.go @@ -31,7 +31,7 @@ func TestMain(m *testing.M) { Nogo: "@//:nogo", Main: ` -- BUILD.bazel -- -load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo") +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "nogo") nogo( name = "nogo", @@ -39,6 +39,7 @@ nogo( ":foofuncname", ":importfmt", ":visibility", + ":cgogen", ], # config = "", visibility = ["//visibility:public"], @@ -71,6 +72,14 @@ go_library( visibility = ["//visibility:public"], ) +go_library( + name = "cgogen", + srcs = ["cgogen.go"], + importpath = "cgogenanalyzer", + deps = ["@org_golang_x_tools//go/analysis"], + visibility = ["//visibility:public"], +) + go_library( name = "has_errors", srcs = ["has_errors.go"], @@ -85,6 +94,13 @@ go_library( deps = [":dep"], ) +go_library( + name = "uses_cgo_clean", + srcs = ["examplepkg/uses_cgo_clean.go"], + importpath = "uses_cgo_clean", + cgo = True, +) + go_library( name = "uses_cgo_with_errors", srcs = [ @@ -108,8 +124,14 @@ go_library( importpath = "dep", ) +go_binary( + name = "type_check_fail", + srcs = ["type_check_fail.go"], + pure = "on", +) + -- foofuncname.go -- -// importfmt checks for functions named "Foo". +// foofuncname checks for functions named "Foo". // It has the same package name as another check to test the checks with // the same package name do not conflict. package importfmt @@ -279,6 +301,40 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil } +-- cgogen.go -- +// cgogen reports diagnostics on files generated by cgo. +package cgogen + +import ( + "go/ast" + "strings" + + "golang.org/x/tools/go/analysis" +) + +const doc = "report synthetic diagnostics on files generated by cgo" + +var Analyzer = &analysis.Analyzer{ + Name: "cgogen", + Run: run, + Doc: doc, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, f := range pass.Files { + ast.Inspect(f, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.Comment: + if strings.HasPrefix(n.Text, "//go:linkname") || strings.HasPrefix(n.Text, "//go:cgo_import") { + pass.Reportf(n.Pos(), "nogo must not run on code generated by cgo") + } + return true + } + return true + }) + } + return nil, nil +} -- config.json -- { @@ -384,6 +440,16 @@ func Foo() bool { // This should fail foofuncname return Bar() } +-- type_check_fail.go -- +package type_check_fail + +import ( + "strings" +) + +func Foo() bool { + return strings.Split("a,b,c", ",") // This fails type-checking +} `, }) } @@ -393,6 +459,7 @@ func Test(t *testing.T) { desc, config, target string wantSuccess bool includes, excludes []string + bazelArgs []string }{ { desc: "default_config", @@ -457,11 +524,25 @@ func Test(t *testing.T) { // note the cross platform regex :) `examplepkg[\\/]pure_src_with_err_calling_native.go:.*function must not be named Foo \(foofuncname\)`, }, + }, { + desc: "type_check_fail", + config: "config.json", + target: "//:type_check_fail", + wantSuccess: false, + includes: []string{ + "4 analyzers skipped due to type-checking error: type_check_fail.go:8:10:", + }, + // Ensure that nogo runs even though compilation fails + bazelArgs: []string{"--keep_going"}, }, { desc: "no_errors", target: "//:no_errors", wantSuccess: true, excludes: []string{"no_errors.go"}, + }, { + desc: "uses_cgo_clean", + target: "//:uses_cgo_clean", + wantSuccess: true, }, } { t.Run(test.desc, func(t *testing.T) { @@ -473,13 +554,13 @@ func Test(t *testing.T) { defer replaceInFile("BUILD.bazel", customConfig, origConfig) } - cmd := bazel_testing.BazelCmd("build", test.target) + cmd := bazel_testing.BazelCmd(append([]string{"build", test.target}, test.bazelArgs...)...) stderr := &bytes.Buffer{} cmd.Stderr = stderr if err := cmd.Run(); err == nil && !test.wantSuccess { - t.Fatal("unexpected success") + t.Fatalf("unexpected success\n%s", stderr) } else if err != nil && test.wantSuccess { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v\n%s", err, stderr) } for _, pattern := range test.includes { 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..801388b022 --- /dev/null +++ b/tests/core/nogo/tests/tests_test.go @@ -0,0 +1,150 @@ +// 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 ( + "strings" + "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"], +) + +go_test( + name = "diagnostic_external_test", + size = "small", + srcs = ["diagnostic_external_test.go"], +) + +go_test( + name = "diagnostic_internal_test", + size = "small", + srcs = ["diagnostic_internal_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) { +} +-- diagnostic_external_test.go -- +package diagnostic_test + +import ( + "testing" +) + +func TestFoo(t *testing.T) { + if TestFoo == nil { + t.Fatal("TestFoo is nil") + } +} +-- diagnostic_internal_test.go -- +package diagnostic + +import ( + "testing" +) + +func TestFoo(t *testing.T) { + if TestFoo == nil { + t.Fatal("TestFoo is nil") + } +} +`, + Nogo: `@//:nogo`, + }) +} + +func TestExternalTestWithFullImportpath(t *testing.T) { + if out, err := bazel_testing.BazelOutput("test", "//:simple_test"); err != nil { + println(string(out)) + t.Fatal(err) + } +} + +func TestEmptyExternalTest(t *testing.T) { + if out, err := bazel_testing.BazelOutput("test", "//:super_simple_test"); err != nil { + println(string(out)) + t.Fatal(err) + } +} + +func TestDiagnosticInExternalTest(t *testing.T) { + if _, err := bazel_testing.BazelOutput("test", "//:diagnostic_external_test"); err == nil { + t.Fatal("unexpected success") + } else if !strings.Contains(err.Error(), "diagnostic_external_test.go:8:8: comparison of function TestFoo == nil is always false (nilfunc)") { + println(err.Error()) + t.Fatal("unexpected output") + } +} + +func TestDiagnosticInInternalTest(t *testing.T) { + if _, err := bazel_testing.BazelOutput("test", "//:diagnostic_internal_test"); err == nil { + t.Fatal("unexpected success") + } else if !strings.Contains(err.Error(), "diagnostic_internal_test.go:8:8: comparison of function TestFoo == nil is always false (nilfunc)") { + println(err.Error()) + t.Fatal("unexpected output") + } +} diff --git a/tests/integration/reproducibility/reproducibility_test.go b/tests/integration/reproducibility/reproducibility_test.go index 2e9bc59144..0feeee4714 100644 --- a/tests/integration/reproducibility/reproducibility_test.go +++ b/tests/integration/reproducibility/reproducibility_test.go @@ -189,6 +189,8 @@ func Test(t *testing.T) { defer wg.Done() cmd := bazel_testing.BazelCmd("build", "//:all", + "--copt=-Irelative/path/does/not/exist", + "--linkopt=-Lrelative/path/does/not/exist", "@io_bazel_rules_go//go/tools/builders:go_path", "@go_sdk//:builder", ) @@ -200,49 +202,92 @@ func Test(t *testing.T) { } wg.Wait() - // Hash files in each bazel-bin directory. - dirHashes := make([][]fileHash, len(dirs)) - errs := make([]error, len(dirs)) - wg.Add(len(dirs)) - for i := range dirs { - go func(i int) { - defer wg.Done() - dirHashes[i], errs[i] = hashFiles(filepath.Join(dirs[i], "bazel-bin")) - }(i) - } - wg.Wait() - for _, err := range errs { - if err != nil { + t.Run("Check Hashes", func(t *testing.T) { + // Hash files in each bazel-bin directory. + dirHashes := make([][]fileHash, len(dirs)) + errs := make([]error, len(dirs)) + wg.Add(len(dirs)) + for i := range dirs { + go func(i int) { + defer wg.Done() + dirHashes[i], errs[i] = hashFiles(filepath.Join(dirs[i], "bazel-out/"), func(root, path string) bool { + // Hash everything but actions stdout/stderr and the volatile-status.txt file + // as they are expected to change + return strings.HasPrefix(path, filepath.Join(root, "_tmp")) || + path == filepath.Join(root, "volatile-status.txt") + }) + }(i) + } + wg.Wait() + for _, err := range errs { + if err != nil { + t.Fatal(err) + } + } + + // Compare dir0 and dir1. They should be identical. + if err := compareHashes(dirHashes[0], dirHashes[1]); err != nil { t.Fatal(err) } - } - // Compare dir0 and dir1. They should be identical. - if err := compareHashes(dirHashes[0], dirHashes[1]); err != nil { - t.Fatal(err) - } + // Compare dir0 and dir2. They should be different. + if err := compareHashes(dirHashes[0], dirHashes[2]); err == nil { + t.Fatalf("dir0 and dir2 are the same)", len(dirHashes[0])) + } + }) - // Compare dir0 and dir2. They should be different. - if err := compareHashes(dirHashes[0], dirHashes[2]); err == nil { - t.Fatalf("dir0 and dir2 are the same)", len(dirHashes[0])) - } + t.Run("Check builder", func(t *testing.T) { + // Check that the go_sdk path doesn't appear in the builder binary. This path is different + // nominally different per workspace (but in these tests, the go_sdk paths are all set to the same + // path in WORKSPACE) -- so if this path is in the builder binary, then builds between workspaces + // would be partially non cacheable. + builder_file, err := os.Open(filepath.Join(dirs[0], "bazel-bin", "external", "go_sdk", "builder")) + if err != nil { + t.Fatal(err) + } + defer builder_file.Close() + builder_data, err := ioutil.ReadAll(builder_file) + if err != nil { + t.Fatal(err) + } + if bytes.Index(builder_data, []byte("go_sdk")) != -1 { + t.Fatalf("Found go_sdk path in builder binary, builder tool won't be reproducible") + } + }) - // Check that the go_sdk path doesn't appear in the builder binary. This path is different - // nominally different per workspace (but in these tests, the go_sdk paths are all set to the same - // path in WORKSPACE) -- so if this path is in the builder binary, then builds between workspaces - // would be partially non cacheable. - builder_file, err := os.Open(filepath.Join(dirs[0], "bazel-bin", "external", "go_sdk", "builder")) - if err != nil { - t.Fatal(err) - } - defer builder_file.Close() - builder_data, err := ioutil.ReadAll(builder_file) - if err != nil { - t.Fatal(err) - } - if bytes.Index(builder_data, []byte("go_sdk")) != -1 { - t.Fatalf("Found go_sdk path in builder binary, builder tool won't be reproducible") - } + t.Run("Check stdlib", func(t *testing.T) { + for _, dir := range dirs { + found := false + root, err := os.Readlink(filepath.Join(dir, "bazel-out/")) + if err != nil { + t.Fatal(err) + } + + filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !strings.Contains(path, "stdlib_/pkg") { + return nil + } + if !strings.HasSuffix(path, ".a") { + return nil + } + data, err := ioutil.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if bytes.Index(data, []byte("__GO_BAZEL_CC_PLACEHOLDER__")) == -1 { + found = true + return filepath.SkipAll + } + return nil + }) + if !found { + t.Fatal("Placeholder not found in stdlib of " + dir) + } + } + }) } func copyTree(dstRoot, srcRoot string) error { @@ -311,7 +356,7 @@ type fileHash struct { rel, hash string } -func hashFiles(dir string) ([]fileHash, error) { +func hashFiles(dir string, exclude func(root, path string) bool) ([]fileHash, error) { // Follow top-level symbolic link root := dir for { @@ -348,7 +393,15 @@ func hashFiles(dir string) ([]fileHash, error) { return err } } + if info.IsDir() { + if exclude(root, path) { + return filepath.SkipDir + } + return nil + } + + if exclude(root, path) { return nil }