Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use -importcfg files for compiling and linking #1654

Merged
merged 5 commits into from
Aug 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions go/private/actions/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,8 @@ load(
"shell",
)

def _importpath(v):
return v.data.importpath

def _searchpath(v):
return v.data.searchpath

def _importmap(v):
return "{}={}".format(v.data.importpath, v.data.importmap)
def _archive(v):
return "{}={}={}".format(v.data.importpath, v.data.importmap, v.data.file.path)

def emit_compile(
go,
Expand Down Expand Up @@ -60,8 +54,7 @@ def emit_compile(

builder_args = go.builder_args(go)
builder_args.add_all(sources, before_each = "-src")
builder_args.add_all(archives, before_each = "-dep", map_each = _importpath)
builder_args.add_all(archives, before_each = "-importmap", map_each = _importmap)
builder_args.add_all(archives, before_each = "-arc", map_each = _archive)
builder_args.add_all(["-o", out_lib])
builder_args.add_all(["-package_list", go.package_list])
if testfilter:
Expand All @@ -71,8 +64,7 @@ def emit_compile(
if asmhdr:
tool_args.add_all(["-asmhdr", asmhdr])
outputs.append(asmhdr)
tool_args.add_all(archives, before_each = "-I", map_each = _searchpath)
tool_args.add_all(["-trimpath", ".", "-I", "."])
tool_args.add_all(["-trimpath", "."])

#TODO: Check if we really need this expand make variables in here
#TODO: If we really do then it needs to be moved all the way back out to the rule
Expand Down
6 changes: 4 additions & 2 deletions go/private/actions/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,11 @@ def emit_link(

builder_args.add_all(
[struct(archive = archive, test_archives = test_archives)],
before_each = "-dep",
before_each = "-arc",
map_each = _map_archive,
)
builder_args.add_all(test_archives, before_each = "-dep", map_each = _format_archive)
builder_args.add_all(test_archives, before_each = "-arc", map_each = _format_archive)
builder_args.add("-package_list", go.package_list)

# Build a list of rpaths for dynamic libraries we need to find.
# rpaths are relative paths from the binary to directories where libraries
Expand Down Expand Up @@ -148,6 +149,7 @@ def emit_link(
go.crosstool,
stamp_inputs,
go.sdk.tools,
[go.sdk.package_list],
go.stdlib.libs,
),
outputs = [executable],
Expand Down
2 changes: 2 additions & 0 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ load(
load(
"@io_bazel_rules_go//go/private:mode.bzl",
"get_mode",
"installsuffix",
"mode_string",
)
load(
Expand Down Expand Up @@ -86,6 +87,7 @@ def _builder_args(go):
args.use_param_file("-param=%s")
args.set_param_file_format("multiline")
args.add("-sdk", go.sdk.root_file.dirname)
args.add("-installsuffix", installsuffix(go.mode))
args.add_joined("-tags", go.tags, join_with = ",")
return args

Expand Down
8 changes: 8 additions & 0 deletions go/private/mode.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,11 @@ def get_mode(ctx, host_only, go_toolchain, go_context_data):
goos = goos,
goarch = goarch,
)

def installsuffix(mode):
s = mode.goos + "_" + mode.goarch
if mode.race:
s += "_race"
elif mode.msan:
s += "_msan"
return s
3 changes: 0 additions & 3 deletions go/private/rules/sdk.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ def _build_package_list(ctx, srcs, root_file, out):
if not pkg_src_dir.startswith(src_dir):
continue
pkg_name = pkg_src_dir[len(src_dir):]
if any([prefix in pkg_name for prefix in ("vendor/", "cmd/")]):
continue
packages[pkg_name] = None
content = "\n".join(sorted(packages.keys())) + "\n"
ctx.actions.write(out, content)

135 changes: 99 additions & 36 deletions go/tools/builders/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,22 @@ package main

import (
"bytes"
"errors"
"flag"
"fmt"
"go/build"
"io"
"io/ioutil"
"log"
"os"
"path/filepath"
"strings"
)

type archive struct {
importPath, importMap, file string
}

func run(args []string) error {
// Parse arguments.
args, err := readParamsFiles(args)
Expand All @@ -37,12 +43,10 @@ func run(args []string) error {
builderArgs, toolArgs := splitArgs(args)
flags := flag.NewFlagSet("GoCompile", flag.ExitOnError)
unfiltered := multiFlag{}
deps := multiFlag{}
importmap := multiFlag{}
archives := archiveMultiFlag{}
goenv := envFlags(flags)
flags.Var(&unfiltered, "src", "A source file to be filtered and compiled")
flags.Var(&deps, "dep", "Import path of a direct dependency")
flags.Var(&importmap, "importmap", "Import maps of a direct dependency")
flags.Var(&archives, "arc", "Import path, package path, and file name of a direct dependency, separated by '='")
output := flags.String("o", "", "The output object file to write")
packageList := flags.String("package_list", "", "The file containing the list of standard library packages")
testfilter := flags.String("testfilter", "off", "Controls test package filtering")
Expand All @@ -52,6 +56,7 @@ func run(args []string) error {
if err := goenv.checkFlags(); err != nil {
return err
}
*output = abs(*output)

// Filter sources using build constraints.
var matcher func(f *goMetadata) bool
Expand Down Expand Up @@ -93,36 +98,30 @@ func run(args []string) error {
files = append(files, &goMetadata{filename: emptyPath})
}

// Check that the filtered sources don't import anything outside of deps.
strictdeps := deps
var importmapArgs []string
for _, mapping := range importmap {
i := strings.Index(mapping, "=")
if i < 0 {
return fmt.Errorf("Invalid importmap %v: no = separator", mapping)
}
source := mapping[:i]
actual := mapping[i+1:]
if source == "" || actual == "" || source == actual {
continue
}
importmapArgs = append(importmapArgs, "-importmap", mapping)
strictdeps = append(strictdeps, source)
// Check that the filtered sources don't import anything outside of
// the standard library and the direct dependencies.
_, stdImports, err := checkDirectDeps(files, archives, *packageList)
if err != nil {
return err
}
if err := checkDirectDeps(build.Default, files, strictdeps, *packageList); err != nil {

// Build an importcfg file for the compiler.
importcfgName, err := buildImportcfgFile(archives, stdImports, goenv.installSuffix, filepath.Dir(*output))
if err != nil {
return err
}
defer os.Remove(importcfgName)

// Compile the filtered files.
goargs := goenv.goTool("compile")
goargs = append(goargs, importmapArgs...)
goargs = append(goargs, "-importcfg", importcfgName)
goargs = append(goargs, "-pack", "-o", *output)
goargs = append(goargs, toolArgs...)
goargs = append(goargs, "--")
for _, f := range files {
goargs = append(goargs, f.filename)
}
absArgs(goargs, []string{"-I", "-o", "-trimpath"})
absArgs(goargs, []string{"-I", "-o", "-trimpath", "-importcfg"})
return goenv.runCommand(goargs)
}

Expand All @@ -134,41 +133,105 @@ func main() {
}
}

func checkDirectDeps(bctx build.Context, files []*goMetadata, deps []string, packageList string) error {
func checkDirectDeps(files []*goMetadata, archives []archive, packageList string) (depImports, stdImports []string, err error) {
packagesTxt, err := ioutil.ReadFile(packageList)
if err != nil {
log.Fatal(err)
}
stdlib := map[string]bool{}
stdlibSet := map[string]bool{}
for _, line := range strings.Split(string(packagesTxt), "\n") {
line = strings.TrimSpace(line)
if line != "" {
stdlib[line] = true
stdlibSet[line] = true
}
}

depSet := make(map[string]bool)
for _, d := range deps {
depSet[d] = true
depSet := map[string]bool{}
depList := make([]string, len(archives))
for i, arc := range archives {
depSet[arc.importPath] = true
depList[i] = arc.importPath
}

derr := depsError{known: deps}
importSet := map[string]bool{}

derr := depsError{known: depList}
for _, f := range files {
for _, path := range f.imports {
if path == "C" || stdlib[path] || isRelative(path) {
// Standard paths don't need to be listed as dependencies (for now).
// Relative paths aren't supported yet. We don't emit errors here, but
// they will certainly break something else.
if path == "C" || isRelative(path) || importSet[path] {
// TODO(#1645): Support local (relative) import paths. We don't emit
// errors for them here, but they will probably break something else.
continue
}
if stdlibSet[path] {
stdImports = append(stdImports, path)
continue
}
if !depSet[path] {
derr.missing = append(derr.missing, missingDep{f.filename, path})
if depSet[path] {
depImports = append(depImports, path)
continue
}
derr.missing = append(derr.missing, missingDep{f.filename, path})
}
}
if len(derr.missing) > 0 {
return derr
return nil, nil, derr
}
return depImports, stdImports, nil
}

func buildImportcfgFile(archives []archive, stdImports []string, installSuffix, dir string) (string, error) {
buf := &bytes.Buffer{}
goroot, ok := os.LookupEnv("GOROOT")
if !ok {
return "", errors.New("GOROOT not set")
}
for _, imp := range stdImports {
path := filepath.Join(goroot, "pkg", installSuffix, filepath.FromSlash(imp))
fmt.Fprintf(buf, "packagefile %s=%s.a\n", imp, path)
}
for _, arc := range archives {
if arc.importPath != arc.importMap {
fmt.Fprintf(buf, "importmap %s=%s\n", arc.importPath, arc.importMap)
}
fmt.Fprintf(buf, "packagefile %s=%s\n", arc.importMap, arc.file)
}
f, err := ioutil.TempFile(dir, "importcfg")
if err != nil {
return "", err
}
filename := f.Name()
if _, err := io.Copy(f, buf); err != nil {
f.Close()
os.Remove(filename)
return "", err
}
if err := f.Close(); err != nil {
os.Remove(filename)
return "", err
}
return filename, nil
}

type archiveMultiFlag []archive

func (m *archiveMultiFlag) String() string {
if m == nil || len(*m) == 0 {
return ""
}
return fmt.Sprint(*m)
}

func (m *archiveMultiFlag) Set(v string) error {
parts := strings.Split(v, "=")
if len(parts) != 3 {
return fmt.Errorf("badly formed -arc flag: %s", v)
}
*m = append(*m, archive{
importPath: parts[0],
importMap: parts[1],
file: abs(parts[2]),
})
return nil
}

Expand Down
6 changes: 6 additions & 0 deletions go/tools/builders/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ type env struct {
// platform. This may be different than GOROOT.
sdk string

// installSuffix is the name of the directory below GOROOT/pkg that contains
// the .a files for the standard library we should build against.
// For example, linux_amd64_race.
installSuffix string

// verbose indicates whether subprocess command lines should be printed.
verbose bool
}
Expand All @@ -56,6 +61,7 @@ func envFlags(flags *flag.FlagSet) *env {
env := &env{}
flags.StringVar(&env.sdk, "sdk", "", "Path to the Go SDK.")
flags.Var(&tagFlag{}, "tags", "List of build tags considered true.")
flags.StringVar(&env.installSuffix, "installsuffix", "", "Standard library under GOROOT/pkg")
flags.BoolVar(&env.verbose, "v", false, "Whether subprocess command lines should be printed")
return env
}
Expand Down
Loading