]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: stop creating nested temp directory trees
authorRuss Cox <rsc@golang.org>
Fri, 9 Jun 2017 19:46:47 +0000 (15:46 -0400)
committerRuss Cox <rsc@golang.org>
Sat, 30 Sep 2017 00:56:08 +0000 (00:56 +0000)
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 <iant@golang.org>
src/cmd/go/internal/test/test.go
src/cmd/go/internal/work/build.go

index bcb659b13115c95576923bafe0c8eb0e37398640..6fed6ac837a0fa15707ce189830028dc6bc63fb0 100644 (file)
@@ -773,29 +773,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
        }
        testBinary := elem + ".test"
 
-       // The ptest package needs to be importable under the
-       // same import path that p has, but we cannot put it in
-       // the usual place in the temporary tree, because then
-       // other tests will see it as the real package.
-       // Instead we make a _test directory under the import path
-       // and then repeat the import path there. We tell the
-       // compiler and linker to look in that _test directory first.
-       //
-       // That is, if the package under test is unicode/utf8,
-       // then the normal place to write the package archive is
-       // $WORK/unicode/utf8.a, but we write the test package archive to
-       // $WORK/unicode/utf8/_test/unicode/utf8.a.
-       // We write the external test package archive to
-       // $WORK/unicode/utf8/_test/unicode/utf8_test.a.
-       testDir := filepath.Join(b.WorkDir, filepath.FromSlash(p.ImportPath+"/_test"))
-       ptestObj := work.BuildToolchain.Pkgpath(testDir, p)
-
-       // Create the directory for the .a files.
-       ptestDir, _ := filepath.Split(ptestObj)
-       if err := b.Mkdir(ptestDir); err != nil {
-               return nil, nil, nil, err
-       }
-
        // Should we apply coverage analysis locally,
        // only for this package and only for this test?
        // 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
                ptest.Internal.Target = ""
                ptest.Imports = str.StringList(p.Imports, p.TestImports)
                ptest.Internal.Imports = append(append([]*load.Package{}, p.Internal.Imports...), imports...)
-               ptest.Internal.Pkgdir = testDir
                ptest.Internal.Fake = true
                ptest.Internal.ForceLibrary = true
                ptest.Stale = true
@@ -857,7 +833,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
                                        ImportPos: p.Internal.Build.XTestImportPos,
                                },
                                Imports:  ximports,
-                               Pkgdir:   testDir,
                                Fake:     true,
                                External: true,
                        },
@@ -867,19 +842,23 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
                }
        }
 
+       testDir := b.NewObjdir()
+       if err := b.Mkdir(testDir); err != nil {
+               return nil, nil, nil, err
+       }
+
        // Action for building pkg.test.
        pmain = &load.Package{
                PackagePublic: load.PackagePublic{
                        Name:       "main",
                        Dir:        testDir,
                        GoFiles:    []string{"_testmain.go"},
-                       ImportPath: "testmain",
+                       ImportPath: p.ImportPath + " (testmain)",
                        Root:       p.Root,
                        Stale:      true,
                },
                Internal: load.PackageInternal{
                        Build:     &build.Package{Name: "main"},
-                       Pkgdir:    testDir,
                        Fake:      true,
                        OmitDebug: !testC && !testNeedBinary,
                },
@@ -942,18 +921,21 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 
        if ptest != p && localCover {
                // We have made modifications to the package p being tested
-               // and are rebuilding p (as ptest), writing it to the testDir tree.
-               // Arrange to rebuild, writing to that same tree, all packages q
-               // such that the test depends on q, and q depends on p.
+               // and are rebuilding p (as ptest).
+               // Arrange to rebuild all packages q such that
+               // the test depends on q and q depends on p.
                // This makes sure that q sees the modifications to p.
                // Strictly speaking, the rebuild is only necessary if the
                // modifications to p change its export metadata, but
                // determining that is a bit tricky, so we rebuild always.
+               // TODO(rsc): Once we get export metadata changes
+               // handled properly, look into the expense of dropping
+               // "&& localCover" above.
                //
                // This will cause extra compilation, so for now we only do it
                // when testCover is set. The conditions are more general, though,
                // and we may find that we need to do it always in the future.
-               recompileForTest(pmain, p, ptest, testDir)
+               recompileForTest(pmain, p, ptest)
        }
 
        t.NeedCgo = forceCgo
@@ -965,9 +947,9 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
        }
 
        if !cfg.BuildN {
-               // writeTestmain writes _testmain.go. This must happen after recompileForTest,
-               // because recompileForTest modifies XXX.
-               if err := writeTestmain(filepath.Join(testDir, "_testmain.go"), t); err != nil {
+               // writeTestmain writes _testmain.go,
+               // using the test description gathered in t.
+               if err := writeTestmain(testDir+"_testmain.go", t); err != nil {
                        return nil, nil, nil, err
                }
        }
@@ -976,23 +958,11 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 
        if ptest != p {
                a := b.Action(work.ModeBuild, work.ModeBuild, ptest)
-               a.Objdir = testDir + string(filepath.Separator) + "_obj_test" + string(filepath.Separator)
-               a.Objpkg = ptestObj
-               a.Target = ptestObj
                a.Link = false
        }
 
-       if pxtest != nil {
-               a := b.Action(work.ModeBuild, work.ModeBuild, pxtest)
-               a.Objdir = testDir + string(filepath.Separator) + "_obj_xtest" + string(filepath.Separator)
-               a.Objpkg = work.BuildToolchain.Pkgpath(testDir, pxtest)
-               a.Target = a.Objpkg
-       }
-
        a := b.Action(work.ModeBuild, work.ModeBuild, pmain)
-       a.Objdir = testDir + string(filepath.Separator)
-       a.Objpkg = filepath.Join(testDir, "main.a")
-       a.Target = filepath.Join(testDir, testBinary) + cfg.ExeSuffix
+       a.Target = testDir + testBinary + cfg.ExeSuffix
        if cfg.Goos == "windows" {
                // There are many reserved words on Windows that,
                // 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
                // we could just do this always on Windows.
                for _, bad := range windowsBadWords {
                        if strings.Contains(testBinary, bad) {
-                               a.Target = filepath.Join(testDir, "test.test") + cfg.ExeSuffix
+                               a.Target = testDir + "test.test" + cfg.ExeSuffix
                                break
                        }
                }
@@ -1056,6 +1026,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
                        Func:    builderCleanTest,
                        Deps:    []*work.Action{runAction},
                        Package: p,
+                       Objdir:  testDir,
                }
                printAction = &work.Action{
                        Func:    builderPrintTest,
@@ -1085,7 +1056,7 @@ Search:
        return stk
 }
 
-func recompileForTest(pmain, preal, ptest *load.Package, testDir string) {
+func recompileForTest(pmain, preal, ptest *load.Package) {
        // The "test copy" of preal is ptest.
        // For each package that depends on preal, make a "test copy"
        // that depends on ptest. And so on, up the dependency tree.
@@ -1098,19 +1069,19 @@ func recompileForTest(pmain, preal, ptest *load.Package, testDir string) {
                                return
                        }
                        didSplit = true
-                       if p.Internal.Pkgdir != testDir {
-                               p1 := new(load.Package)
-                               testCopy[p] = p1
-                               *p1 = *p
-                               p1.Internal.Imports = make([]*load.Package, len(p.Internal.Imports))
-                               copy(p1.Internal.Imports, p.Internal.Imports)
-                               p = p1
-                               p.Internal.Pkgdir = testDir
-                               p.Internal.Target = ""
-                               p.Internal.Fake = true
-                               p.Stale = true
-                               p.StaleReason = "depends on package being tested"
+                       if testCopy[p] != nil {
+                               panic("recompileForTest loop")
                        }
+                       p1 := new(load.Package)
+                       testCopy[p] = p1
+                       *p1 = *p
+                       p1.Internal.Imports = make([]*load.Package, len(p.Internal.Imports))
+                       copy(p1.Internal.Imports, p.Internal.Imports)
+                       p = p1
+                       p.Internal.Target = ""
+                       p.Internal.Fake = true
+                       p.Stale = true
+                       p.StaleReason = "depends on package being tested"
                }
 
                // Update p.Internal.Imports to use test copies.
@@ -1288,9 +1259,10 @@ func builderCleanTest(b *work.Builder, a *work.Action) error {
        if cfg.BuildWork {
                return nil
        }
-       run := a.Deps[0]
-       testDir := filepath.Join(b.WorkDir, filepath.FromSlash(run.Package.ImportPath+"/_test"))
-       os.RemoveAll(testDir)
+       if cfg.BuildX {
+               b.Showcmd("", "rm -r %s", a.Objdir)
+       }
+       os.RemoveAll(a.Objdir)
        return nil
 }
 
index 0300831ebfa3d7729359b1e70bac7faa88e765ed..932626d060e42cfb41f534c465dda85c4e07f816 100644 (file)
@@ -658,6 +658,9 @@ type Builder struct {
        flagCache   map[[2]string]bool   // a cache of supported compiler flags
        Print       func(args ...interface{}) (int, error)
 
+       objdirSeq int // counter for NewObjdir
+       pkgSeq    int
+
        output    sync.Mutex
        scriptDir string // current directory in printed script
 
@@ -683,7 +686,6 @@ type Action struct {
 
        // Generated files, directories.
        Link   bool   // target is executable, not just package
-       Pkgdir string // the -I or -L argument to use when importing this package
        Objdir string // directory for intermediate objects
        Objpkg string // the intermediate package .a file created during the action
        Target string // goal of the action: the created package or executable
@@ -746,6 +748,19 @@ func (b *Builder) Init() {
        }
 }
 
+// NewObjdir returns the name of a fresh object directory under b.WorkDir.
+// It is up to the caller to call b.Mkdir on the result at an appropriate time.
+// The result ends in a slash, so that file names in that directory
+// can be constructed with direct string addition.
+//
+// NewObjdir must be called only from a single goroutine at a time,
+// so it is safe to call during action graph construction, but it must not
+// be called during action graph execution.
+func (b *Builder) NewObjdir() string {
+       b.objdirSeq++
+       return filepath.Join(b.WorkDir, fmt.Sprintf("b%03d", b.objdirSeq)) + string(filepath.Separator)
+}
+
 // readpkglist returns the list of packages that were built into the shared library
 // at shlibpath. For the native toolchain this list is stored, newline separated, in
 // 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
                return a
        }
 
-       a = &Action{Package: p, Pkgdir: p.Internal.Build.PkgRoot}
-       if p.Internal.Pkgdir != "" { // overrides p.t
-               a.Pkgdir = p.Internal.Pkgdir
-       }
+       a = &Action{Package: p}
        b.actionCache[key] = a
 
        for _, p1 := range p.Internal.Imports {
@@ -885,13 +897,9 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo
                // Imported via local path. No permanent target.
                mode = ModeBuild
        }
-       work := p.Internal.Pkgdir
-       if work == "" {
-               work = b.WorkDir
-       }
-       a.Objdir = filepath.Join(work, a.Package.ImportPath, "_obj") + string(filepath.Separator)
-       a.Objpkg = BuildToolchain.Pkgpath(work, a.Package)
-       a.Link = p.Name == "main"
+       a.Objdir = b.NewObjdir()
+       a.Objpkg = a.Objdir + "_pkg_.a"
+       a.Link = p.Name == "main" && !p.Internal.ForceLibrary
 
        switch mode {
        case ModeInstall:
@@ -915,8 +923,7 @@ func (b *Builder) action1(mode BuildMode, depMode BuildMode, p *load.Package, lo
                                Package: a.Package,
                                Deps:    []*Action{a.Deps[0]},
                                Func:    (*Builder).installHeader,
-                               Pkgdir:  a.Pkgdir,
-                               Objdir:  a.Objdir,
+                               Objdir:  a.Deps[0].Objdir,
                                Target:  hdrTarget,
                        }
                        a.Deps = append(a.Deps, ah)
@@ -1654,70 +1661,23 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) {
        // with aggressive buffering, cleaning incrementally like
        // this keeps the intermediate objects from hitting the disk.
        if !cfg.BuildWork {
-               defer os.RemoveAll(a1.Objdir)
-               defer os.Remove(a1.Target)
+               defer func() {
+                       if cfg.BuildX {
+                               b.Showcmd("", "rm -r %s", a1.Objdir)
+                       }
+                       os.RemoveAll(a1.Objdir)
+                       if _, err := os.Stat(a1.Target); err == nil {
+                               if cfg.BuildX {
+                                       b.Showcmd("", "rm %s", a1.Target)
+                               }
+                               os.Remove(a1.Target)
+                       }
+               }()
        }
 
        return b.moveOrCopyFile(a, a.Target, a1.Target, perm, false)
 }
 
-// includeArgs returns the -I or -L directory list for access
-// to the results of the list of actions.
-func (b *Builder) includeArgs(flag string, all []*Action) []string {
-       inc := []string{}
-       incMap := map[string]bool{
-               b.WorkDir:     true, // handled later
-               cfg.GOROOTpkg: true,
-               "":            true, // ignore empty strings
-       }
-
-       // Look in the temporary space for results of test-specific actions.
-       // This is the $WORK/my/package/_test directory for the
-       // package being built, so there are few of these.
-       for _, a1 := range all {
-               if a1.Package == nil {
-                       continue
-               }
-               if dir := a1.Pkgdir; dir != a1.Package.Internal.Build.PkgRoot && !incMap[dir] {
-                       incMap[dir] = true
-                       inc = append(inc, flag, dir)
-               }
-       }
-
-       // Also look in $WORK for any non-test packages that have
-       // been built but not installed.
-       inc = append(inc, flag, b.WorkDir)
-
-       // Finally, look in the installed package directories for each action.
-       // First add the package dirs corresponding to GOPATH entries
-       // in the original GOPATH order.
-       need := map[string]*build.Package{}
-       for _, a1 := range all {
-               if a1.Package != nil && a1.Pkgdir == a1.Package.Internal.Build.PkgRoot {
-                       need[a1.Package.Internal.Build.Root] = a1.Package.Internal.Build
-               }
-       }
-       for _, root := range cfg.Gopath {
-               if p := need[root]; p != nil && !incMap[p.PkgRoot] {
-                       incMap[p.PkgRoot] = true
-                       inc = append(inc, flag, p.PkgTargetRoot)
-               }
-       }
-
-       // Then add anything that's left.
-       for _, a1 := range all {
-               if a1.Package == nil {
-                       continue
-               }
-               if dir := a1.Pkgdir; dir == a1.Package.Internal.Build.PkgRoot && !incMap[dir] {
-                       incMap[dir] = true
-                       inc = append(inc, flag, a1.Package.Internal.Build.PkgTargetRoot)
-               }
-       }
-
-       return inc
-}
-
 // moveOrCopyFile is like 'mv src dst' or 'cp src dst'.
 func (b *Builder) moveOrCopyFile(a *Action, dst, src string, perm os.FileMode, force bool) error {
        if cfg.BuildN {
@@ -1842,6 +1802,9 @@ func (b *Builder) installHeader(a *Action) error {
        if _, err := os.Stat(src); os.IsNotExist(err) {
                // If the file does not exist, there are no exported
                // functions, and we do not install anything.
+               if cfg.BuildX {
+                       b.Showcmd("", "# %s not created", src)
+               }
                return nil
        }