From 81555cb4f3521b53f9de4ce15f64b77cc9df61b9 Mon Sep 17 00:00:00 2001 From: Richard Musiol Date: Sun, 5 Aug 2018 18:52:15 +0200 Subject: [PATCH 01/31] cmd/compile/internal/gc: add nil check for closure call on wasm This commit adds an explicit nil check for closure calls on wasm, so calling a nil func causes a proper panic instead of crashing on the WebAssembly level. Change-Id: I6246844f316677976cdd420618be5664444c25ae Reviewed-on: https://go-review.googlesource.com/127759 Run-TryBot: Richard Musiol TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/gc/ssa.go | 4 ++++ test/closure4.go | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test/closure4.go diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 553713a1e9..af43da6275 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -3515,6 +3515,10 @@ func (s *state) call(n *Node, k callKind) *ssa.Value { break } closure = s.expr(fn) + if thearch.LinkArch.Family == sys.Wasm { + // TODO(neelance): On other architectures this should be eliminated by the optimization steps + s.nilCheck(closure) + } case OCALLMETH: if fn.Op != ODOTMETH { Fatalf("OCALLMETH: n.Left not an ODOTMETH: %v", fn) diff --git a/test/closure4.go b/test/closure4.go new file mode 100644 index 0000000000..ec4e0a18eb --- /dev/null +++ b/test/closure4.go @@ -0,0 +1,21 @@ +// run + +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Check that calling a nil func causes a proper panic. + +package main + +func main() { + defer func() { + err := recover() + if err == nil { + panic("panic expected") + } + }() + + var f func() + f() +} From daeb0b4f538872a37787626816c47afcf71bfd2c Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 13 Aug 2018 17:25:49 -0700 Subject: [PATCH 02/31] go/printer: revert "make empty lines break table alignment" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c116265eb3f2b1a8549e7ceef73b780439404030. The change, while addressing issue #26352, introduced another regression (#26930), which is worse. Reverting this change in favor of a better fix for the original issue. Updates #26352. Fixes #26930. Change-Id: I71ad12a8212992cce5c1e73907d1f7460f98d9e8 Reviewed-on: https://go-review.googlesource.com/129255 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Daniel Martí Reviewed-by: Brad Fitzpatrick --- src/go/printer/nodes.go | 17 ++++------------- src/go/printer/testdata/alignment.golden | 9 --------- src/go/printer/testdata/alignment.input | 9 --------- 3 files changed, 4 insertions(+), 31 deletions(-) diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go index 3723f30e56..18f2371d24 100644 --- a/src/go/printer/nodes.go +++ b/src/go/printer/nodes.go @@ -221,22 +221,13 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp // If the previous line and the current line had single- // line-expressions and the key sizes are small or the // ratio between the current key and the geometric mean - // does not exceed a threshold, align columns and do not use - // formfeed. - // If the previous line was an empty line, break the alignment. - // (The text/tabwriter will break alignment after an empty line - // even if we don't do anything here, but we can't see that; yet - // we need to reset the variables used in the geomean - // computation after an alignment break. Do it explicitly - // instead so we're aware of the break. Was issue #26352.) + // if the previous key sizes does not exceed a threshold, + // align columns and do not use formfeed. if prevSize > 0 && size > 0 { const smallSize = 40 - switch { - case prevLine+1 < line: - useFF = true - case count == 0, prevSize <= smallSize && size <= smallSize: + if count == 0 || prevSize <= smallSize && size <= smallSize { useFF = false - default: + } else { const r = 2.5 // threshold geomean := math.Exp(lnsum / float64(count)) // count > 0 ratio := float64(size) / geomean diff --git a/src/go/printer/testdata/alignment.golden b/src/go/printer/testdata/alignment.golden index 302b32e766..c65defe6ae 100644 --- a/src/go/printer/testdata/alignment.golden +++ b/src/go/printer/testdata/alignment.golden @@ -128,12 +128,3 @@ func main() { abcdefghijklmnopqrstuvwxyz: "foo", } } - -// ---------------------------------------------------------------------------- -// Examples from issue #26352. -var _ = map[int]string{ - 1: "", - - 12345678901234567890123456789: "", - 12345678901234567890123456789012345678: "", -} diff --git a/src/go/printer/testdata/alignment.input b/src/go/printer/testdata/alignment.input index 83361cc7c1..9b0aae6bec 100644 --- a/src/go/printer/testdata/alignment.input +++ b/src/go/printer/testdata/alignment.input @@ -128,12 +128,3 @@ func main() { abcdefghijklmnopqrstuvwxyz: "foo", } } - -// ---------------------------------------------------------------------------- -// Examples from issue #26352. -var _ = map[int]string{ - 1: "", - - 12345678901234567890123456789: "", - 12345678901234567890123456789012345678: "", -} From 469ada6ed9cbf921a268f867d3fde0f311532ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20M=C3=B6hrmann?= Date: Sat, 28 Jul 2018 16:46:22 +0200 Subject: [PATCH 03/31] runtime: go fmt runtime2.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I29a6125c9ef285fc365c4e11ab158b79224ae333 Reviewed-on: https://go-review.googlesource.com/126602 Run-TryBot: Martin Möhrmann TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/runtime/runtime2.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index a3193b63c5..ad47d1275e 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -842,11 +842,11 @@ var ( lfenceBeforeRdtsc bool // Set in runtime.cpuinit. - support_erms bool - support_popcnt bool - support_sse2 bool - support_sse41 bool - arm64_support_atomics bool + support_erms bool + support_popcnt bool + support_sse2 bool + support_sse41 bool + arm64_support_atomics bool goarm uint8 // set by cmd/link on arm systems framepointer_enabled bool // set by cmd/link From c882f4b6b13d4d38d354bec4614d4402031ec1b1 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 13 Aug 2018 17:57:39 -0700 Subject: [PATCH 04/31] go/printer: consider empty lines in table layout computation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In previous versions of Go including 1.10, an empty line would break the alignment of elements within an expression list. golang.org/cl/104755 changed the heuristic, with the side effect that empty lines no longer broke the table alignment. A prior fix (https://go-review.googlesource.com/c/go/+/125260, reverted) introduced another regression (#26930) which this change doesn't produce. Added test cases for both #26352 and #26930. Fixes #26352. Updates #26930. Change-Id: I371f48e6f3620ebbab53f2128ec5e58bcd4a62f1 Reviewed-on: https://go-review.googlesource.com/129256 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Daniel Martí Reviewed-by: Alan Donovan --- src/go/printer/nodes.go | 41 ++++++++++++-------- src/go/printer/testdata/alignment.golden | 42 ++++++++++++++++++++ src/go/printer/testdata/alignment.input | 49 ++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 16 deletions(-) diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go index 18f2371d24..1de7cd81b2 100644 --- a/src/go/printer/nodes.go +++ b/src/go/printer/nodes.go @@ -32,8 +32,10 @@ import ( // Print as many newlines as necessary (but at least min newlines) to get to // the current line. ws is printed before the first line break. If newSection -// is set, the first line break is printed as formfeed. Returns true if any -// line break was printed; returns false otherwise. +// is set, the first line break is printed as formfeed. Returns 0 if no line +// breaks were printed, returns 1 if there was exactly one newline printed, +// and returns a value > 1 if there was a formfeed or more than one newline +// printed. // // TODO(gri): linebreak may add too many lines if the next statement at "line" // is preceded by comments because the computation of n assumes @@ -43,7 +45,7 @@ import ( // linebreaks. At the moment there is no easy way to know about // future (not yet interspersed) comments in this function. // -func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (printedBreak bool) { +func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (nbreaks int) { n := nlimit(line - p.pos.Line) if n < min { n = min @@ -53,11 +55,12 @@ func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (prin if newSection { p.print(formfeed) n-- + nbreaks = 2 } + nbreaks += n for ; n > 0; n-- { p.print(newline) } - printedBreak = true } return } @@ -173,7 +176,7 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp // The first linebreak is always a formfeed since this section must not // depend on any previous formatting. prevBreak := -1 // index of last expression that was followed by a linebreak - if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) { + if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) > 0 { ws = ignore prevBreak = 0 } @@ -234,14 +237,6 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp useFF = r*ratio <= 1 || r <= ratio } } - if useFF { - lnsum = 0 - count = 0 - } - if size > 0 { - lnsum += math.Log(float64(size)) - count++ - } needsLinebreak := 0 < prevLine && prevLine < line if i > 0 { @@ -257,11 +252,20 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp // Lines are broken using newlines so comments remain aligned // unless useFF is set or there are multiple expressions on // the same line in which case formfeed is used. - if p.linebreak(line, 0, ws, useFF || prevBreak+1 < i) { + nbreaks := p.linebreak(line, 0, ws, useFF || prevBreak+1 < i) + if nbreaks > 0 { ws = ignore prevBreak = i needsBlank = false // we got a line break instead } + // If there was a new section or more than one new line + // (which means that the tabwriter will implicitly break + // the section), reset the geomean variables since we are + // starting a new group of elements with the next element. + if nbreaks > 1 { + lnsum = 0 + count = 0 + } } if needsBlank { p.print(blank) @@ -281,6 +285,11 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp p.expr0(x, depth) } + if size > 0 { + lnsum += math.Log(float64(size)) + count++ + } + prevLine = line } @@ -338,7 +347,7 @@ func (p *printer) parameters(fields *ast.FieldList) { p.print(token.COMMA) } // separator if needed (linebreak or blank) - if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) { + if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) > 0 { // break line if the opening "(" or previous parameter ended on a different line ws = ignore } else if i > 0 { @@ -709,7 +718,7 @@ func (p *printer) binaryExpr(x *ast.BinaryExpr, prec1, cutoff, depth int) { if xline != yline && xline > 0 && yline > 0 { // at least one line break, but respect an extra empty line // in the source - if p.linebreak(yline, 1, ws, true) { + if p.linebreak(yline, 1, ws, true) > 0 { ws = ignore printBlank = false // no blank after line break } diff --git a/src/go/printer/testdata/alignment.golden b/src/go/printer/testdata/alignment.golden index c65defe6ae..96086ed906 100644 --- a/src/go/printer/testdata/alignment.golden +++ b/src/go/printer/testdata/alignment.golden @@ -128,3 +128,45 @@ func main() { abcdefghijklmnopqrstuvwxyz: "foo", } } + +// ---------------------------------------------------------------------------- +// Examples from issue #26352. + +var _ = map[int]string{ + 1: "", + + 12345678901234567890123456789: "", + 12345678901234567890123456789012345678: "", +} + +func f() { + _ = map[int]string{ + 1: "", + + 12345678901234567: "", + 12345678901234567890123456789012345678901: "", + } +} + +// ---------------------------------------------------------------------------- +// Examples from issue #26930. + +var _ = S{ + F1: []string{}, + F2____: []string{}, +} + +var _ = S{ + F1: []string{}, + F2____: []string{}, +} + +var _ = S{ + F1____: []string{}, + F2: []string{}, +} + +var _ = S{ + F1____: []string{}, + F2: []string{}, +} diff --git a/src/go/printer/testdata/alignment.input b/src/go/printer/testdata/alignment.input index 9b0aae6bec..323d2689e0 100644 --- a/src/go/printer/testdata/alignment.input +++ b/src/go/printer/testdata/alignment.input @@ -128,3 +128,52 @@ func main() { abcdefghijklmnopqrstuvwxyz: "foo", } } + +// ---------------------------------------------------------------------------- +// Examples from issue #26352. + +var _ = map[int]string{ + 1: "", + + 12345678901234567890123456789: "", + 12345678901234567890123456789012345678: "", +} + +func f() { + _ = map[int]string{ + 1: "", + + 12345678901234567: "", + 12345678901234567890123456789012345678901: "", + } +} + +// ---------------------------------------------------------------------------- +// Examples from issue #26930. + +var _ = S{ + F1: []string{ + }, + F2____: []string{}, +} + +var _ = S{ + F1: []string{ + + + }, + F2____: []string{}, +} + +var _ = S{ + F1____: []string{ + }, + F2: []string{}, +} + +var _ = S{ + F1____: []string{ + + }, + F2: []string{}, +} From 6502c112863bda754ca3a52ead739f7d6c259117 Mon Sep 17 00:00:00 2001 From: Alberto Donizetti Date: Thu, 16 Aug 2018 16:55:32 +0200 Subject: [PATCH 05/31] cmd/go: fix typos in go help mod subpages mkalldocs.sh was run and it also picked up a doc change introduced in CL 128935, where it wasn't run. Fixes #27030 Change-Id: Ie13fdb71cd7d5481366a02eb711ca48f094026fd Reviewed-on: https://go-review.googlesource.com/129576 Reviewed-by: Brad Fitzpatrick --- src/cmd/go/alldocs.go | 7 ++++--- src/cmd/go/internal/list/list.go | 2 +- src/cmd/go/internal/modcmd/download.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 1292596697..70d655747c 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -722,7 +722,8 @@ // The -compiled flag causes list to set CompiledGoFiles to the Go source // files presented to the compiler. Typically this means that it repeats // the files listed in GoFiles and then also adds the Go code generated -// by processing CgoFiles and SwigFiles. +// by processing CgoFiles and SwigFiles. The Imports list contains the +// union of all imports from both GoFiles and CompiledGoFiles. // // The -deps flag causes list to iterate over not just the named packages // but also all their dependencies. It visits them in a depth-first post-order @@ -842,7 +843,7 @@ // module paths match the pattern. // A query of the form path@version specifies the result of that query, // which is not limited to active modules. -// See 'go help module' for more about module queries. +// See 'go help modules' for more about module queries. // // The template function "module" takes a single string argument // that must be a module path or query and returns the specified @@ -912,7 +913,7 @@ // Dir string // absolute path to cached source root directory // } // -// See 'go help module' for more about module queries. +// See 'go help modules' for more about module queries. // // // Edit go.mod from tools or scripts diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index e75270fa55..423516aad7 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -268,7 +268,7 @@ A pattern containing "..." specifies the active modules whose module paths match the pattern. A query of the form path@version specifies the result of that query, which is not limited to active modules. -See 'go help module' for more about module queries. +See 'go help modules' for more about module queries. The template function "module" takes a single string argument that must be a module path or query and returns the specified diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go index 0a457a56f2..2f072d73cf 100644 --- a/src/cmd/go/internal/modcmd/download.go +++ b/src/cmd/go/internal/modcmd/download.go @@ -41,7 +41,7 @@ corresponding to this Go struct: Dir string // absolute path to cached source root directory } -See 'go help module' for more about module queries. +See 'go help modules' for more about module queries. `, } From a68b713ef600a14e15b59f5bac424be73e1acaf5 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 15 Aug 2018 16:21:00 -0700 Subject: [PATCH 06/31] runtime: load errno as signed 32-bit the function libc_errno returns a pointer to a signed-32 bit quantity, not a 64-bit quantity. Fixes #27004 Change-Id: I0623835ee34fd9655532251f096022a5accb58cd Reviewed-on: https://go-review.googlesource.com/129475 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/runtime/sys_darwin_amd64.s | 4 ++-- src/runtime/sys_darwin_arm64.s | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/runtime/sys_darwin_amd64.s b/src/runtime/sys_darwin_amd64.s index 2a2e7379ca..db74352613 100644 --- a/src/runtime/sys_darwin_amd64.s +++ b/src/runtime/sys_darwin_amd64.s @@ -306,7 +306,7 @@ TEXT runtime·mmap_trampoline(SB),NOSPLIT,$0 CMPQ AX, $-1 JNE ok CALL libc_error(SB) - MOVQ (AX), DX // errno + MOVLQSX (AX), DX // errno XORL AX, AX ok: MOVQ AX, 32(BX) @@ -371,7 +371,7 @@ TEXT runtime·kevent_trampoline(SB),NOSPLIT,$0 CMPQ AX, $-1 JNE ok CALL libc_error(SB) - MOVQ (AX), AX // errno + MOVLQSX (AX), AX // errno NEGQ AX // caller wants it as a negative error code ok: POPQ BP diff --git a/src/runtime/sys_darwin_arm64.s b/src/runtime/sys_darwin_arm64.s index 4f9d0b8d58..d7ba116b84 100644 --- a/src/runtime/sys_darwin_arm64.s +++ b/src/runtime/sys_darwin_arm64.s @@ -70,7 +70,7 @@ TEXT runtime·mmap_trampoline(SB),NOSPLIT,$0 CMP R0, R2 BNE ok BL libc_error(SB) - MOVD (R0), R1 + MOVW (R0), R1 MOVD $0, R0 ok: MOVD R0, 32(R19) // ret 1 p @@ -277,7 +277,7 @@ TEXT runtime·kevent_trampoline(SB),NOSPLIT,$0 CMP R0, R2 BNE ok BL libc_error(SB) - MOVD (R0), R0 // errno + MOVW (R0), R0 // errno NEG R0, R0 // caller wants it as a negative error code ok: RET From dea36a6f751783c3c511e2eb0e1de3696daec1f7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 16 Aug 2018 09:54:11 -0700 Subject: [PATCH 07/31] cmd/go: disable TestAccidentalGitCheckout for now It seems it might not have ever worked. Updates #22983 Change-Id: Icc022539aa2555486a65900abf97dfa30f92a1ea Reviewed-on: https://go-review.googlesource.com/129615 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Andrew Bonventre --- src/cmd/go/go_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index debe4867e6..a7be617af9 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1293,6 +1293,7 @@ func TestGetGitDefaultBranch(t *testing.T) { func TestAccidentalGitCheckout(t *testing.T) { testenv.MustHaveExternalNetwork(t) + testenv.SkipFlaky(t, 22983) // this test might not have ever worked; see issue. if _, err := exec.LookPath("git"); err != nil { t.Skip("skipping because git binary not found") } From c265c893de73727b3a54511acf0e3f60593c1fa6 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 10 Aug 2018 20:22:21 -0400 Subject: [PATCH 08/31] cmd/go: ignore /tmp/go.mod Two different people have created /tmp/go.mod for experimentation and then had other tests that create fresh work directories below /tmp fail unexpectedly because the go command finds /tmp/go.mod. Refuse to use /tmp/go.mod. /tmp/anything/go.mod is fine. Fixes #26708. Change-Id: I2a4f61ea63099cff59fbf9e8798e5dcefefd5557 Reviewed-on: https://go-review.googlesource.com/129063 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/modload/init.go | 16 ++++++++++++++-- src/cmd/go/testdata/script/mod_enabled.txt | 9 +++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index 5e9db0f9ea..169bb5fdb6 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -150,8 +150,20 @@ func Init() { ModRoot = cwd } else { ModRoot, _ = FindModuleRoot(cwd, "", MustUseModules) - if ModRoot == "" && !MustUseModules { - return + if !MustUseModules { + if ModRoot == "" { + return + } + if search.InDir(ModRoot, os.TempDir()) == "." { + // If you create /tmp/go.mod for experimenting, + // then any tests that create work directories under /tmp + // will find it and get modules when they're not expecting them. + // It's a bit of a peculiar thing to disallow but quite mysterious + // when it happens. See golang.org/issue/26708. + ModRoot = "" + fmt.Fprintf(os.Stderr, "go: warning: ignoring go.mod in system temp root %v\n", os.TempDir()) + return + } } } diff --git a/src/cmd/go/testdata/script/mod_enabled.txt b/src/cmd/go/testdata/script/mod_enabled.txt index 4901b9c5e6..8eef870b02 100644 --- a/src/cmd/go/testdata/script/mod_enabled.txt +++ b/src/cmd/go/testdata/script/mod_enabled.txt @@ -65,6 +65,15 @@ cd $GOPATH/foo/bar/baz go env GOMOD ! stdout .+ +# GO111MODULE=auto should ignore and warn about /tmp/go.mod +env GO111MODULE=auto +cp $GOPATH/src/x/y/z/go.mod $WORK/tmp/go.mod +mkdir $WORK/tmp/mydir +cd $WORK/tmp/mydir +go env GOMOD +! stdout .+ +stderr '^go: warning: ignoring go.mod in system temp root ' + -- $GOPATH/src/x/y/z/go.mod -- module x/y/z -- $GOPATH/src/x/y/z/w/w.txt -- From 974d5364a643e107c4dea1df20293c67c32bfc04 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 10 Aug 2018 20:05:44 -0400 Subject: [PATCH 09/31] cmd/go: ignore import "C" files in module loader in non-cgo mode Obviously, including files that import "C" when cgo is disabled is wrong. The package load step correctly excludes them and finds no files at all, which then causes a failure. Fixes #26927. Change-Id: I00e6d6450e783d467d20bde99e91240ecb0db837 Reviewed-on: https://go-review.googlesource.com/129062 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills Reviewed-by: David du Colombier <0intro@gmail.com> --- src/cmd/go/internal/imports/scan.go | 7 +++++++ src/cmd/go/testdata/script/mod_patterns.txt | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/src/cmd/go/internal/imports/scan.go b/src/cmd/go/internal/imports/scan.go index 095bb64a8d..bae6b934bc 100644 --- a/src/cmd/go/internal/imports/scan.go +++ b/src/cmd/go/internal/imports/scan.go @@ -37,6 +37,7 @@ func scanFiles(files []string, tags map[string]bool, explicitFiles bool) ([]stri imports := make(map[string]bool) testImports := make(map[string]bool) numFiles := 0 +Files: for _, name := range files { r, err := os.Open(name) if err != nil { @@ -48,6 +49,12 @@ func scanFiles(files []string, tags map[string]bool, explicitFiles bool) ([]stri if err != nil { return nil, nil, fmt.Errorf("reading %s: %v", name, err) } + // import "C" is implicit requirement of cgo tag + for _, path := range list { + if path == `"C"` && !tags["cgo"] && !tags["*"] { + continue Files + } + } if !explicitFiles && !ShouldBuild(data, tags) { continue } diff --git a/src/cmd/go/testdata/script/mod_patterns.txt b/src/cmd/go/testdata/script/mod_patterns.txt index 36d738a867..a43fe82489 100644 --- a/src/cmd/go/testdata/script/mod_patterns.txt +++ b/src/cmd/go/testdata/script/mod_patterns.txt @@ -43,6 +43,10 @@ stdout example.com/m/useunsafe [cgo] stdout example.com/m/useC [!cgo] ! stdout example.com/m/useC +env CGO_ENABLED=0 +go list -f '{{.ImportPath}}: {{.Match}}' all ... example.com/m/... ./... ./xyz... +! stdout example.com/m/useC + -- m/go.mod -- module example.com/m From 751ea9369a9bd3b30daafced4d1c172541076617 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 10:54:20 -0400 Subject: [PATCH 10/31] cmd/go: document import "C" check from CL 129062 Added this locally but then broke the first rule of Gerrit and clicked Submit instead of running "git submit". Change-Id: I83c28d9151c566e9b2092e2613d67731a5d64beb Reviewed-on: https://go-review.googlesource.com/129678 Run-TryBot: Russ Cox Reviewed-by: Russ Cox --- src/cmd/go/internal/imports/scan.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/imports/scan.go b/src/cmd/go/internal/imports/scan.go index bae6b934bc..d944e95724 100644 --- a/src/cmd/go/internal/imports/scan.go +++ b/src/cmd/go/internal/imports/scan.go @@ -49,12 +49,19 @@ Files: if err != nil { return nil, nil, fmt.Errorf("reading %s: %v", name, err) } - // import "C" is implicit requirement of cgo tag + + // import "C" is implicit requirement of cgo tag. + // When listing files on the command line (explicitFiles=true) + // we do not apply build tag filtering but we still do apply + // cgo filtering, so no explicitFiles check here. + // Why? Because we always have, and it's not worth breaking + // that behavior now. for _, path := range list { if path == `"C"` && !tags["cgo"] && !tags["*"] { continue Files } } + if !explicitFiles && !ShouldBuild(data, tags) { continue } From 876c6d1f274a3d508fa97558ca6765cf324c2939 Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Wed, 15 Aug 2018 16:48:44 -0700 Subject: [PATCH 11/31] cmd/compile: make duplicate anonymous interface output deterministic Ranging through a map is non-deterministic and there can be duplicate entries in the set (with the same name) which don't have identical definitions in some cases. Fixes #27013 Change-Id: I378c48bc359c10b25b9238e0c663b498455b19fd Reviewed-on: https://go-review.googlesource.com/129515 Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/reflect.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index b9124b6317..935c3b0503 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -34,8 +34,9 @@ type ptabEntry struct { // runtime interface and reflection data structures var ( - signatsetmu sync.Mutex // protects signatset + signatmu sync.Mutex // protects signatset and signatslice signatset = make(map[*types.Type]struct{}) + signatslice []*types.Type itabs []itabEntry ptabs []ptabEntry @@ -960,9 +961,9 @@ func typesymprefix(prefix string, t *types.Type) *types.Sym { // This function is for looking up type-related generated functions // (e.g. eq and hash). Make sure they are indeed generated. - signatsetmu.Lock() + signatmu.Lock() addsignat(t) - signatsetmu.Unlock() + signatmu.Unlock() //print("algsym: %s -> %+S\n", p, s); @@ -974,9 +975,9 @@ func typenamesym(t *types.Type) *types.Sym { Fatalf("typenamesym %v", t) } s := typesym(t) - signatsetmu.Lock() + signatmu.Lock() addsignat(t) - signatsetmu.Unlock() + signatmu.Unlock() return s } @@ -1447,7 +1448,10 @@ func itabsym(it *obj.LSym, offset int64) *obj.LSym { // addsignat ensures that a runtime type descriptor is emitted for t. func addsignat(t *types.Type) { - signatset[t] = struct{}{} + if _, ok := signatset[t]; !ok { + signatset[t] = struct{}{} + signatslice = append(signatslice, t) + } } func addsignats(dcls []*Node) { @@ -1462,14 +1466,15 @@ func addsignats(dcls []*Node) { func dumpsignats() { // Process signatset. Use a loop, as dtypesym adds // entries to signatset while it is being processed. - signats := make([]typeAndStr, len(signatset)) - for len(signatset) > 0 { + signats := make([]typeAndStr, len(signatslice)) + for len(signatslice) > 0 { signats = signats[:0] // Transfer entries to a slice and sort, for reproducible builds. - for t := range signatset { + for _, t := range signatslice { signats = append(signats, typeAndStr{t: t, short: typesymname(t), regular: t.String()}) delete(signatset, t) } + signatslice = signatslice[:0] sort.Sort(typesByString(signats)) for _, ts := range signats { t := ts.t From 2482451f76f47707e29b19cafdcaf754badd024b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 16 Aug 2018 14:39:13 +0100 Subject: [PATCH 12/31] cmd/vet: don't suggest ... if it breaks a program MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is possible to write a function that seems to wrap a print/printf call, but then doesn't. For example, if the string parameter we thought was the format is used as another argument. One option would be to make vet's print analysis smarter, to detect when format strings are indeed used like we initially suspected. However, I've opted for a simpler solution - check if the print/printf call is already using more than one variadic argument, in which case using an ellipsis in the last one would break the program: // too many arguments in call to fmt.Printf fmt.Printf(format, arg0, args...) Fixes #26979. Change-Id: I39371f1cec8483cfd2770a91670c1e80cbb9efdf Reviewed-on: https://go-review.googlesource.com/129575 Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Reviewed-by: Russ Cox --- src/cmd/vet/print.go | 14 ++++++++++++++ src/cmd/vet/testdata/print.go | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go index 90fd4ed379..a55da1d3c8 100644 --- a/src/cmd/vet/print.go +++ b/src/cmd/vet/print.go @@ -259,6 +259,20 @@ func checkPrintfFwd(pkg *Package, w *printfWrapper, call *ast.CallExpr, kind int } if !call.Ellipsis.IsValid() { + typ, ok := pkg.types[call.Fun].Type.(*types.Signature) + if !ok { + return + } + if len(call.Args) > typ.Params().Len() { + // If we're passing more arguments than what the + // print/printf function can take, adding an ellipsis + // would break the program. For example: + // + // func foo(arg1 string, arg2 ...interface{} { + // fmt.Printf("%s %v", arg1, arg2) + // } + return + } if !vcfg.VetxOnly { desc := "printf" if kind == kindPrint { diff --git a/src/cmd/vet/testdata/print.go b/src/cmd/vet/testdata/print.go index 7c0cbcf05a..ecafed5fa2 100644 --- a/src/cmd/vet/testdata/print.go +++ b/src/cmd/vet/testdata/print.go @@ -446,6 +446,10 @@ func (*ptrStringer) BadWrapf(x int, format string, args ...interface{}) string { return fmt.Sprintf(format, args) // ERROR "missing ... in args forwarded to printf-like function" } +func (*ptrStringer) WrapfFalsePositive(x int, arg1 string, arg2 ...interface{}) string { + return fmt.Sprintf("%s %v", arg1, arg2) +} + type embeddedStringer struct { foo string ptrStringer From 3e0f5f934eb474196297fddf957157422f93a2d7 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 11:07:30 -0400 Subject: [PATCH 13/31] doc/code: drop mentions of GOPATH/pkg directory It's already half gone and later will be all gone. It's not worth explaining in an introduction doc. Fixes #24506 Updates #4719 Change-Id: Ie48128b3aa090d84e0e734aa45f14a4480292913 Reviewed-on: https://go-review.googlesource.com/129679 Reviewed-by: Brad Fitzpatrick --- doc/code.html | 51 ++++++--------------------------------------------- 1 file changed, 6 insertions(+), 45 deletions(-) diff --git a/doc/code.html b/doc/code.html index 4e8c54a1c5..b6d41ef68c 100644 --- a/doc/code.html +++ b/doc/code.html @@ -44,18 +44,16 @@ control repositories.

Workspaces

-A workspace is a directory hierarchy with three directories at its root: +A workspace is a directory hierarchy with two directories at its root:

    -
  • src contains Go source files, -
  • pkg contains package objects, and +
  • src contains Go source files, and
  • bin contains executable commands.

-The go tool builds source packages and installs the resulting -binaries to the pkg and bin directories. +The go tool builds and installs binaries to the bin directory.

@@ -72,10 +70,6 @@ To give you an idea of how a workspace looks in practice, here's an example: bin/ hello # command executable outyet # command executable -pkg/ - linux_amd64/ - github.com/golang/example/ - stringutil.a # package object src/ github.com/golang/example/ .git/ # Git repository metadata @@ -374,9 +368,8 @@ $ go build

-This won't produce an output file. To do that, you must use go -install, which places the package object inside the pkg -directory of the workspace. +This won't produce an output file. +Instead it saves the compiled package in the local build cache.

@@ -400,19 +393,13 @@ func main() {

-Whenever the go tool installs a package or binary, it also -installs whatever dependencies it has. -So when you install the hello program +Install the hello program:

 $ go install github.com/user/hello
 
-

-the stringutil package will be installed as well, automatically. -

-

Running the new version of the program, you should see a new, reversed message:

@@ -429,10 +416,6 @@ After the steps above, your workspace should look like this:
 bin/
     hello                 # command executable
-pkg/
-    linux_amd64/          # this will reflect your OS and architecture
-        github.com/user/
-            stringutil.a  # package object
 src/
     github.com/user/
         hello/
@@ -441,22 +424,6 @@ src/
             reverse.go    # package source
 
-

-Note that go install placed the stringutil.a object -in a directory inside pkg/linux_amd64 that mirrors its source -directory. -This is so that future invocations of the go tool can find the -package object and avoid recompiling the package unnecessarily. -The linux_amd64 part is there to aid in cross-compilation, -and will reflect the operating system and architecture of your system. -

- -

-Go command executables are statically linked; the package objects need not -be present to run Go programs. -

- -

Package names

@@ -597,12 +564,6 @@ tree should now look like this:

 bin/
     hello                           # command executable
-pkg/
-    linux_amd64/
-        github.com/golang/example/
-            stringutil.a            # package object
-        github.com/user/
-            stringutil.a            # package object
 src/
     github.com/golang/example/
 	.git/                       # Git repository metadata

From c81d216d844f165b729fa3dbb0c3d834eb4268a8 Mon Sep 17 00:00:00 2001
From: Russ Cox 
Date: Fri, 17 Aug 2018 10:51:53 -0400
Subject: [PATCH 14/31] go/doc: allow interior dot in heading, as in "go.mod"

Only the expected headings are affected.
Diffing the output of "go run headscan.go" before and after:

$ diff head1 head2
26a27,28
> 	Edit go.mod from tools or scripts
> 	Make go.mod semantically consistent
168c170
< 141 headings found
---
> 143 headings found
$

Fixes #26938.

Change-Id: I204fd982a60773aa26880cd19eed890c373b8ab6
Reviewed-on: https://go-review.googlesource.com/129677
Run-TryBot: Russ Cox 
TryBot-Result: Gobot Gobot 
Reviewed-by: Ian Lance Taylor 
---
 src/go/doc/comment.go | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/go/doc/comment.go b/src/go/doc/comment.go
index 7c4490e7c3..d068d8960c 100644
--- a/src/go/doc/comment.go
+++ b/src/go/doc/comment.go
@@ -232,7 +232,7 @@ func heading(line string) string {
 	}
 
 	// exclude lines with illegal characters. we allow "(),"
-	if strings.ContainsAny(line, ".;:!?+*/=[]{}_^°&§~%#@<\">\\") {
+	if strings.ContainsAny(line, ";:!?+*/=[]{}_^°&§~%#@<\">\\") {
 		return ""
 	}
 
@@ -248,6 +248,18 @@ func heading(line string) string {
 		b = b[i+2:]
 	}
 
+	// allow "." when followed by non-space
+	for b := line;; {
+		i := strings.IndexRune(b, '.')
+		if i < 0 {
+			break
+		}
+		if i+1 >= len(b) || b[i+1] == ' ' {
+			return "" // not followed by non-space
+		}
+		b = b[i+1:]
+	}
+
 	return line
 }
 

From 08d10f9af1429d19633722e36f7bfeda6c000aa5 Mon Sep 17 00:00:00 2001
From: Alan Donovan 
Date: Thu, 16 Aug 2018 16:31:45 -0400
Subject: [PATCH 15/31] doc: describe golang.org/x/go/packages in go1.11
 release notes

Also, rename an HTML element ID to avoid duplicate.

Fixes golang/go#27038

Change-Id: Icc064a1cc86ddc794fc085d98b4cde3effff8ad0
Reviewed-on: https://go-review.googlesource.com/129635
Reviewed-by: Andrew Bonventre 
Reviewed-by: Ian Cottrell 
---
 doc/go1.11.html | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/go1.11.html b/doc/go1.11.html
index a1249db475..89649f34c0 100644
--- a/doc/go1.11.html
+++ b/doc/go1.11.html
@@ -167,7 +167,18 @@ Do not send CLs removing the interior tags from such phrases.
 

Package loading

- TODO: Note about go/build versus golang.org/x/tools/go/packages. + The new package + golang.org/x/tools/go/packages + provides a simple API for locating and loading packages of Go source code. + Although not yet part of the standard library, for many tasks it + effectively replaces the go/build + package, whose API is unable to fully support modules. + Because it runs an external query command such as + go list + to obtain information about Go packages, it enables the construction of + analysis tools that work equally well with alternative build systems + such as Bazel + and Buck.

Build cache requirement

@@ -793,7 +804,7 @@ for k := range m { -
runtime
+
runtime

From d46587c4eaaf64a7e9cb0797fcfa238f5138b170 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 10 Aug 2018 13:26:32 -0400 Subject: [PATCH 16/31] cmd/go: distinguish patterns from the results of matching them To date the go command has always just treated the command line package patterns as a []string, expanded by pattern matching into another []string. As a result, the code is not always clear about whether a particular []string contains patterns or results. A few different important bugs are caused by not keeping this distinction clear enough. This CL sets us up well for fixing those, by introducing an explicit search.Match struct holding the results of matching a single pattern. The added clarity here also makes it clear how to avoid duplicate warnings about unmatched packages. Fixes #26925. (Test in followup CL.) Change-Id: Ic2f0606f7ab8b3734a40e22d3cb1e6f58d031061 Reviewed-on: https://go-review.googlesource.com/129058 Run-TryBot: Russ Cox Reviewed-by: Alan Donovan --- src/cmd/go/internal/get/get.go | 45 +++----- src/cmd/go/internal/load/pkg.go | 45 +++----- src/cmd/go/internal/modcmd/why.go | 24 ++-- src/cmd/go/internal/modget/get.go | 13 ++- src/cmd/go/internal/modload/load.go | 163 +++++++++++++++++---------- src/cmd/go/internal/search/search.go | 119 ++++++++++--------- src/cmd/go/internal/work/build.go | 10 +- 7 files changed, 217 insertions(+), 202 deletions(-) diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go index fd97c6dcb6..47953f09a4 100644 --- a/src/cmd/go/internal/get/get.go +++ b/src/cmd/go/internal/get/get.go @@ -163,9 +163,8 @@ func runGet(cmd *base.Command, args []string) { if *getT { mode |= load.GetTestDeps } - args = downloadPaths(args) - for _, arg := range args { - download(arg, nil, &stk, mode) + for _, pkg := range downloadPaths(args) { + download(pkg, nil, &stk, mode) } base.ExitIfErrors() @@ -184,8 +183,7 @@ func runGet(cmd *base.Command, args []string) { // This leads to duplicated loads of the standard packages. load.ClearCmdCache() - args = load.ImportPaths(args) - load.PackagesForBuild(args) + pkgs := load.PackagesForBuild(args) // Phase 3. Install. if *getD { @@ -195,7 +193,7 @@ func runGet(cmd *base.Command, args []string) { return } - work.InstallPackages(args) + work.InstallPackages(args, pkgs) } // downloadPaths prepares the list of paths to pass to download. @@ -203,34 +201,21 @@ func runGet(cmd *base.Command, args []string) { // for a particular pattern, downloadPaths leaves it in the result list, // in the hope that we can figure out the repository from the // initial ...-free prefix. -func downloadPaths(args []string) []string { - for _, arg := range args { +func downloadPaths(patterns []string) []string { + for _, arg := range patterns { if strings.Contains(arg, "@") { base.Fatalf("go: cannot use path@version syntax in GOPATH mode") } } - - args = load.ImportPathsForGoGet(args) - var out []string - for _, a := range args { - if strings.Contains(a, "...") { - var expand []string - // Use matchPackagesInFS to avoid printing - // warnings. They will be printed by the - // eventual call to importPaths instead. - if build.IsLocalImport(a) { - expand = search.MatchPackagesInFS(a) - } else { - expand = search.MatchPackages(a) - } - if len(expand) > 0 { - out = append(out, expand...) - continue - } + var pkgs []string + for _, m := range search.ImportPathsQuiet(patterns) { + if len(m.Pkgs) == 0 && strings.Contains(m.Pattern, "...") { + pkgs = append(pkgs, m.Pattern) + } else { + pkgs = append(pkgs, m.Pkgs...) } - out = append(out, a) } - return out + return pkgs } // downloadCache records the import paths we have already @@ -311,9 +296,9 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int) // for p has been replaced in the package cache. if wildcardOkay && strings.Contains(arg, "...") { if build.IsLocalImport(arg) { - args = search.MatchPackagesInFS(arg) + args = search.MatchPackagesInFS(arg).Pkgs } else { - args = search.MatchPackages(arg) + args = search.MatchPackages(arg).Pkgs } isWildcard = true } diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index bef27b33ad..3a3a38651c 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -35,7 +35,7 @@ var ( ModBinDir func() string // return effective bin directory ModLookup func(path string) (dir, realPath string, err error) // lookup effective meaning of import ModPackageModuleInfo func(path string) *modinfo.ModulePublic // return module info for Package struct - ModImportPaths func(args []string) []string // expand import paths + ModImportPaths func(args []string) []*search.Match // expand import paths ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary ModInfoProg func(info string) []byte // wrap module info in .go code for binary ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files @@ -1829,54 +1829,41 @@ func Packages(args []string) []*Package { // *Package for every argument, even the ones that // cannot be loaded at all. // The packages that fail to load will have p.Error != nil. -func PackagesAndErrors(args []string) []*Package { - if len(args) > 0 && strings.HasSuffix(args[0], ".go") { - return []*Package{GoFilesPackage(args)} +func PackagesAndErrors(patterns []string) []*Package { + if len(patterns) > 0 && strings.HasSuffix(patterns[0], ".go") { + return []*Package{GoFilesPackage(patterns)} } - args = ImportPaths(args) + matches := ImportPaths(patterns) var ( pkgs []*Package stk ImportStack - seenArg = make(map[string]bool) seenPkg = make(map[*Package]bool) ) - for _, arg := range args { - if seenArg[arg] { - continue + for _, m := range matches { + for _, pkg := range m.Pkgs { + p := LoadPackage(pkg, &stk) + if seenPkg[p] { + continue + } + seenPkg[p] = true + pkgs = append(pkgs, p) } - seenArg[arg] = true - pkg := LoadPackage(arg, &stk) - if seenPkg[pkg] { - continue - } - seenPkg[pkg] = true - pkgs = append(pkgs, pkg) } return pkgs } -func ImportPaths(args []string) []string { - if cmdlineMatchers == nil { - SetCmdlinePatterns(search.CleanImportPaths(args)) - } +func ImportPaths(args []string) []*search.Match { if ModInit(); cfg.ModulesEnabled { return ModImportPaths(args) } return search.ImportPaths(args) } -func ImportPathsForGoGet(args []string) []string { - if cmdlineMatchers == nil { - SetCmdlinePatterns(search.CleanImportPaths(args)) - } - return search.ImportPathsNoDotExpansion(args) -} - -// packagesForBuild is like 'packages' but fails if any of -// the packages or their dependencies have errors +// PackagesForBuild is like Packages but exits +// if any of the packages or their dependencies have errors // (cannot be built). func PackagesForBuild(args []string) []*Package { pkgs := PackagesAndErrors(args) diff --git a/src/cmd/go/internal/modcmd/why.go b/src/cmd/go/internal/modcmd/why.go index 6923685599..03e0a039bc 100644 --- a/src/cmd/go/internal/modcmd/why.go +++ b/src/cmd/go/internal/modcmd/why.go @@ -100,20 +100,22 @@ func runWhy(cmd *base.Command, args []string) { sep = "\n" } } else { - pkgs := modload.ImportPaths(args) // resolve to packages - loadALL() // rebuild graph, from main module (not from named packages) + matches := modload.ImportPaths(args) // resolve to packages + loadALL() // rebuild graph, from main module (not from named packages) sep := "" - for _, path := range pkgs { - why := modload.Why(path) - if why == "" { - vendoring := "" - if *whyVendor { - vendoring = " to vendor" + for _, m := range matches { + for _, path := range m.Pkgs { + why := modload.Why(path) + if why == "" { + vendoring := "" + if *whyVendor { + vendoring = " to vendor" + } + why = "(main module does not need" + vendoring + " package " + path + ")\n" } - why = "(main module does not need" + vendoring + " package " + path + ")\n" + fmt.Printf("%s# %s\n%s", sep, path, why) + sep = "\n" } - fmt.Printf("%s# %s\n%s", sep, path, why) - sep = "\n" } } } diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go index f4a92686a5..90a5bd8130 100644 --- a/src/cmd/go/internal/modget/get.go +++ b/src/cmd/go/internal/modget/get.go @@ -229,7 +229,7 @@ func runGet(cmd *base.Command, args []string) { // and a list of install targets (for the "go install" at the end). var tasks []*task var install []string - for _, arg := range search.CleanImportPaths(args) { + for _, arg := range search.CleanPatterns(args) { // Argument is module query path@vers, or else path with implicit @latest. path := arg vers := "" @@ -519,8 +519,9 @@ func runGet(cmd *base.Command, args []string) { // Note that 'go get -u' without any arguments results in len(install) == 1: // search.CleanImportPaths returns "." for empty args. work.BuildInit() - var pkgs []string - for _, p := range load.PackagesAndErrors(install) { + pkgs := load.PackagesAndErrors(install) + var todo []*load.Package + for _, p := range pkgs { // Ignore "no Go source files" errors for 'go get' operations on modules. if p.Error != nil { if len(args) == 0 && getU != "" && strings.HasPrefix(p.Error.Err, "no Go files") { @@ -534,14 +535,14 @@ func runGet(cmd *base.Command, args []string) { continue } } - pkgs = append(pkgs, p.ImportPath) + todo = append(todo, p) } // If -d was specified, we're done after the download: no build. // (The load.PackagesAndErrors is what did the download // of the named packages and their dependencies.) - if len(pkgs) > 0 && !*getD { - work.InstallPackages(pkgs) + if len(todo) > 0 && !*getD { + work.InstallPackages(install, todo) } } } diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 389c643db2..e408e478d3 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -27,6 +27,7 @@ import ( "cmd/go/internal/par" "cmd/go/internal/search" "cmd/go/internal/semver" + "cmd/go/internal/str" ) // buildList is the list of modules to use for building packages. @@ -50,24 +51,46 @@ var loaded *loader // ImportPaths returns the set of packages matching the args (patterns), // adding modules to the build list as needed to satisfy new imports. -func ImportPaths(args []string) []string { +func ImportPaths(patterns []string) []*search.Match { InitMod() - cleaned := search.CleanImportPaths(args) + var matches []*search.Match + for _, pattern := range search.CleanPatterns(patterns) { + m := &search.Match{ + Pattern: pattern, + Literal: !strings.Contains(pattern, "...") && !search.IsMetaPackage(pattern), + } + if m.Literal { + m.Pkgs = []string{pattern} + } + matches = append(matches, m) + } + + fsDirs := make([][]string, len(matches)) loaded = newLoader() - var paths []string - loaded.load(func() []string { - var roots []string - paths = nil - for _, pkg := range cleaned { + updateMatches := func(iterating bool) { + for i, m := range matches { switch { - case build.IsLocalImport(pkg) || filepath.IsAbs(pkg): - list := []string{pkg} - if strings.Contains(pkg, "...") { - // TODO: Where is the go.mod cutoff? - list = warnPattern(pkg, search.AllPackagesInFS(pkg)) + case build.IsLocalImport(m.Pattern) || filepath.IsAbs(m.Pattern): + // Evaluate list of file system directories on first iteration. + if fsDirs[i] == nil { + var dirs []string + if m.Literal { + dirs = []string{m.Pattern} + } else { + dirs = search.MatchPackagesInFS(m.Pattern).Pkgs + } + fsDirs[i] = dirs } - for _, pkg := range list { + + // Make a copy of the directory list and translate to import paths. + // Note that whether a directory corresponds to an import path + // changes as the build list is updated, and a directory can change + // from not being in the build list to being in it and back as + // the exact version of a particular module increases during + // the loader iterations. + m.Pkgs = str.StringList(fsDirs[i]) + for i, pkg := range m.Pkgs { dir := pkg if !filepath.IsAbs(dir) { dir = filepath.Join(cwd, pkg) @@ -93,38 +116,53 @@ func ImportPaths(args []string) []string { } else if path := pathInModuleCache(dir); path != "" { pkg = path } else { - base.Errorf("go: directory %s outside available modules", base.ShortPath(dir)) - continue + if !iterating { + base.Errorf("go: directory %s outside available modules", base.ShortPath(dir)) + } + pkg = "" } - roots = append(roots, pkg) - paths = append(paths, pkg) + m.Pkgs[i] = pkg } - case pkg == "all": + case strings.Contains(m.Pattern, "..."): + m.Pkgs = matchPackages(m.Pattern, loaded.tags, true, buildList) + + case m.Pattern == "all": loaded.testAll = true - // TODO: Don't print warnings multiple times. - roots = append(roots, warnPattern("all", matchPackages("...", loaded.tags, false, []module.Version{Target}))...) - paths = append(paths, "all") // will expand after load completes + if iterating { + // Enumerate the packages in the main module. + // We'll load the dependencies as we find them. + m.Pkgs = matchPackages("...", loaded.tags, false, []module.Version{Target}) + } else { + // Starting with the packages in the main module, + // enumerate the full list of "all". + m.Pkgs = loaded.computePatternAll(m.Pkgs) + } - case search.IsMetaPackage(pkg): // std, cmd - list := search.AllPackages(pkg) - roots = append(roots, list...) - paths = append(paths, list...) + case search.IsMetaPackage(m.Pattern): // std, cmd + if len(m.Pkgs) == 0 { + m.Pkgs = search.MatchPackages(m.Pattern).Pkgs + } + } + } + } - case strings.Contains(pkg, "..."): - // TODO: Don't we need to reevaluate this one last time once the build list stops changing? - list := warnPattern(pkg, matchPackages(pkg, loaded.tags, true, buildList)) - roots = append(roots, list...) - paths = append(paths, list...) - - default: - roots = append(roots, pkg) - paths = append(paths, pkg) + loaded.load(func() []string { + var roots []string + updateMatches(true) + for _, m := range matches { + for _, pkg := range m.Pkgs { + if pkg != "" { + roots = append(roots, pkg) + } } } return roots }) + // One last pass to finalize wildcards. + updateMatches(false) + // A given module path may be used as itself or as a replacement for another // module, but not both at the same time. Otherwise, the aliasing behavior is // too subtle (see https://golang.org/issue/26607), and we don't want to @@ -142,33 +180,10 @@ func ImportPaths(args []string) []string { } } base.ExitIfErrors() - WriteGoMod() - // Process paths to produce final paths list. - // Remove duplicates and expand "all". - have := make(map[string]bool) - var final []string - for _, path := range paths { - if have[path] { - continue - } - have[path] = true - if path == "all" { - for _, pkg := range loaded.pkgs { - if e, ok := pkg.err.(*ImportMissingError); ok && e.Module.Path == "" { - continue // Package doesn't actually exist, so don't report it. - } - if !have[pkg.path] { - have[pkg.path] = true - final = append(final, pkg.path) - } - } - continue - } - final = append(final, path) - } - return final + search.WarnUnmatched(matches) + return matches } // pathInModuleCache returns the import path of the directory dir, @@ -581,6 +596,36 @@ func (ld *loader) doPkg(item interface{}) { } } +// computePatternAll returns the list of packages matching pattern "all", +// starting with a list of the import paths for the packages in the main module. +func (ld *loader) computePatternAll(paths []string) []string { + seen := make(map[*loadPkg]bool) + var all []string + var walk func(*loadPkg) + walk = func(pkg *loadPkg) { + if seen[pkg] { + return + } + seen[pkg] = true + if pkg.testOf == nil { + all = append(all, pkg.path) + } + for _, p := range pkg.imports { + walk(p) + } + if p := pkg.test; p != nil { + walk(p) + } + } + for _, path := range paths { + walk(ld.pkg(path, false)) + } + sort.Strings(all) + + fmt.Fprintf(os.Stderr, "ALL %v -> %v\n", paths, all) + return all +} + // scanDir is like imports.ScanDir but elides known magic imports from the list, // so that we do not go looking for packages that don't really exist. // diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go index b020f600c1..60ae73696b 100644 --- a/src/cmd/go/internal/search/search.go +++ b/src/cmd/go/internal/search/search.go @@ -17,32 +17,22 @@ import ( "strings" ) -// AllPackages returns all the packages that can be found +// A Match represents the result of matching a single package pattern. +type Match struct { + Pattern string // the pattern itself + Literal bool // whether it is a literal (no wildcards) + Pkgs []string // matching packages (dirs or import paths) +} + +// MatchPackages returns all the packages that can be found // under the $GOPATH directories and $GOROOT matching pattern. // The pattern is either "all" (all packages), "std" (standard packages), // "cmd" (standard commands), or a path including "...". -func AllPackages(pattern string) []string { - pkgs := MatchPackages(pattern) - if len(pkgs) == 0 { - fmt.Fprintf(os.Stderr, "warning: %q matched no packages\n", pattern) +func MatchPackages(pattern string) *Match { + m := &Match{ + Pattern: pattern, + Literal: false, } - return pkgs -} - -// AllPackagesInFS is like allPackages but is passed a pattern -// beginning ./ or ../, meaning it should scan the tree rooted -// at the given directory. There are ... in the pattern too. -func AllPackagesInFS(pattern string) []string { - pkgs := MatchPackagesInFS(pattern) - if len(pkgs) == 0 { - fmt.Fprintf(os.Stderr, "warning: %q matched no packages\n", pattern) - } - return pkgs -} - -// MatchPackages returns a list of package paths matching pattern -// (see go help packages for pattern syntax). -func MatchPackages(pattern string) []string { match := func(string) bool { return true } treeCanMatch := func(string) bool { return true } if !IsMetaPackage(pattern) { @@ -56,7 +46,6 @@ func MatchPackages(pattern string) []string { if !cfg.BuildContext.CgoEnabled { have["runtime/cgo"] = true // ignore during walk } - var pkgs []string for _, src := range cfg.BuildContext.SrcDirs() { if (pattern == "std" || pattern == "cmd") && src != cfg.GOROOTsrc { @@ -123,11 +112,11 @@ func MatchPackages(pattern string) []string { return nil } - pkgs = append(pkgs, name) + m.Pkgs = append(m.Pkgs, name) return nil }) } - return pkgs + return m } var modRoot string @@ -136,10 +125,16 @@ func SetModRoot(dir string) { modRoot = dir } -// MatchPackagesInFS returns a list of package paths matching pattern, -// which must begin with ./ or ../ -// (see go help packages for pattern syntax). -func MatchPackagesInFS(pattern string) []string { +// MatchPackagesInFS is like allPackages but is passed a pattern +// beginning ./ or ../, meaning it should scan the tree rooted +// at the given directory. There are ... in the pattern too. +// (See go help packages for pattern syntax.) +func MatchPackagesInFS(pattern string) *Match { + m := &Match{ + Pattern: pattern, + Literal: false, + } + // Find directory to begin the scan. // Could be smarter but this one optimization // is enough for now, since ... is usually at the @@ -168,7 +163,6 @@ func MatchPackagesInFS(pattern string) []string { } } - var pkgs []string filepath.Walk(dir, func(path string, fi os.FileInfo, err error) error { if err != nil || !fi.IsDir() { return nil @@ -218,10 +212,10 @@ func MatchPackagesInFS(pattern string) []string { } return nil } - pkgs = append(pkgs, name) + m.Pkgs = append(m.Pkgs, name) return nil }) - return pkgs + return m } // TreeCanMatchPattern(pattern)(name) reports whether @@ -308,36 +302,53 @@ func replaceVendor(x, repl string) string { return strings.Join(elem, "/") } -// ImportPaths returns the import paths to use for the given command line. -func ImportPaths(args []string) []string { - args = CleanImportPaths(args) - var out []string - for _, a := range args { +// WarnUnmatched warns about patterns that didn't match any packages. +func WarnUnmatched(matches []*Match) { + for _, m := range matches { + if len(m.Pkgs) == 0 { + fmt.Fprintf(os.Stderr, "go: warning: %q matched no packages\n", m.Pattern) + } + } +} + +// ImportPaths returns the matching paths to use for the given command line. +// It calls ImportPathsQuiet and then WarnUnmatched. +func ImportPaths(patterns []string) []*Match { + matches := ImportPathsQuiet(patterns) + WarnUnmatched(matches) + return matches +} + +// ImportPathsQuiet is like ImportPaths but does not warn about patterns with no matches. +func ImportPathsQuiet(patterns []string) []*Match { + var out []*Match + for _, a := range CleanPatterns(patterns) { if IsMetaPackage(a) { - out = append(out, AllPackages(a)...) + out = append(out, MatchPackages(a)) continue } if strings.Contains(a, "...") { if build.IsLocalImport(a) { - out = append(out, AllPackagesInFS(a)...) + out = append(out, MatchPackagesInFS(a)) } else { - out = append(out, AllPackages(a)...) + out = append(out, MatchPackages(a)) } continue } - out = append(out, a) + out = append(out, &Match{Pattern: a, Literal: true, Pkgs: []string{a}}) } return out } -// CleanImportPaths returns the import paths to use for the given -// command line, but it does no wildcard expansion. -func CleanImportPaths(args []string) []string { - if len(args) == 0 { +// CleanPatterns returns the patterns to use for the given +// command line. It canonicalizes the patterns but does not +// evaluate any matches. +func CleanPatterns(patterns []string) []string { + if len(patterns) == 0 { return []string{"."} } var out []string - for _, a := range args { + for _, a := range patterns { // Arguments are supposed to be import paths, but // as a courtesy to Windows developers, rewrite \ to / // in command-line arguments. Handles .\... and so on. @@ -359,22 +370,6 @@ func CleanImportPaths(args []string) []string { return out } -// ImportPathsNoDotExpansion returns the import paths to use for the given -// command line, but it does no ... expansion. -// TODO(rsc): Delete once old go get is gone. -func ImportPathsNoDotExpansion(args []string) []string { - args = CleanImportPaths(args) - var out []string - for _, a := range args { - if IsMetaPackage(a) { - out = append(out, AllPackages(a)...) - continue - } - out = append(out, a) - } - return out -} - // IsMetaPackage checks if name is a reserved package name that expands to multiple packages. func IsMetaPackage(name string) bool { return name == "std" || name == "cmd" || name == "all" diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index 891f81e116..ed41ce5d07 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -414,7 +414,7 @@ func libname(args []string, pkgs []*load.Package) (string, error) { func runInstall(cmd *base.Command, args []string) { BuildInit() - InstallPackages(args) + InstallPackages(args, load.PackagesForBuild(args)) } // omitTestOnly returns pkgs with test-only packages removed. @@ -434,12 +434,12 @@ func omitTestOnly(pkgs []*load.Package) []*load.Package { return list } -func InstallPackages(args []string) { +func InstallPackages(patterns []string, pkgs []*load.Package) { if cfg.GOBIN != "" && !filepath.IsAbs(cfg.GOBIN) { base.Fatalf("cannot install, GOBIN must be an absolute path") } - pkgs := omitTestOnly(pkgsFilter(load.PackagesForBuild(args))) + pkgs = omitTestOnly(pkgsFilter(pkgs)) for _, p := range pkgs { if p.Target == "" { switch { @@ -500,7 +500,7 @@ func InstallPackages(args []string) { // tools above did not apply, and a is just a simple Action // with a list of Deps, one per package named in pkgs, // the same as in runBuild. - a = b.buildmodeShared(ModeInstall, ModeInstall, args, pkgs, a) + a = b.buildmodeShared(ModeInstall, ModeInstall, patterns, pkgs, a) } b.Do(a) @@ -515,7 +515,7 @@ func InstallPackages(args []string) { // One way to view this behavior is that it is as if 'go install' first // runs 'go build' and the moves the generated file to the install dir. // See issue 9645. - if len(args) == 0 && len(pkgs) == 1 && pkgs[0].Name == "main" { + if len(patterns) == 0 && len(pkgs) == 1 && pkgs[0].Name == "main" { // Compute file 'go build' would have created. // If it exists and is an executable file, remove it. _, targ := filepath.Split(pkgs[0].ImportPath) From 2ce6da0be30c1888120a7f7e2a596c6de1892c0a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 10 Aug 2018 00:01:48 -0400 Subject: [PATCH 17/31] cmd/go: fix -gcflags, -ldflags not applying to current directory A flag setting like -gcflags=-e applies only to the packages named on the command line, not to their dependencies. The way we used to implement this was to remember the command line arguments, reinterpret them as pattern matches instead of package argument generators (globs), and apply them during package load. The reason for this complexity was to address a command-line like: go build -gcflags=-e fmt runtime The load of fmt will load dependencies, including runtime, and the load of runtime will reuse the result of the earlier load. Because we were computing the effective -gcflags for each package during the load, we had to have a way to tell, when encountering runtime during the load of fmt, that runtime had been named on the command line, even though we hadn't gotten that far. That would be easy if the only possible arguments were import paths, but we also need to handle go build -gcflags=-e fmt runt... go build -gcflags=-e fmt $GOROOT/src/runtime go build -gcflags=-e fmt $GOROOT/src/runt... and so on. The match predicates usually did their job well, but not always. In particular, thanks to symlinks and case-insensitive file systems and unusual ways to spell file paths, it's always been possible in various corner cases to give an argument that evalutes to the runtime package during loading but failed to match it when reused to determine "was this package named on the command line?" CL 109235 fixed one instance of this problem by making a directory pattern match case-insensitive on Windows, but that is incorrect in some other cases and doesn't address the root problem, namely that there will probably always be odd corner cases where pattern matching and pattern globbing are not exactly aligned. This CL eliminates the assumption that pattern matching and pattern globbing are always completely in agreement, by simply marking the packages named on the command line after the package load returns them. This means delaying the computation of tool flags until after the load too, for a few different ways packages are loaded. The different load entry points add some complexity, which is why the original approach seemed more attractive, but the original approach had complexity that we simply didn't recognize at the time. This CL then rolls back the CL 109235 pattern-matching change, but it keeps the test introduced in that CL. That test still passes. In addition to fixing ambiguity due to case-sensitive file systems, this new approach also very likely fixes various ambiguities that might arise from abuse of symbolic links. Fixes #24232. Fixes #24456. Fixes #24750. Fixes #25046. Fixes #25878. Change-Id: I0b09825785dfb5112fb11494cff8527ebf57966f Reviewed-on: https://go-review.googlesource.com/129059 Run-TryBot: Russ Cox Reviewed-by: Alan Donovan --- src/cmd/go/go_test.go | 31 ------- src/cmd/go/internal/get/get.go | 4 +- src/cmd/go/internal/load/flag.go | 50 ------------ src/cmd/go/internal/load/pkg.go | 80 +++++++++++++------ src/cmd/go/internal/load/search.go | 9 +-- .../go/testdata/script/gcflags_patterns.txt | 71 ++++++++++++++++ 6 files changed, 128 insertions(+), 117 deletions(-) create mode 100644 src/cmd/go/testdata/script/gcflags_patterns.txt diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index a7be617af9..3ca50bb475 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -5648,37 +5648,6 @@ func TestRelativePkgdir(t *testing.T) { tg.run("build", "-i", "-pkgdir=.", "runtime") } -func TestGcflagsPatterns(t *testing.T) { - skipIfGccgo(t, "gccgo has no standard packages") - tg := testgo(t) - defer tg.cleanup() - tg.setenv("GOPATH", "") - tg.setenv("GOCACHE", "off") - - tg.run("build", "-n", "-v", "-gcflags= \t\r\n -e", "fmt") - tg.grepStderr("^# fmt", "did not rebuild fmt") - tg.grepStderrNot("^# reflect", "incorrectly rebuilt reflect") - - tg.run("build", "-n", "-v", "-gcflags=-e", "fmt", "reflect") - tg.grepStderr("^# fmt", "did not rebuild fmt") - tg.grepStderr("^# reflect", "did not rebuild reflect") - tg.grepStderrNot("^# runtime", "incorrectly rebuilt runtime") - - tg.run("build", "-n", "-x", "-v", "-gcflags= \t\r\n reflect \t\r\n = \t\r\n -N", "fmt") - tg.grepStderr("^# fmt", "did not rebuild fmt") - tg.grepStderr("^# reflect", "did not rebuild reflect") - tg.grepStderr("compile.* -N .*-p reflect", "did not build reflect with -N flag") - tg.grepStderrNot("compile.* -N .*-p fmt", "incorrectly built fmt with -N flag") - - tg.run("test", "-c", "-n", "-gcflags=-N", "-ldflags=-X=x.y=z", "strings") - tg.grepStderr("compile.* -N .*compare_test.go", "did not compile strings_test package with -N flag") - tg.grepStderr("link.* -X=x.y=z", "did not link strings.test binary with -X flag") - - tg.run("test", "-c", "-n", "-gcflags=strings=-N", "-ldflags=strings=-X=x.y=z", "strings") - tg.grepStderr("compile.* -N .*compare_test.go", "did not compile strings_test package with -N flag") - tg.grepStderr("link.* -X=x.y=z", "did not link strings.test binary with -X flag") -} - func TestGoTestMinusN(t *testing.T) { // Intent here is to verify that 'go test -n' works without crashing. // This reuses flag_test.go, but really any test would do. diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go index 47953f09a4..e4148bceb0 100644 --- a/src/cmd/go/internal/get/get.go +++ b/src/cmd/go/internal/get/get.go @@ -240,7 +240,7 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int) } load1 := func(path string, mode int) *load.Package { if parent == nil { - return load.LoadPackage(path, stk) + return load.LoadPackageNoFlags(path, stk) } return load.LoadImport(path, parent.Dir, parent, stk, nil, mode|load.ResolveModule) } @@ -329,7 +329,7 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int) base.Run(cfg.BuildToolexec, str.StringList(base.Tool("fix"), files)) // The imports might have changed, so reload again. - p = load.ReloadPackage(arg, stk) + p = load.ReloadPackageNoFlags(arg, stk) if p.Error != nil { base.Errorf("%s", p.Error) return diff --git a/src/cmd/go/internal/load/flag.go b/src/cmd/go/internal/load/flag.go index d9177b0de3..7534e65f54 100644 --- a/src/cmd/go/internal/load/flag.go +++ b/src/cmd/go/internal/load/flag.go @@ -6,7 +6,6 @@ package load import ( "cmd/go/internal/base" - "cmd/go/internal/search" "cmd/go/internal/str" "fmt" "strings" @@ -92,52 +91,3 @@ func (f *PerPackageFlag) For(p *Package) []string { } return flags } - -var ( - cmdlineMatchers []func(*Package) bool - cmdlineMatcherLiterals []func(*Package) bool -) - -// SetCmdlinePatterns records the set of patterns given on the command line, -// for use by the PerPackageFlags. -func SetCmdlinePatterns(args []string) { - setCmdlinePatterns(args, base.Cwd) -} - -func setCmdlinePatterns(args []string, cwd string) { - if len(args) == 0 { - args = []string{"."} - } - cmdlineMatchers = nil // allow reset for testing - cmdlineMatcherLiterals = nil - for _, arg := range args { - cmdlineMatchers = append(cmdlineMatchers, MatchPackage(arg, cwd)) - } - for _, arg := range args { - if !strings.Contains(arg, "...") && !search.IsMetaPackage(arg) { - cmdlineMatcherLiterals = append(cmdlineMatcherLiterals, MatchPackage(arg, cwd)) - } - } -} - -// isCmdlinePkg reports whether p is a package listed on the command line. -func isCmdlinePkg(p *Package) bool { - for _, m := range cmdlineMatchers { - if m(p) { - return true - } - } - return false -} - -// isCmdlinePkgLiteral reports whether p is a package listed as -// a literal package argument on the command line -// (as opposed to being the result of expanding a wildcard). -func isCmdlinePkgLiteral(p *Package) bool { - for _, m := range cmdlineMatcherLiterals { - if m(p) { - return true - } - } - return false -} diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 3a3a38651c..b7257e77e3 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -374,15 +374,17 @@ func ClearPackageCachePartial(args []string) { } } -// reloadPackage is like loadPackage but makes sure +// ReloadPackageNoFlags is like LoadPackageNoFlags but makes sure // not to use the package cache. -func ReloadPackage(arg string, stk *ImportStack) *Package { +// It is only for use by GOPATH-based "go get". +// TODO(rsc): When GOPATH-based "go get" is removed, delete this function. +func ReloadPackageNoFlags(arg string, stk *ImportStack) *Package { p := packageCache[arg] if p != nil { delete(packageCache, p.Dir) delete(packageCache, p.ImportPath) } - return LoadPackage(arg, stk) + return LoadPackageNoFlags(arg, stk) } // dirToImportPath returns the pseudo-import path we use for a package @@ -431,6 +433,9 @@ const ( // but possibly a local import path (an absolute file system path or one beginning // with ./ or ../). A local relative path is interpreted relative to srcDir. // It returns a *Package describing the package found in that directory. +// LoadImport does not set tool flags and should only be used by +// this package, as part of a bigger load operation, and by GOPATH-based "go get". +// TODO(rsc): When GOPATH-based "go get" is removed, unexport this function. func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package { stk.Push(path) defer stk.Pop() @@ -1185,27 +1190,6 @@ var foldPath = make(map[string]string) func (p *Package) load(stk *ImportStack, bp *build.Package, err error) { p.copyBuild(bp) - // Decide whether p was listed on the command line. - // Given that load is called while processing the command line, - // you might think we could simply pass a flag down into load - // saying whether we are loading something named on the command - // line or something to satisfy an import. But the first load of a - // package named on the command line may be as a dependency - // of an earlier package named on the command line, not when we - // get to that package during command line processing. - // For example "go test fmt reflect" will load reflect as a dependency - // of fmt before it attempts to load as a command-line argument. - // Because loads are cached, the later load will be a no-op, - // so it is important that the first load can fill in CmdlinePkg correctly. - // Hence the call to a separate matching check here. - p.Internal.CmdlinePkg = isCmdlinePkg(p) - p.Internal.CmdlinePkgLiteral = isCmdlinePkgLiteral(p) - - p.Internal.Asmflags = BuildAsmflags.For(p) - p.Internal.Gcflags = BuildGcflags.For(p) - p.Internal.Ldflags = BuildLdflags.For(p) - p.Internal.Gccgoflags = BuildGccgoflags.For(p) - // The localPrefix is the path we interpret ./ imports relative to. // Synthesized main packages sometimes override this. if p.Internal.Local { @@ -1740,11 +1724,31 @@ func ClearCmdCache() { } } +// LoadPackage loads the package named by arg. +func LoadPackage(arg string, stk *ImportStack) *Package { + p := loadPackage(arg, stk) + setToolFlags(p) + return p +} + +// LoadPackageNoFlags is like LoadPackage +// but does not guarantee that the build tool flags are set in the result. +// It is only for use by GOPATH-based "go get" +// and is only appropriate for preliminary loading of packages. +// A real load using LoadPackage or (more likely) +// Packages, PackageAndErrors, or PackagesForBuild +// must be done before passing the package to any build +// steps, so that the tool flags can be set properly. +// TODO(rsc): When GOPATH-based "go get" is removed, delete this function. +func LoadPackageNoFlags(arg string, stk *ImportStack) *Package { + return loadPackage(arg, stk) +} + // loadPackage is like loadImport but is used for command-line arguments, // not for paths found in import statements. In addition to ordinary import paths, // loadPackage accepts pseudo-paths beginning with cmd/ to denote commands // in the Go command directory, as well as paths to those directories. -func LoadPackage(arg string, stk *ImportStack) *Package { +func loadPackage(arg string, stk *ImportStack) *Package { if build.IsLocalImport(arg) { dir := arg if !filepath.IsAbs(dir) { @@ -1843,7 +1847,14 @@ func PackagesAndErrors(patterns []string) []*Package { for _, m := range matches { for _, pkg := range m.Pkgs { - p := LoadPackage(pkg, &stk) + p := loadPackage(pkg, &stk) + p.Internal.CmdlinePkg = true + if m.Literal { + // Note: do not set = m.Literal unconditionally + // because maybe we'll see p matching both + // a literal and also a non-literal pattern. + p.Internal.CmdlinePkgLiteral = true + } if seenPkg[p] { continue } @@ -1852,9 +1863,24 @@ func PackagesAndErrors(patterns []string) []*Package { } } + // Now that CmdlinePkg is set correctly, + // compute the effective flags for all loaded packages + // (not just the ones matching the patterns but also + // their dependencies). + setToolFlags(pkgs...) + return pkgs } +func setToolFlags(pkgs ...*Package) { + for _, p := range PackageList(pkgs) { + p.Internal.Asmflags = BuildAsmflags.For(p) + p.Internal.Gcflags = BuildGcflags.For(p) + p.Internal.Ldflags = BuildLdflags.For(p) + p.Internal.Gccgoflags = BuildGccgoflags.For(p) + } +} + func ImportPaths(args []string) []*search.Match { if ModInit(); cfg.ModulesEnabled { return ModImportPaths(args) @@ -1986,5 +2012,7 @@ func GoFilesPackage(gofiles []string) *Package { } } + setToolFlags(pkg) + return pkg } diff --git a/src/cmd/go/internal/load/search.go b/src/cmd/go/internal/load/search.go index d379c7b021..cf09c7b0a8 100644 --- a/src/cmd/go/internal/load/search.go +++ b/src/cmd/go/internal/load/search.go @@ -6,7 +6,6 @@ package load import ( "path/filepath" - "runtime" "strings" "cmd/go/internal/search" @@ -28,13 +27,7 @@ func MatchPackage(pattern, cwd string) func(*Package) bool { } dir = filepath.Join(cwd, dir) if pattern == "" { - return func(p *Package) bool { - // TODO(rsc): This is wrong. See golang.org/issue/25878. - if runtime.GOOS != "windows" { - return p.Dir == dir - } - return strings.EqualFold(p.Dir, dir) - } + return func(p *Package) bool { return p.Dir == dir } } matchPath := search.MatchPattern(pattern) return func(p *Package) bool { diff --git a/src/cmd/go/testdata/script/gcflags_patterns.txt b/src/cmd/go/testdata/script/gcflags_patterns.txt new file mode 100644 index 0000000000..fe2cf6f0fb --- /dev/null +++ b/src/cmd/go/testdata/script/gcflags_patterns.txt @@ -0,0 +1,71 @@ +[!gc] skip 'using -gcflags and -ldflags' + +# -gcflags=-e applies to named packages, not dependencies +go build -n -v -gcflags=-e z1 z2 +stderr 'compile.* -e .*-p z1' +stderr 'compile.* -e .*-p z2' +stderr 'compile.* -p y' +! stderr 'compile.* -e .*-p [^z]' + +# -gcflags can specify package=flags, and can be repeated; last match wins +go build -n -v -gcflags=-e -gcflags=z1=-N z1 z2 +stderr 'compile.* -N .*-p z1' +! stderr 'compile.* -e .*-p z1' +! stderr 'compile.* -N .*-p z2' +stderr 'compile.* -e .*-p z2' +stderr 'compile.* -p y' +! stderr 'compile.* -e .*-p [^z]' +! stderr 'compile.* -N .*-p [^z]' + +# -gcflags can have arbitrary spaces around the flags +go build -n -v -gcflags=' z1 = -e ' z1 +stderr 'compile.* -e .*-p z1' + +# -ldflags for implicit test package applies to test binary +go test -c -n -gcflags=-N -ldflags=-X=x.y=z z1 +stderr 'compile.* -N .*z_test.go' +stderr 'link.* -X=x.y=z' + +# -ldflags for explicit test package applies to test binary +go test -c -n -gcflags=z1=-N -ldflags=z1=-X=x.y=z z1 +stderr 'compile.* -N .*z_test.go' +stderr 'link.* -X=x.y=z' + +# -ldflags applies to link of command +go build -n -ldflags=-X=math.pi=3 my/cmd/prog +stderr 'link.* -X=math.pi=3' + +# -ldflags applies to link of command even with strange directory name +go build -n -ldflags=-X=math.pi=3 my/cmd/prog/ +stderr 'link.* -X=math.pi=3' + +# -ldflags applies to current directory +cd my/cmd/prog +go build -n -ldflags=-X=math.pi=3 +stderr 'link.* -X=math.pi=3' + +# -ldflags applies to current directory even if GOPATH is funny +[windows] cd $WORK/GoPath/src/my/cmd/prog +[darwin] cd $WORK/GoPath/src/my/cmd/prog +go build -n -ldflags=-X=math.pi=3 +stderr 'link.* -X=math.pi=3' + +-- z1/z.go -- +package z1 +import _ "y" +import _ "z2" + +-- z1/z_test.go -- +package z1_test +import "testing" +func Test(t *testing.T) {} + +-- z2/z.go -- +package z2 + +-- y/y.go -- +package y + +-- my/cmd/prog/prog.go -- +package main +func main() {} From 8dd27b1864f334fa82e0ead5bd8b9448e295e316 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 12:40:18 -0400 Subject: [PATCH 18/31] cmd/go: report which patterns match each package in list It's important for some uses of go/packages, as well as for some of go/packages's internal use, to be able to tell which results from go list output correspond to which patterns, keeping in mind that a single package might have been matched by multiple patterns. Also adds test for #26925. Change-Id: I708ac162f65d9946fe6afb244b08dc7b04d2b530 Reviewed-on: https://go-review.googlesource.com/129060 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Alan Donovan --- src/cmd/go/internal/list/list.go | 37 +++++++------- src/cmd/go/internal/load/pkg.go | 15 +++--- src/cmd/go/internal/modload/load.go | 17 ++++--- src/cmd/go/testdata/script/mod_patterns.txt | 53 ++++++++------------- 4 files changed, 56 insertions(+), 66 deletions(-) diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index 423516aad7..186b006c12 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -47,24 +47,25 @@ syntax of package template. The default output is equivalent to -f '{{.ImportPath}}'. The struct being passed to the template is: type Package struct { - Dir string // directory containing package sources - ImportPath string // import path of package in dir - ImportComment string // path in import comment on package statement - Name string // package name - Doc string // package documentation string - Target string // install path - Shlib string // the shared library that contains this package (only set when -linkshared) - Goroot bool // is this package in the Go root? - Standard bool // is this package part of the standard Go library? - Stale bool // would 'go install' do anything for this package? - StaleReason string // explanation for Stale==true - Root string // Go root or Go path dir containing this package - ConflictDir string // this directory shadows Dir in $GOPATH - BinaryOnly bool // binary-only package: cannot be recompiled from sources - ForTest string // package is only for use in named test - DepOnly bool // package is only a dependency, not explicitly listed - Export string // file containing export data (when using -export) - Module *Module // info about package's containing module, if any (can be nil) + Dir string // directory containing package sources + ImportPath string // import path of package in dir + ImportComment string // path in import comment on package statement + Name string // package name + Doc string // package documentation string + Target string // install path + Shlib string // the shared library that contains this package (only set when -linkshared) + Goroot bool // is this package in the Go root? + Standard bool // is this package part of the standard Go library? + Stale bool // would 'go install' do anything for this package? + StaleReason string // explanation for Stale==true + Root string // Go root or Go path dir containing this package + ConflictDir string // this directory shadows Dir in $GOPATH + BinaryOnly bool // binary-only package: cannot be recompiled from sources + ForTest string // package is only for use in named test + Export string // file containing export data (when using -export) + Module *Module // info about package's containing module, if any (can be nil) + Match []string // command-line patterns matching this package + DepOnly bool // package is only a dependency, not explicitly listed // Source files GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index b7257e77e3..43887b0008 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -60,15 +60,17 @@ type PackagePublic struct { Doc string `json:",omitempty"` // package documentation string Target string `json:",omitempty"` // installed target for this package (may be executable) Shlib string `json:",omitempty"` // the shared library that contains this package (only set when -linkshared) - Goroot bool `json:",omitempty"` // is this package found in the Go root? - Standard bool `json:",omitempty"` // is this package part of the standard Go library? Root string `json:",omitempty"` // Go root or Go path dir containing this package ConflictDir string `json:",omitempty"` // Dir is hidden by this other directory - BinaryOnly bool `json:",omitempty"` // package cannot be recompiled ForTest string `json:",omitempty"` // package is only for use in named test - DepOnly bool `json:",omitempty"` // package is only as a dependency, not explicitly listed Export string `json:",omitempty"` // file containing export data (set by go list -export) Module *modinfo.ModulePublic `json:",omitempty"` // info about package's module, if any + Match []string `json:",omitempty"` // command-line patterns matching this package + Goroot bool `json:",omitempty"` // is this package found in the Go root? + Standard bool `json:",omitempty"` // is this package part of the standard Go library? + DepOnly bool `json:",omitempty"` // package is only as a dependency, not explicitly listed + BinaryOnly bool `json:",omitempty"` // package cannot be recompiled + Incomplete bool `json:",omitempty"` // was there an error loading this package or dependencies? // Stale and StaleReason remain here *only* for the list command. // They are only initialized in preparation for list execution. @@ -107,7 +109,7 @@ type PackagePublic struct { Deps []string `json:",omitempty"` // all (recursively) imported dependencies // Error information - Incomplete bool `json:",omitempty"` // was there an error loading this package or dependencies? + // Incomplete is above, packed into the other bools Error *PackageError `json:",omitempty"` // error loading this package (not dependencies) DepsErrors []*PackageError `json:",omitempty"` // errors loading dependencies @@ -1848,6 +1850,7 @@ func PackagesAndErrors(patterns []string) []*Package { for _, m := range matches { for _, pkg := range m.Pkgs { p := loadPackage(pkg, &stk) + p.Match = append(p.Match, m.Pattern) p.Internal.CmdlinePkg = true if m.Literal { // Note: do not set = m.Literal unconditionally @@ -1937,7 +1940,6 @@ func PackagesForBuild(args []string) []*Package { func GoFilesPackage(gofiles []string) *Package { ModInit() - // TODO: Remove this restriction. for _, f := range gofiles { if !strings.HasSuffix(f, ".go") { base.Fatalf("named files must be .go files") @@ -1998,6 +2000,7 @@ func GoFilesPackage(gofiles []string) *Package { pkg.Internal.LocalPrefix = dirToImportPath(dir) pkg.ImportPath = "command-line-arguments" pkg.Target = "" + pkg.Match = gofiles if pkg.Name == "main" { _, elem := filepath.Split(gofiles[0]) diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index e408e478d3..b00f81458f 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -56,14 +56,14 @@ func ImportPaths(patterns []string) []*search.Match { var matches []*search.Match for _, pattern := range search.CleanPatterns(patterns) { - m := &search.Match{ - Pattern: pattern, - Literal: !strings.Contains(pattern, "...") && !search.IsMetaPackage(pattern), - } - if m.Literal { - m.Pkgs = []string{pattern} - } - matches = append(matches, m) + m := &search.Match{ + Pattern: pattern, + Literal: !strings.Contains(pattern, "...") && !search.IsMetaPackage(pattern), + } + if m.Literal { + m.Pkgs = []string{pattern} + } + matches = append(matches, m) } fsDirs := make([][]string, len(matches)) @@ -622,7 +622,6 @@ func (ld *loader) computePatternAll(paths []string) []string { } sort.Strings(all) - fmt.Fprintf(os.Stderr, "ALL %v -> %v\n", paths, all) return all } diff --git a/src/cmd/go/testdata/script/mod_patterns.txt b/src/cmd/go/testdata/script/mod_patterns.txt index a43fe82489..4fa436ba2d 100644 --- a/src/cmd/go/testdata/script/mod_patterns.txt +++ b/src/cmd/go/testdata/script/mod_patterns.txt @@ -5,43 +5,30 @@ cd m # 'go list all' should list all of the packages used (directly or indirectly) by # the packages in the main module, but no other packages from the standard # library or active modules. -go list all -stdout example.com/m/useunicode -stdout example.com/m/useunsafe -[cgo] stdout example.com/m/useC -[!cgo] ! stdout example.com/m/useC -stdout '^unicode$' -stdout '^unsafe$' -! stdout index/suffixarray - +# # 'go list ...' should list packages in all active modules and the standard library. # But not cmd/* - see golang.org/issue/26924. -go list ... -stdout example.com/unused/useerrors -stdout example.com/m/useunsafe -[cgo] stdout example.com/m/useC -[!cgo] ! stdout example.com/m/useC -stdout '^unicode$' -stdout '^unsafe$' -stdout index/suffixarray -! stdout cmd/pprof - -# 'go list example.com/m/...' should list packages in all modules that begin with -# "example.com/m/". -go list example.com/m/... -stdout example.com/m/useunicode -stdout example.com/m/useunsafe -! stdout example.com/[^m] -! stdout ^[^e] -[cgo] stdout example.com/m/useC -[!cgo] ! stdout example.com/m/useC - +# +# 'go list example.com/m/...' should list packages in all modules that begin with 'example.com/m/'. +# # 'go list ./...' should list only packages in the current module, not other active modules. -go list ./... -stdout example.com/m/useunicode -stdout example.com/m/useunsafe -[cgo] stdout example.com/m/useC +# +# Warnings about unmatched patterns should only be printed once. +# +# And the go command should be able to keep track of all this! +go list -f '{{.ImportPath}}: {{.Match}}' all ... example.com/m/... ./... ./xyz... +stdout 'example.com/m/useunicode: \[all \.\.\. example.com/m/... ./...\]' +stdout 'example.com/m/useunsafe: \[all \.\.\. example.com/m/... ./...\]' +[cgo] stdout 'example.com/m/useC: \[all \.\.\. example.com/m/... ./...\]' [!cgo] ! stdout example.com/m/useC +stdout 'example.com/unused/useerrors: \[\.\.\.\]' # but not "all" +stdout 'example.com/m/nested/useencoding: \[\.\.\. example.com/m/...\]' # but NOT "all" or "./..." +stdout '^unicode: \[all \.\.\.\]' +stdout '^unsafe: \[all \.\.\.\]' +stdout 'index/suffixarray: \[\.\.\.\]' +! stdout cmd/pprof # golang.org/issue/26924 + +stderr -count=1 '^go: warning: "./xyz..." matched no packages$' env CGO_ENABLED=0 go list -f '{{.ImportPath}}: {{.Match}}' all ... example.com/m/... ./... ./xyz... From 64fae252868fe6ab97f743cfadcb54cee8ccca02 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 10 Aug 2018 16:28:48 -0400 Subject: [PATCH 19/31] cmd/go: do not turn list ./nonexist into a network lookup If you're in a directory corresponding to x/y and you run go list ./z, we do at some point want to turn that into x/y/z. But if ./z does not exist that will make the go command check the network to see if it can find x/y/z. That's clearly wrong: ./z means that directory, nothing else. And it turns a typo into a long delay, which is even worse. Fixes #26874. Change-Id: Iec15fa7b359af11b6a4fc6cb082e593658fb6e41 Reviewed-on: https://go-review.googlesource.com/129061 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Alan Donovan --- src/cmd/go/internal/modload/load.go | 14 ++++++++++++++ src/cmd/go/testdata/script/mod_fs_patterns.txt | 12 ++++++++++++ src/cmd/go/testdata/script/mod_list_dir.txt | 5 ++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index b00f81458f..5ca2ed2d10 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -116,10 +116,24 @@ func ImportPaths(patterns []string) []*search.Match { } else if path := pathInModuleCache(dir); path != "" { pkg = path } else { + pkg = "" if !iterating { base.Errorf("go: directory %s outside available modules", base.ShortPath(dir)) } + } + info, err := os.Stat(dir) + if err != nil || !info.IsDir() { + // If the directory does not exist, + // don't turn it into an import path + // that will trigger a lookup. pkg = "" + if !iterating { + if err != nil { + base.Errorf("go: no such directory %v", m.Pattern) + } else { + base.Errorf("go: %s is not a directory", m.Pattern) + } + } } m.Pkgs[i] = pkg } diff --git a/src/cmd/go/testdata/script/mod_fs_patterns.txt b/src/cmd/go/testdata/script/mod_fs_patterns.txt index b5350c3eed..d7d3e0321b 100644 --- a/src/cmd/go/testdata/script/mod_fs_patterns.txt +++ b/src/cmd/go/testdata/script/mod_fs_patterns.txt @@ -28,6 +28,18 @@ stdout ^m/vendor$ stdout ^m/y$ ! stdout ^m/y/z +# non-existent directory should not prompt lookups +! go build -mod=readonly example.com/nonexist +stderr 'import lookup disabled' + +! go build -mod=readonly ./nonexist +! stderr 'import lookup disabled' +stderr '^go: no such directory ./nonexist' + +! go build -mod=readonly ./go.mod +! stderr 'import lookup disabled' +stderr '^go: ./go.mod is not a directory' + -- x/go.mod -- module m diff --git a/src/cmd/go/testdata/script/mod_list_dir.txt b/src/cmd/go/testdata/script/mod_list_dir.txt index 29cde71fb8..800f277559 100644 --- a/src/cmd/go/testdata/script/mod_list_dir.txt +++ b/src/cmd/go/testdata/script/mod_list_dir.txt @@ -9,11 +9,14 @@ go list -f '{{.ImportPath}}' $GOROOT/src/math stdout ^math$ go list -f '{{.ImportPath}}' . stdout ^x$ +! go list -f '{{.ImportPath}}' $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 +stderr '^go: no such directory.*quote@v1.5.2' +go mod download rsc.io/quote@v1.5.2 go list -f '{{.ImportPath}}' $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 stdout '^rsc.io/quote$' go list -f '{{.ImportPath}}' $GOPATH/pkg/mod/rsc.io/sampler@v1.3.0 stdout '^rsc.io/sampler$' -go get rsc.io/sampler@v1.3.1 +go get -d rsc.io/sampler@v1.3.1 go list -f '{{.ImportPath}}' $GOPATH/pkg/mod/rsc.io/sampler@v1.3.1 stdout '^rsc.io/sampler$' ! go list -f '{{.ImportPath}}' $GOPATH/pkg/mod/rsc.io/sampler@v1.3.0 From 0a842d55609f5deb25889f151e49744c4af3ec80 Mon Sep 17 00:00:00 2001 From: David du Colombier <0intro@gmail.com> Date: Sat, 18 Aug 2018 00:05:46 +0200 Subject: [PATCH 20/31] os: handle TMPDIR in TempDir on Plan 9 CL 129063 added a test in TestScript/mod_enabled, which was failing on Plan 9. The test was failing because the Init function of the cmd/go/internal/modload package was expecting ModRoot to be part of os.TempDir. However, ModRoot was set to TMPDIR, while os.TempDir is returning /tmp on Plan 9. This change fixes the implementation of os.TempDir on Plan 9 to handle the TMPDIR environment variable, similarly to Unix. Fixes #27065. Change-Id: Id6ff926c5c379f63cab2dfc378fa6c15293fd453 Reviewed-on: https://go-review.googlesource.com/129775 Reviewed-by: Brad Fitzpatrick --- src/os/file_plan9.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/os/file_plan9.go b/src/os/file_plan9.go index 3a0b774aa2..2c74403434 100644 --- a/src/os/file_plan9.go +++ b/src/os/file_plan9.go @@ -478,7 +478,12 @@ func (f *File) Chown(uid, gid int) error { } func tempDir() string { - return "/tmp" + dir := Getenv("TMPDIR") + if dir == "" { + dir = "/tmp" + } + return dir + } // Chdir changes the current working directory to the file, From 16a72125d53b31e2f70a6922bc398dcbba354b7a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 15:53:35 -0400 Subject: [PATCH 21/31] cmd/go/internal/modfetch: correct TestCodeRepo expectation The proxy protocol was simplified to only send (and only receive) the Path and Version fields in the JSON blob, not Name and Short. (Those make sense when querying a VCS repo directly, but not when talking about extracted modules.) So don't expect them in the test. Fixes #27042. Change-Id: I3daacd668126e2227dcc8e6b89ee0cf0e3c8497c Reviewed-on: https://go-review.googlesource.com/129684 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/modfetch/coderepo_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cmd/go/internal/modfetch/coderepo_test.go b/src/cmd/go/internal/modfetch/coderepo_test.go index c46705105d..79b82786cb 100644 --- a/src/cmd/go/internal/modfetch/coderepo_test.go +++ b/src/cmd/go/internal/modfetch/coderepo_test.go @@ -228,10 +228,9 @@ var codeRepoTests = []struct { path: "swtch.com/testmod", rev: "v1.0.0", version: "v1.0.0", - name: "v1.0.0", - short: "v1.0.0", - time: time.Date(1972, 7, 18, 12, 34, 56, 0, time.UTC), - gomod: "module \"swtch.com/testmod\"\n", + // NO name or short - we intentionally ignore those in the proxy protocol + time: time.Date(1972, 7, 18, 12, 34, 56, 0, time.UTC), + gomod: "module \"swtch.com/testmod\"\n", }, { // redirect to googlesource From 4864decf04f3691045bc6f95effa9c6e2ba0ad33 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 14:47:31 -0400 Subject: [PATCH 22/31] cmd/go: remove go mod fix, add go help go.mod "go mod fix" does work already done by nearly every other go command. It was also confusing why we had both "go mod fix" and "go mod tidy". Delete "go mod fix". The main reason we kept "go mod fix" this long was for the discussion of automatic go.mod updates in its documentation, which is now moved into a new "go help go.mod". Fixes #26831. Change-Id: Ic95ca8918449ab79791d27998e02eb3377ac7972 Reviewed-on: https://go-review.googlesource.com/129682 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/alldocs.go | 156 ++++++++++-------- src/cmd/go/internal/modcmd/fix.go | 65 -------- src/cmd/go/internal/modcmd/mod.go | 1 - src/cmd/go/internal/modload/help.go | 77 ++++++++- src/cmd/go/main.go | 1 + src/cmd/go/testdata/script/mod_get_commit.txt | 4 +- 6 files changed, 163 insertions(+), 141 deletions(-) delete mode 100644 src/cmd/go/internal/modcmd/fix.go diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 70d655747c..c67e3f5a1c 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -40,6 +40,7 @@ // cache build and test caching // environment environment variables // filetype file types +// go.mod the go.mod file // gopath GOPATH environment variable // gopath-get legacy GOPATH go get // goproxy module proxy protocol @@ -622,24 +623,25 @@ // to -f '{{.ImportPath}}'. The struct being passed to the template is: // // type Package struct { -// Dir string // directory containing package sources -// ImportPath string // import path of package in dir -// ImportComment string // path in import comment on package statement -// Name string // package name -// Doc string // package documentation string -// Target string // install path -// Shlib string // the shared library that contains this package (only set when -linkshared) -// Goroot bool // is this package in the Go root? -// Standard bool // is this package part of the standard Go library? -// Stale bool // would 'go install' do anything for this package? -// StaleReason string // explanation for Stale==true -// Root string // Go root or Go path dir containing this package -// ConflictDir string // this directory shadows Dir in $GOPATH -// BinaryOnly bool // binary-only package: cannot be recompiled from sources -// ForTest string // package is only for use in named test -// DepOnly bool // package is only a dependency, not explicitly listed -// Export string // file containing export data (when using -export) -// Module *Module // info about package's containing module, if any (can be nil) +// Dir string // directory containing package sources +// ImportPath string // import path of package in dir +// ImportComment string // path in import comment on package statement +// Name string // package name +// Doc string // package documentation string +// Target string // install path +// Shlib string // the shared library that contains this package (only set when -linkshared) +// Goroot bool // is this package in the Go root? +// Standard bool // is this package part of the standard Go library? +// Stale bool // would 'go install' do anything for this package? +// StaleReason string // explanation for Stale==true +// Root string // Go root or Go path dir containing this package +// ConflictDir string // this directory shadows Dir in $GOPATH +// BinaryOnly bool // binary-only package: cannot be recompiled from sources +// ForTest string // package is only for use in named test +// Export string // file containing export data (when using -export) +// Module *Module // info about package's containing module, if any (can be nil) +// Match []string // command-line patterns matching this package +// DepOnly bool // package is only a dependency, not explicitly listed // // // Source files // GoFiles []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) @@ -874,7 +876,6 @@ // // download download modules to local cache // edit edit go.mod from tools or scripts -// fix make go.mod semantically consistent // graph print module requirement graph // init initialize new module in current directory // tidy add missing and remove unused modules @@ -998,52 +999,6 @@ // by invoking 'go mod edit' with -require, -exclude, and so on. // // -// Make go.mod semantically consistent -// -// Usage: -// -// go mod fix -// -// Fix updates go.mod to use canonical version identifiers and -// to be semantically consistent. For example, consider this go.mod file: -// -// module M -// -// require ( -// A v1 -// B v1.0.0 -// C v1.0.0 -// D v1.2.3 -// E dev -// ) -// -// exclude D v1.2.3 -// -// First, fix rewrites non-canonical version identifiers to semver form, so -// A's v1 becomes v1.0.0 and E's dev becomes the pseudo-version for the latest -// commit on the dev branch, perhaps v0.0.0-20180523231146-b3f5c0f6e5f1. -// -// Next, fix updates requirements to respect exclusions, so the requirement -// on the excluded D v1.2.3 is updated to use the next available version of D, -// perhaps D v1.2.4 or D v1.3.0. -// -// Finally, fix removes redundant or misleading requirements. -// For example, if A v1.0.0 itself requires B v1.2.0 and C v1.0.0, then go.mod's -// requirement of B v1.0.0 is misleading (superseded by A's need for v1.2.0), -// and its requirement of C v1.0.0 is redundant (implied by A's need for the -// same version), so both will be removed. If module M contains packages -// that directly import packages from B or C, then the requirements will be -// kept but updated to the actual versions being used. -// -// Although fix runs the fix-up operation in isolation, the fix-up also -// runs automatically any time a go command uses the module graph, -// to update go.mod to reflect reality. Because the module graph defines -// the meaning of import statements, any commands that load packages -// also use and therefore fix the module graph. For example, -// go build, go get, go install, go list, go test, go mod graph, go mod tidy, -// and other commands all effectively imply go mod fix. -// -// // Print module requirement graph // // Usage: @@ -1620,6 +1575,73 @@ // command. // // +// The go.mod file +// +// A module version is defined by a tree of source files, with a go.mod +// file in its root. When the go command is run, it looks in the current +// directory and then successive parent directories to find the go.mod +// marking the root of the main (current) module. +// +// The go.mod file itself is line-oriented, with // comments but +// no /* */ comments. Each line holds a single directive, made up of a +// verb followed by arguments. For example: +// +// module my/thing +// require other/thing v1.0.2 +// require new/thing v2.3.4 +// exclude old/thing v1.2.3 +// replace bad/thing v1.4.5 => good/thing v1.4.5 +// +// The verbs are module, to define the module path; require, to require +// a particular module at a given version or later; exclude, to exclude +// a particular module version from use; and replace, to replace a module +// version with a different module version. Exclude and replace apply only +// in the main module's go.mod and are ignored in dependencies. +// See https://research.swtch.com/vgo-mvs for details. +// +// The leading verb can be factored out of adjacent lines to create a block, +// like in Go imports: +// +// require ( +// new/thing v2.3.4 +// old/thing v1.2.3 +// ) +// +// The go.mod file is designed both to be edited directly and to be +// easily updated by tools. The 'go mod edit' command can be used to +// parse and edit the go.mod file from programs and tools. +// See 'go help mod edit'. +// +// The go command automatically updates go.mod each time it uses the +// module graph, to make sure go.mod always accurately reflects reality +// and is properly formatted. +// +// The update rewrites non-canonical version identifiers to semver form, +// so A's v1 becomes v1.0.0 and E's dev becomes the pseudo-version for the +// latest commit on the dev branch, perhaps v0.0.0-20180523231146-b3f5c0f6e5f1. +// +// The update modifies requirements to respect exclusions, so the +// requirement on the excluded D v1.2.3 is updated to use the next +// available version of D, perhaps D v1.2.4 or D v1.3.0. +// +// The update removes redundant or misleading requirements. +// For example, if A v1.0.0 itself requires B v1.2.0 and C v1.0.0, +// then go.mod's requirement of B v1.0.0 is misleading (superseded by +// A's need for v1.2.0), and its requirement of C v1.0.0 is redundant +// (implied by A's need for the same version), so both will be removed. +// If module M contains packages that directly import packages from B or +// C, then the requirements will be kept but updated to the actual +// versions being used. +// +// Finally, the update reformats the go.mod in a canonical formatting, so +// that future mechanical changes will result in minimal diffs. +// +// Because the module graph defines the meaning of import statements, any +// commands that load packages also use and therefore update go.mod, +// including go build, go get, go install, go list, go test, go mod graph, +// go mod tidy, and go mod why. +// +// // GOPATH environment variable // // The Go path is used to resolve import statements. @@ -2095,7 +2117,7 @@ // The go.mod file can also specify replacements and excluded versions // that only apply when building the module directly; they are ignored // when the module is incorporated into a larger build. -// For more about the go.mod file, see https://research.swtch.com/vgo-module. +// For more about the go.mod file, see 'go help go.mod'. // // To start a new module, simply create a go.mod file in the root of the // module's directory tree, containing only a module statement. @@ -2350,8 +2372,6 @@ // about how source code in version control systems is mapped to // module file trees. // -// TODO: Add documentation to go command. -// // Module downloading and verification // // The go command maintains, in the main module's root directory alongside diff --git a/src/cmd/go/internal/modcmd/fix.go b/src/cmd/go/internal/modcmd/fix.go deleted file mode 100644 index bfb51456a6..0000000000 --- a/src/cmd/go/internal/modcmd/fix.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// go mod fix - -package modcmd - -import ( - "cmd/go/internal/base" - "cmd/go/internal/modload" -) - -var cmdFix = &base.Command{ - UsageLine: "go mod fix", - Short: "make go.mod semantically consistent", - Long: ` -Fix updates go.mod to use canonical version identifiers and -to be semantically consistent. For example, consider this go.mod file: - - module M - - require ( - A v1 - B v1.0.0 - C v1.0.0 - D v1.2.3 - E dev - ) - - exclude D v1.2.3 - -First, fix rewrites non-canonical version identifiers to semver form, so -A's v1 becomes v1.0.0 and E's dev becomes the pseudo-version for the latest -commit on the dev branch, perhaps v0.0.0-20180523231146-b3f5c0f6e5f1. - -Next, fix updates requirements to respect exclusions, so the requirement -on the excluded D v1.2.3 is updated to use the next available version of D, -perhaps D v1.2.4 or D v1.3.0. - -Finally, fix removes redundant or misleading requirements. -For example, if A v1.0.0 itself requires B v1.2.0 and C v1.0.0, then go.mod's -requirement of B v1.0.0 is misleading (superseded by A's need for v1.2.0), -and its requirement of C v1.0.0 is redundant (implied by A's need for the -same version), so both will be removed. If module M contains packages -that directly import packages from B or C, then the requirements will be -kept but updated to the actual versions being used. - -Although fix runs the fix-up operation in isolation, the fix-up also -runs automatically any time a go command uses the module graph, -to update go.mod to reflect reality. Because the module graph defines -the meaning of import statements, any commands that load packages -also use and therefore fix the module graph. For example, -go build, go get, go install, go list, go test, go mod graph, go mod tidy, -and other commands all effectively imply go mod fix. - `, - Run: runFix, -} - -func runFix(cmd *base.Command, args []string) { - if len(args) != 0 { - base.Fatalf("go mod fix: fix takes no arguments") - } - modload.LoadBuildList() // writes go.mod -} diff --git a/src/cmd/go/internal/modcmd/mod.go b/src/cmd/go/internal/modcmd/mod.go index c1d0c13a10..f150cc9728 100644 --- a/src/cmd/go/internal/modcmd/mod.go +++ b/src/cmd/go/internal/modcmd/mod.go @@ -21,7 +21,6 @@ See 'go help modules' for an overview of module functionality. Commands: []*base.Command{ cmdDownload, cmdEdit, - cmdFix, cmdGraph, cmdInit, cmdTidy, diff --git a/src/cmd/go/internal/modload/help.go b/src/cmd/go/internal/modload/help.go index 64c70b7d7b..9a12b24482 100644 --- a/src/cmd/go/internal/modload/help.go +++ b/src/cmd/go/internal/modload/help.go @@ -6,8 +6,7 @@ package modload import "cmd/go/internal/base" -// TODO(rsc): The links out to research.swtch.com here should all be -// replaced eventually with links to proper documentation. +// TODO(rsc): The "module code layout" section needs to be written. var HelpModules = &base.Command{ UsageLine: "modules", @@ -81,7 +80,7 @@ depends on specific versions of golang.org/x/text and gopkg.in/yaml.v2: The go.mod file can also specify replacements and excluded versions that only apply when building the module directly; they are ignored when the module is incorporated into a larger build. -For more about the go.mod file, see https://research.swtch.com/vgo-module. +For more about the go.mod file, see 'go help go.mod'. To start a new module, simply create a go.mod file in the root of the module's directory tree, containing only a module statement. @@ -336,8 +335,6 @@ For now, see https://research.swtch.com/vgo-module for information about how source code in version control systems is mapped to module file trees. -TODO: Add documentation to go command. - Module downloading and verification The go command maintains, in the main module's root directory alongside @@ -381,3 +378,73 @@ top-level vendor directory is used; vendor directories in other locations are still ignored. `, } + +var HelpGoMod = &base.Command{ + UsageLine: "go.mod", + Short: "the go.mod file", + Long: ` +A module version is defined by a tree of source files, with a go.mod +file in its root. When the go command is run, it looks in the current +directory and then successive parent directories to find the go.mod +marking the root of the main (current) module. + +The go.mod file itself is line-oriented, with // comments but +no /* */ comments. Each line holds a single directive, made up of a +verb followed by arguments. For example: + + module my/thing + require other/thing v1.0.2 + require new/thing v2.3.4 + exclude old/thing v1.2.3 + replace bad/thing v1.4.5 => good/thing v1.4.5 + +The verbs are module, to define the module path; require, to require +a particular module at a given version or later; exclude, to exclude +a particular module version from use; and replace, to replace a module +version with a different module version. Exclude and replace apply only +in the main module's go.mod and are ignored in dependencies. +See https://research.swtch.com/vgo-mvs for details. + +The leading verb can be factored out of adjacent lines to create a block, +like in Go imports: + + require ( + new/thing v2.3.4 + old/thing v1.2.3 + ) + +The go.mod file is designed both to be edited directly and to be +easily updated by tools. The 'go mod edit' command can be used to +parse and edit the go.mod file from programs and tools. +See 'go help mod edit'. + +The go command automatically updates go.mod each time it uses the +module graph, to make sure go.mod always accurately reflects reality +and is properly formatted. + +The update rewrites non-canonical version identifiers to semver form, +so A's v1 becomes v1.0.0 and E's dev becomes the pseudo-version for the +latest commit on the dev branch, perhaps v0.0.0-20180523231146-b3f5c0f6e5f1. + +The update modifies requirements to respect exclusions, so the +requirement on the excluded D v1.2.3 is updated to use the next +available version of D, perhaps D v1.2.4 or D v1.3.0. + +The update removes redundant or misleading requirements. +For example, if A v1.0.0 itself requires B v1.2.0 and C v1.0.0, +then go.mod's requirement of B v1.0.0 is misleading (superseded by +A's need for v1.2.0), and its requirement of C v1.0.0 is redundant +(implied by A's need for the same version), so both will be removed. +If module M contains packages that directly import packages from B or +C, then the requirements will be kept but updated to the actual +versions being used. + +Finally, the update reformats the go.mod in a canonical formatting, so +that future mechanical changes will result in minimal diffs. + +Because the module graph defines the meaning of import statements, any +commands that load packages also use and therefore update go.mod, +including go build, go get, go install, go list, go test, go mod graph, +go mod tidy, and go mod why. + `, +} diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go index 59d367edaa..0639b4d2ca 100644 --- a/src/cmd/go/main.go +++ b/src/cmd/go/main.go @@ -64,6 +64,7 @@ func init() { help.HelpCache, help.HelpEnvironment, help.HelpFileType, + modload.HelpGoMod, help.HelpGopath, get.HelpGopathGet, modfetch.HelpGoproxy, diff --git a/src/cmd/go/testdata/script/mod_get_commit.txt b/src/cmd/go/testdata/script/mod_get_commit.txt index 2608397404..589a791fd4 100644 --- a/src/cmd/go/testdata/script/mod_get_commit.txt +++ b/src/cmd/go/testdata/script/mod_get_commit.txt @@ -45,8 +45,8 @@ grep 'rsc.io/quote v1.5.1' go.mod go mod edit -require rsc.io/quote@23179ee grep 'rsc.io/quote 23179ee' go.mod -# but go mod fix fixes them -go mod fix +# but other commands fix them +go mod graph grep 'rsc.io/quote v1.5.1' go.mod -- go.mod -- From 850c964be21d8aadcb0c79be89e62762b0604fbf Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 15:40:55 -0400 Subject: [PATCH 23/31] cmd/go: treat VCS errors as hard errors in module search If we're looking for a module for a/b/c/d/e, we check for a module named a/b/c/d/e, then a/b/c/d, then a/b/c, then a/b, then a. If we know the source repo for a/b/c and that fails, we should report that error instead of continuing the loop: a/b and a are useless, and the error from a/b/c contains important information. The errors are now a bit more verbose than I'd like but they will suffice for Go 1.11. $ go get github.com/bradfitz/private/sonos go get github.com/bradfitz/private/sonos: git ls-remote -q origin in /Users/rsc/pkg/mod/cache/vcs/61e3c76780847e514802ec6af8f940f641c6017f711444f05c59cb17ac46d456: exit status 128: remote: Repository not found. fatal: repository 'https://github.com/bradfitz/private/' not found $ go list launchpad.net/gocheck can't load package: package launchpad.net/gocheck: unknown import path "launchpad.net/gocheck": bzr branch --use-existing-dir https://launchpad.net/~niemeyer/gocheck/trunk . in /Users/rsc/pkg/mod/cache/vcs/f46ce2ae80d31f9b0a29099baa203e3b6d269dace4e5357a2cf74bd109e13339: exec: "bzr": executable file not found in $PATH $ Fixes #26885. Fixes #26982. Change-Id: I2f9cf1853d2d68af18adad668c80513b6ba220d6 Reviewed-on: https://go-review.googlesource.com/129683 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/modfetch/codehost/vcs.go | 14 ++++++++++++++ src/cmd/go/internal/modfetch/repo.go | 3 +++ src/cmd/go/internal/modload/import.go | 4 ++++ src/cmd/go/internal/modload/query.go | 6 ++++++ src/cmd/go/testdata/script/mod_vcs_missing.txt | 11 +++++++++++ 5 files changed, 38 insertions(+) create mode 100644 src/cmd/go/testdata/script/mod_vcs_missing.txt diff --git a/src/cmd/go/internal/modfetch/codehost/vcs.go b/src/cmd/go/internal/modfetch/codehost/vcs.go index 03def8e082..9e862a0ef8 100644 --- a/src/cmd/go/internal/modfetch/codehost/vcs.go +++ b/src/cmd/go/internal/modfetch/codehost/vcs.go @@ -22,6 +22,17 @@ import ( "cmd/go/internal/str" ) +// A VCSError indicates an error using a version control system. +// The implication of a VCSError is that we know definitively where +// to get the code, but we can't access it due to the error. +// The caller should report this error instead of continuing to probe +// other possible module paths. +type VCSError struct { + Err error +} + +func (e *VCSError) Error() string { return e.Err.Error() } + func NewRepo(vcs, remote string) (Repo, error) { type key struct { vcs string @@ -33,6 +44,9 @@ func NewRepo(vcs, remote string) (Repo, error) { } c := vcsRepoCache.Do(key{vcs, remote}, func() interface{} { repo, err := newVCSRepo(vcs, remote) + if err != nil { + err = &VCSError{err} + } return cached{repo, err} }).(cached) diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go index 003479461c..c8b133574e 100644 --- a/src/cmd/go/internal/modfetch/repo.go +++ b/src/cmd/go/internal/modfetch/repo.go @@ -237,6 +237,9 @@ func lookup(path string) (r Repo, err error) { func lookupCodeRepo(rr *get.RepoRoot) (codehost.Repo, error) { code, err := codehost.NewRepo(rr.VCS, rr.Repo) if err != nil { + if _, ok := err.(*codehost.VCSError); ok { + return nil, err + } return nil, fmt.Errorf("lookup %s: %v", rr.Root, err) } return code, nil diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 3b954f18fe..78ae83e4bf 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -14,6 +14,7 @@ import ( "strings" "cmd/go/internal/cfg" + "cmd/go/internal/modfetch/codehost" "cmd/go/internal/module" "cmd/go/internal/par" "cmd/go/internal/search" @@ -133,6 +134,9 @@ func Import(path string) (m module.Version, dir string, err error) { m, _, err = QueryPackage(path, "latest", Allowed) if err != nil { + if _, ok := err.(*codehost.VCSError); ok { + return module.Version{}, "", err + } return module.Version{}, "", &ImportMissingError{ImportPath: path} } return m, "", &ImportMissingError{ImportPath: path, Module: m} diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go index bd3141865c..3b550f1db7 100644 --- a/src/cmd/go/internal/modload/query.go +++ b/src/cmd/go/internal/modload/query.go @@ -6,6 +6,7 @@ package modload import ( "cmd/go/internal/modfetch" + "cmd/go/internal/modfetch/codehost" "cmd/go/internal/module" "cmd/go/internal/semver" "fmt" @@ -223,6 +224,11 @@ func QueryPackage(path, query string, allowed func(module.Version) bool) (module for p := path; p != "."; p = pathpkg.Dir(p) { info, err := Query(p, query, allowed) if err != nil { + if _, ok := err.(*codehost.VCSError); ok { + // A VCSError means we know where to find the code, + // we just can't. Abort search. + return module.Version{}, nil, err + } if finalErr == errMissing { finalErr = err } diff --git a/src/cmd/go/testdata/script/mod_vcs_missing.txt b/src/cmd/go/testdata/script/mod_vcs_missing.txt new file mode 100644 index 0000000000..fb146b4415 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_vcs_missing.txt @@ -0,0 +1,11 @@ +[exec:bzr] skip 'tests NOT having bzr' +[!net] skip + +env GO111MODULE=on +env GOPROXY= + +! go list launchpad.net/gocheck +stderr '"bzr": executable file not found' + +-- go.mod -- +module m From 714c141c4f2db626ea470a27cfd35f86b0c77c07 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 21:10:19 -0400 Subject: [PATCH 24/31] cmd/go: update TestGoGetUpdateWithWildcard expected behavior If you run go get -u github.com/rsc/foo/bar... then the go get command has always worked hard to make sure that it applies the wildcard after downloading rsc/foo. (If it applied the wildcard only before downloading rsc/foo, it would match nothing if you had an empty GOPATH before, and you'd still have an empty afterward, which is clearly useless.) The goal has always been that if you run the same go get command twice, the second command doesn't find anything new to do. CL 19892 worked around an "internal error" failure but broke the rule about the first command doing everything the second command would. Suppose you had github.com/rsc/foo already, with just github.com/rsc/foo/bar, and you run go get -u github.com/rsc/... The wildcard first matches github.com/rsc/foo/bar, but suppose updating the repo pulls down github.com/rsc/foo/baz, which in turn depends on the non-existent package github.com/rsc/quux. We need to reevaluate the wildcard after the download. The new pattern match refactoring makes this easier and happened to have corrected the behavior, but we missed a long test that expected the old behavior. Fix that long test. Change-Id: I088473e7a90925e5c0f9697da9554a11456ddd08 Reviewed-on: https://go-review.googlesource.com/129796 Run-TryBot: Russ Cox Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/cmd/go/go_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 3ca50bb475..da2dfd3bfb 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -4188,9 +4188,10 @@ func TestGoGetUpdateWithWildcard(t *testing.T) { tg.setenv("GOPATH", tg.path(".")) const aPkgImportPath = "github.com/tmwh/go-get-issue-14450/a" tg.run("get", aPkgImportPath) - tg.run("get", "-u", ".../") - tg.grepStderrNot("cannot find package", "did not update packages given wildcard path") + tg.runFail("get", "-u", ".../") + tg.grepStderr("cannot find package.*d-dependency/e", "should have detected e missing") + // Even though get -u failed, the source for others should be downloaded. var expectedPkgPaths = []string{ "src/github.com/tmwh/go-get-issue-14450/b", "src/github.com/tmwh/go-get-issue-14450-b-dependency/c", From 239b8f2edc70046484d5bcc5bf0cac06f8e1ace4 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 21:25:52 -0400 Subject: [PATCH 25/31] cmd/go: fix and reenable TestAccidentalGitCheckout This is an important security problem so we shouldn't disable the test. The second half was failing on case-sensitive file systems but the first half is still good. Fixes #22983. Change-Id: I437bb4c9f78eb3177aa8b619e2357b2539566ca9 Reviewed-on: https://go-review.googlesource.com/129797 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/go_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index da2dfd3bfb..ec6a72c66a 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1291,9 +1291,9 @@ func TestGetGitDefaultBranch(t *testing.T) { tg.grepStdout(`\* another-branch`, "not on correct default branch") } +// Security issue. Don't disable. See golang.org/issue/22125. func TestAccidentalGitCheckout(t *testing.T) { testenv.MustHaveExternalNetwork(t) - testenv.SkipFlaky(t, 22983) // this test might not have ever worked; see issue. if _, err := exec.LookPath("git"); err != nil { t.Skip("skipping because git binary not found") } @@ -1302,13 +1302,17 @@ func TestAccidentalGitCheckout(t *testing.T) { defer tg.cleanup() tg.parallel() tg.tempDir("src") + tg.setenv("GOPATH", tg.path(".")) tg.runFail("get", "-u", "vcs-test.golang.org/go/test1-svn-git") tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") - tg.runFail("get", "-u", "vcs-test.golang.org/go/test2-svn-git/test2main") - tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") + if _, err := os.Stat(tg.path("SrC")); err == nil { + // This case only triggers on a case-insensitive file system. + tg.runFail("get", "-u", "vcs-test.golang.org/go/test2-svn-git/test2main") + tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") + } } func TestErrorMessageForSyntaxErrorInTestGoFileSaysFAIL(t *testing.T) { From cc880de4bebf9b9242a8f5d1d7e9cf01863bb701 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 22:32:22 -0400 Subject: [PATCH 26/31] cmd/go: allow 'go run x.go' to use nearby internal imports in module mode In GOPATH mode the rule has always been that 'go run x.go' can import whatever the package in x.go's directory would be able to import. Apply the same rule here. The bad import path was triggering other mysterious errors during 'go run' in other circumstances. Setting it correctly fixes those too. Fixes #26046. Fixes #27022. Change-Id: I0a9b0a154a20f48add5a199da85572e7ffe0cde4 Reviewed-on: https://go-review.googlesource.com/129798 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/load/pkg.go | 56 ++++++++++--------- src/cmd/go/internal/modload/init.go | 1 + src/cmd/go/internal/modload/load.go | 22 ++++++++ .../go/testdata/script/mod_run_internal.txt | 46 +++++++++++++++ src/cmd/go/testdata/script/mod_run_path.txt | 15 +++++ 5 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_run_internal.txt create mode 100644 src/cmd/go/testdata/script/mod_run_path.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 43887b0008..ec2fa730c6 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -39,6 +39,7 @@ var ( ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary ModInfoProg func(info string) []byte // wrap module info in .go code for binary ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files + ModDirImportPath func(string) string // return effective import path for directory ) var IgnoreImports bool // control whether we ignore imports in packages @@ -567,11 +568,11 @@ func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPo } // Checked on every import because the rules depend on the code doing the importing. - if perr := disallowInternal(srcDir, parentPath, p, stk); perr != p { + if perr := disallowInternal(srcDir, parent, parentPath, p, stk); perr != p { return setErrorPos(perr, importPos) } if mode&ResolveImport != 0 { - if perr := disallowVendor(srcDir, parentPath, origPath, p, stk); perr != p { + if perr := disallowVendor(srcDir, parent, parentPath, origPath, p, stk); perr != p { return setErrorPos(perr, importPos) } } @@ -932,7 +933,7 @@ func reusePackage(p *Package, stk *ImportStack) *Package { // is allowed to import p. // If the import is allowed, disallowInternal returns the original package p. // If not, it returns a new package containing just an appropriate error. -func disallowInternal(srcDir, importerPath string, p *Package, stk *ImportStack) *Package { +func disallowInternal(srcDir string, importer *Package, importerPath string, p *Package, stk *ImportStack) *Package { // golang.org/s/go14internal: // An import of a path containing the element “internal” // is disallowed if the importing code is outside the tree @@ -990,10 +991,16 @@ func disallowInternal(srcDir, importerPath string, p *Package, stk *ImportStack) return p } } else { - // p is in a module, so make it available based on the import path instead + // p is in a module, so make it available based on the importer's import path instead // of the file path (https://golang.org/issue/23970). - parent := p.ImportPath[:i] - if str.HasPathPrefix(importerPath, parent) { + if importerPath == "." { + // The importer is a list of command-line files. + // Pretend that the import path is the import path of the + // directory containing them. + importerPath = ModDirImportPath(importer.Dir) + } + parentOfInternal := p.ImportPath[:i] + if str.HasPathPrefix(importerPath, parentOfInternal) { return p } } @@ -1031,7 +1038,7 @@ func findInternal(path string) (index int, ok bool) { // is allowed to import p as path. // If the import is allowed, disallowVendor returns the original package p. // If not, it returns a new package containing just an appropriate error. -func disallowVendor(srcDir, importerPath, path string, p *Package, stk *ImportStack) *Package { +func disallowVendor(srcDir string, importer *Package, importerPath, path string, p *Package, stk *ImportStack) *Package { // The stack includes p.ImportPath. // If that's the only thing on the stack, we started // with a name given on the command line, not an @@ -1040,26 +1047,18 @@ func disallowVendor(srcDir, importerPath, path string, p *Package, stk *ImportSt return p } - if p.Standard && ModPackageModuleInfo != nil && importerPath != "" { - // Modules must not import vendor packages in the standard library, - // but the usual vendor visibility check will not catch them - // because the module loader presents them with an ImportPath starting - // with "golang_org/" instead of "vendor/". - if mod := ModPackageModuleInfo(importerPath); mod != nil { - dir := p.Dir - if relDir, err := filepath.Rel(p.Root, p.Dir); err == nil { - dir = relDir - } - if _, ok := FindVendor(filepath.ToSlash(dir)); ok { - perr := *p - perr.Error = &PackageError{ - ImportStack: stk.Copy(), - Err: "use of vendored package " + path + " not allowed", - } - perr.Incomplete = true - return &perr - } + // Modules must not import vendor packages in the standard library, + // but the usual vendor visibility check will not catch them + // because the module loader presents them with an ImportPath starting + // with "golang_org/" instead of "vendor/". + if p.Standard && !importer.Standard && strings.HasPrefix(p.ImportPath, "golang_org") { + perr := *p + perr.Error = &PackageError{ + ImportStack: stk.Copy(), + Err: "use of vendored package " + path + " not allowed", } + perr.Incomplete = true + return &perr } if perr := disallowVendorVisibility(srcDir, p, stk); perr != p { @@ -1991,6 +1990,11 @@ func GoFilesPackage(gofiles []string) *Package { } bp, err := ctxt.ImportDir(dir, 0) + if ModDirImportPath != nil { + // Use the effective import path of the directory + // for deciding visibility during pkg.load. + bp.ImportPath = ModDirImportPath(dir) + } pkg := new(Package) pkg.Internal.Local = true pkg.Internal.CmdlineFiles = true diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go index 169bb5fdb6..f995bad13b 100644 --- a/src/cmd/go/internal/modload/init.go +++ b/src/cmd/go/internal/modload/init.go @@ -175,6 +175,7 @@ func Init() { load.ModPackageBuildInfo = PackageBuildInfo load.ModInfoProg = ModInfoProg load.ModImportFromFiles = ImportFromFiles + load.ModDirImportPath = DirImportPath search.SetModRoot(ModRoot) } diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go index 5ca2ed2d10..285daa8f4f 100644 --- a/src/cmd/go/internal/modload/load.go +++ b/src/cmd/go/internal/modload/load.go @@ -248,6 +248,28 @@ func ImportFromFiles(gofiles []string) { WriteGoMod() } +// DirImportPath returns the effective import path for dir, +// provided it is within the main module, or else returns ".". +func DirImportPath(dir string) string { + if !filepath.IsAbs(dir) { + dir = filepath.Join(cwd, dir) + } else { + dir = filepath.Clean(dir) + } + + if dir == ModRoot { + return Target.Path + } + if strings.HasPrefix(dir, ModRoot+string(filepath.Separator)) { + suffix := filepath.ToSlash(dir[len(ModRoot):]) + if strings.HasPrefix(suffix, "/vendor/") { + return strings.TrimPrefix(suffix, "/vendor/") + } + return Target.Path + suffix + } + return "." +} + // LoadBuildList loads and returns the build list from go.mod. // The loading of the build list happens automatically in ImportPaths: // LoadBuildList need only be called if ImportPaths is not diff --git a/src/cmd/go/testdata/script/mod_run_internal.txt b/src/cmd/go/testdata/script/mod_run_internal.txt new file mode 100644 index 0000000000..653ad282be --- /dev/null +++ b/src/cmd/go/testdata/script/mod_run_internal.txt @@ -0,0 +1,46 @@ +env GO111MODULE=on + +go list -e -f '{{.Incomplete}}' runbad1.go +stdout true +! go run runbad1.go +stderr 'use of internal package m/x/internal not allowed' + +go list -e -f '{{.Incomplete}}' runbad2.go +stdout true +! go run runbad2.go +stderr 'use of internal package m/x/internal/y not allowed' + +go list -e -f '{{.Incomplete}}' runok.go +stdout false +go run runok.go + +-- go.mod -- +module m + +-- x/internal/internal.go -- +package internal + +-- x/internal/y/y.go -- +package y + +-- internal/internal.go -- +package internal + +-- internal/z/z.go -- +package z + +-- runbad1.go -- +package main +import _ "m/x/internal" +func main() {} + +-- runbad2.go -- +package main +import _ "m/x/internal/y" +func main() {} + +-- runok.go -- +package main +import _ "m/internal" +import _ "m/internal/z" +func main() {} diff --git a/src/cmd/go/testdata/script/mod_run_path.txt b/src/cmd/go/testdata/script/mod_run_path.txt new file mode 100644 index 0000000000..4369ee4131 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_run_path.txt @@ -0,0 +1,15 @@ +# Test that go run does not get confused by conflict +# between go.mod's module path and what you'd +# expect from GOPATH. golang.org/issue/26046. + +env GO111MODULE=on + +cd $GOPATH/src/example.com/hello +go run main.go + +-- $GOPATH/src/example.com/hello/go.mod -- +module example.com/hello/v2 + +-- $GOPATH/src/example.com/hello/main.go -- +package main +func main() {} From 5d750db02415e9b3ff5624ecbaf1f5d141145bde Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 Aug 2018 23:08:24 -0400 Subject: [PATCH 27/31] cmd/go: fix go mod tidy crash on empty module Fixes #27066. Change-Id: Iede4385ad86b42d7d90814965b161a7e64d29833 Reviewed-on: https://go-review.googlesource.com/129799 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/modcmd/tidy.go | 3 ++- src/cmd/go/testdata/script/mod_tidy.txt | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/modcmd/tidy.go b/src/cmd/go/internal/modcmd/tidy.go index 54f47e764f..f2063a9ea6 100644 --- a/src/cmd/go/internal/modcmd/tidy.go +++ b/src/cmd/go/internal/modcmd/tidy.go @@ -44,10 +44,11 @@ func runTidy(cmd *base.Command, args []string) { // LoadALL adds missing modules. // Remove unused modules. - used := map[module.Version]bool{modload.Target: true} + used := make(map[module.Version]bool) for _, pkg := range modload.LoadALL() { used[modload.PackageModule(pkg)] = true } + used[modload.Target] = true // note: LoadALL initializes Target inGoMod := make(map[string]bool) for _, r := range modload.ModFile().Require { diff --git a/src/cmd/go/testdata/script/mod_tidy.txt b/src/cmd/go/testdata/script/mod_tidy.txt index 86434af7f3..449aa073a7 100644 --- a/src/cmd/go/testdata/script/mod_tidy.txt +++ b/src/cmd/go/testdata/script/mod_tidy.txt @@ -10,6 +10,10 @@ go list -m all stdout '^w.1 v1.2.0' stdout '^z.1 v1.2.0' +# empty tidy should not crash +cd triv +go mod tidy + -- go.mod -- module m @@ -55,3 +59,6 @@ module z -- z/sub/sub.go -- package sub + +-- triv/go.mod -- +module triv From c5046bca7b06ab73225040abdc53430511049b56 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sat, 18 Aug 2018 00:41:52 -0400 Subject: [PATCH 28/31] cmd/go/internal/modconv: fix TestConvertLegacyConfig expectations It was a bug to find that commit in the Masterminds/semver repo. It's not part of the main repo but only part of an unmerged pull request. The code was updated to try not to look at unmerged pull requests, but the test was not. Worse, whether the code succeeds at not looking at unmerged pull requests apparently depends on the git version. Sigh. Fixes #26754. Fixes #27043. Change-Id: Ib9e07f565906de4f1169244911a258396688f14d Reviewed-on: https://go-review.googlesource.com/129800 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/internal/modconv/convert_test.go | 50 ++++++++++++--------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/cmd/go/internal/modconv/convert_test.go b/src/cmd/go/internal/modconv/convert_test.go index f430380871..ad27abb8ef 100644 --- a/src/cmd/go/internal/modconv/convert_test.go +++ b/src/cmd/go/internal/modconv/convert_test.go @@ -61,28 +61,36 @@ func TestConvertLegacyConfig(t *testing.T) { vers string gomod string }{ - { - // Gopkg.lock parsing. - "github.com/golang/dep", "v0.4.0", - `module github.com/golang/dep + /* + Different versions of git seem to find or not find + github.com/Masterminds/semver's a93e51b5a57e, + which is an unmerged pull request. + We'd rather not provide access to unmerged pull requests, + so the line is removed from the golden file here, + but some git commands still find it somehow. - require ( - github.com/Masterminds/semver v0.0.0-20170726230514-a93e51b5a57e - github.com/Masterminds/vcs v1.11.1 - github.com/armon/go-radix v0.0.0-20160115234725-4239b77079c7 - github.com/boltdb/bolt v1.3.1 - github.com/go-yaml/yaml v0.0.0-20170407172122-cd8b52f8269e - github.com/golang/protobuf v0.0.0-20170901042739-5afd06f9d81a - github.com/jmank88/nuts v0.3.0 - github.com/nightlyone/lockfile v0.0.0-20170707060451-e83dc5e7bba0 - github.com/pelletier/go-toml v0.0.0-20171218135716-b8b5e7696574 - github.com/pkg/errors v0.8.0 - github.com/sdboyer/constext v0.0.0-20170321163424-836a14457353 - golang.org/x/net v0.0.0-20170828231752-66aacef3dd8a - golang.org/x/sync v0.0.0-20170517211232-f52d1811a629 - golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea - )`, - }, + { + // Gopkg.lock parsing. + "github.com/golang/dep", "v0.4.0", + `module github.com/golang/dep + + require ( + github.com/Masterminds/vcs v1.11.1 + github.com/armon/go-radix v0.0.0-20160115234725-4239b77079c7 + github.com/boltdb/bolt v1.3.1 + github.com/go-yaml/yaml v0.0.0-20170407172122-cd8b52f8269e + github.com/golang/protobuf v0.0.0-20170901042739-5afd06f9d81a + github.com/jmank88/nuts v0.3.0 + github.com/nightlyone/lockfile v0.0.0-20170707060451-e83dc5e7bba0 + github.com/pelletier/go-toml v0.0.0-20171218135716-b8b5e7696574 + github.com/pkg/errors v0.8.0 + github.com/sdboyer/constext v0.0.0-20170321163424-836a14457353 + golang.org/x/net v0.0.0-20170828231752-66aacef3dd8a + golang.org/x/sync v0.0.0-20170517211232-f52d1811a629 + golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea + )`, + }, + */ // TODO: https://github.com/docker/distribution uses vendor.conf From bf80e3b5640447d213f68e0423ad37f34bf7a222 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sat, 18 Aug 2018 00:42:58 -0400 Subject: [PATCH 29/31] cmd/go: fix module get -insecure Need to actually use the flag for it to take effect. Fixes #27049. Change-Id: I57227b45f46f9dd67ecbf87c11bb2d08124bcfa0 Reviewed-on: https://go-review.googlesource.com/129801 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/go/go_test.go | 45 ++++++++++++++++++++-------- src/cmd/go/internal/modfetch/repo.go | 12 ++++++-- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index ec6a72c66a..85cae90f87 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -3532,24 +3532,43 @@ func TestImportLocal(t *testing.T) { } func TestGoGetInsecure(t *testing.T) { - testenv.MustHaveExternalNetwork(t) + test := func(t *testing.T, modules bool) { + testenv.MustHaveExternalNetwork(t) - tg := testgo(t) - defer tg.cleanup() - tg.makeTempdir() - tg.setenv("GOPATH", tg.path(".")) - tg.failSSH() + tg := testgo(t) + defer tg.cleanup() + tg.makeTempdir() + tg.failSSH() - const repo = "insecure.go-get-issue-15410.appspot.com/pkg/p" + if modules { + tg.setenv("GOPATH", tg.path("gp")) + tg.tempFile("go.mod", "module m") + tg.cd(tg.path(".")) + tg.setenv("GO111MODULE", "on") + } else { + tg.setenv("GOPATH", tg.path(".")) + tg.setenv("GO111MODULE", "off") + } - // Try go get -d of HTTP-only repo (should fail). - tg.runFail("get", "-d", repo) + const repo = "insecure.go-get-issue-15410.appspot.com/pkg/p" - // Try again with -insecure (should succeed). - tg.run("get", "-d", "-insecure", repo) + // Try go get -d of HTTP-only repo (should fail). + tg.runFail("get", "-d", repo) - // Try updating without -insecure (should fail). - tg.runFail("get", "-d", "-u", "-f", repo) + // Try again with -insecure (should succeed). + tg.run("get", "-d", "-insecure", repo) + + // Try updating without -insecure (should fail). + tg.runFail("get", "-d", "-u", "-f", repo) + + if modules { + tg.run("list", "-m", "...") + tg.grepStdout("insecure.go-get-issue", "should find insecure module") + } + } + + t.Run("gopath", func(t *testing.T) { test(t, false) }) + t.Run("modules", func(t *testing.T) { test(t, true) }) } func TestGoGetUpdateInsecure(t *testing.T) { diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go index c8b133574e..0ea8c1f0e3 100644 --- a/src/cmd/go/internal/modfetch/repo.go +++ b/src/cmd/go/internal/modfetch/repo.go @@ -216,7 +216,11 @@ func lookup(path string) (r Repo, err error) { return lookupProxy(path) } - rr, err := get.RepoRootForImportPath(path, get.PreferMod, web.Secure) + security := web.Secure + if get.Insecure { + security = web.Insecure + } + rr, err := get.RepoRootForImportPath(path, get.PreferMod, security) if err != nil { // We don't know where to find code for a module with this path. return nil, err @@ -257,7 +261,11 @@ func ImportRepoRev(path, rev string) (Repo, *RevInfo, error) { // Note: Because we are converting a code reference from a legacy // version control system, we ignore meta tags about modules // and use only direct source control entries (get.IgnoreMod). - rr, err := get.RepoRootForImportPath(path, get.IgnoreMod, web.Secure) + security := web.Secure + if get.Insecure { + security = web.Insecure + } + rr, err := get.RepoRootForImportPath(path, get.IgnoreMod, security) if err != nil { return nil, nil, err } From edf81050a8cfdcdaff47664f51c52441b754480e Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sat, 18 Aug 2018 14:50:50 -0400 Subject: [PATCH 30/31] doc/go1.11: Delve 1.1.0 added support for method calls Change-Id: I5f887f9831378cf76f5a9f447f481ea24c63f390 Reviewed-on: https://go-review.googlesource.com/129803 Reviewed-by: Brad Fitzpatrick --- doc/go1.11.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/go1.11.html b/doc/go1.11.html index 89649f34c0..fae1c5ff14 100644 --- a/doc/go1.11.html +++ b/doc/go1.11.html @@ -273,9 +273,8 @@ func f(v interface{}) { This is useful, for example, to call String methods when paused at a breakpoint. - - This is currently only supported by Delve. + This is currently only supported by Delve (version 1.1.0 and up).

Test

From e0faedbb5344eb6f8f704005fe88961cdc6cf5f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 19 Aug 2018 15:58:35 +0100 Subject: [PATCH 31/31] cmd/go: add missing newlines in printf formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are all errors given by module-aware cmd/go, so they must end with a newline. It looks like they were omitted by mistake. Fixes #27081. Change-Id: I19b5803bb48a6d5dd52e857f483278fe20fe246b Reviewed-on: https://go-review.googlesource.com/129780 Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/go/internal/modfile/rule.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cmd/go/internal/modfile/rule.go b/src/cmd/go/internal/modfile/rule.go index f669575c86..e11f0a6e31 100644 --- a/src/cmd/go/internal/modfile/rule.go +++ b/src/cmd/go/internal/modfile/rule.go @@ -250,7 +250,7 @@ func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, f arrow = 1 } if len(args) < arrow+2 || len(args) > arrow+3 || args[arrow] != "=>" { - fmt.Fprintf(errs, "%s:%d: usage: %s module/path [v1.2.3] => other/module v1.4\n\t or %s module/path [v1.2.3] => ../local/directory", f.Syntax.Name, line.Start.Line, verb, verb) + fmt.Fprintf(errs, "%s:%d: usage: %s module/path [v1.2.3] => other/module v1.4\n\t or %s module/path [v1.2.3] => ../local/directory\n", f.Syntax.Name, line.Start.Line, verb, verb) return } s, err := parseString(&args[0]) @@ -287,11 +287,11 @@ func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, f nv := "" if len(args) == arrow+2 { if !IsDirectoryPath(ns) { - fmt.Fprintf(errs, "%s:%d: replacement module without version must be directory path (rooted or starting with ./ or ../)", f.Syntax.Name, line.Start.Line) + fmt.Fprintf(errs, "%s:%d: replacement module without version must be directory path (rooted or starting with ./ or ../)\n", f.Syntax.Name, line.Start.Line) return } if filepath.Separator == '/' && strings.Contains(ns, `\`) { - fmt.Fprintf(errs, "%s:%d: replacement directory appears to be Windows path (on a non-windows system)", f.Syntax.Name, line.Start.Line) + fmt.Fprintf(errs, "%s:%d: replacement directory appears to be Windows path (on a non-windows system)\n", f.Syntax.Name, line.Start.Line) return } } @@ -303,7 +303,7 @@ func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, f return } if IsDirectoryPath(ns) { - fmt.Fprintf(errs, "%s:%d: replacement module directory path %q cannot have version", f.Syntax.Name, line.Start.Line, ns) + fmt.Fprintf(errs, "%s:%d: replacement module directory path %q cannot have version\n", f.Syntax.Name, line.Start.Line, ns) return } }