Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: bazel-contrib/rules_go
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: master
Choose a base ref
...
head repository: cockroachdb/rules_go
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Choose a head ref
Can’t automatically merge. Don’t worry, you can still create the pull request.
  • 11 commits
  • 7 files changed
  • 5 contributors

Commits on Nov 11, 2021

  1. Copy the full SHA
    01b4592 View commit details
  2. Stage Go sources in a directory named after the package.

    This is a workaround for the issue described at
    cockroachdb/cockroach#64379 -- this is
    unnecessarily inefficient, but it does work and avoids some regressions
    `rules_go` would otherwise introduce.
    rickystewart committed Nov 11, 2021
    Copy the full SHA
    18a0557 View commit details
  3. Copy the full SHA
    4c7fdb1 View commit details
  4. Copy the full SHA
    442d7c1 View commit details

Commits on Nov 23, 2021

  1. Save test output in test2json format

    If `GO_TEST_JSON_OUTPUT_FILE` environment variable is set, write the
    test output in test2json format.
    rail committed Nov 23, 2021

    Verified

    This commit was signed with the committer’s verified signature.
    rail Rail Aliiev
    Copy the full SHA
    1ce9e34 View commit details

Commits on Jan 14, 2022

  1. Copy the full SHA
    6b31297 View commit details
  2. compilepkg: fix stored file path truncation

    This fixes the issue described in cockroachdb/cockroach#64379 where
    the internal paths of the files don't include the
    `github.com/cockroachdb/cockroach` prefix.
    
    We fix this by using the rewrite syntax of `-trimpath`, e.g.
    `-trimpath=/sandbox/execroot=>github.com/cockroachdb/cockroach`.
    
    Figuring out the replacement part is not trivial. We use the package
    path and strip out the relative path of the source file directory.
    RaduBerinde authored and rickystewart committed Jan 14, 2022
    Copy the full SHA
    23b381c View commit details

Commits on Feb 20, 2022

  1. sdk: symlink everything under lib/ when using local SDK

    We rely on the packaged zoneinfo zip in CRDB, found under
    lib/time/zoneinfo.zip. When using the remote SDK, we untar the entire
    thing into our sandbox. When pointing to a local SDK however, we ignored
    everything under lib, so failed to build.
    
    We'll need this diff in order to build CRDB with modified go
    installations, or at least iterating on it locally.
    
    +cc cockroachdb/cockroach#56178
    irfansharif committed Feb 20, 2022
    Copy the full SHA
    3f0fb1e View commit details

Commits on Feb 22, 2022

  1. Merge pull request #4 from cockroachdb/220219.local-sdk-lib

    sdk: symlink everything under lib/ when using local SDK
    irfansharif authored Feb 22, 2022

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    f96cc04 View commit details

Commits on Apr 7, 2022

  1. Translate paths in compilepkg output.

    When cgo2 is involved in a build, the go srcs are linked/copied
    into a temporary directory to keep everything in the same
    directory. Those paths are unrecognizable to IDEs, being absolute
    paths outside the project directory.
    
    This adds a translation step to replace paths in the compiler output,
    translating them back to the input paths that were given to the tool
    in the first place.
    
    This doesn't affect -trimpath, which affects runtime error messages
    but not compilation-time error messages; for the same reason, this
    change can't use that flag to do its dirty work.
    
    Fixes cockroachdb/cockroach#76377.
    
    Release note: None
    mari-crl committed Apr 7, 2022
    Copy the full SHA
    2a778e9 View commit details
  2. Merge pull request #5 from mari-crl/pathtranslation

    Translate paths in compilepkg output.
    mari-crl authored Apr 7, 2022

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    58cb947 View commit details
Showing with 100 additions and 17 deletions.
  1. +1 −1 go/private/sdk.bzl
  2. +12 −8 go/tools/builders/cgo2.go
  3. +21 −7 go/tools/builders/compilepkg.go
  4. +40 −1 go/tools/builders/env.go
  5. +7 −0 go/tools/builders/generate_test_main.go
  6. +14 −0 go/tools/builders/link.go
  7. +5 −0 go/tools/bzltestutil/wrap.go
2 changes: 1 addition & 1 deletion go/private/sdk.bzl
Original file line number Diff line number Diff line change
@@ -219,7 +219,7 @@ def _remote_sdk(ctx, urls, strip_prefix, sha256):
)

def _local_sdk(ctx, path):
for entry in ["src", "pkg", "bin"]:
for entry in ["src", "pkg", "bin", "lib"]:
ctx.symlink(path + "/" + entry, entry)

def _sdk_build_file(ctx, platform):
20 changes: 12 additions & 8 deletions go/tools/builders/cgo2.go
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ import (
)

// cgo2 processes a set of mixed source files with cgo.
func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSrcs, hSrcs []string, packagePath, packageName string, cc string, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags []string, cgoExportHPath string) (srcDir string, allGoSrcs, cObjs []string, err error) {
func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSrcs, hSrcs []string, packagePath, packageName string, cc string, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags []string, cgoExportHPath string) (allGoSrcsDir string, allGoSrcs []pathPair, cObjs []string, err error) {
// Report an error if the C/C++ toolchain wasn't configured.
if cc == "" {
err := cgoError(cgoSrcs[:])
@@ -96,7 +96,7 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr

// If cgo sources are in different directories, gather them into a temporary
// directory so we can use -srcdir.
srcDir = filepath.Dir(cgoSrcs[0])
srcDir := filepath.Dir(cgoSrcs[0])
srcsInSingleDir := true
for _, src := range cgoSrcs[1:] {
if filepath.Dir(src) != srcDir {
@@ -151,13 +151,16 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr
return "", nil, nil, err
}
}
genGoSrcs := make([]string, 1+len(cgoSrcs))
genGoSrcs[0] = filepath.Join(workDir, "_cgo_gotypes.go")
// genGoSrcs contains the go source files generated by the cgo compiler plus 2 additional files, one at each end:
// genGoSrcs[0] is the gotypes file, while genGoSrcs[-1] is the imports file.
genGoSrcs := make([]pathPair, 2+len(cgoSrcs))
genGoSrcs[0].workingPath = filepath.Join(workDir, "_cgo_gotypes.go")
genCSrcs := make([]string, 1+len(cgoSrcs))
genCSrcs[0] = filepath.Join(workDir, "_cgo_export.c")
for i, src := range cgoSrcs {
stem := strings.TrimSuffix(filepath.Base(src), ".go")
genGoSrcs[i+1] = filepath.Join(workDir, stem+".cgo1.go")
genGoSrcs[i+1].inputPath = src
genGoSrcs[i+1].workingPath = filepath.Join(workDir, stem+".cgo1.go")
genCSrcs[i+1] = filepath.Join(workDir, stem+".cgo2.c")
}
cgoMainC := filepath.Join(workDir, "_cgo_main.c")
@@ -200,7 +203,7 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr
if err := goenv.runCommand(args); err != nil {
return "", nil, nil, err
}
genGoSrcs = append(genGoSrcs, cgoImportsGo)
genGoSrcs[len(genGoSrcs)-1].workingPath = cgoImportsGo

// Copy regular Go source files into the work directory so that we can
// use -trimpath=workDir.
@@ -209,9 +212,10 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr
return "", nil, nil, err
}

allGoSrcs = make([]string, len(goSrcs)+len(genGoSrcs))
allGoSrcs = make([]pathPair, len(goSrcs)+len(genGoSrcs))
for i := range goSrcs {
allGoSrcs[i] = filepath.Join(workDir, goBases[i])
allGoSrcs[i].inputPath = goSrcs[i]
allGoSrcs[i].workingPath = filepath.Join(workDir, goBases[i])
}
copy(allGoSrcs[len(goSrcs):], genGoSrcs)
return workDir, allGoSrcs, cObjs, nil
28 changes: 21 additions & 7 deletions go/tools/builders/compilepkg.go
Original file line number Diff line number Diff line change
@@ -267,24 +267,38 @@ func compileArchive(
// If we have cgo, generate separate C and go files, and compile the
// C files.
var objFiles []string
var goSrcsMapping []pathPair
if cgoEnabled && haveCgo {
// TODO(#2006): Compile .s and .S files with cgo2, not the Go assembler.
// If cgo is not enabled or we don't have other cgo sources, don't
// compile .S files.
var srcDir string
srcDir, goSrcs, objFiles, err = cgo2(goenv, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, nil, hSrcs, packagePath, packageName, cc, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags, cgoExportHPath)
srcDir, goSrcsMapping, objFiles, err = cgo2(goenv, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, nil, hSrcs, packagePath, packageName, cc, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags, cgoExportHPath)
if err != nil {
return err
}
goSrcs = make([]string, len(goSrcsMapping))
for i, v := range goSrcsMapping {
goSrcs[i] = v.workingPath
}

gcFlags = append(gcFlags, "-trimpath="+srcDir)
gcFlags = append(gcFlags, fmt.Sprintf("-trimpath=%s=>%s", abs(srcDir), packagePath))
} else {
if cgoExportHPath != "" {
if err := ioutil.WriteFile(cgoExportHPath, nil, 0666); err != nil {
return err
}
}
gcFlags = append(gcFlags, "-trimpath=.")
// We want the source files to show up (e.g. in stack traces) with the package
// path. We use -trimpath to replace the root path with the correct prefix
// of the package path.
root := abs(".")
relSrcPath, err := filepath.Rel(root, srcs.goSrcs[0].filename)
if err != nil {
return err
}
rootPkgPath := filepath.Clean(strings.TrimSuffix(packagePath, filepath.Dir(relSrcPath)))
gcFlags = append(gcFlags, fmt.Sprintf("-trimpath=%s=>%s", root, rootPkgPath))
}

// Check that the filtered sources don't import anything outside of
@@ -386,7 +400,7 @@ func compileArchive(
}

// Compile the filtered .go files.
if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, outPath); err != nil {
if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, goSrcsMapping, outPath); err != nil {
return err
}

@@ -456,7 +470,7 @@ func compileArchive(
return appendFiles(goenv, outXPath, []string{pkgDefPath})
}

func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, outPath string) error {
func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, paths []pathPair, outPath string) error {
args := goenv.goTool("compile")
args = append(args, "-p", packagePath, "-importcfg", importcfgPath, "-pack")
if embedcfgPath != "" {
@@ -472,8 +486,8 @@ func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPa
args = append(args, "-o", outPath)
args = append(args, "--")
args = append(args, srcs...)
absArgs(args, []string{"-I", "-o", "-trimpath", "-importcfg"})
return goenv.runCommand(args)
absArgs(args, []string{"-I", "-o", "-importcfg"})
return goenv.runCommandAndReplacePaths(args, paths)
}

func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string, deps []archive, packagePath, importcfgPath, outFactsPath string) error {
41 changes: 40 additions & 1 deletion go/tools/builders/env.go
Original file line number Diff line number Diff line change
@@ -62,6 +62,17 @@ type env struct {
shouldPreserveWorkDir bool
}

// pathPair maps the input path given to the builder to the
// working path, usually in a temporary directory, given to
// tools like cgo2.
type pathPair struct {
// inputPath is the path given to the builder.
// Files generated by the builder will have an inputPath of "".
inputPath string
// workingPath is the path given to the tool itself.
workingPath string
}

// envFlags registers flags common to multiple builders and returns an env
// configured with those flags.
func envFlags(flags *flag.FlagSet) *env {
@@ -128,7 +139,8 @@ func (e *env) goCmd(cmd string, args ...string) []string {
}

// runCommand executes a subprocess that inherits stdout, stderr, and the
// environment from this process.
// environment from this process. Paths under the working directory will
// be relativized in the output.
func (e *env) runCommand(args []string) error {
cmd := exec.Command(args[0], args[1:]...)
// Redirecting stdout to stderr. This mirrors behavior in the go command:
@@ -141,6 +153,22 @@ func (e *env) runCommand(args []string) error {
return err
}

// runCommandAndReplacePaths executes a subprocess that inherits stdout,
// stderr, and the environment from this process. The stderr stream
// will be filtered to replace the given pairs of paths, and then paths
// under the working directory will be relativized in the output.
func (e *env) runCommandAndReplacePaths(args []string, paths []pathPair) error {
cmd := exec.Command(args[0], args[1:]...)
// Redirecting stdout to stderr. This mirrors behavior in the go command:
// https://go.googlesource.com/go/+/refs/tags/go1.15.2/src/cmd/go/internal/work/exec.go#1958
buf := &bytes.Buffer{}
cmd.Stdout = buf
cmd.Stderr = buf
err := runAndLogCommand(cmd, e.verbose)
os.Stderr.Write(relativizePaths(replacePaths(paths, buf.Bytes())))
return err
}

// runCommandToFile executes a subprocess and writes the output to the given
// writer.
func (e *env) runCommandToFile(w io.Writer, args []string) error {
@@ -356,6 +384,17 @@ func relativizePaths(output []byte) []byte {
return bytes.ReplaceAll(output, dirBytes, []byte{'.'})
}

// replacePaths converts any instances in data of the destPath of any
// element of the paths to the corresponding sourcePath.
func replacePaths(paths []pathPair, data []byte) []byte {
for _, pair := range paths {
if pair.inputPath != "" && pair.workingPath != pair.inputPath {
data = bytes.ReplaceAll(data, []byte(pair.workingPath), []byte(pair.inputPath))
}
}
return data
}

// formatCommand formats cmd as a string that can be pasted into a shell.
// Spaces in environment variables and arguments are escaped as needed.
func formatCommand(cmd *exec.Cmd) string {
7 changes: 7 additions & 0 deletions go/tools/builders/generate_test_main.go
Original file line number Diff line number Diff line change
@@ -159,9 +159,16 @@ func testsInShard() []testing.InternalTest {
}
func main() {
// NOTE(ricky): Bazel sets the TEST_TMPDIR env variable, but Cockroach
// tests generally consult TMPDIR.
err := os.Setenv("TMPDIR", os.Getenv("TEST_TMPDIR"))
if err != nil {
panic(err)
}
if bzltestutil.ShouldWrap() {
err := bzltestutil.Wrap("{{.Pkgname}}")
if xerr, ok := err.(*exec.ExitError); ok {
log.Printf("Test %v exited with error code %v", os.Getenv("TEST_TARGET"), xerr.ExitCode())
os.Exit(xerr.ExitCode())
} else if err != nil {
log.Print(err)
14 changes: 14 additions & 0 deletions go/tools/builders/link.go
Original file line number Diff line number Diff line change
@@ -30,7 +30,21 @@ import (
"strings"
)

// NOTE(ricky): The go executable freaks out if the PATH contains relative
// paths. Unfortunately, that happens normally as part of the Bazel build
// process when using custom toolchains. As a result, we munge all the paths in
// PATH to be absolute.
// https://github.com/golang/go/commit/953d1feca9b21af075ad5fc8a3dad096d3ccc3a0
func mungePath() {
var newPaths []string
for _, path := range strings.Split(os.Getenv("PATH"), string(os.PathListSeparator)) {
newPaths = append(newPaths, abs(path))
}
os.Setenv("PATH", strings.Join(newPaths, string(os.PathListSeparator)))
}

func link(args []string) error {
mungePath()
// Parse arguments.
args, err := expandParamsFiles(args)
if err != nil {
5 changes: 5 additions & 0 deletions go/tools/bzltestutil/wrap.go
Original file line number Diff line number Diff line change
@@ -85,6 +85,11 @@ func Wrap(pkg string) error {
return fmt.Errorf("error while generating testreport: %s", werr)
}
}
if out, ok := os.LookupEnv("GO_TEST_JSON_OUTPUT_FILE"); ok {
if err := ioutil.WriteFile(out, jsonBuffer.Bytes(), 0664); err != nil {
return fmt.Errorf("error writing test json: %s", err)
}
}
return err
}