Skip to content

Commit ee4fbbc

Browse files
committed
cmd/go: stop creating nested temp directory trees
Now that we have -importcfg, there's no need for the temporary directory trees that mirror the import path structure, and we can drop a bunch of complex code that was building and maintaining that structure. This should fix "file name too long" errors on systems with low limits. (For example #10651 and #17070, although we fixed those by adding code to deal with very long file names on Windows instead.) Change-Id: I11e221c6c1edeb81c3b2f1d89988f5235aa2bbb9 Reviewed-on: https://go-review.googlesource.com/56280 Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent a1fb024 commit ee4fbbc

File tree

2 files changed

+72
-137
lines changed

2 files changed

+72
-137
lines changed

src/cmd/go/internal/test/test.go

Lines changed: 36 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -773,29 +773,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
773773
}
774774
testBinary := elem + ".test"
775775

776-
// The ptest package needs to be importable under the
777-
// same import path that p has, but we cannot put it in
778-
// the usual place in the temporary tree, because then
779-
// other tests will see it as the real package.
780-
// Instead we make a _test directory under the import path
781-
// and then repeat the import path there. We tell the
782-
// compiler and linker to look in that _test directory first.
783-
//
784-
// That is, if the package under test is unicode/utf8,
785-
// then the normal place to write the package archive is
786-
// $WORK/unicode/utf8.a, but we write the test package archive to
787-
// $WORK/unicode/utf8/_test/unicode/utf8.a.
788-
// We write the external test package archive to
789-
// $WORK/unicode/utf8/_test/unicode/utf8_test.a.
790-
testDir := filepath.Join(b.WorkDir, filepath.FromSlash(p.ImportPath+"/_test"))
791-
ptestObj := work.BuildToolchain.Pkgpath(testDir, p)
792-
793-
// Create the directory for the .a files.
794-
ptestDir, _ := filepath.Split(ptestObj)
795-
if err := b.Mkdir(ptestDir); err != nil {
796-
return nil, nil, nil, err
797-
}
798-
799776
// Should we apply coverage analysis locally,
800777
// only for this package and only for this test?
801778
// Yes, if -cover is on but -coverpkg has not specified
@@ -812,7 +789,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
812789
ptest.Internal.Target = ""
813790
ptest.Imports = str.StringList(p.Imports, p.TestImports)
814791
ptest.Internal.Imports = append(append([]*load.Package{}, p.Internal.Imports...), imports...)
815-
ptest.Internal.Pkgdir = testDir
816792
ptest.Internal.Fake = true
817793
ptest.Internal.ForceLibrary = true
818794
ptest.Stale = true
@@ -857,7 +833,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
857833
ImportPos: p.Internal.Build.XTestImportPos,
858834
},
859835
Imports: ximports,
860-
Pkgdir: testDir,
861836
Fake: true,
862837
External: true,
863838
},
@@ -867,19 +842,23 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
867842
}
868843
}
869844

845+
testDir := b.NewObjdir()
846+
if err := b.Mkdir(testDir); err != nil {
847+
return nil, nil, nil, err
848+
}
849+
870850
// Action for building pkg.test.
871851
pmain = &load.Package{
872852
PackagePublic: load.PackagePublic{
873853
Name: "main",
874854
Dir: testDir,
875855
GoFiles: []string{"_testmain.go"},
876-
ImportPath: "testmain",
856+
ImportPath: p.ImportPath + " (testmain)",
877857
Root: p.Root,
878858
Stale: true,
879859
},
880860
Internal: load.PackageInternal{
881861
Build: &build.Package{Name: "main"},
882-
Pkgdir: testDir,
883862
Fake: true,
884863
OmitDebug: !testC && !testNeedBinary,
885864
},
@@ -942,18 +921,21 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
942921

943922
if ptest != p && localCover {
944923
// We have made modifications to the package p being tested
945-
// and are rebuilding p (as ptest), writing it to the testDir tree.
946-
// Arrange to rebuild, writing to that same tree, all packages q
947-
// such that the test depends on q, and q depends on p.
924+
// and are rebuilding p (as ptest).
925+
// Arrange to rebuild all packages q such that
926+
// the test depends on q and q depends on p.
948927
// This makes sure that q sees the modifications to p.
949928
// Strictly speaking, the rebuild is only necessary if the
950929
// modifications to p change its export metadata, but
951930
// determining that is a bit tricky, so we rebuild always.
931+
// TODO(rsc): Once we get export metadata changes
932+
// handled properly, look into the expense of dropping
933+
// "&& localCover" above.
952934
//
953935
// This will cause extra compilation, so for now we only do it
954936
// when testCover is set. The conditions are more general, though,
955937
// and we may find that we need to do it always in the future.
956-
recompileForTest(pmain, p, ptest, testDir)
938+
recompileForTest(pmain, p, ptest)
957939
}
958940

959941
t.NeedCgo = forceCgo
@@ -965,9 +947,9 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
965947
}
966948

967949
if !cfg.BuildN {
968-
// writeTestmain writes _testmain.go. This must happen after recompileForTest,
969-
// because recompileForTest modifies XXX.
970-
if err := writeTestmain(filepath.Join(testDir, "_testmain.go"), t); err != nil {
950+
// writeTestmain writes _testmain.go,
951+
// using the test description gathered in t.
952+
if err := writeTestmain(testDir+"_testmain.go", t); err != nil {
971953
return nil, nil, nil, err
972954
}
973955
}
@@ -976,23 +958,11 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
976958

977959
if ptest != p {
978960
a := b.Action(work.ModeBuild, work.ModeBuild, ptest)
979-
a.Objdir = testDir + string(filepath.Separator) + "_obj_test" + string(filepath.Separator)
980-
a.Objpkg = ptestObj
981-
a.Target = ptestObj
982961
a.Link = false
983962
}
984963

985-
if pxtest != nil {
986-
a := b.Action(work.ModeBuild, work.ModeBuild, pxtest)
987-
a.Objdir = testDir + string(filepath.Separator) + "_obj_xtest" + string(filepath.Separator)
988-
a.Objpkg = work.BuildToolchain.Pkgpath(testDir, pxtest)
989-
a.Target = a.Objpkg
990-
}
991-
992964
a := b.Action(work.ModeBuild, work.ModeBuild, pmain)
993-
a.Objdir = testDir + string(filepath.Separator)
994-
a.Objpkg = filepath.Join(testDir, "main.a")
995-
a.Target = filepath.Join(testDir, testBinary) + cfg.ExeSuffix
965+
a.Target = testDir + testBinary + cfg.ExeSuffix
996966
if cfg.Goos == "windows" {
997967
// There are many reserved words on Windows that,
998968
// if used in the name of an executable, cause Windows
@@ -1018,7 +988,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
1018988
// we could just do this always on Windows.
1019989
for _, bad := range windowsBadWords {
1020990
if strings.Contains(testBinary, bad) {
1021-
a.Target = filepath.Join(testDir, "test.test") + cfg.ExeSuffix
991+
a.Target = testDir + "test.test" + cfg.ExeSuffix
1022992
break
1023993
}
1024994
}
@@ -1056,6 +1026,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
10561026
Func: builderCleanTest,
10571027
Deps: []*work.Action{runAction},
10581028
Package: p,
1029+
Objdir: testDir,
10591030
}
10601031
printAction = &work.Action{
10611032
Func: builderPrintTest,
@@ -1085,7 +1056,7 @@ Search:
10851056
return stk
10861057
}
10871058

1088-
func recompileForTest(pmain, preal, ptest *load.Package, testDir string) {
1059+
func recompileForTest(pmain, preal, ptest *load.Package) {
10891060
// The "test copy" of preal is ptest.
10901061
// For each package that depends on preal, make a "test copy"
10911062
// that depends on ptest. And so on, up the dependency tree.
@@ -1098,19 +1069,19 @@ func recompileForTest(pmain, preal, ptest *load.Package, testDir string) {
10981069
return
10991070
}
11001071
didSplit = true
1101-
if p.Internal.Pkgdir != testDir {
1102-
p1 := new(load.Package)
1103-
testCopy[p] = p1
1104-
*p1 = *p
1105-
p1.Internal.Imports = make([]*load.Package, len(p.Internal.Imports))
1106-
copy(p1.Internal.Imports, p.Internal.Imports)
1107-
p = p1
1108-
p.Internal.Pkgdir = testDir
1109-
p.Internal.Target = ""
1110-
p.Internal.Fake = true
1111-
p.Stale = true
1112-
p.StaleReason = "depends on package being tested"
1072+
if testCopy[p] != nil {
1073+
panic("recompileForTest loop")
11131074
}
1075+
p1 := new(load.Package)
1076+
testCopy[p] = p1
1077+
*p1 = *p
1078+
p1.Internal.Imports = make([]*load.Package, len(p.Internal.Imports))
1079+
copy(p1.Internal.Imports, p.Internal.Imports)
1080+
p = p1
1081+
p.Internal.Target = ""
1082+
p.Internal.Fake = true
1083+
p.Stale = true
1084+
p.StaleReason = "depends on package being tested"
11141085
}
11151086

11161087
// Update p.Internal.Imports to use test copies.
@@ -1288,9 +1259,10 @@ func builderCleanTest(b *work.Builder, a *work.Action) error {
12881259
if cfg.BuildWork {
12891260
return nil
12901261
}
1291-
run := a.Deps[0]
1292-
testDir := filepath.Join(b.WorkDir, filepath.FromSlash(run.Package.ImportPath+"/_test"))
1293-
os.RemoveAll(testDir)
1262+
if cfg.BuildX {
1263+
b.Showcmd("", "rm -r %s", a.Objdir)
1264+
}
1265+
os.RemoveAll(a.Objdir)
12941266
return nil
12951267
}
12961268

src/cmd/go/internal/work/build.go

Lines changed: 36 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,9 @@ type Builder struct {
658658
flagCache map[[2]string]bool // a cache of supported compiler flags
659659
Print func(args ...interface{}) (int, error)
660660

661+
objdirSeq int // counter for NewObjdir
662+
pkgSeq int
663+
661664
output sync.Mutex
662665
scriptDir string // current directory in printed script
663666

@@ -683,7 +686,6 @@ type Action struct {
683686

684687
// Generated files, directories.
685688
Link bool // target is executable, not just package
686-
Pkgdir string // the -I or -L argument to use when importing this package
687689
Objdir string // directory for intermediate objects
688690
Objpkg string // the intermediate package .a file created during the action
689691
Target string // goal of the action: the created package or executable
@@ -746,6 +748,19 @@ func (b *Builder) Init() {
746748
}
747749
}
748750

751+
// NewObjdir returns the name of a fresh object directory under b.WorkDir.
752+
// It is up to the caller to call b.Mkdir on the result at an appropriate time.
753+
// The result ends in a slash, so that file names in that directory
754+
// can be constructed with direct string addition.
755+
//
756+
// NewObjdir must be called only from a single goroutine at a time,
757+
// so it is safe to call during action graph construction, but it must not
758+
// be called during action graph execution.
759+
func (b *Builder) NewObjdir() string {
760+
b.objdirSeq++
761+
return filepath.Join(b.WorkDir, fmt.Sprintf("b%03d", b.objdirSeq)) + string(filepath.Separator)
762+
}
763+
749764
// readpkglist returns the list of packages that were built into the shared library
750765
// at shlibpath. For the native toolchain this list is stored, newline separated, in
751766
// an ELF note with name "Go\x00\x00" and type 1. For GCCGO it is extracted from the
@@ -816,10 +831,7 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo
816831
return a
817832
}
818833

819-
a = &Action{Package: p, Pkgdir: p.Internal.Build.PkgRoot}
820-
if p.Internal.Pkgdir != "" { // overrides p.t
821-
a.Pkgdir = p.Internal.Pkgdir
822-
}
834+
a = &Action{Package: p}
823835
b.actionCache[key] = a
824836

825837
for _, p1 := range p.Internal.Imports {
@@ -885,13 +897,9 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo
885897
// Imported via local path. No permanent target.
886898
mode = ModeBuild
887899
}
888-
work := p.Internal.Pkgdir
889-
if work == "" {
890-
work = b.WorkDir
891-
}
892-
a.Objdir = filepath.Join(work, a.Package.ImportPath, "_obj") + string(filepath.Separator)
893-
a.Objpkg = BuildToolchain.Pkgpath(work, a.Package)
894-
a.Link = p.Name == "main"
900+
a.Objdir = b.NewObjdir()
901+
a.Objpkg = a.Objdir + "_pkg_.a"
902+
a.Link = p.Name == "main" && !p.Internal.ForceLibrary
895903

896904
switch mode {
897905
case ModeInstall:
@@ -915,8 +923,7 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo
915923
Package: a.Package,
916924
Deps: []*Action{a.Deps[0]},
917925
Func: (*Builder).installHeader,
918-
Pkgdir: a.Pkgdir,
919-
Objdir: a.Objdir,
926+
Objdir: a.Deps[0].Objdir,
920927
Target: hdrTarget,
921928
}
922929
a.Deps = append(a.Deps, ah)
@@ -1654,70 +1661,23 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) {
16541661
// with aggressive buffering, cleaning incrementally like
16551662
// this keeps the intermediate objects from hitting the disk.
16561663
if !cfg.BuildWork {
1657-
defer os.RemoveAll(a1.Objdir)
1658-
defer os.Remove(a1.Target)
1664+
defer func() {
1665+
if cfg.BuildX {
1666+
b.Showcmd("", "rm -r %s", a1.Objdir)
1667+
}
1668+
os.RemoveAll(a1.Objdir)
1669+
if _, err := os.Stat(a1.Target); err == nil {
1670+
if cfg.BuildX {
1671+
b.Showcmd("", "rm %s", a1.Target)
1672+
}
1673+
os.Remove(a1.Target)
1674+
}
1675+
}()
16591676
}
16601677

16611678
return b.moveOrCopyFile(a, a.Target, a1.Target, perm, false)
16621679
}
16631680

1664-
// includeArgs returns the -I or -L directory list for access
1665-
// to the results of the list of actions.
1666-
func (b *Builder) includeArgs(flag string, all []*Action) []string {
1667-
inc := []string{}
1668-
incMap := map[string]bool{
1669-
b.WorkDir: true, // handled later
1670-
cfg.GOROOTpkg: true,
1671-
"": true, // ignore empty strings
1672-
}
1673-
1674-
// Look in the temporary space for results of test-specific actions.
1675-
// This is the $WORK/my/package/_test directory for the
1676-
// package being built, so there are few of these.
1677-
for _, a1 := range all {
1678-
if a1.Package == nil {
1679-
continue
1680-
}
1681-
if dir := a1.Pkgdir; dir != a1.Package.Internal.Build.PkgRoot && !incMap[dir] {
1682-
incMap[dir] = true
1683-
inc = append(inc, flag, dir)
1684-
}
1685-
}
1686-
1687-
// Also look in $WORK for any non-test packages that have
1688-
// been built but not installed.
1689-
inc = append(inc, flag, b.WorkDir)
1690-
1691-
// Finally, look in the installed package directories for each action.
1692-
// First add the package dirs corresponding to GOPATH entries
1693-
// in the original GOPATH order.
1694-
need := map[string]*build.Package{}
1695-
for _, a1 := range all {
1696-
if a1.Package != nil && a1.Pkgdir == a1.Package.Internal.Build.PkgRoot {
1697-
need[a1.Package.Internal.Build.Root] = a1.Package.Internal.Build
1698-
}
1699-
}
1700-
for _, root := range cfg.Gopath {
1701-
if p := need[root]; p != nil && !incMap[p.PkgRoot] {
1702-
incMap[p.PkgRoot] = true
1703-
inc = append(inc, flag, p.PkgTargetRoot)
1704-
}
1705-
}
1706-
1707-
// Then add anything that's left.
1708-
for _, a1 := range all {
1709-
if a1.Package == nil {
1710-
continue
1711-
}
1712-
if dir := a1.Pkgdir; dir == a1.Package.Internal.Build.PkgRoot && !incMap[dir] {
1713-
incMap[dir] = true
1714-
inc = append(inc, flag, a1.Package.Internal.Build.PkgTargetRoot)
1715-
}
1716-
}
1717-
1718-
return inc
1719-
}
1720-
17211681
// moveOrCopyFile is like 'mv src dst' or 'cp src dst'.
17221682
func (b *Builder) moveOrCopyFile(a *Action, dst, src string, perm os.FileMode, force bool) error {
17231683
if cfg.BuildN {
@@ -1842,6 +1802,9 @@ func (b *Builder) installHeader(a *Action) error {
18421802
if _, err := os.Stat(src); os.IsNotExist(err) {
18431803
// If the file does not exist, there are no exported
18441804
// functions, and we do not install anything.
1805+
if cfg.BuildX {
1806+
b.Showcmd("", "# %s not created", src)
1807+
}
18451808
return nil
18461809
}
18471810

0 commit comments

Comments
 (0)