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

gb project importer #818

Closed
wants to merge 7 commits into from
Closed
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
205 changes: 205 additions & 0 deletions cmd/dep/gb_importer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
// 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 main

import (
"encoding/json"
"io/ioutil"
"log"
"os"
"path/filepath"

"github.com/golang/dep"
fb "github.com/golang/dep/internal/feedback"
"github.com/golang/dep/internal/gps"
"github.com/pkg/errors"
)

// gbImporter imports gb configuration into the dep configuration format.
type gbImporter struct {
manifest gbManifest
logger *log.Logger
verbose bool
sm gps.SourceManager
}

func newGbImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *gbImporter {
return &gbImporter{
logger: logger,
verbose: verbose,
sm: sm,
}
}

// gbManifest represents the manifest file for GB projects
type gbManifest struct {
Dependencies []gbDependency `json:"dependencies"`
}

type gbDependency struct {
Importpath string `json:"importpath"`
Repository string `json:"repository"`

// All gb vendored dependencies have a specific revision
Revision string `json:"revision"`

// Branch may be HEAD or an actual branch. In the case of HEAD, that means
// the user vendored a dependency by specifying a tag or a specific revision
// which results in a detached HEAD
Branch string `json:"branch"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't look like Branch is being used. If it's set to a real branch name that we can find in the repo, should it be used as the constraint in the manifest?

Copy link
Author

Choose a reason for hiding this comment

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

I added support for using the branch as the initial constraint (if it's not HEAD); but I don't have a solid repository to use in the tests. Ideally there'd be a blessed test repository with two branches, then I can have a test that pins to a revision that is not on the master branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would https://github.com/carolynvs/deptest work for your test? It has a master and v2 branch, and it's "blessed" for using in dep's tests.

Copy link
Author

Choose a reason for hiding this comment

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

I'll add a test that uses it and see how the tooling responds. Thanks.

}

func (i *gbImporter) Name() string {
return "gb"
}

func (i *gbImporter) HasDepMetadata(dir string) bool {
// gb stores the manifest in the vendor tree
var m = filepath.Join(dir, "vendor", "manifest")
if _, err := os.Stat(m); err != nil {
return false
}

return true
}

func (i *gbImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
err := i.load(dir)
if err != nil {
return nil, nil, err
}

return i.convert(pr)
}

// load the gb manifest
func (i *gbImporter) load(projectDir string) error {
i.logger.Println("Detected gb manifest file...")
var mf = filepath.Join(projectDir, "vendor", "manifest")
if i.verbose {
i.logger.Printf(" Loading %s", mf)
}

var buf []byte
var err error
if buf, err = ioutil.ReadFile(mf); err != nil {
return errors.Wrapf(err, "Unable to read %s", mf)
}
if err := json.Unmarshal(buf, &i.manifest); err != nil {
return errors.Wrapf(err, "Unable to parse %s", mf)
}

return nil
}

// convert the gb manifest into dep configuration files.
func (i *gbImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
i.logger.Println("Converting from gb manifest...")

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

lock := &dep.Lock{}

for _, pkg := range i.manifest.Dependencies {
if pkg.Importpath == "" {
return nil, nil, errors.New("Invalid gb configuration, package import path is required")
}

if pkg.Revision == "" {
return nil, nil, errors.New("Invalid gb configuration, package revision is required")
}

// Deduce the project root. This is necessary because gb manifests can have
// multiple entries for the same project root, one for each imported subpackage
var root gps.ProjectRoot
var err error
if root, err = i.sm.DeduceProjectRoot(pkg.Importpath); err != nil {
return nil, nil, err
}

// Set the proper import path back on the dependency
pkg.Importpath = string(root)

// If we've already locked this project root then we can skip
if projectExistsInLock(lock, pkg.Importpath) {
continue
}

// Otherwise, attempt to convert this specific package, which returns a constraint and a lock
pc, lp, err := i.convertOne(pkg)
if err != nil {
return nil, nil, err
}

manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Source: pc.Ident.Source, Constraint: pc.Constraint}
lock.P = append(lock.P, lp)

}

return manifest, lock, nil
}

func (i *gbImporter) convertOne(pkg gbDependency) (pc gps.ProjectConstraint, lp gps.LockedProject, err error) {
/*
gb's vendor plugin (gb vendor), which manages the vendor tree and manifest
file, supports fetching by a specific tag or revision, but if you specify
either of those it's a detached checkout and the "branch" field is HEAD.
The only time the "branch" field is not "HEAD" is if you do not specify a
tag or revision, otherwise it's either "master" or the value of the -branch
flag

This means that, generally, the only possible "constraint" we can really specify is
the branch name if the branch name is not HEAD. Otherwise, it's a specific revision.

However, if we can infer a tag that points to the revision or the branch, we may be able
to use that as the constraint

So, the order of operations to convert a single dependency in a gb manifest is:
- Find a specific version for the revision (and branch, if set)
- If there's a branch available, use that as the constraint
- If there's no branch, but we found a version from step 1, use the version as the constraint
Copy link
Collaborator

Choose a reason for hiding this comment

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

The inline doc is very helpful! 👍

*/
pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Importpath), Source: pkg.Repository}

// Generally, gb tracks revisions
var revision = gps.Revision(pkg.Revision)

// But if the branch field is not "HEAD", we can use that as the initial constraint
var constraint gps.Constraint
if pkg.Branch != "" && pkg.Branch != "HEAD" {
constraint = gps.NewBranch(pkg.Branch)
}

// See if we can get a version from that constraint
version, err := lookupVersionForLockedProject(pc.Ident, constraint, revision, i.sm)
if err != nil {
// Log the error, but don't fail it. It's okay if we can't find a version
i.logger.Println(err.Error())
}

// If the constraint is nil (no branch), but there's a version, infer a constraint from there
if constraint == nil && version != nil {
constraint, err = i.sm.InferConstraint(version.String(), pc.Ident)
if err != nil {
return
}
}

// If there's *still* no constraint, set the constraint to the revision
Copy link
Author

Choose a reason for hiding this comment

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

Is this the proper behavior? Should there just not be a constraint if there's no branch specified and no tag that points to the revision?

Copy link
Collaborator

@carolynvs carolynvs Jul 28, 2017

Choose a reason for hiding this comment

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

In general, dep tries to prevent setting a constraint to a specific revision, that's what the lock is for. The manifest tries to only hold hints on how to update a package, like "I am following the v2 branch" or "Anything less than 2.0.0 please".

Instead of not adding a constraint, what we should do in this case (i.e. they haven't picked a branch and are using a revision that isn't tagged) is to set constraint = gps.Any().

The result will be an entry in the manifest without any restriction on it:

[[constraint]]
  name = "github.com/carolyn/lovesponies"

if constraint == nil {
constraint = revision
}

pc.Constraint = constraint

lp = gps.NewLockedProject(pc.Ident, version, nil)

fb.NewConstraintFeedback(pc, fb.DepTypeImported).LogFeedback(i.logger)
fb.NewLockedProjectFeedback(lp, fb.DepTypeImported).LogFeedback(i.logger)

return
}
Loading