In some cases walkFn is being called after the fastWalk function has
returned. This often happens when an error was encountered early on in
scanning directories with many entries.
It is caused by fastWalk not waiting for its workers to complete their
work. A sync.WaitGroup is used to wait for all workers to finish when
the function returns.
Updates golang/go#16399
Change-Id: I695d30c18e4878b789520b9d8a650f9688d896ac
Reviewed-on: https://go-review.googlesource.com/40092
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Change the shouldTraverse function to no longer keep a global map of
which inodes it's seen. Instead, whenever a symlink is seen for a path
name, check every directory entry in that path name and see if any are
the same inode as the current one, detecting any loop just from the
name itself.
More details of why the test was flaky are in the bug.
Fixesgolang/go#18142
Change-Id: I869f7a13d130c63d78b7af81802a16c4b4b2f3bd
Reviewed-on: https://go-review.googlesource.com/37947
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change remove an unnecessary string conversion of
a variable that is already a string.
Change-Id: I64d9a6c059276fa22f1be9f2dba02bbeb63cb8fb
Reviewed-on: https://go-review.googlesource.com/35560
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL only copies files and updates build tags.
Substantive changes will come in follow-ups.
This is a workaround for git's lack of rename/copy tracking.
Tested with go1.6, go1.7, and tip (go1.8).
Change-Id: Id88a05273fb963586b228d5e5dfacab32133a960
Reviewed-on: https://go-review.googlesource.com/32630
Reviewed-by: Robert Griesemer <gri@golang.org>
This allows the caller to indicate they want certain
import paths to sort into another group after 3rd-party
imports when added by goimports. For example, running
'goimports -local example.com/' might produce
import (
"database/sql"
"io"
"strconv"
"golang.org/x/net/context"
"example.com/foo/bar"
"example.com/foo/baz"
)
Resolvesgolang/go#12420
Change-Id: If6d88599f6cca2f102313bce95ba6ac46ffec1fe
Reviewed-on: https://go-review.googlesource.com/25145
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
filepath.Rel will always fail if one parameter starts with
drive letter and the other does not. Make both filepath.Rel
parameters absolute paths, to give it chance to succeed.
Fixes broken tools build on windows.
Change-Id: Ibefcfe283f28977503323f01bc3a698478227f84
Reviewed-on: https://go-review.googlesource.com/25120
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This brings goimports from 160ms to 100ms on my laptop, and under 50ms
on my Linux machine.
Using cmd/trace, I noticed that filepath.Walk is inherently slow.
See https://golang.org/issue/16399 for details.
Instead, this CL introduces a new (private) filepath.Walk
implementation, optimized for speed and avoiding unnecessary work.
In addition to avoid an Lstat per file, it also reads directories
concurrently. The old goimports code did that too, but now that logic
is removed from goimports and the code is simplified.
This also adds some profiling command line flags to goimports that I
found useful.
Updates golang/go#16367 (goimports is slow)
Updates golang/go#16399 (filepath.Walk is slow)
Change-Id: I708d570cbaad3fa9ad75a12054f5a932ee159b84
Reviewed-on: https://go-review.googlesource.com/25001
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This means a directory /gopath/src/foo/Go-Bar is considered as maybe
containing package "bar" or even "gobar".
Also, more tests.
Fixesgolang/go#16402
Change-Id: I14208d738e3a081cb6d9bcd83d777280e118f4e7
Reviewed-on: https://go-review.googlesource.com/25030
Reviewed-by: Andrew Gerrand <adg@golang.org>
These are mostly comments for CL 24941 that didn't
get sent quickly enough.
strings.Trim(`\"`) works, but by accident.
It trims all leading and trailing "s and \s,
but there are never leading or trailing \s.
Semantic line breaks and punctuation cleanup.
The reflow of comments in the pkgName == "main" if
block is to silence this spurious vet failure:
fix.go:247: +build comment must appear before package clause and be followed by a blank line
Plain sync.Once values are enough.
Change-Id: I241f3025031b6f21605da78ea52066713a203327
Reviewed-on: https://go-review.googlesource.com/24983
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Each $GOPATH entry may have a file $GOPATH/src/.goimportsignore which
may contain blank lines, #comment lines, or lines naming a directory
relative to the configuration file to ignore when scanning. No
globbing or regex patterns are allowed.
Updates golang/go#16367 (goimports speed)
Fixesgolang/go#16386 (add mechanism to ignore directories)
Change-Id: I8f1a88ae6c4d0ed3075444d70aec3e2228c5ce6a
Reviewed-on: https://go-review.googlesource.com/24971
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When goimports was run on a file like:
package main
import (
"example.net/foo"
"example.net/bar"
)
var _, _ = foo.Foo, bar.Bar
... even though there looks to be no work to do, it still needs to
verify that "example.net/foo" is really package "foo" (even though it
looks like it) and "example.net/bar" is really package
"bar". (Packages in the standard library are hard-coded since the
previous commit and not verified for consistency since they're always consistent)
To do that verification for non-std packages, go/build.Import was
being used before, but Import reads all files in the directory to make
sure they're consistent. That's unnecessary. Instead, stop after the
first file. If example.net/foo has foo.go with "package foo" and
just_kidding.go with "package other", we never read that far to find
the inconsistency. Oh well. Prefer speed.
Updates golang/go#16367
Change-Id: I9fc3fefbee0e8a6bc287bf2a565257fb9523fd5c
Reviewed-on: https://go-review.googlesource.com/24948
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
I felt the burn of my laptop on my legs, spinning away while processing
goimports, and felt that it was time to make goimports great again.
Over the past few years goimports fell into a slow state of disrepair
with too many feature additions and no attention to the performance
death by a thousand cuts. This was particularly terrible on OS X with
its lackluster filesystem buffering.
This CL makes goimports stronger, together with various optimizations
and more visibility into what goimports is doing.
* adds more internal documentation
* avoids scanning $GOPATH for answers when running goimports on a file
under $GOROOT (for Go core hackers)
* don't read all $GOROOT & $GOPATH directories' Go code looking for
their package names until much later. Require the package name of
missing imports to be present in the last two directory path
components. Then only try importing them in order from best to
worst (shortest to longest, as before), so we can stop early.
* when adding imports, add names to imports when the imported package name
doesn't match the baes of its import path. For example:
import foo "example.net/foo/v1"
* don't read all *.go files in a package directory once the first file
in a directory has revealed itself to be a package we're not looking
for. For example, if we're looking for the right "client" for "client.Foo",
we used to consider a directory "bar/client" as a candidate and read
all 50 of its *.go files instead of stopping after its first *.go
file had a "package main" line.
* add some fast paths to remove allocations
* add some fast paths to remove disk I/O when looking up the base
package name of a standard library import (of existing imports in a
file, which are very common)
* adds a special case for import "C", to avoid some disk I/O.
* add a -verbose flag to goimports for debugging
On my Mac laptop with a huge $GOPATH, with a test file like:
package foo
import (
"fmt"
"net/http"
)
/*
*/
import "C"
var _ = cloudbilling.New
var _ = http.NewRequest
var _ = client.New
... this took like 10 seconds before, and now 1.3 seconds. (Still
slow; disk-based caching can come later)
Updates golang/go#16367 (goimports is slow)
Updates golang/go#16384 (refactor TestRename is broken on Windows)
Change-Id: I97e85d3016afc9f2ad5501f97babad30c7989183
Reviewed-on: https://go-review.googlesource.com/24941
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In Go 1.7, math/rand.Read was added. Previously, the only package
containing "rand.Read" was "crypto/rand".
goimports was updated to know that, and zstdlib.go contains a note
that it's ambiguous:
"rand.Perm": "math/rand",
"rand.Prime": "crypto/rand",
"rand.Rand": "math/rand",
// "rand.Read" is ambiguous
"rand.Reader": "crypto/rand",
"rand.Seed": "math/rand",
"rand.Source": "math/rand",
The intention originally was that such ambiguous things would
never be resolved, even randomly.
But a later change added support for build.Default.SrcDirs, which
meant GOROOT was also searched for ambiguous things. Or maybe I forget
the history.
In any case, when goimports tried to resolve "rand.Read", the
findImportStdlib check was returning nothing, which lead to the
$GOROOT being searched, where math/rand was picked by chance. That's a
dangerous default when the intentional might've been crypto/rand.
Special case it and prefer crypto/rand if there's no more specific
clue either way.
Change-Id: Ib5f8f297f72fa309d5ca9b15a37493df2e17567c
Reviewed-on: https://go-review.googlesource.com/24847
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The go tool will import a package with a import path that traverses
symlinks with no problems, but goimports would remove that import
because it would fail to recognize the package as existent.
Fixesgolang/go#14845
Note: if the file you are currently processing also has a name inside
the symlink, you might have to use the "long" path for vendoring to
work, as it wouldn't be recognized as "deeper" than the vendor folder
otherwise. For example in this tree:
.
├── myfile.go
└── x
├── vendor
│ └── mypkg
│ └── foo.go
└── y -> ..
If myfile.go imports mypkg, you will have to process it as
./x/y/myfile.go, not ./myfile.go.
Change-Id: Ic8f41baed3f28d4e9b813160d91aef09ece1fc9f
Reviewed-on: https://go-review.googlesource.com/23803
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since CL 17728 goimports was using the file path to try finding packages
inside its /vendor directory. This CL builds upon this previous work to
also consider packages inside /vendor when converting an import path to
a package name.
Change-Id: I173472ac51e3f681d758ec52add60ffc04c62c1a
Reviewed-on: https://go-review.googlesource.com/22020
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The prior commit fixed this test for tip (1.7) but broke it for 1.6.
Change-Id: Ic39d215ac8bc60dba4b66201f1f16713b2e02d9c
Reviewed-on: https://go-review.googlesource.com/21144
Reviewed-by: Alan Donovan <adonovan@google.com>
go vet detected that the value of pkgIndexOnce
is copied through assignments in the test.
This patch fixes that by converting it to *sync.Once instead.
Change-Id: I90b2252942625b2f8a93eb5da24d16c6a8a56e4e
Reviewed-on: https://go-review.googlesource.com/20910
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
These changes will need to be reverted once we use
vendoring in the stdlib again, but it's trivial to
do so when the time comes.
TBR=adonovan
Fixes#14823.
Change-Id: I2173c48d5466874492affc679332f6484bf96592
Reviewed-on: https://go-review.googlesource.com/20695
Reviewed-by: Robert Griesemer <gri@golang.org>
DeleteNamedImport deletes the import with a given
name and path from a parsed file, if present.
imports uses this function to delete named imports.
Fixesgolang/go#8149.
Change-Id: I84539d5627191c45f06db2d008507aee4d3b1eb2
Reviewed-on: https://go-review.googlesource.com/19951
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Editor modes that invoke the goimports command on temporary copies
of actual source files will need to invoke goimports -srcdir now to say
where the real source directory is. Otherwise goimports will not consider
vendored or internal packages when looking for new imports.
In lieu of a test for cmd/goimports (because it has no tests),
a command transcript:
$ cd /tmp
$ cat x.go
package p
var _ = hpack.HuffmanDecode
$
$ GOPATH= goimports < x.go
package p
var _ = hpack.HuffmanDecode
$ GOPATH= goimports x.go
package p
var _ = hpack.HuffmanDecode
$
But with the new flag:
$ GOPATH= goimports -srcdir $GOROOT/src/math < x.go
package p
import "golang.org/x/net/http2/hpack"
var _ = hpack.HuffmanDecode
$ GOPATH= goimports -srcdir $GOROOT/src/math x.go
package p
import "golang.org/x/net/http2/hpack"
var _ = hpack.HuffmanDecode
$
The tests in this CL and the above transcript assume that
$GOROOT/src/vendor/golang.org/x/net/http2/hpack exists.
It did in 40a26c9, but it does not today.
It will again soon (once Go 1.7 opens).
For golang/go#12278 (original request).
Change-Id: I27b136041f54edcde4bf474215b48ebb0417f34d
Reviewed-on: https://go-review.googlesource.com/17728
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
AddImport and AddNamedImport attempt to place new
imports in roughly the correct place--and thus the
correct group--by matching prefixes. Matching prefixes
byte-by-byte led to "regexp" being grouped with "rsc.io/p".
Instead, match prefixes by segments.
Fixesgolang/go#9961.
Change-Id: I52b7c58a9a2fbe85c2b5297e50c87d409364bda3
Reviewed-on: https://go-review.googlesource.com/8090
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The previous logic would descend into "_"-prefixed names, "testdata" folders,
and skipped folders beginning with a digit.
Fixesgolang/go#8958.
LGTM=bradfitz
R=golang-codereviews, dave, bradfitz
CC=golang-codereviews
https://golang.org/cl/167000043
