diff --git a/cmd/dep/testdata/harness_tests/ensure/add/desync/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/add/desync/final/Gopkg.lock index 7d186b5b67..6872265e9a 100644 --- a/cmd/dep/testdata/harness_tests/ensure/add/desync/final/Gopkg.lock +++ b/cmd/dep/testdata/harness_tests/ensure/add/desync/final/Gopkg.lock @@ -19,7 +19,7 @@ [[projects]] branch = "master" - digest = "1:0dba41ffdf62b10cbbd79009edceb0eaf635031e854fb456fdd5be154802f8d3" + digest = "1:c683a5f3a422ecd929d76af63de214178e6caa41cbfdf4522112f7f9173d0cae" name = "github.com/sdboyer/deptesttres" packages = ["."] pruneopts = "" diff --git a/cmd/dep/testdata/harness_tests/ensure/add/exists-imports/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/add/exists-imports/final/Gopkg.lock index 39b9ac12c9..9102af515f 100644 --- a/cmd/dep/testdata/harness_tests/ensure/add/exists-imports/final/Gopkg.lock +++ b/cmd/dep/testdata/harness_tests/ensure/add/exists-imports/final/Gopkg.lock @@ -3,7 +3,7 @@ [[projects]] branch = "master" - digest = "1:0dba41ffdf62b10cbbd79009edceb0eaf635031e854fb456fdd5be154802f8d3" + digest = "1:c683a5f3a422ecd929d76af63de214178e6caa41cbfdf4522112f7f9173d0cae" name = "github.com/sdboyer/deptesttres" packages = ["."] pruneopts = "" diff --git a/cmd/dep/testdata/harness_tests/ensure/add/exists-manifest-constraint/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/add/exists-manifest-constraint/final/Gopkg.lock index 0e7a36581a..1074b6a1f8 100644 --- a/cmd/dep/testdata/harness_tests/ensure/add/exists-manifest-constraint/final/Gopkg.lock +++ b/cmd/dep/testdata/harness_tests/ensure/add/exists-manifest-constraint/final/Gopkg.lock @@ -11,7 +11,7 @@ [[projects]] branch = "master" - digest = "1:0dba41ffdf62b10cbbd79009edceb0eaf635031e854fb456fdd5be154802f8d3" + digest = "1:c683a5f3a422ecd929d76af63de214178e6caa41cbfdf4522112f7f9173d0cae" name = "github.com/sdboyer/deptesttres" packages = ["."] pruneopts = "" diff --git a/gps/verify/digest.go b/gps/verify/digest.go index 413879fe5a..ba8add7da6 100644 --- a/gps/verify/digest.go +++ b/gps/verify/digest.go @@ -14,6 +14,7 @@ import ( "io" "os" "path/filepath" + "sort" "strconv" "strings" @@ -152,14 +153,10 @@ type dirWalkClosure struct { // // Other than the `vendor` and VCS directories mentioned above, the calculated // hash includes the pathname to every discovered file system node, whether it -// is an empty directory, a non-empty directory, empty file, non-empty file, or -// symbolic link. If a symbolic link, the referent name is included. If a -// non-empty file, the file's contents are included. If a non-empty directory, -// the contents of the directory are included. +// is an empty directory, a non-empty directory, an empty file, or a non-empty file. // -// While filepath.Walk could have been used, that standard library function -// skips symbolic links, and for now, we want the hash to include the symbolic -// link referents. +// Symbolic links are excluded, as they are not considered valid elements in the +// definition of a Go module. func DigestFromDirectory(osDirname string) (VersionedDigest, error) { osDirname = filepath.Clean(osDirname) @@ -173,9 +170,14 @@ func DigestFromDirectory(osDirname string) (VersionedDigest, error) { someHash: sha256.New(), } - err := DirWalk(osDirname, func(osPathname string, info os.FileInfo, err error) error { + err := filepath.Walk(osDirname, func(osPathname string, info os.FileInfo, err error) error { if err != nil { - return err // DirWalk received an error during initial Lstat + return err + } + + // Completely ignore symlinks. + if info.Mode()&os.ModeSymlink != 0 { + return nil } var osRelative string @@ -207,11 +209,9 @@ func DigestFromDirectory(osDirname string) (VersionedDigest, error) { switch { case modeType&os.ModeDir > 0: mt = os.ModeDir - // DirWalkFunc itself does not need to enumerate children, because - // DirWalk will do that for us. + // This func does not need to enumerate children, because + // filepath.Walk will do that for us. shouldSkip = true - case modeType&os.ModeSymlink > 0: - mt = os.ModeSymlink case modeType&os.ModeNamedPipe > 0: mt = os.ModeNamedPipe shouldSkip = true @@ -225,9 +225,8 @@ func DigestFromDirectory(osDirname string) (VersionedDigest, error) { // Write the relative pathname to hash because the hash is a function of // the node names, node types, and node contents. Added benefit is that - // empty directories, named pipes, sockets, devices, and symbolic links - // will also affect final hash value. Use `filepath.ToSlash` to ensure - // relative pathname is os-agnostic. + // empty directories, named pipes, sockets, and devices. Use + // `filepath.ToSlash` to ensure relative pathname is os-agnostic. writeBytesWithNull(closure.someHash, []byte(filepath.ToSlash(osRelative))) binary.LittleEndian.PutUint32(closure.someModeBytes, uint32(mt)) // encode the type of mode @@ -237,15 +236,6 @@ func DigestFromDirectory(osDirname string) (VersionedDigest, error) { return nil // nothing more to do for some of the node types } - if mt == os.ModeSymlink { // okay to check for equivalence because we set to this value - osRelative, err = os.Readlink(osPathname) // read the symlink referent - if err != nil { - return errors.Wrap(err, "cannot Readlink") - } - writeBytesWithNull(closure.someHash, []byte(filepath.ToSlash(osRelative))) // write referent to hash - return nil // proceed to next node in queue - } - // If we get here, node is a regular file. fh, err := os.Open(osPathname) if err != nil { @@ -541,3 +531,25 @@ func CheckDepTree(osDirname string, wantDigests map[string]VersionedDigest) (map return slashStatus, nil } + +// sortedChildrenFromDirname returns a lexicographically sorted list of child +// nodes for the specified directory. +func sortedChildrenFromDirname(osDirname string) ([]string, error) { + fh, err := os.Open(osDirname) + if err != nil { + return nil, errors.Wrap(err, "cannot Open") + } + + osChildrenNames, err := fh.Readdirnames(0) // 0: read names of all children + if err != nil { + return nil, errors.Wrap(err, "cannot Readdirnames") + } + sort.Strings(osChildrenNames) + + // Close the file handle to the open directory without masking possible + // previous error value. + if er := fh.Close(); err == nil { + err = errors.Wrap(er, "cannot Close") + } + return osChildrenNames, err +} diff --git a/gps/verify/digest_test.go b/gps/verify/digest_test.go index 36fccb3dcb..9d479d93b7 100644 --- a/gps/verify/digest_test.go +++ b/gps/verify/digest_test.go @@ -153,6 +153,7 @@ func TestVerifyDepTree(t *testing.T) { } checkStatus := func(t *testing.T, status map[string]VendorStatus, key string, want VendorStatus) { + t.Helper() got, ok := status[key] if !ok { t.Errorf("Want key: %q", key) diff --git a/gps/verify/dirwalk.go b/gps/verify/dirwalk.go deleted file mode 100644 index 4010c4a03f..0000000000 --- a/gps/verify/dirwalk.go +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright 2017 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. - -package verify - -import ( - "os" - "path/filepath" - "sort" - "strings" - - "github.com/pkg/errors" -) - -// DirWalkFunc is the type of the function called for each file system node -// visited by DirWalk. The path argument contains the argument to DirWalk as a -// prefix; that is, if DirWalk is called with "dir", which is a directory -// containing the file "a", the walk function will be called with the argument -// "dir/a", using the correct os.PathSeparator for the Go Operating System -// architecture, GOOS. The info argument is the os.FileInfo for the named path. -// -// If there was a problem walking to the file or directory named by path, the -// incoming error will describe the problem and the function can decide how to -// handle that error (and DirWalk will not descend into that directory). If an -// error is returned, processing stops. The sole exception is when the function -// returns the special value filepath.SkipDir. If the function returns -// filepath.SkipDir when invoked on a directory, DirWalk skips the directory's -// contents entirely. If the function returns filepath.SkipDir when invoked on a -// non-directory file system node, DirWalk skips the remaining files in the -// containing directory. -type DirWalkFunc func(osPathname string, info os.FileInfo, err error) error - -// DirWalk walks the file tree rooted at osDirname, calling for each file system -// node in the tree, including root. All errors that arise visiting nodes are -// filtered by walkFn. The nodes are walked in lexical order, which makes the -// output deterministic but means that for very large directories DirWalk can be -// inefficient. Unlike filepath.Walk, DirWalk does follow symbolic links. -func DirWalk(osDirname string, walkFn DirWalkFunc) error { - osDirname = filepath.Clean(osDirname) - - // Ensure parameter is a directory - fi, err := os.Stat(osDirname) - if err != nil { - return errors.Wrap(err, "cannot read node") - } - if !fi.IsDir() { - return errors.Errorf("cannot walk non directory: %q", osDirname) - } - - // Initialize a work queue with the empty string, which signifies the - // starting directory itself. - queue := []string{""} - - var osRelative string // os-specific relative pathname under directory name - - // As we enumerate over the queue and encounter a directory, its children - // will be added to the work queue. - for len(queue) > 0 { - // Unshift a pathname from the queue (breadth-first traversal of - // hierarchy) - osRelative, queue = queue[0], queue[1:] - osPathname := filepath.Join(osDirname, osRelative) - - // walkFn needs to choose how to handle symbolic links, therefore obtain - // lstat rather than stat. - fi, err = os.Lstat(osPathname) - if err == nil { - err = walkFn(osPathname, fi, nil) - } else { - err = walkFn(osPathname, nil, errors.Wrap(err, "cannot read node")) - } - - if err != nil { - if err == filepath.SkipDir { - if fi.Mode()&os.ModeSymlink > 0 { - // Resolve symbolic link referent to determine whether node - // is directory or not. - fi, err = os.Stat(osPathname) - if err != nil { - return errors.Wrap(err, "cannot visit node") - } - } - // If current node is directory, then skip this - // directory. Otherwise, skip all nodes in the same parent - // directory. - if !fi.IsDir() { - // Consume nodes from queue while they have the same parent - // as the current node. - osParent := filepath.Dir(osPathname) + osPathSeparator - for len(queue) > 0 && strings.HasPrefix(queue[0], osParent) { - queue = queue[1:] // drop sibling from queue - } - } - - continue - } - return errors.Wrap(err, "DirWalkFunction") // wrap error returned by walkFn - } - - if fi.IsDir() { - osChildrenNames, err := sortedChildrenFromDirname(osPathname) - if err != nil { - return errors.Wrap(err, "cannot get list of directory children") - } - for _, osChildName := range osChildrenNames { - switch osChildName { - case ".", "..": - // skip - default: - queue = append(queue, filepath.Join(osRelative, osChildName)) - } - } - } - } - return nil -} - -// sortedChildrenFromDirname returns a lexicographically sorted list of child -// nodes for the specified directory. -func sortedChildrenFromDirname(osDirname string) ([]string, error) { - fh, err := os.Open(osDirname) - if err != nil { - return nil, errors.Wrap(err, "cannot Open") - } - - osChildrenNames, err := fh.Readdirnames(0) // 0: read names of all children - if err != nil { - return nil, errors.Wrap(err, "cannot Readdirnames") - } - sort.Strings(osChildrenNames) - - // Close the file handle to the open directory without masking possible - // previous error value. - if er := fh.Close(); err == nil { - err = errors.Wrap(er, "cannot Close") - } - return osChildrenNames, err -}