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

move deduceConstraint to SourceManager #761

Merged
merged 2 commits into from
Jul 6, 2017
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
70 changes: 1 addition & 69 deletions cmd/dep/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ package main

import (
"bytes"
"encoding/hex"
"flag"
"fmt"
"go/build"
"log"
"path/filepath"
"strconv"
"strings"

"github.com/golang/dep"
Expand Down Expand Up @@ -298,79 +296,13 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai
}

pi := gps.ProjectIdentifier{ProjectRoot: pr, Source: source}
c, err := deduceConstraint(versionStr, pi, sm)
c, err := sm.InferConstraint(versionStr, pi)
if err != nil {
return emptyPC, err
}
return gps.ProjectConstraint{Ident: pi, Constraint: c}, nil
}

// deduceConstraint tries to puzzle out what kind of version is given in a string -
// semver, a revision, or as a fallback, a plain tag
func deduceConstraint(s string, pi gps.ProjectIdentifier, sm gps.SourceManager) (gps.Constraint, error) {
if s == "" {
// Find the default branch
versions, err := sm.ListVersions(pi)
if err != nil {
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
}

gps.SortPairedForUpgrade(versions)
for _, v := range versions {
if v.Type() == gps.IsBranch {
return v.Unpair(), nil
}
}
}

// always semver if we can
c, err := gps.NewSemverConstraintIC(s)
if err == nil {
return c, nil
}

slen := len(s)
if slen == 40 {
if _, err = hex.DecodeString(s); err == nil {
// Whether or not it's intended to be a SHA1 digest, this is a
// valid byte sequence for that, so go with Revision. This
// covers git and hg
return gps.Revision(s), nil
}
}
// Next, try for bzr, which has a three-component GUID separated by
// dashes. There should be two, but the email part could contain
// internal dashes
if strings.Count(s, "-") >= 2 {
// Work from the back to avoid potential confusion from the email
i3 := strings.LastIndex(s, "-")
// Skip if - is last char, otherwise this would panic on bounds err
if slen == i3+1 {
return gps.NewVersion(s), nil
}

i2 := strings.LastIndex(s[:i3], "-")
if _, err = strconv.ParseUint(s[i2+1:i3], 10, 64); err == nil {
// Getting this far means it'd pretty much be nuts if it's not a
// bzr rev, so don't bother parsing the email.
return gps.Revision(s), nil
}
}

// call out to network and get the package's versions
versions, err := sm.ListVersions(pi)
if err != nil {
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
}

for _, version := range versions {
if s == version.String() {
return version.Unpair(), nil
}
}
return nil, errors.Errorf("%s is not a valid version for the package %s(%s)", s, pi.ProjectRoot, pi.Source)
}

func checkErrors(m map[string]pkgtree.PackageOrErr) error {
noGoErrors, pkgErrors := 0, 0
for _, poe := range m {
Expand Down
4 changes: 2 additions & 2 deletions cmd/dep/ensure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestDeduceConstraint(t *testing.T) {

pi := gps.ProjectIdentifier{ProjectRoot: "github.com/sdboyer/deptest"}
for str, want := range constraints {
got, err := deduceConstraint(str, pi, sm)
got, err := sm.InferConstraint(str, pi)
h.Must(err)

wantT := reflect.TypeOf(want)
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestDeduceConstraint_InvalidInput(t *testing.T) {

pi := gps.ProjectIdentifier{ProjectRoot: "github.com/sdboyer/deptest"}
for _, str := range constraints {
_, err := deduceConstraint(str, pi, sm)
_, err := sm.InferConstraint(str, pi)
if err == nil {
t.Errorf("expected %s to produce an error", str)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/glide_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (g *glideImporter) buildProjectConstraint(pkg glidePackage) (pc gps.Project
}

pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Name), Source: pkg.Repository}
pc.Constraint, err = deduceConstraint(pkg.Reference, pc.Ident, g.sm)
pc.Constraint, err = g.sm.InferConstraint(pkg.Reference, pc.Ident)
if err != nil {
return
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/dep/godep_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e
// create a project constraint
func (g *godepImporter) buildProjectConstraint(pkg godepPackage) (pc gps.ProjectConstraint, err error) {
pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)}
pc.Constraint, err = deduceConstraint(pkg.Comment, pc.Ident, g.sm)
pc.Constraint, err = g.sm.InferConstraint(pkg.Comment, pc.Ident)
if err != nil {
return
}
Expand Down
9 changes: 9 additions & 0 deletions internal/gps/solve_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,15 @@ func (sm *depspecSourceManager) ignore() map[string]bool {
return sm.ig
}

// InferConstraint tries to puzzle out what kind of version is given in a string -
// semver, a revision, or as a fallback, a plain tag. This current implementation
// is a panic because there's no current circumstance under which the depspecSourceManager
// is useful outside of the gps solving tests, and it shouldn't be used anywhere else without a conscious and intentional
// expansion of its semantics.
func (sm *depspecSourceManager) InferConstraint(s string, pi ProjectIdentifier) (Constraint, error) {
panic("depsecSourceManager is only for gps solving tests")
}

type depspecBridge struct {
*bridge
}
Expand Down
73 changes: 73 additions & 0 deletions internal/gps/source_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@ package gps

import (
"context"
"encoding/hex"
"fmt"
"os"
"os/signal"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"sync/atomic"
"time"

"github.com/golang/dep/internal/gps/pkgtree"
"github.com/pkg/errors"
"github.com/sdboyer/constext"
)

Expand Down Expand Up @@ -72,6 +75,10 @@ type SourceManager interface {
// no longer safe to call methods against it; all method calls will
// immediately result in errors.
Release()

// InferConstraint tries to puzzle out what kind of version is given in a string -
// semver, a revision, or as a fallback, a plain tag
InferConstraint(s string, pi ProjectIdentifier) (Constraint, error)
}

// A ProjectAnalyzer is responsible for analyzing a given path for Manifest and
Expand Down Expand Up @@ -455,6 +462,72 @@ func (sm *SourceMgr) DeduceProjectRoot(ip string) (ProjectRoot, error) {
return ProjectRoot(pd.root), err
}

// InferConstraint tries to puzzle out what kind of version is given in a string -
// semver, a revision, or as a fallback, a plain tag
func (sm *SourceMgr) InferConstraint(s string, pi ProjectIdentifier) (Constraint, error) {
if s == "" {
// Find the default branch
versions, err := sm.ListVersions(pi)
if err != nil {
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
}

SortPairedForUpgrade(versions)
for _, v := range versions {
if v.Type() == IsBranch {
return v.Unpair(), nil
}
}
}

// always semver if we can
c, err := NewSemverConstraintIC(s)
if err == nil {
return c, nil
}

slen := len(s)
Copy link
Member

Choose a reason for hiding this comment

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

By moving this into *sourceMgr, we create an opportunity to do better validation here. Rather than deducing whether it's a revision from what general rules we have about the patterns of the bytes, we can actually hand this off to a sourceGateway, which can validate that it's an appropriate revision for that source type.

However, that may end up being somewhat involved to really do well, so we can defer it to a later issue. (Let's open up a follow-up prior to merging this issue for doing that, though).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've created #800 to track this.

if slen == 40 {
if _, err = hex.DecodeString(s); err == nil {
// Whether or not it's intended to be a SHA1 digest, this is a
// valid byte sequence for that, so go with Revision. This
// covers git and hg
return Revision(s), nil
}
}
// Next, try for bzr, which has a three-component GUID separated by
// dashes. There should be two, but the email part could contain
// internal dashes
if strings.Count(s, "-") >= 2 {
// Work from the back to avoid potential confusion from the email
i3 := strings.LastIndex(s, "-")
// Skip if - is last char, otherwise this would panic on bounds err
if slen == i3+1 {
return NewVersion(s), nil
}

i2 := strings.LastIndex(s[:i3], "-")
if _, err = strconv.ParseUint(s[i2+1:i3], 10, 64); err == nil {
// Getting this far means it'd pretty much be nuts if it's not a
// bzr rev, so don't bother parsing the email.
return Revision(s), nil
}
}

// call out to network and get the package's versions
versions, err := sm.ListVersions(pi)
if err != nil {
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
}

for _, version := range versions {
if s == version.String() {
return version.Unpair(), nil
}
}
return nil, errors.Errorf("%s is not a valid version for the package %s(%s)", s, pi.ProjectRoot, pi.Source)
}

type timeCount struct {
count int
start time.Time
Expand Down