Skip to content

Commit

Permalink
gopackagesdriver: fix interface to work with golangci-lint (bazelbuil…
Browse files Browse the repository at this point in the history
…d#3523)

Unlike gopls which uses `file=` queries exclusively, golangci-lint uses
the more generic `go list` query syntax. This change allow for file
paths not prefixed with `file=` to be queried correctly.

Additionally, some bugs have been fixed up:
- Packages with different IDs and identical import paths work now.
- stdlib now includes CGO processed files.
- WKT for protobufs are no longer skipped when the proto wrapper is
  used.`
  • Loading branch information
Thomas Rampelberg committed Apr 25, 2023
1 parent 2a71311 commit d887d3a
Show file tree
Hide file tree
Showing 12 changed files with 252 additions and 139 deletions.
49 changes: 30 additions & 19 deletions go/private/actions/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,46 @@ def _should_use_sdk_stdlib(go):
not go.mode.gc_goopts and
go.mode.link == LINKMODE_NORMAL)

def _build_stdlib_list_json(go):
def _build_stdlib_list_json(go, env):
out = go.declare_file(go, "stdlib.pkg.json")
args = go.builder_args(go, "stdliblist")
args.add("-sdk", go.sdk.root_file.dirname)
args.add("-out", out)

go.actions.run(
inputs = go.sdk_files,
outputs = [out],
mnemonic = "GoStdlibList",
executable = go.toolchain._builder,
arguments = [args],
env = go.env,
env = env,
)
return out

def _cgo_env(go):
env = go.env

# NOTE(#2545): avoid unnecessary dynamic link
# go std library doesn't use C++, so should not have -lstdc++
# Also drop coverage flags as nothing in the stdlib is compiled with
# coverage - we disable it for all CGo code anyway.
ldflags = [
option
for option in extldflags_from_cc_toolchain(go)
if option not in ("-lstdc++", "-lc++") and option not in COVERAGE_OPTIONS_DENYLIST
]
env.update({
"CGO_ENABLED": "1",
"CC": go.cgo_tools.c_compiler_path,
"CGO_CFLAGS": " ".join(go.cgo_tools.c_compile_options),
"CGO_LDFLAGS": " ".join(ldflags),
})

return env

def _sdk_stdlib(go):
return GoStdLib(
_list_json = _build_stdlib_list_json(go),
_list_json = _build_stdlib_list_json(go, _cgo_env(go)),
libs = go.sdk.libs,
root_file = go.sdk.root_file,
)
Expand All @@ -96,26 +118,15 @@ def _build_stdlib(go):
if not go.mode.pure:
args.add("-package", "runtime/cgo")
args.add_all(link_mode_args(go.mode))

env = go.env
if go.mode.pure:
env.update({"CGO_ENABLED": "0"})
else:
# NOTE(#2545): avoid unnecessary dynamic link
# go std library doesn't use C++, so should not have -lstdc++
# Also drop coverage flags as nothing in the stdlib is compiled with
# coverage - we disable it for all CGo code anyway.
ldflags = [
option
for option in extldflags_from_cc_toolchain(go)
if option not in ("-lstdc++", "-lc++") and option not in COVERAGE_OPTIONS_DENYLIST
]
env.update({
"CGO_ENABLED": "1",
"CC": go.cgo_tools.c_compiler_path,
"CGO_CFLAGS": " ".join(go.cgo_tools.c_compile_options),
"CGO_LDFLAGS": " ".join(ldflags),
})
env = _cgo_env(go)

args.add("-gcflags", quote_opts(go.mode.gc_goopts))

inputs = (go.sdk.srcs +
go.sdk.headers +
go.sdk.tools +
Expand All @@ -131,7 +142,7 @@ def _build_stdlib(go):
env = env,
)
return GoStdLib(
_list_json = _build_stdlib_list_json(go),
_list_json = _build_stdlib_list_json(go, env),
libs = [pkg],
root_file = pkg,
)
77 changes: 66 additions & 11 deletions go/tools/builders/stdliblist.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,45 @@ func outputBasePath(cloneBase, p string) string {
return filepath.Join("__BAZEL_OUTPUT_BASE__", dir)
}

// absoluteSourcesPaths replace cloneBase of the absolution
// paths with the label for all source files in a package
// absoluteSourcesPaths replace cloneBase of the absolution
// paths with the label for all source files in a package
func absoluteSourcesPaths(cloneBase, pkgDir string, srcs []string) []string {
ret := make([]string, 0, len(srcs))
pkgDir = outputBasePath(cloneBase, pkgDir)
for _, src := range srcs {
ret = append(ret, filepath.Join(pkgDir, src))
absPath := src

// Generated files will already have an absolute path. These come from
// the compiler's cache.
if !filepath.IsAbs(src) {
absPath = filepath.Join(pkgDir, src)
}

ret = append(ret, absPath)
}
return ret
}

// filterGoFiles keeps only files either ending in .go or those without an
// extension (which are from the cache). This is a work around for
// https://golang.org/issue/28749: cmd/go puts assembly, C, and C++ files in
// CompiledGoFiles.
func filterGoFiles(srcs []string) []string {
ret := make([]string, 0, len(srcs))
for _, f := range srcs {
if ext := filepath.Ext(f); ext != ".go" && ext != "" {
continue
}

ret = append(ret, f)
}

return ret
}

func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage {
// Don't use generated files from the stdlib
goFiles := absoluteSourcesPaths(cloneBase, pkg.Dir, pkg.GoFiles)
compiledGoFiles := absoluteSourcesPaths(cloneBase, pkg.Dir, pkg.CompiledGoFiles)

newPkg := &flatPackage{
ID: stdlibPackageID(pkg.ImportPath),
Expand All @@ -139,13 +164,31 @@ func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage {
Imports: map[string]string{},
Standard: pkg.Standard,
GoFiles: goFiles,
CompiledGoFiles: goFiles,
CompiledGoFiles: filterGoFiles(compiledGoFiles),
}

// imports
//
// Imports contains the IDs of all imported packages.
// ImportsMap records (path, ID) only where they differ.
ids := make(map[string]bool)
for _, id := range pkg.Imports {
ids[id] = true
}
for _, imp := range pkg.Imports {
newPkg.Imports[imp] = stdlibPackageID(imp)

for path, id := range pkg.ImportMap {
newPkg.Imports[path] = stdlibPackageID(id)
delete(ids, id)
}
// We don't support CGo for now
delete(newPkg.Imports, "C")

for id := range ids {
if id == "C" {
continue
}

newPkg.Imports[id] = stdlibPackageID(id)
}

return newPkg
}

Expand Down Expand Up @@ -200,12 +243,18 @@ func stdliblist(args []string) error {
}
os.Setenv("PATH", strings.Join(absPaths, string(os.PathListSeparator)))
os.Setenv("GOROOT", newGoRoot)

cgoEnabled := os.Getenv("CGO_ENABLED") == "1"
// Make sure we have an absolute path to the C compiler.
// TODO(#1357): also take absolute paths of includes and other paths in flags.
os.Setenv("CC", quotePathIfNeeded(abs(os.Getenv("CC"))))
ccEnv, ok := os.LookupEnv("CC")
if cgoEnabled && !ok {
return fmt.Errorf("CC must be set")
}
os.Setenv("CC", quotePathIfNeeded(abs(ccEnv)))

// We want to keep the cache around so that the processed files can be used by other tools.
cachePath := abs(*out + ".gocache")
defer os.RemoveAll(cachePath)
os.Setenv("GOCACHE", cachePath)
os.Setenv("GOMODCACHE", cachePath)
os.Setenv("GOPATH", cachePath)
Expand All @@ -214,6 +263,11 @@ func stdliblist(args []string) error {
if len(build.Default.BuildTags) > 0 {
listArgs = append(listArgs, "-tags", strings.Join(build.Default.BuildTags, ","))
}

if cgoEnabled {
listArgs = append(listArgs, "-compiled=true")
}

listArgs = append(listArgs, "-json", "builtin", "std", "runtime/cgo")

jsonFile, err := os.Create(*out)
Expand All @@ -226,6 +280,7 @@ func stdliblist(args []string) error {
if err := goenv.runCommandToFile(jsonData, os.Stderr, listArgs); err != nil {
return err
}

encoder := json.NewEncoder(jsonFile)
decoder := json.NewDecoder(jsonData)
for decoder.More() {
Expand Down
13 changes: 12 additions & 1 deletion go/tools/gopackagesdriver/aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ DEPS_ATTRS = [
PROTO_COMPILER_ATTRS = [
"compiler",
"compilers",
"library",
]

def bazel_supports_canonical_label_literals():
Expand Down Expand Up @@ -68,6 +69,10 @@ def _go_archive_to_pkg(archive):
for src in archive.data.orig_srcs
if not src.path.endswith(".go")
],
Imports = {
pkg.data.importpath: str(pkg.data.label)
for pkg in archive.direct
},
)

def make_pkg_json(ctx, name, pkg_info):
Expand All @@ -84,7 +89,13 @@ def _go_pkg_info_aspect_impl(target, ctx):
transitive_compiled_go_files = []

for attr in DEPS_ATTRS + PROTO_COMPILER_ATTRS:
for dep in getattr(ctx.rule.attr, attr, []) or []:
deps = getattr(ctx.rule.attr, attr, []) or []

# Some attrs are not iterable, ensure that deps is always iterable.
if type(deps) != "list":
deps = [deps]

for dep in deps:
if GoPkgInfo in dep:
pkg_info = dep[GoPkgInfo]
transitive_json_files.append(pkg_info.pkg_json_files)
Expand Down
1 change: 1 addition & 0 deletions go/tools/gopackagesdriver/bazel.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func (b *Bazel) Build(ctx context.Context, args ...string) ([]string, error) {
if err := decoder.Decode(&namedSet); err != nil {
return nil, fmt.Errorf("unable to decode %s: %w", jsonFile.Name(), err)
}

if namedSet.NamedSetOfFiles != nil {
for _, f := range namedSet.NamedSetOfFiles.Files {
fileUrl, err := url.Parse(f.URI)
Expand Down
Loading

0 comments on commit d887d3a

Please sign in to comment.