From 6be11768a48b2f6714db7b0ac26496895d601a6a Mon Sep 17 00:00:00 2001 From: Forest Eckhardt Date: Fri, 30 Aug 2019 15:28:42 -0400 Subject: [PATCH] Implements and tests a SDK roll forward strategy that mimics Microsoft's roll forward policy [#167848284] --- .../integration/deploy_dotnetcore_app_test.go | 3 +- src/dotnetcore/supply/mocks_test.go | 32 ---------------- src/dotnetcore/supply/supply.go | 37 ++++++++++++++++--- src/dotnetcore/supply/supply_test.go | 16 ++++---- 4 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/dotnetcore/integration/deploy_dotnetcore_app_test.go b/src/dotnetcore/integration/deploy_dotnetcore_app_test.go index 89972a94f..5c1168705 100644 --- a/src/dotnetcore/integration/deploy_dotnetcore_app_test.go +++ b/src/dotnetcore/integration/deploy_dotnetcore_app_test.go @@ -89,8 +89,7 @@ var _ = Describe("CF Dotnet Buildpack", func() { }) - XContext("when the sdk is missing", func() { - //TODO: https://www.pivotaltracker.com/story/show/167848284 + Context("when the sdk is missing", func() { BeforeEach(func() { app = ReplaceFileTemplate(filepath.Join(bpDir, "fixtures", "source_2.1_global_json_templated"), "global.json", "sdk_version", "2.1.500") }) diff --git a/src/dotnetcore/supply/mocks_test.go b/src/dotnetcore/supply/mocks_test.go index 486fd780e..9373da554 100644 --- a/src/dotnetcore/supply/mocks_test.go +++ b/src/dotnetcore/supply/mocks_test.go @@ -36,7 +36,6 @@ func (m *MockCommand) EXPECT() *MockCommandMockRecorder { // Execute mocks base method func (m *MockCommand) Execute(arg0 string, arg1, arg2 io.Writer, arg3 string, arg4 ...string) error { - m.ctrl.T.Helper() varargs := []interface{}{arg0, arg1, arg2, arg3} for _, a := range arg4 { varargs = append(varargs, a) @@ -48,14 +47,12 @@ func (m *MockCommand) Execute(arg0 string, arg1, arg2 io.Writer, arg3 string, ar // Execute indicates an expected call of Execute func (mr *MockCommandMockRecorder) Execute(arg0, arg1, arg2, arg3 interface{}, arg4 ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() varargs := append([]interface{}{arg0, arg1, arg2, arg3}, arg4...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Execute", reflect.TypeOf((*MockCommand)(nil).Execute), varargs...) } // Output mocks base method func (m *MockCommand) Output(arg0, arg1 string, arg2 ...string) (string, error) { - m.ctrl.T.Helper() varargs := []interface{}{arg0, arg1} for _, a := range arg2 { varargs = append(varargs, a) @@ -68,7 +65,6 @@ func (m *MockCommand) Output(arg0, arg1 string, arg2 ...string) (string, error) // Output indicates an expected call of Output func (mr *MockCommandMockRecorder) Output(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() varargs := append([]interface{}{arg0, arg1}, arg2...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Output", reflect.TypeOf((*MockCommand)(nil).Output), varargs...) } @@ -98,7 +94,6 @@ func (m *MockManifest) EXPECT() *MockManifestMockRecorder { // AllDependencyVersions mocks base method func (m *MockManifest) AllDependencyVersions(arg0 string) []string { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AllDependencyVersions", arg0) ret0, _ := ret[0].([]string) return ret0 @@ -106,13 +101,11 @@ func (m *MockManifest) AllDependencyVersions(arg0 string) []string { // AllDependencyVersions indicates an expected call of AllDependencyVersions func (mr *MockManifestMockRecorder) AllDependencyVersions(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AllDependencyVersions", reflect.TypeOf((*MockManifest)(nil).AllDependencyVersions), arg0) } // DefaultVersion mocks base method func (m *MockManifest) DefaultVersion(arg0 string) (libbuildpack.Dependency, error) { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DefaultVersion", arg0) ret0, _ := ret[0].(libbuildpack.Dependency) ret1, _ := ret[1].(error) @@ -121,7 +114,6 @@ func (m *MockManifest) DefaultVersion(arg0 string) (libbuildpack.Dependency, err // DefaultVersion indicates an expected call of DefaultVersion func (mr *MockManifestMockRecorder) DefaultVersion(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DefaultVersion", reflect.TypeOf((*MockManifest)(nil).DefaultVersion), arg0) } @@ -150,7 +142,6 @@ func (m *MockInstaller) EXPECT() *MockInstallerMockRecorder { // FetchDependency mocks base method func (m *MockInstaller) FetchDependency(arg0 libbuildpack.Dependency, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "FetchDependency", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -158,13 +149,11 @@ func (m *MockInstaller) FetchDependency(arg0 libbuildpack.Dependency, arg1 strin // FetchDependency indicates an expected call of FetchDependency func (mr *MockInstallerMockRecorder) FetchDependency(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchDependency", reflect.TypeOf((*MockInstaller)(nil).FetchDependency), arg0, arg1) } // InstallDependency mocks base method func (m *MockInstaller) InstallDependency(arg0 libbuildpack.Dependency, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "InstallDependency", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -172,13 +161,11 @@ func (m *MockInstaller) InstallDependency(arg0 libbuildpack.Dependency, arg1 str // InstallDependency indicates an expected call of InstallDependency func (mr *MockInstallerMockRecorder) InstallDependency(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InstallDependency", reflect.TypeOf((*MockInstaller)(nil).InstallDependency), arg0, arg1) } // InstallOnlyVersion mocks base method func (m *MockInstaller) InstallOnlyVersion(arg0, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "InstallOnlyVersion", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -186,7 +173,6 @@ func (m *MockInstaller) InstallOnlyVersion(arg0, arg1 string) error { // InstallOnlyVersion indicates an expected call of InstallOnlyVersion func (mr *MockInstallerMockRecorder) InstallOnlyVersion(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InstallOnlyVersion", reflect.TypeOf((*MockInstaller)(nil).InstallOnlyVersion), arg0, arg1) } @@ -215,7 +201,6 @@ func (m *MockStager) EXPECT() *MockStagerMockRecorder { // BuildDir mocks base method func (m *MockStager) BuildDir() string { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "BuildDir") ret0, _ := ret[0].(string) return ret0 @@ -223,13 +208,11 @@ func (m *MockStager) BuildDir() string { // BuildDir indicates an expected call of BuildDir func (mr *MockStagerMockRecorder) BuildDir() *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BuildDir", reflect.TypeOf((*MockStager)(nil).BuildDir)) } // CacheDir mocks base method func (m *MockStager) CacheDir() string { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CacheDir") ret0, _ := ret[0].(string) return ret0 @@ -237,13 +220,11 @@ func (m *MockStager) CacheDir() string { // CacheDir indicates an expected call of CacheDir func (mr *MockStagerMockRecorder) CacheDir() *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CacheDir", reflect.TypeOf((*MockStager)(nil).CacheDir)) } // DepDir mocks base method func (m *MockStager) DepDir() string { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DepDir") ret0, _ := ret[0].(string) return ret0 @@ -251,13 +232,11 @@ func (m *MockStager) DepDir() string { // DepDir indicates an expected call of DepDir func (mr *MockStagerMockRecorder) DepDir() *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DepDir", reflect.TypeOf((*MockStager)(nil).DepDir)) } // DepsIdx mocks base method func (m *MockStager) DepsIdx() string { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DepsIdx") ret0, _ := ret[0].(string) return ret0 @@ -265,13 +244,11 @@ func (m *MockStager) DepsIdx() string { // DepsIdx indicates an expected call of DepsIdx func (mr *MockStagerMockRecorder) DepsIdx() *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DepsIdx", reflect.TypeOf((*MockStager)(nil).DepsIdx)) } // LinkDirectoryInDepDir mocks base method func (m *MockStager) LinkDirectoryInDepDir(arg0, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "LinkDirectoryInDepDir", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -279,13 +256,11 @@ func (m *MockStager) LinkDirectoryInDepDir(arg0, arg1 string) error { // LinkDirectoryInDepDir indicates an expected call of LinkDirectoryInDepDir func (mr *MockStagerMockRecorder) LinkDirectoryInDepDir(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkDirectoryInDepDir", reflect.TypeOf((*MockStager)(nil).LinkDirectoryInDepDir), arg0, arg1) } // AddBinDependencyLink mocks base method func (m *MockStager) AddBinDependencyLink(arg0, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddBinDependencyLink", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -293,13 +268,11 @@ func (m *MockStager) AddBinDependencyLink(arg0, arg1 string) error { // AddBinDependencyLink indicates an expected call of AddBinDependencyLink func (mr *MockStagerMockRecorder) AddBinDependencyLink(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddBinDependencyLink", reflect.TypeOf((*MockStager)(nil).AddBinDependencyLink), arg0, arg1) } // WriteEnvFile mocks base method func (m *MockStager) WriteEnvFile(arg0, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "WriteEnvFile", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -307,13 +280,11 @@ func (m *MockStager) WriteEnvFile(arg0, arg1 string) error { // WriteEnvFile indicates an expected call of WriteEnvFile func (mr *MockStagerMockRecorder) WriteEnvFile(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteEnvFile", reflect.TypeOf((*MockStager)(nil).WriteEnvFile), arg0, arg1) } // WriteProfileD mocks base method func (m *MockStager) WriteProfileD(arg0, arg1 string) error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "WriteProfileD", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -321,13 +292,11 @@ func (m *MockStager) WriteProfileD(arg0, arg1 string) error { // WriteProfileD indicates an expected call of WriteProfileD func (mr *MockStagerMockRecorder) WriteProfileD(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteProfileD", reflect.TypeOf((*MockStager)(nil).WriteProfileD), arg0, arg1) } // SetStagingEnvironment mocks base method func (m *MockStager) SetStagingEnvironment() error { - m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetStagingEnvironment") ret0, _ := ret[0].(error) return ret0 @@ -335,6 +304,5 @@ func (m *MockStager) SetStagingEnvironment() error { // SetStagingEnvironment indicates an expected call of SetStagingEnvironment func (mr *MockStagerMockRecorder) SetStagingEnvironment() *gomock.Call { - mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetStagingEnvironment", reflect.TypeOf((*MockStager)(nil).SetStagingEnvironment)) } diff --git a/src/dotnetcore/supply/supply.go b/src/dotnetcore/supply/supply.go index b06dfa289..e9bd46c79 100644 --- a/src/dotnetcore/supply/supply.go +++ b/src/dotnetcore/supply/supply.go @@ -9,6 +9,7 @@ import ( "log" "os" "path/filepath" + "strconv" "strings" "github.com/cloudfoundry/dotnet-core-buildpack/src/dotnetcore/config" @@ -292,13 +293,37 @@ func (s *Supplier) commandsInProjFiles(commands []string) (bool, error) { // Turn a semver string into major.minor.x // Will turn a.b.c into a.b.x // Will not modify strings that don't match a.b.c -func majorMinorOnly(version string) string { +func sdkRollForward(version string, versions []string) (string, error) { + var featureLine string + var highestPatch string parts := strings.SplitN(version, ".", 3) if len(parts) == 3 { - parts[2] = "x" // ignore patch version - return strings.Join(parts, ".") + featureLine = parts[2][:1] } - return version + + for _, v := range versions { + versionSplit := strings.SplitN(v, ".", 3) + if len(versionSplit) == 3 && versionSplit[2][:1] == featureLine { + if highestPatch == "" { + highestPatch = versionSplit[2][1:] + } else { + current, err := strconv.Atoi(highestPatch) + comp, err := strconv.Atoi(versionSplit[2][1:]) + if err != nil { + return "", err + } + if current < comp { + highestPatch = versionSplit[2][1:] + } + } + } + } + + if highestPatch == "" { + return "", fmt.Errorf("could not find sdk in same feature line") + } + + return fmt.Sprintf("%s.%s.%s%s", parts[0], parts[1], featureLine, highestPatch), nil } func (s *Supplier) pickVersionToInstall() (string, error) { @@ -327,7 +352,7 @@ func (s *Supplier) pickVersionToInstall() (string, error) { return globalJSONVersion, nil } s.Log.Warning("SDK %s in global.json is not available", globalJSONVersion) - installVersion, err := project.FindMatchingVersionWithPreview(majorMinorOnly(globalJSONVersion), allVersions) + installVersion, err := sdkRollForward(globalJSONVersion, allVersions) if err == nil { s.Log.Info("falling back to latest version in version line") return installVersion, nil @@ -408,7 +433,7 @@ func (s *Supplier) suppliedVersion(allVersions []string) (string, error) { } s.Log.Warning("SDK %s in global.json is not available", globalJSONVersion) // TODO: Reevaluate this logic in light of patch lines? https://docs.microsoft.com/en-us/dotnet/core/versions/ - installVersion, err := project.FindMatchingVersionWithPreview(majorMinorOnly(globalJSONVersion), allVersions) + installVersion, err := sdkRollForward(globalJSONVersion, allVersions) if err != nil { return "", nil } diff --git a/src/dotnetcore/supply/supply_test.go b/src/dotnetcore/supply/supply_test.go index 1eb53ea67..d57a29aa7 100644 --- a/src/dotnetcore/supply/supply_test.go +++ b/src/dotnetcore/supply/supply_test.go @@ -336,12 +336,12 @@ var _ = Describe("Supply", func() { Context("that is missing, but matches existing version lines", func() { BeforeEach(func() { - Expect(ioutil.WriteFile(filepath.Join(buildDir, "global.json"), []byte(`{"sdk": {"version": "1.2.3"}}`), 0644)).To(Succeed()) - mockManifest.EXPECT().AllDependencyVersions("dotnet-sdk").Return([]string{"1.1.1", "1.2.5", "1.2.6", "1.3.7"}) + Expect(ioutil.WriteFile(filepath.Join(buildDir, "global.json"), []byte(`{"sdk": {"version": "1.2.301"}}`), 0644)).To(Succeed()) + mockManifest.EXPECT().AllDependencyVersions("dotnet-sdk").Return([]string{"1.1.113", "1.2.303", "1.2.608", "1.3.709"}) }) - It("installs the latest of the same version line", func() { - dep := libbuildpack.Dependency{Name: "dotnet-sdk", Version: "1.2.6"} + It("installs the latest of the same feature line", func() { + dep := libbuildpack.Dependency{Name: "dotnet-sdk", Version: "1.2.303"} mockInstaller.EXPECT().InstallDependency(dep, filepath.Join(depsDir, depsIdx, "dotnet-sdk")) Expect(supplier.InstallDotnetSdk()).To(Succeed()) @@ -423,12 +423,12 @@ var _ = Describe("Supply", func() { Context("that is missing, but matches existing version lines", func() { BeforeEach(func() { - Expect(ioutil.WriteFile(filepath.Join(buildDir, "global.json"), []byte(`{"sdk": {"version": "1.2.3"}}`), 0644)).To(Succeed()) - mockManifest.EXPECT().AllDependencyVersions("dotnet-sdk").Return([]string{"1.1.1", "1.2.5", "1.2.6", "1.3.7"}) + Expect(ioutil.WriteFile(filepath.Join(buildDir, "global.json"), []byte(`{"sdk": {"version": "1.2.301"}}`), 0644)).To(Succeed()) + mockManifest.EXPECT().AllDependencyVersions("dotnet-sdk").Return([]string{"1.1.113", "1.2.303", "1.2.608", "1.3.709"}) }) - It("installs the latest of the same version line", func() { - dep := libbuildpack.Dependency{Name: "dotnet-sdk", Version: "1.2.6"} + It("installs the latest of the same feature line", func() { + dep := libbuildpack.Dependency{Name: "dotnet-sdk", Version: "1.2.303"} mockInstaller.EXPECT().InstallDependency(dep, filepath.Join(depsDir, depsIdx, "dotnet-sdk")) Expect(supplier.InstallDotnetSdk()).To(Succeed())