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

Commit ac944f7

Browse files
carolynvscarolynvs
committed
Interpret strings as branches > semver constraints
* Now that deduceConstraint hits the source manager even when given a semver-ish string, I’ve updated the tests to always pass one in.
1 parent c36bccf commit ac944f7

File tree

2 files changed

+94
-80
lines changed

2 files changed

+94
-80
lines changed

‎cmd/dep/ensure.go

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -304,39 +304,19 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai
304304
return gps.ProjectConstraint{Ident: pi, Constraint: c}, nil
305305
}
306306

307-
// deduceConstraint tries to puzzle out what kind of version is given in a string -
308-
// semver, a revision, or as a fallback, a plain tag
307+
// deduceConstraint tries to puzzle out what kind of version is given in a string.
308+
// Preference is given first for revisions, then branches, then semver constraints, then plain tags.
309309
func deduceConstraint(s string, pi gps.ProjectIdentifier, sm gps.SourceManager) (gps.Constraint, error) {
310-
if s == "" {
311-
// Find the default branch
312-
versions, err := sm.ListVersions(pi)
313-
if err != nil {
314-
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
315-
}
316-
317-
gps.SortPairedForUpgrade(versions)
318-
for _, v := range versions {
319-
if v.Type() == gps.IsBranch {
320-
return v.Unpair(), nil
321-
}
322-
}
323-
}
324-
325-
// always semver if we can
326-
c, err := gps.NewSemverConstraintIC(s)
327-
if err == nil {
328-
return c, nil
329-
}
330-
331310
slen := len(s)
332311
if slen == 40 {
333-
if _, err = hex.DecodeString(s); err == nil {
312+
if _, err := hex.DecodeString(s); err == nil {
334313
// Whether or not it's intended to be a SHA1 digest, this is a
335314
// valid byte sequence for that, so go with Revision. This
336315
// covers git and hg
337316
return gps.Revision(s), nil
338317
}
339318
}
319+
340320
// Next, try for bzr, which has a three-component GUID separated by
341321
// dashes. There should be two, but the email part could contain
342322
// internal dashes
@@ -349,24 +329,45 @@ func deduceConstraint(s string, pi gps.ProjectIdentifier, sm gps.SourceManager)
349329
}
350330

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

359-
// call out to network and get the package's versions
339+
// Lookup the string in the repository
340+
var version gps.PairedVersion
360341
versions, err := sm.ListVersions(pi)
361342
if err != nil {
362343
return nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist
363344
}
364-
365-
for _, version := range versions {
366-
if s == version.String() {
367-
return version.Unpair(), nil
345+
gps.SortPairedForUpgrade(versions)
346+
for _, v := range versions {
347+
// Pick the default branch if no constraint is given
348+
if s == "" || s == v.String() {
349+
version = v
368350
}
369351
}
352+
353+
// Try to interpret as a semver constraint
354+
c, err := gps.NewSemverConstraintIC(s)
355+
356+
// Branch
357+
if version != nil && version.Type() == gps.IsBranch {
358+
return version.Unpair(), nil
359+
}
360+
361+
// Semver Constraint
362+
if c != nil && err == nil {
363+
return c, nil
364+
}
365+
366+
// Tag
367+
if version != nil {
368+
return version.Unpair(), nil
369+
}
370+
370371
return nil, errors.Errorf("%s is not a valid version for the package %s(%s)", s, pi.ProjectRoot, pi.Source)
371372
}
372373

‎cmd/dep/glide_importer_test.go

Lines changed: 63 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,24 @@ func newTestContext(h *test.Helper) *dep.Ctx {
3838
}
3939

4040
func TestGlideConfig_Import(t *testing.T) {
41-
t.Parallel()
42-
4341
h := test.NewHelper(t)
4442
defer h.Cleanup()
4543

46-
cacheDir := "gps-repocache"
47-
h.TempDir(cacheDir)
48-
h.TempDir("src")
44+
ctx := newTestContext(h)
45+
sm, err := ctx.SourceManager()
46+
h.Must(err)
47+
defer sm.Release()
48+
4949
h.TempDir(filepath.Join("src", testGlideProjectRoot))
5050
h.TempCopy(filepath.Join(testGlideProjectRoot, glideYamlName), "glide/glide.yaml")
5151
h.TempCopy(filepath.Join(testGlideProjectRoot, glideLockName), "glide/glide.lock")
52-
5352
projectRoot := h.Path(testGlideProjectRoot)
54-
sm, err := gps.NewSourceManager(h.Path(cacheDir))
55-
h.Must(err)
56-
defer sm.Release()
5753

5854
// Capture stderr so we can verify output
5955
verboseOutput := &bytes.Buffer{}
60-
logger := log.New(verboseOutput, "", 0)
56+
ctx.Err = log.New(verboseOutput, "", 0)
6157

62-
g := newGlideImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run
58+
g := newGlideImporter(ctx.Err, false, sm) // Disable verbose so that we don't print values that change each test run
6359
if !g.HasDepMetadata(projectRoot) {
6460
t.Fatal("Expected the importer to detect the glide configuration files")
6561
}
@@ -90,24 +86,19 @@ func TestGlideConfig_Import(t *testing.T) {
9086
}
9187

9288
func TestGlideConfig_Import_MissingLockFile(t *testing.T) {
93-
t.Parallel()
94-
9589
h := test.NewHelper(t)
9690
defer h.Cleanup()
9791

98-
cacheDir := "gps-repocache"
99-
h.TempDir(cacheDir)
100-
h.TempDir("src")
92+
ctx := newTestContext(h)
93+
sm, err := ctx.SourceManager()
94+
h.Must(err)
95+
defer sm.Release()
96+
10197
h.TempDir(filepath.Join("src", "glidetest"))
10298
h.TempCopy(filepath.Join("glidetest", glideYamlName), "glide/glide.yaml")
103-
10499
projectRoot := h.Path("glidetest")
105-
sm, err := gps.NewSourceManager(h.Path(cacheDir))
106-
h.Must(err)
107-
defer sm.Release()
108100

109-
logger := log.New(os.Stderr, "", 0)
110-
g := newGlideImporter(logger, true, sm)
101+
g := newGlideImporter(ctx.Err, true, sm)
111102
if !g.HasDepMetadata(projectRoot) {
112103
t.Fatal("The glide importer should gracefully handle when only glide.yaml is present")
113104
}
@@ -125,19 +116,17 @@ func TestGlideConfig_Import_MissingLockFile(t *testing.T) {
125116
}
126117

127118
func TestGlideConfig_Convert_Project(t *testing.T) {
128-
t.Parallel()
129-
130119
h := test.NewHelper(t)
131120
defer h.Cleanup()
132121

133-
pkg := "github.com/sdboyer/deptest"
134-
repo := "https://github.com/sdboyer/deptest.git"
135-
136122
ctx := newTestContext(h)
137123
sm, err := ctx.SourceManager()
138124
h.Must(err)
139125
defer sm.Release()
140126

127+
pkg := "github.com/sdboyer/deptest"
128+
repo := "https://github.com/sdboyer/deptest.git"
129+
141130
g := newGlideImporter(ctx.Err, true, sm)
142131
g.yaml = glideYaml{
143132
Imports: []glidePackage{
@@ -216,18 +205,16 @@ func TestGlideConfig_Convert_Project(t *testing.T) {
216205
}
217206

218207
func TestGlideConfig_Convert_TestProject(t *testing.T) {
219-
t.Parallel()
220-
221208
h := test.NewHelper(t)
222209
defer h.Cleanup()
223210

224-
pkg := "github.com/sdboyer/deptest"
225-
226211
ctx := newTestContext(h)
227212
sm, err := ctx.SourceManager()
228213
h.Must(err)
229214
defer sm.Release()
230215

216+
pkg := "github.com/sdboyer/deptest"
217+
231218
g := newGlideImporter(ctx.Err, true, sm)
232219
g.yaml = glideYaml{
233220
TestImports: []glidePackage{
@@ -266,12 +253,17 @@ func TestGlideConfig_Convert_TestProject(t *testing.T) {
266253
}
267254

268255
func TestGlideConfig_Convert_Ignore(t *testing.T) {
269-
t.Parallel()
256+
h := test.NewHelper(t)
257+
defer h.Cleanup()
270258

271-
pkg := "github.com/sdboyer/deptest"
259+
ctx := newTestContext(h)
260+
sm, err := ctx.SourceManager()
261+
h.Must(err)
262+
defer sm.Release()
272263

273-
logger := log.New(os.Stderr, "", 0)
274-
g := newGlideImporter(logger, true, nil)
264+
pkg := "github.com/sdboyer/deptest"
265+
266+
g := newGlideImporter(ctx.Err, true, sm)
275267
g.yaml = glideYaml{
276268
Ignores: []string{pkg},
277269
}
@@ -291,10 +283,15 @@ func TestGlideConfig_Convert_Ignore(t *testing.T) {
291283
}
292284

293285
func TestGlideConfig_Convert_ExcludeDir(t *testing.T) {
294-
t.Parallel()
286+
h := test.NewHelper(t)
287+
defer h.Cleanup()
295288

296-
logger := log.New(os.Stderr, "", 0)
297-
g := newGlideImporter(logger, true, nil)
289+
ctx := newTestContext(h)
290+
sm, err := ctx.SourceManager()
291+
h.Must(err)
292+
defer sm.Release()
293+
294+
g := newGlideImporter(ctx.Err, true, sm)
298295
g.yaml = glideYaml{
299296
ExcludeDirs: []string{"samples"},
300297
}
@@ -314,10 +311,15 @@ func TestGlideConfig_Convert_ExcludeDir(t *testing.T) {
314311
}
315312

316313
func TestGlideConfig_Convert_ExcludeDir_IgnoresMismatchedPackageName(t *testing.T) {
317-
t.Parallel()
314+
h := test.NewHelper(t)
315+
defer h.Cleanup()
318316

319-
logger := log.New(os.Stderr, "", 0)
320-
g := newGlideImporter(logger, true, nil)
317+
ctx := newTestContext(h)
318+
sm, err := ctx.SourceManager()
319+
h.Must(err)
320+
defer sm.Release()
321+
322+
g := newGlideImporter(ctx.Err, true, sm)
321323
g.yaml = glideYaml{
322324
Name: "github.com/golang/mismatched-package-name",
323325
ExcludeDirs: []string{"samples"},
@@ -338,27 +340,33 @@ func TestGlideConfig_Convert_ExcludeDir_IgnoresMismatchedPackageName(t *testing.
338340
}
339341

340342
func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) {
341-
t.Parallel()
342-
343343
testCases := map[string]glidePackage{
344344
"specified an os": {OS: "windows"},
345345
"specified an arch": {Arch: "i686"},
346346
}
347347

348348
for wantWarning, pkg := range testCases {
349349
t.Run(wantWarning, func(t *testing.T) {
350+
h := test.NewHelper(t)
351+
defer h.Cleanup()
352+
350353
pkg.Name = "github.com/sdboyer/deptest"
351354
pkg.Reference = "v1.0.0"
352355

356+
ctx := newTestContext(h)
357+
sm, err := ctx.SourceManager()
358+
h.Must(err)
359+
defer sm.Release()
360+
353361
// Capture stderr so we can verify warnings
354362
verboseOutput := &bytes.Buffer{}
355-
logger := log.New(verboseOutput, "", 0)
356-
g := newGlideImporter(logger, true, nil)
363+
ctx.Err = log.New(verboseOutput, "", 0)
364+
g := newGlideImporter(ctx.Err, true, sm)
357365
g.yaml = glideYaml{
358366
Imports: []glidePackage{pkg},
359367
}
360368

361-
_, _, err := g.convert(testGlideProjectRoot)
369+
_, _, err = g.convert(testGlideProjectRoot)
362370
if err != nil {
363371
t.Fatal(err)
364372
}
@@ -372,15 +380,20 @@ func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) {
372380
}
373381

374382
func TestGlideConfig_Convert_BadInput_EmptyPackageName(t *testing.T) {
375-
t.Parallel()
383+
h := test.NewHelper(t)
384+
defer h.Cleanup()
385+
386+
ctx := newTestContext(h)
387+
sm, err := ctx.SourceManager()
388+
h.Must(err)
389+
defer sm.Release()
376390

377-
logger := log.New(os.Stderr, "", 0)
378-
g := newGlideImporter(logger, true, nil)
391+
g := newGlideImporter(ctx.Err, true, sm)
379392
g.yaml = glideYaml{
380393
Imports: []glidePackage{{Name: ""}},
381394
}
382395

383-
_, _, err := g.convert(testGlideProjectRoot)
396+
_, _, err = g.convert(testGlideProjectRoot)
384397
if err == nil {
385398
t.Fatal("Expected conversion to fail because the package name is empty")
386399
}

0 commit comments

Comments
 (0)