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

Add importer for govendor #815

Merged
merged 6 commits into from
Nov 26, 2017
Merged

Add importer for govendor #815

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
52b7df1
internal/importers: add govendor importer
kyleconroy Jul 15, 2017
6c922d6
internal/importers: add govendor importer
otoolec Jul 15, 2017
90a984d
internal/importers: add govendor importer
Jul 15, 2017
9033856
internal/importers: add govendor importer
joey-clypd Jul 15, 2017
d77cf70
internal/importers: add govendor importer
kyleconroy Jul 15, 2017
56cefc4
internal/importers: add govendor importer
carolynvs Oct 6, 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
Prev Previous commit
Next Next commit
internal/importers: add govendor importer 
Add an `Any` constraint for projects

Standardize warnings around ignored build tags

Also make few minor changes to logging messages.

Add govendor to the list of supported tools

Sort the list of organized tools alphabetically as well.

Use manifest constructor function
  • Loading branch information
@kyleconroy @carolynvs
kyleconroy authored and carolynvs committed Nov 15, 2017
commit d77cf70f5ba1ede4a5da2601f367b0317c788916
49 changes: 19 additions & 30 deletions cmd/dep/govendor_importer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ type govendorPackage struct {
// See the vendor spec for definitions.
Origin string
Path string
Tree bool
Revision string
Version string
}
Expand All @@ -75,7 +74,7 @@ func (g *govendorImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest
}

func (g *govendorImporter) load(projectDir string) error {
g.logger.Println("Detected govendor configuration files...")
g.logger.Println("Detected govendor configuration file...")
v := filepath.Join(projectDir, govendorDir, govendorName)
if g.verbose {
g.logger.Printf(" Loading %s", v)
Expand All @@ -94,9 +93,7 @@ func (g *govendorImporter) load(projectDir string) error {
func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
g.logger.Println("Converting from vendor.json...")

manifest := &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
}
manifest := dep.NewManifest()

if len(g.file.Ignore) > 0 {
// Govendor has three use cases here
Expand All @@ -107,14 +104,14 @@ func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock
// Dep doesn't support build tags right now: https://github.com/golang/dep/issues/120
for _, i := range strings.Split(g.file.Ignore, " ") {
if !strings.Contains(i, "/") {
g.logger.Printf("Warning: Not able to convert ignoring of build tag '%v'", i)
g.logger.Printf(" Govendor was configured to ignore the %s build tag, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.", i)
continue
}
_, err := g.sm.DeduceProjectRoot(i)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you help me understand why we are deducing the project root, then ignoring the result? I assume it has to do with partial paths or something, but it's not clear to me so maybe I'm wrong. 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was to do with case 3. It's very possible that this ignore directive is a package prefix, which isn't supported by dep. I'm updated the error message to make this obvious.

if err == nil {
manifest.Ignored = append(manifest.Ignored, i)
} else {
g.logger.Printf("Warning: Not able to convert ignoring of build tag '%v'\n", i)
g.logger.Printf(" Govendor was configured to ignore the %s package prefix, but that isn't supported by dep yet, and will be ignored.", i)
}
}
}
Expand All @@ -136,7 +133,7 @@ func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock
pkg.Path = string(pr)

// Check if it already existing in locked projects
if projectExistsInLock(lock, pkg.Path) {
if projectExistsInLock(lock, pr) {
continue
}

Expand All @@ -149,7 +146,7 @@ func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock
if pkg.Version == "" {
// When no version is specified try to get the corresponding version
pi := gps.ProjectIdentifier{
ProjectRoot: gps.ProjectRoot(pkg.Path),
ProjectRoot: pr,
}
if pkg.Origin != "" {
pi.Source = pkg.Origin
Copy link
Collaborator

Choose a reason for hiding this comment

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

source is the alternate location from which dep will download/clone/checkout the source code for the dependency, and if the origin looks something like "github.com/MSOpenTech/azure-sdk-for-go/vendor/crypto/tls" I doubt dep will know what to do with it.

I don't think this will work, but please let me know how it goes! 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct that it won't work for that case, but I don't think we have a way to determine which source values will work. If it fails, you can just update the vendor.json file to remove the source parameter. Do you have any other ideas on what we can do?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we know that any origin with a vendor segment is not going to work with dep, how about we check for those, and print a warning when we find an unsupported origin that it was ignored.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we know that any origin with a vendor segment is not going to work with dep, how about we check for those, and print a warning when we find an unsupported origin that it was ignored.

Expand All @@ -167,14 +164,12 @@ func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock
}
}

if pkg.Version != "" {
// If there's a version, use it to create project constraint
pc, err := g.buildProjectConstraint(pkg)
if err != nil {
return nil, nil, err
}
manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint}
// If there's a version, use it to create project constraint
pc, err := g.buildProjectConstraint(pkg)
if err != nil {
return nil, nil, err
}
manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint}

lp := g.buildLockedProject(pkg, manifest)
lock.P = append(lock.P, lp)
Expand All @@ -184,20 +179,15 @@ func (g *govendorImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock
}

func (g *govendorImporter) buildProjectConstraint(pkg *govendorPackage) (pc gps.ProjectConstraint, err error) {
if pkg.Path == "" {
err = errors.New("Invalid vendor configuration, package path is required")
return
}

ref := pkg.Version
if ref == "" {
ref = pkg.Revision
}

pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Path), Source: pkg.Path}
pc.Constraint, err = g.sm.InferConstraint(ref, pc.Ident)
if err != nil {
return

if pkg.Version != "" {
pc.Constraint, err = g.sm.InferConstraint(pkg.Version, pc.Ident)
if err != nil {
return
}
} else {
pc.Constraint = gps.Any()
}

f := fb.NewConstraintFeedback(pc, fb.DepTypeImported)
Expand All @@ -206,7 +196,6 @@ func (g *govendorImporter) buildProjectConstraint(pkg *govendorPackage) (pc gps.
return
}

// buildLockedProject uses the package Rev and Comment to create lock project
func (g *govendorImporter) buildLockedProject(pkg *govendorPackage, manifest *dep.Manifest) gps.LockedProject {
pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Path)}
revision := gps.Revision(pkg.Revision)
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ specified, use the current directory.
When configuration for another dependency management tool is detected, it is
imported into the initial manifest and lock. Use the -skip-tools flag to
disable this behavior. The following external tools are supported:
glide, godep, vndr, govend, gb, gvt.
glide, godep, vndr, govend, gb, gvt, govendor.

Any dependencies that are not constrained by external configuration use the
GOPATH analysis below.
Expand Down
4 changes: 2 additions & 2 deletions cmd/dep/testdata/govendor/golden.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Detected govendor configuration files...
Detected govendor configuration file...
Converting from vendor.json...
Warning: Not able to convert ignoring of build tag 'test'
Govendor was configured to ignore the test build tag, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.
Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest
Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest
Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos
Expand Down
2 changes: 1 addition & 1 deletion docs/FAQ.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ about what's going on.
During `dep init` configuration from other dependency managers is detected
and imported, unless `-skip-tools` is specified.

The following tools are supported: `glide`, `godep`, `vndr`, `govend`, `gb` and `gvt`.
The following tools are supported: `glide`, `godep`, `vndr`, `govend`, `gb`, `gvt` and `govendor`.

See [#186](https://github.com/golang/dep/issues/186#issuecomment-306363441) for
how to add support for another tool.
Expand Down