From a75d6d2f3a3477df54ffb21294c6673032b8d4fc Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 24 May 2017 12:26:48 -0500 Subject: [PATCH 1/5] Support implied carets in gps --- internal/gps/constraint_test.go | 22 +++++++++++++++++ internal/gps/constraints.go | 44 +++++++++++++++++++++++++++++++++ internal/gps/version.go | 20 +++++++++++++++ internal/gps/version_unifier.go | 7 ++++++ 4 files changed, 93 insertions(+) diff --git a/internal/gps/constraint_test.go b/internal/gps/constraint_test.go index ab99063919..0aaf5f1709 100644 --- a/internal/gps/constraint_test.go +++ b/internal/gps/constraint_test.go @@ -674,6 +674,28 @@ func TestSemverConstraintOps(t *testing.T) { } } +func TestSemverConstraint_ImpliedCaret(t *testing.T) { + c, _ := NewSemverConstraintIC("1.0.0") + + wantS := "^1.0.0" + gotS := c.String() + if wantS != gotS { + t.Errorf("Expected string %s, got %s", wantS, gotS) + } + + wantI := "1.0.0" + gotI := c.ImpliedCaretString() + if wantI != gotI { + t.Errorf("Expected implied string %s, got %s", wantI, gotI) + } + + wantT := "svc-^1.0.0" + gotT := c.typedString() + if wantT != gotT { + t.Errorf("Expected type string %s, got %s", wantT, gotT) + } +} + // Test that certain types of cross-version comparisons work when they are // expressed as a version union (but that others don't). func TestVersionUnion(t *testing.T) { diff --git a/internal/gps/constraints.go b/internal/gps/constraints.go index eb89467e00..89762231b1 100644 --- a/internal/gps/constraints.go +++ b/internal/gps/constraints.go @@ -25,6 +25,14 @@ var ( type Constraint interface { fmt.Stringer + // ImpliedCaretString converts the Constraint to a string in the same manner + // as String(), but treats the empty operator as equivalent to ^, rather + // than =. + // + // In the same way that String() is the inverse of NewConstraint(), this + // method is the inverse of to NewSemverConstraintIC(). + ImpliedCaretString() string + // Matches indicates if the provided Version is allowed by the Constraint. Matches(Version) bool @@ -64,6 +72,24 @@ func NewSemverConstraint(body string) (Constraint, error) { return semverConstraint{c: c}, nil } +// NewSemverConstraintIC attempts to construct a semver Constraint object from the +// input string, defaulting to a caret, ^, when no constraint is specified. +// +// If the input string cannot be made into a valid semver Constraint, an error +// is returned. +func NewSemverConstraintIC(body string) (Constraint, error) { + c, err := semver.NewConstraintIC(body) + if err != nil { + return nil, err + } + // If we got a simple semver.Version, simplify by returning our + // corresponding type + if sv, ok := c.(semver.Version); ok { + return semVersion{sv: sv}, nil + } + return semverConstraint{c: c}, nil +} + type semverConstraint struct { c semver.Constraint } @@ -72,6 +98,16 @@ func (c semverConstraint) String() string { return c.c.String() } +// ImpliedCaretString converts the Constraint to a string in the same manner +// as String(), but treats the empty operator as equivalent to ^, rather +// than =. +// +// In the same way that String() is the inverse of NewConstraint(), this +// method is the inverse of to NewSemverConstraintIC(). +func (c semverConstraint) ImpliedCaretString() string { + return c.c.ImpliedCaretString() +} + func (c semverConstraint) typedString() string { return fmt.Sprintf("svc-%s", c.c.String()) } @@ -153,6 +189,10 @@ func (anyConstraint) String() string { return "*" } +func (anyConstraint) ImpliedCaretString() string { + return "*" +} + func (anyConstraint) typedString() string { return "any-*" } @@ -177,6 +217,10 @@ func (noneConstraint) String() string { return "" } +func (noneConstraint) ImpliedCaretString() string { + return "" +} + func (noneConstraint) typedString() string { return "none-" } diff --git a/internal/gps/version.go b/internal/gps/version.go index a75e38aad6..060c5b6971 100644 --- a/internal/gps/version.go +++ b/internal/gps/version.go @@ -119,6 +119,10 @@ func (r Revision) String() string { return string(r) } +func (r Revision) ImpliedCaretString() string { + return r.String() +} + func (r Revision) typedString() string { return "r-" + string(r) } @@ -195,6 +199,10 @@ func (v branchVersion) String() string { return string(v.name) } +func (v branchVersion) ImpliedCaretString() string { + return v.String() +} + func (v branchVersion) typedString() string { return fmt.Sprintf("b-%s", v.String()) } @@ -272,6 +280,10 @@ func (v plainVersion) String() string { return string(v) } +func (v plainVersion) ImpliedCaretString() string { + return v.String() +} + func (v plainVersion) typedString() string { return fmt.Sprintf("pv-%s", v.String()) } @@ -355,6 +367,10 @@ func (v semVersion) String() string { return str } +func (v semVersion) ImpliedCaretString() string { + return v.sv.ImpliedCaretString() +} + func (v semVersion) typedString() string { return fmt.Sprintf("sv-%s", v.String()) } @@ -439,6 +455,10 @@ func (v versionPair) String() string { return v.v.String() } +func (v versionPair) ImpliedCaretString() string { + return v.v.ImpliedCaretString() +} + func (v versionPair) typedString() string { return fmt.Sprintf("%s-%s", v.Unpair().typedString(), v.Underlying().typedString()) } diff --git a/internal/gps/version_unifier.go b/internal/gps/version_unifier.go index 7f9dc5d646..d9cfb2a9ef 100644 --- a/internal/gps/version_unifier.go +++ b/internal/gps/version_unifier.go @@ -184,6 +184,13 @@ func (vtu versionTypeUnion) String() string { panic("versionTypeUnion should never be turned into a string; it is solver internal-only") } +// This should generally not be called, but is required for the interface. If it +// is called, we have a bigger problem (the type has escaped the solver); thus, +// panic. +func (vtu versionTypeUnion) ImpliedCaretString() string { + panic("versionTypeUnion should never be turned into a string; it is solver internal-only") +} + func (vtu versionTypeUnion) typedString() string { panic("versionTypeUnion should never be turned into a string; it is solver internal-only") } From ecf43b900ce71cc6deddce9808c4c3a139c61d40 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 24 May 2017 12:07:14 -0500 Subject: [PATCH 2/5] Always use implied carets in dep --- cmd/dep/ensure.go | 2 +- cmd/dep/ensure_test.go | 2 +- cmd/dep/init.go | 3 +-- cmd/dep/init_test.go | 2 +- manifest.go | 6 +++--- manifest_test.go | 4 ++-- testdata/manifest/golden.toml | 2 +- 7 files changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 5a28839430..a5657edb29 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -329,7 +329,7 @@ func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstrai // semver, a revision, or as a fallback, a plain tag func deduceConstraint(s string) gps.Constraint { // always semver if we can - c, err := gps.NewSemverConstraint(s) + c, err := gps.NewSemverConstraintIC(s) if err == nil { return c } diff --git a/cmd/dep/ensure_test.go b/cmd/dep/ensure_test.go index 5d1a4ee232..5ccd3d840f 100644 --- a/cmd/dep/ensure_test.go +++ b/cmd/dep/ensure_test.go @@ -13,7 +13,7 @@ import ( func TestDeduceConstraint(t *testing.T) { t.Parallel() - sv, err := gps.NewSemverConstraint("v1.2.3") + sv, err := gps.NewSemverConstraintIC("v1.2.3") if err != nil { t.Fatal(err) } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index 189ad039c0..43061623dc 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -319,8 +319,7 @@ func getProjectPropertiesFromVersion(v gps.Version) gps.ProjectProperties { case gps.IsBranch, gps.IsVersion: pp.Constraint = v case gps.IsSemver: - // TODO: remove "^" when https://github.com/golang/dep/issues/225 is ready. - c, err := gps.NewSemverConstraint("^" + v.String()) + c, err := gps.NewSemverConstraintIC(v.String()) if err != nil { panic(err) } diff --git a/cmd/dep/init_test.go b/cmd/dep/init_test.go index 37b6cfdf18..0c177de296 100644 --- a/cmd/dep/init_test.go +++ b/cmd/dep/init_test.go @@ -27,7 +27,7 @@ func TestContains(t *testing.T) { func TestGetProjectPropertiesFromVersion(t *testing.T) { t.Parallel() - wantSemver, _ := gps.NewSemverConstraint("^v1.0.0") + wantSemver, _ := gps.NewSemverConstraintIC("v1.0.0") cases := []struct { version, want gps.Constraint }{ diff --git a/manifest.go b/manifest.go index 557659bdc4..d92c72d397 100644 --- a/manifest.go +++ b/manifest.go @@ -162,7 +162,7 @@ func toProject(raw rawProject) (n gps.ProjectRoot, pp gps.ProjectProperties, err } // always semver if we can - pp.Constraint, err = gps.NewSemverConstraint(raw.Version) + pp.Constraint, err = gps.NewSemverConstraintIC(raw.Version) if err != nil { // but if not, fall back on plain versions pp.Constraint = gps.NewVersion(raw.Version) @@ -236,7 +236,7 @@ func toRawProject(name gps.ProjectRoot, project gps.ProjectProperties) rawProjec case gps.IsBranch: raw.Branch = v.String() case gps.IsSemver, gps.IsVersion: - raw.Version = v.String() + raw.Version = v.ImpliedCaretString() } return raw } @@ -248,7 +248,7 @@ func toRawProject(name gps.ProjectRoot, project gps.ProjectProperties) rawProjec // if !gps.IsAny(pp.Constraint) && !gps.IsNone(pp.Constraint) { if !gps.IsAny(project.Constraint) && project.Constraint != nil { // Has to be a semver range. - raw.Version = project.Constraint.String() + raw.Version = project.Constraint.ImpliedCaretString() } return raw } diff --git a/manifest_test.go b/manifest_test.go index 7ccb021008..8c2b68545a 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -25,7 +25,7 @@ func TestReadManifest(t *testing.T) { t.Fatalf("Should have read Manifest correctly, but got err %q", err) } - c, _ := gps.NewSemverConstraint(">=0.12.0, <1.0.0") + c, _ := gps.NewSemverConstraint("^0.12.0") want := Manifest{ Dependencies: map[gps.ProjectRoot]gps.ProjectProperties{ gps.ProjectRoot("github.com/golang/dep/internal/gps"): { @@ -61,7 +61,7 @@ func TestWriteManifest(t *testing.T) { golden := "manifest/golden.toml" want := h.GetTestFileString(golden) - c, _ := gps.NewSemverConstraint(">=0.12.0, <1.0.0") + c, _ := gps.NewSemverConstraint("^0.12.0") m := &Manifest{ Dependencies: map[gps.ProjectRoot]gps.ProjectProperties{ gps.ProjectRoot("github.com/golang/dep/internal/gps"): { diff --git a/testdata/manifest/golden.toml b/testdata/manifest/golden.toml index 98b7cd3e09..8bab2107ee 100644 --- a/testdata/manifest/golden.toml +++ b/testdata/manifest/golden.toml @@ -6,7 +6,7 @@ ignored = ["github.com/foo/bar"] [[dependencies]] name = "github.com/golang/dep/internal/gps" - version = ">=0.12.0, <1.0.0" + version = "0.12.0" [[overrides]] branch = "master" From 5285e629160a93476fc2fe421ed344b3991932e4 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 24 May 2017 12:08:33 -0500 Subject: [PATCH 3/5] Preserve previous test behavior before implied caret --- .../testdata/harness_tests/ensure/override/case1/testcase.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/dep/testdata/harness_tests/ensure/override/case1/testcase.json b/cmd/dep/testdata/harness_tests/ensure/override/case1/testcase.json index efbbc392f7..4f1ce1d26b 100644 --- a/cmd/dep/testdata/harness_tests/ensure/override/case1/testcase.json +++ b/cmd/dep/testdata/harness_tests/ensure/override/case1/testcase.json @@ -1,7 +1,7 @@ { "commands": [ ["init"], - ["ensure", "-override", "github.com/sdboyer/deptest@1.0.0"] + ["ensure", "-override", "github.com/sdboyer/deptest@=1.0.0"] ], "error-expected": "", "vendor-final": [ From 8dcaab5ad14429c042596846b9d343e63730436f Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Tue, 23 May 2017 10:38:35 -0500 Subject: [PATCH 4/5] Fix deduceConstraint test to not rely upon the types being comparable --- cmd/dep/ensure_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/dep/ensure_test.go b/cmd/dep/ensure_test.go index 5ccd3d840f..927cac2492 100644 --- a/cmd/dep/ensure_test.go +++ b/cmd/dep/ensure_test.go @@ -5,6 +5,7 @@ package main import ( + "reflect" "testing" "github.com/golang/dep/internal/gps" @@ -31,10 +32,16 @@ func TestDeduceConstraint(t *testing.T) { "20120425195858-psty8c35ve2oej8t": gps.NewVersion("20120425195858-psty8c35ve2oej8t"), } - for str, expected := range constraints { - c := deduceConstraint(str) - if c != expected { - t.Fatalf("expected: %#v, got %#v for %s", expected, c, str) + for str, want := range constraints { + got := deduceConstraint(str) + + wantT := reflect.TypeOf(want) + gotT := reflect.TypeOf(got) + if wantT != gotT { + t.Errorf("expected type: %s, got %s, for input %s", wantT, gotT, str) + } + if got.String() != want.String() { + t.Errorf("expected value: %s, got %s for input %s", want, got, str) } } } From 16e350fdadd15d682490a458b9d7d183c4d3cccd Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 24 May 2017 15:14:11 -0500 Subject: [PATCH 5/5] Update testdata now that init omits carets in the manifest --- .../harness_tests/ensure/empty/case1/final/Gopkg.toml | 2 +- .../harness_tests/ensure/override/case1/final/Gopkg.toml | 2 +- cmd/dep/testdata/harness_tests/init/case1/final/Gopkg.toml | 2 +- cmd/dep/testdata/harness_tests/init/case2/final/Gopkg.toml | 4 ++-- .../testdata/harness_tests/init/skip-hidden/final/Gopkg.toml | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/dep/testdata/harness_tests/ensure/empty/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/empty/case1/final/Gopkg.toml index 26987273ec..c34f3e6502 100644 --- a/cmd/dep/testdata/harness_tests/ensure/empty/case1/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/ensure/empty/case1/final/Gopkg.toml @@ -53,4 +53,4 @@ [[dependencies]] name = "github.com/sdboyer/deptest" - version = "^1.0.0" + version = "1.0.0" diff --git a/cmd/dep/testdata/harness_tests/ensure/override/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/override/case1/final/Gopkg.toml index 26987273ec..c34f3e6502 100644 --- a/cmd/dep/testdata/harness_tests/ensure/override/case1/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/ensure/override/case1/final/Gopkg.toml @@ -53,4 +53,4 @@ [[dependencies]] name = "github.com/sdboyer/deptest" - version = "^1.0.0" + version = "1.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/case1/final/Gopkg.toml index 6deaa21764..7cbbec2985 100644 --- a/cmd/dep/testdata/harness_tests/init/case1/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/init/case1/final/Gopkg.toml @@ -1,4 +1,4 @@ [[dependencies]] name = "github.com/sdboyer/deptest" - version = "^0.8.0" + version = "0.8.0" diff --git a/cmd/dep/testdata/harness_tests/init/case2/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/case2/final/Gopkg.toml index 55921055ae..91f9228623 100644 --- a/cmd/dep/testdata/harness_tests/init/case2/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/init/case2/final/Gopkg.toml @@ -1,8 +1,8 @@ [[dependencies]] name = "github.com/sdboyer/deptest" - version = "^0.8.0" + version = "0.8.0" [[dependencies]] name = "github.com/sdboyer/deptestdos" - version = "^2.0.0" + version = "2.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/skip-hidden/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/skip-hidden/final/Gopkg.toml index d5f3e3c9d6..04a443daa3 100644 --- a/cmd/dep/testdata/harness_tests/init/skip-hidden/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/init/skip-hidden/final/Gopkg.toml @@ -1,4 +1,4 @@ [[dependencies]] name = "github.com/sdboyer/deptest" - version = "^1.0.0" + version = "1.0.0"