Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Guard against errs in stripVendor WalkFunc #1026

Merged
merged 5 commits into from
Aug 17, 2017
Merged

Guard against errs in stripVendor WalkFunc #1026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4373a7
Guard against errs in stripVendor WalkFunc
sdboyer Aug 17, 2017
8f928b1
Accommodate error channel size properly
sdboyer Aug 17, 2017
b4ff96f
Fix remove-before-visit bug
sdboyer Aug 17, 2017
151b3c8
Fix remove-before-visit for Windows, too
sdboyer Aug 17, 2017
8c85179
Drop unnecessary err check at start of walker
sdboyer Aug 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/gps/result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,21 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log
for _, p := range l.Projects() {
wg.Add(1)
go func(p LockedProject) {
defer wg.Done()
to := filepath.FromSlash(filepath.Join(basedir, string(p.Ident().ProjectRoot)))
logger.Printf("Writing out %s@%s", p.Ident().errString(), p.Version())

if err := sm.ExportProject(p.Ident(), p.Version(), to); err != nil {
errCh <- errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot)
return
}

if sv {
filepath.Walk(to, stripVendor)
err := filepath.Walk(to, stripVendor)
if err != nil {
errCh <- errors.Wrapf(err, "failed to strip vendor from %s", p.Ident().ProjectRoot)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the errCh buffer is too small since each routine could send 2 errors. Does that even make sense? Or should the first error terminate the goroutine? (maybe defer the wg.Done() so we can return from there)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes good call

}
wg.Done()
}(p)
}

Expand Down
39 changes: 27 additions & 12 deletions internal/gps/strip_vendor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,39 @@

package gps

import "os"
import (
"os"
"path/filepath"
)

func stripVendor(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}

if info.Name() == "vendor" {
if _, err := os.Lstat(path); err == nil {
if (info.Mode() & os.ModeSymlink) != 0 {
realInfo, err := os.Stat(path)
if err != nil {
return err
}
if realInfo.IsDir() {
return os.Remove(path)
}
if _, err := os.Lstat(path); err != nil {
return err
}

if (info.Mode() & os.ModeSymlink) != 0 {
realInfo, err := os.Stat(path)
if err != nil {
return err
}
if info.IsDir() {
return removeAll(path)
if realInfo.IsDir() {
return os.Remove(path)
}
}

if info.IsDir() {
if err := removeAll(path); err != nil {
return err
}
return filepath.SkipDir
}

return nil
}

return nil
Expand Down
9 changes: 8 additions & 1 deletion internal/gps/strip_vendor_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import (
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a //+build windows at the top of this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the _windows.go file suffix does the same thing.


func stripVendor(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}

if info.Name() == "vendor" {
if _, err := os.Lstat(path); err == nil {
symlink := (info.Mode() & os.ModeSymlink) != 0
Expand All @@ -36,7 +40,10 @@ func stripVendor(path string, info os.FileInfo, err error) error {
}

case dir:
return removeAll(path)
if err := removeAll(path); err != nil {
return err
}
return filepath.SkipDir
}
}
}
Expand Down