Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use glide list #21

Merged
merged 1 commit into from
Sep 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go:
sudo: false

install:
- go get github.com/Masterminds/glide
- GO15VENDOREXPERIMENT=1 go build

script:
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ For a detailed explanation on why Glide doesn't do this see [here](http://engine

## Description

This tool will help you removing from the project vendor directories all the files not needed for building your project. By default it'll keep all the files provided by packages listed in the `glide.lock` file.
This tool will help you removing from the project vendor directories all the files not needed for building your project. By default it'll keep all the files provided by packages returned by the `glide list` command.
If you want to keep only go (including tests) files you can provide the `--only-code` option.
If you want to remove also the go test files you can add the `--no-tests` option.

Expand Down
3 changes: 2 additions & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ platform:
install:
- go version
- go env
- go get github.com/Masterminds/glide

build_script:
- go build

test_script:
- go test .
- PATH=$GOPATH/bin:$PATH go test .

deploy: off
229 changes: 124 additions & 105 deletions gvc.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package main

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"strings"

Expand All @@ -20,12 +22,21 @@ var cmd = &cobra.Command{
}

type options struct {
dryrun bool
onlyCode bool
noTests bool
dryrun bool
onlyCode bool
noTests bool
noLegalFiles bool
keepPatterns []string

// Deprecated
useLockFile bool
noTestImports bool
noLegalFiles bool
keepPatterns []string
}

type packages struct {
Installed []string `json:"installed"`
Missing []string `json:"missing"`
Gopath []string `json:"gopath"`
}

var (
Expand All @@ -40,10 +51,12 @@ const (
func init() {
cmd.PersistentFlags().BoolVar(&opts.dryrun, "dryrun", false, "just output what will be removed")
cmd.PersistentFlags().BoolVar(&opts.onlyCode, "only-code", false, "keep only go files (including go test files)")
cmd.PersistentFlags().BoolVar(&opts.noTestImports, "no-test-imports", false, "remove also testImport vendor directories")
cmd.PersistentFlags().BoolVar(&opts.noTests, "no-tests", false, "remove also go test files (requires --only-code)")
cmd.PersistentFlags().BoolVar(&opts.noLegalFiles, "no-legal-files", false, "remove also licenses and legal files")
cmd.PersistentFlags().StringSliceVar(&opts.keepPatterns, "keep", []string{}, "A pattern to keep additional files inside needed packages. The pattern match will be relative to the deeper vendor dir. Supports double star (**) patterns. (see https://golang.org/pkg/path/filepath/#Match and https://github.com/bmatcuk/doublestar). Can be specified multiple times. For example to keep all the files with json extension use the '**/*.json' pattern.")

cmd.PersistentFlags().BoolVar(&opts.useLockFile, "use-lock-file", false, "use glide.lock instead of glide list to determine imports")
cmd.PersistentFlags().BoolVar(&opts.noTestImports, "no-test-imports", false, "remove also testImport vendor directories")
}

func main() {
Expand All @@ -56,53 +69,87 @@ func glidevc(cmd *cobra.Command, args []string) {
os.Exit(1)
}

if err := cleanup(".", opts); err != nil {
if err := cleanup("."); err != nil {
fmt.Print(err)
os.Exit(1)
}
return
}

func cleanup(path string, opts options) error {
lock, err := LoadGlideLockfile(path)
func glideLockImports(path string) ([]string, error) {
yml, err := ioutil.ReadFile(filepath.Join(path, gpath.LockFile))
if err != nil {
return fmt.Errorf("Could not load lockfile: %v", err)
return nil, err
}

// The package list already have the path converted to the os specific
// path separator, needed for future comparisons.
pkgList := []string{}
repoList := []string{}
adder := func(lock cfg.Locks) {
for _, imp := range lock {
// This converts pkg separator "/" to os specific separator
repoList = append(repoList, filepath.FromSlash(imp.Name))
lock, err := cfg.LockfileFromYaml(yml)
if err != nil {
return nil, err
}

if len(imp.Subpackages) > 0 {
for _, sp := range imp.Subpackages {
// This converts pkg separator "/" to os specific separator
pkgList = append(pkgList, filepath.Join(imp.Name, sp))
}
var imports []string
adder := func(locks cfg.Locks) {
for _, lock := range locks {
for _, subpackage := range lock.Subpackages {
imports = append(imports, lock.Name+"/"+subpackage)
}
// TODO(sgotti) we cannot skip the base import if it has subpackages
// because glide doesn't write "." as a subpackage, otherwise if some
// files in the base import are needed they will be removed.

// This converts pkg separator "/" to os specific separator
pkgList = append(pkgList, filepath.FromSlash(imp.Name))
imports = append(imports, lock.Name)
}
}

adder(lock.Imports)
if !opts.noTestImports {
adder(lock.DevImports)
}

return imports, nil
}

func glideListImports(path string) ([]string, error) {
cmd := exec.Command("glide", "list", "-output", "json", path)
out, err := cmd.Output()
if err != nil {
return nil, err
}

list := &packages{}
err = json.Unmarshal(out, list)
if err != nil {
return nil, err
}

return list.Installed, nil
}

func cleanup(path string) error {
var (
packages []string
err error
)

if opts.useLockFile {
packages, err = glideLockImports(path)
} else {
packages, err = glideListImports(path)
}

// The package list already have the path converted to the os specific
// path separator, needed for future comparisons.
pkgList := []string{}
pkgMap := map[string]struct{}{}
for _, imp := range packages {
Copy link
Owner

Choose a reason for hiding this comment

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

I like how you refactored this part. 👍

if _, found := pkgMap[imp]; !found {
// This converts pkg separator "/" to os specific separator
pkgList = append(pkgList, filepath.FromSlash(imp))
pkgMap[imp] = struct{}{}
}
}

vpath, err := gpath.Vendor()
if err != nil {
return err
}
if vpath == "" {
return fmt.Errorf("cannot fine vendor dir")
return fmt.Errorf("cannot find vendor dir")
}

type pathData struct {
Expand All @@ -124,83 +171,63 @@ func cleanup(path string, opts options) error {
return nil
}

// Short-circuit for test files
if opts.noTests && strings.HasSuffix(path, "_test.go") {
return nil
}

localPath := strings.TrimPrefix(path, searchPath)

lastVendorPath, err := getLastVendorPath(localPath)
if err != nil {
return err
}
if lastVendorPath == "" {
lastVendorPath = localPath
}
lastVendorPathDir := filepath.Dir(lastVendorPath)

keep := false

for _, name := range pkgList {
// If the file's parent directory is a needed package, keep it.
if !info.IsDir() && filepath.Dir(lastVendorPath) == name {
if opts.onlyCode {
if opts.noTests && strings.HasSuffix(path, "_test.go") {
keep = false
continue
}
for _, suffix := range codeSuffixes {
if strings.HasSuffix(path, suffix) {
keep = true
}
}
// Match keep patterns
for _, keepPattern := range opts.keepPatterns {
ok, err := doublestar.Match(keepPattern, lastVendorPath)
// TODO(sgotti) if a bad pattern is encountered stop here. Actually there's no function to verify a pattern before using it, perhaps just a fake match at the start will work.
if err != nil {
return fmt.Errorf("bad pattern: %q", keepPattern)
}
if ok {
keep = true
}
}
} else {
keep = true
}
// If a directory is a needed package then keep it
keep = keep || info.IsDir() && name == lastVendorPath

Copy link
Owner

@sgotti sgotti Sep 21, 2016

Choose a reason for hiding this comment

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

just noticed than in the previous iteration you called break after setting keep to true. This avoided some unneeded looping on the other packages since the file was already marked to be kept. Did you removed this because it's not a big deal and to keep the code cleaner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I figured the comparisons were all operating on things already in memory, so there wouldn't be a huge performance hit, and I felt it made the code easier to follow. It should be pretty easy to add the breaks back if needed.

// The remaining tests are only for files
if info.IsDir() {
continue
}
}

// Keep all the legal files inside top repo dir and required package dirs
for _, name := range append(repoList, pkgList...) {
if !info.IsDir() && filepath.Dir(lastVendorPath) == name {
if !opts.noLegalFiles {
if IsLegalFile(path) {
keep = true
}
}
// Keep legal files in directories that are the parent of a needed package
keep = keep || !opts.noLegalFiles && IsLegalFile(path) && isParentDirectory(lastVendorPathDir, name)

// The remaining tests only apply if the file is in a needed package
if name != lastVendorPathDir {
continue
}
}

// If a directory is a needed package then keep it
if keep == false && info.IsDir() {
for _, name := range pkgList {
if name == lastVendorPath {
keep = true
// Keep everything unless --only-code was specified
keep = keep || !opts.onlyCode

// Always keep code files
for _, suffix := range codeSuffixes {
keep = keep || strings.HasSuffix(path, suffix)
}

// Match keep patterns
for _, keepPattern := range opts.keepPatterns {
ok, err := doublestar.Match(keepPattern, lastVendorPath)
// TODO(sgotti) if a bad pattern is encountered stop here. Actually there's no function to verify a pattern before using it, perhaps just a fake match at the start will work.
if err != nil {
return fmt.Errorf("bad pattern: %q", keepPattern)
}
keep = keep || ok
}
}

if keep {
// Keep also all parents of current path
curpath := localPath
for {
curpath = filepath.Dir(curpath)
if curpath == "." {
break
}
if _, ok := markForKeep[curpath]; ok {
// Already marked for keep
break
}
// Keep all parent directories of current path
for curpath := localPath; curpath != "."; curpath = filepath.Dir(curpath) {
markForKeep[curpath] = pathData{curpath, true}
}

// Mark for keep
// Fix isDir property for current path
markForKeep[localPath] = pathData{localPath, info.IsDir()}
}

Expand Down Expand Up @@ -255,30 +282,22 @@ func cleanup(path string, opts options) error {
}

func getLastVendorPath(path string) (string, error) {
curpath := path
for {
if curpath == "." {
return "", nil
}
for curpath := path; curpath != "."; curpath = filepath.Dir(curpath) {
if filepath.Base(curpath) == "vendor" {
return filepath.Rel(curpath, path)
}
curpath = filepath.Dir(curpath)
}
return path, nil
}

// LoadLockfile loads the contents of a glide.lock file.
func LoadGlideLockfile(base string) (*cfg.Lockfile, error) {
yml, err := ioutil.ReadFile(filepath.Join(base, gpath.LockFile))
if err != nil {
return nil, err
func isParentDirectory(parent, child string) bool {
if !strings.HasSuffix(parent, "/") {
parent += "/"
}
lock, err := cfg.LockfileFromYaml(yml)
if err != nil {
return nil, err
if !strings.HasSuffix(child, "/") {
child += "/"
}

return lock, nil
return strings.HasPrefix(child, parent)
}

// File lists and code took from https://github.com/client9/gosupplychain/blob/master/license.go
Expand Down Expand Up @@ -316,7 +335,7 @@ func IsLegalFile(path string) bool {
}
}
for _, substring := range LegalFileSubstring {
if strings.Index(lowerfile, substring) != -1 && !strings.HasSuffix(lowerfile, goTestSuffix) {
if strings.Contains(lowerfile, substring) && !strings.HasSuffix(lowerfile, goTestSuffix) {
return true
}
}
Expand Down
Loading