Skip to content

Commit e445813

Browse files
authored
Fix trim path problems (#4435)
**What type of PR is this?** Bug fix **What does this PR do? Why is it needed?** There were multiple problems with trim paths: - The trim path was inconsistent between non-cgo and cgo. non-cgo trimmed the working directory, and cgo trimmed the parent of the working directory. - For non-cgo, paths in external repos are not trimmed when --experimental_sibling_repository_layout is enabled. - For cgo, the name of the working directory is included in the output, but this name is not consistent between local and remote builds, breaking reproducibility. These are fixed here by creating the trim path in the same way for both non-cgo and cgo, and trimming the working directory as well as replacing the parent of the working directory with `..`. This means that the resulting paths are relative to the working directory. Additionally, this fixes the existing feature to extend an existing trimpath. It used the wrong separator (`:` instead of `;`), and it applied `abs()` to the entire trimpath argument instead of the individual paths inside it. (See #2994 (comment) and #2994 (comment)) **Which issues(s) does this PR fix?** Fixes: #4434 Fixes: #4161
1 parent 879148f commit e445813

File tree

6 files changed

+212
-27
lines changed

6 files changed

+212
-27
lines changed

go/tools/builders/cgo2.go

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,12 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr
169169
}
170170
hdrIncludes = append(hdrIncludes, "-iquote", workDir) // for _cgo_export.h
171171

172-
execRoot, err := bazelExecRoot()
172+
// Trim the path in //line comments emitted by cgo.
173+
trimPath, err := createTrimPath()
173174
if err != nil {
174175
return "", nil, nil, err
175176
}
176-
// Trim the execroot from the //line comments emitted by cgo.
177-
args := goenv.goTool("cgo", "-srcdir", srcDir, "-objdir", workDir, "-trimpath", execRoot)
177+
args := goenv.goTool("cgo", "-srcdir", srcDir, "-objdir", workDir, "-trimpath", trimPath)
178178
if ldflagsFile != nil {
179179
// The "@" prefix tells cgo to read arguments from the file.
180180
args = append(args, "-ldflags", "@"+ldflagsFile.Name())
@@ -421,18 +421,6 @@ func gatherSrcs(dir string, srcs []string) ([]string, error) {
421421
return copiedBases, nil
422422
}
423423

424-
func bazelExecRoot() (string, error) {
425-
// Bazel executes the builder with a working directory of the form
426-
// .../execroot/<workspace name>. By stripping the last segment, we obtain a
427-
// prefix of all possible source files, even when contained in external
428-
// repositories.
429-
cwd, err := os.Getwd()
430-
if err != nil {
431-
return "", err
432-
}
433-
return filepath.Dir(cwd), nil
434-
}
435-
436424
type cgoError []string
437425

438426
func (e cgoError) Error() string {

go/tools/builders/compilepkg.go

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -353,14 +353,35 @@ func compileArchive(
353353
return err
354354
}
355355
}
356-
gcFlags = append(gcFlags, createTrimPath(gcFlags, srcDir))
356+
gcFlags = append(gcFlags, "-trimpath="+srcDir)
357357
} else {
358358
if cgoExportHPath != "" {
359359
if err := os.WriteFile(cgoExportHPath, nil, 0o666); err != nil {
360360
return err
361361
}
362362
}
363-
gcFlags = append(gcFlags, createTrimPath(gcFlags, "."))
363+
trimPath, err := createTrimPath()
364+
if err != nil {
365+
return err
366+
}
367+
// Preserve an existing -trimpath argument, applying abs() to each prefix.
368+
for i, flag := range gcFlags {
369+
if strings.HasPrefix(flag, "-trimpath=") {
370+
gcFlags = append(gcFlags[:i], gcFlags[i+1:]...)
371+
rewrites := strings.Split(flag[len("-trimpath="):], ";")
372+
for j, rewrite := range rewrites {
373+
prefix, replace := rewrite, ""
374+
if p := strings.LastIndex(rewrite, "=>"); p >= 0 {
375+
prefix, replace = rewrite[:p], rewrite[p:]
376+
}
377+
rewrites[j] = abs(prefix) + replace
378+
}
379+
rewrites = append(rewrites, trimPath)
380+
trimPath = strings.Join(rewrites, ";")
381+
break
382+
}
383+
}
384+
gcFlags = append(gcFlags, "-trimpath="+trimPath)
364385
}
365386

366387
importcfgPath, err := checkImportsAndBuildCfg(goenv, importPath, srcs, deps, packageListPath, recompileInternalDeps, compilingWithCgo, coverMode, workDir)
@@ -531,7 +552,7 @@ func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPa
531552
args = append(args, "-linkobj", outLinkobjPath)
532553
args = append(args, "--")
533554
args = append(args, srcs...)
534-
absArgs(args, []string{"-I", "-o", "-trimpath", "-importcfg"})
555+
absArgs(args, []string{"-I", "-o", "-importcfg"})
535556
return goenv.runCommand(args)
536557
}
537558

@@ -542,14 +563,16 @@ func appendToArchive(goenv *env, pack, outPath string, objFiles []string) error
542563
return goenv.runCommand(args)
543564
}
544565

545-
func createTrimPath(gcFlags []string, path string) string {
546-
for _, flag := range gcFlags {
547-
if strings.HasPrefix(flag, "-trimpath=") {
548-
return flag + ":" + path
549-
}
566+
func createTrimPath() (string, error) {
567+
trimPath, err := os.Getwd()
568+
if err != nil {
569+
return "", err
550570
}
551-
552-
return "-trimpath=" + path
571+
// Create a trim path to make paths relative to the working directory.
572+
// First, attempt to trim the working directory, and if this fails, replace
573+
// the parent of the working directory with "..".
574+
trimPath = fmt.Sprintf("%s;%s=>..", trimPath, filepath.Dir(trimPath))
575+
return trimPath, nil
553576
}
554577

555578
func sanitizePathForIdentifier(path string) string {

tests/core/go_library/BUILD.bazel

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,8 @@ go_library(
208208
"//tests/core/go_test:__pkg__",
209209
],
210210
)
211+
212+
go_bazel_test(
213+
name = "trimpath_test",
214+
srcs = ["trimpath_test.go"],
215+
)

tests/core/go_library/README.rst

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Basic go_library functionality
77
.. _#1772: https://github.com/bazelbuild/rules_go/issues/1772
88
.. _#2058: https://github.com/bazelbuild/rules_go/issues/2058
99
.. _#3558: https://github.com/bazelbuild/rules_go/issues/3558
10+
.. _#4434: https://github.com/bazel-contrib/rules_go/issues/4434
1011

1112
empty
1213
-----
@@ -54,4 +55,9 @@ no_srcs_test
5455
------------
5556

5657
Verifies that `go_library`_ targets without Go source files build concurrently,
57-
even unsandboxed, and reproducibly. Verifies `#3558`_.
58+
even unsandboxed, and reproducibly. Verifies `#3558`_.
59+
60+
trimpath_test
61+
-------------
62+
63+
Verifies that trimpath has the expected effect on paths. Verifies `#4434`_.
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// Copyright 2025 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package trimpath_test
16+
17+
import (
18+
"testing"
19+
20+
"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
21+
)
22+
23+
func TestMain(m *testing.M) {
24+
bazel_testing.TestMain(m, bazel_testing.Args{
25+
Main: `
26+
-- BUILD.bazel --
27+
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
28+
29+
go_library(
30+
name = "maincgo",
31+
srcs = ["maincgo.go"],
32+
cgo = True,
33+
importpath = "example.com/main_repo/maincgo",
34+
visibility = ["//visibility:private"],
35+
)
36+
37+
go_library(
38+
name = "main_lib",
39+
srcs = ["main.go"],
40+
deps = [":maincgo", "@other_repo//:other", "@other_repo//:othercgo"],
41+
importpath = "example.com/main_repo/main",
42+
visibility = ["//visibility:private"],
43+
)
44+
45+
go_binary(
46+
name = "main",
47+
embed = [":main_lib"],
48+
visibility = ["//visibility:public"],
49+
)
50+
-- main.go --
51+
package main
52+
53+
import "runtime"
54+
import "fmt"
55+
import "example.com/main_repo/maincgo"
56+
import "example.com/other_repo/other"
57+
import "example.com/other_repo/othercgo"
58+
59+
func File() string {
60+
_, file, _, _ := runtime.Caller(0)
61+
return file
62+
}
63+
64+
func main() {
65+
fmt.Println(File())
66+
fmt.Println(maincgo.File())
67+
fmt.Println(other.File())
68+
fmt.Println(othercgo.File())
69+
}
70+
-- maincgo.go --
71+
package maincgo
72+
73+
import "C"
74+
import "runtime"
75+
76+
func File() string {
77+
_, file, _, _ := runtime.Caller(0)
78+
return file
79+
}
80+
-- other_repo/WORKSPACE --
81+
-- other_repo/BUILD.bazel --
82+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
83+
84+
go_library(
85+
name = "other",
86+
srcs = ["other.go"],
87+
importpath = "example.com/other_repo/other",
88+
visibility = ["//visibility:public"],
89+
)
90+
91+
go_library(
92+
name = "othercgo",
93+
srcs = ["othercgo.go"],
94+
cgo = True,
95+
importpath = "example.com/other_repo/othercgo",
96+
visibility = ["//visibility:public"],
97+
)
98+
-- other_repo/other.go --
99+
package other
100+
101+
import "runtime"
102+
103+
func File() string {
104+
_, file, _, _ := runtime.Caller(0)
105+
return file
106+
}
107+
-- other_repo/othercgo.go --
108+
package othercgo
109+
110+
import "C"
111+
import "runtime"
112+
113+
func File() string {
114+
_, file, _, _ := runtime.Caller(0)
115+
return file
116+
}
117+
`,
118+
WorkspaceSuffix: `
119+
local_repository(
120+
name = "other_repo",
121+
path = "other_repo",
122+
)
123+
`,
124+
})
125+
}
126+
127+
// These are the expected paths after applying trimpath.
128+
var expectedDefault = `
129+
main.go
130+
maincgo.go
131+
external/other_repo/other.go
132+
external/other_repo/othercgo.go
133+
`
134+
135+
var expectedSibling = `
136+
main.go
137+
maincgo.go
138+
../other_repo/other.go
139+
../other_repo/othercgo.go
140+
`
141+
142+
func TestTrimpath(t *testing.T) {
143+
t.Run("default", func(t *testing.T) {
144+
out, err := bazel_testing.BazelOutput("run", "//:main")
145+
if err != nil {
146+
t.Fatal(err)
147+
}
148+
outStr := "\n" + string(out)
149+
if outStr != expectedDefault {
150+
t.Fatal("actual", outStr, "vs expected", expectedDefault)
151+
}
152+
})
153+
t.Run("experimental_sibling_repository_layout", func(t *testing.T) {
154+
out, err := bazel_testing.BazelOutput("run", "--experimental_sibling_repository_layout", "//:main")
155+
if err != nil {
156+
t.Fatal(err)
157+
}
158+
outStr := "\n" + string(out)
159+
if outStr != expectedSibling {
160+
t.Fatal("actual", outStr, "vs expected", expectedSibling)
161+
}
162+
})
163+
}

tests/core/go_plugin_with_proto_library/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ proto_library(
2525

2626
go_proto_library(
2727
name = "validate",
28-
gc_goopts = ["-trimpath=$(BINDIR)=>."],
28+
gc_goopts = ["-trimpath=$(BINDIR)"],
2929
importpath = "go_plugin_with_proto_library/validate",
3030
proto = ":validate_proto",
3131
)

0 commit comments

Comments
 (0)