From 5bdcb2ec35c3b6eed6d9e282040cb5b861b8a14e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20W=C3=A5reus?= Date: Fri, 29 Sep 2023 16:41:33 +0200 Subject: [PATCH] fix missing targetframework in packages.config and improve lock file generation logic by deleting temp csproj files and using special naming for this custom lock file --- internal/resolution/pm/nuget/cmd_factory.go | 77 ++++++++++--------- .../resolution/pm/nuget/cmd_factory_test.go | 41 ++++++---- internal/resolution/pm/nuget/job.go | 13 ++++ internal/resolution/pm/nuget/job_test.go | 37 ++++++++- .../pm/nuget/testdata/cmd_factory_mock.go | 12 ++- 5 files changed, 125 insertions(+), 55 deletions(-) diff --git a/internal/resolution/pm/nuget/cmd_factory.go b/internal/resolution/pm/nuget/cmd_factory.go index 0073874f..96f67d3f 100644 --- a/internal/resolution/pm/nuget/cmd_factory.go +++ b/internal/resolution/pm/nuget/cmd_factory.go @@ -15,6 +15,7 @@ import ( type ICmdFactory interface { MakeInstallCmd(command string, file string) (*exec.Cmd, error) + GetTempoCsproj() string } type IExecPath interface { @@ -45,17 +46,23 @@ type CmdFactory struct { execPath IExecPath packageConfgRegex string packagesConfigTemplate string + tempoCsproj string } -func NewCmdFactory(execPath IExecPath) CmdFactory { - return CmdFactory{ +func NewCmdFactory(execPath IExecPath) *CmdFactory { + return &CmdFactory{ execPath: execPath, packageConfgRegex: PackagesConfigRegex, packagesConfigTemplate: packagesConfigTemplate, + tempoCsproj: "", } } -func (cmdf CmdFactory) MakeInstallCmd(command string, file string) (*exec.Cmd, error) { +func (cmdf *CmdFactory) GetTempoCsproj() string { + return cmdf.tempoCsproj +} + +func (cmdf *CmdFactory) MakeInstallCmd(command string, file string) (*exec.Cmd, error) { path, err := cmdf.execPath.LookPath(command) @@ -70,11 +77,14 @@ func (cmdf CmdFactory) MakeInstallCmd(command string, file string) (*exec.Cmd, e return nil, err } + fileLockName := "packages.lock.json" if packageConfig.MatchString(file) { file, err = cmdf.convertPackagesConfigToCsproj(file, command) if err != nil { return nil, err } + cmdf.tempoCsproj = file + fileLockName = ".packages.config.nuget.debricked.lock" } fileDir := filepath.Dir(file) @@ -82,7 +92,10 @@ func (cmdf CmdFactory) MakeInstallCmd(command string, file string) (*exec.Cmd, e return &exec.Cmd{ Path: path, Args: []string{command, "restore", + file, "--use-lock-file", + "--lock-file-path", + fileLockName, }, Dir: fileDir, }, err @@ -103,7 +116,7 @@ type Package struct { // that enables debricked to parse out transitive dependencies. // This may add some additional framework dependencies that will not show up if // we only scan the packages.config file. -func (cmdf CmdFactory) convertPackagesConfigToCsproj(filePath string, command string) (string, error) { +func (cmdf *CmdFactory) convertPackagesConfigToCsproj(filePath string, command string) (string, error) { packages, err := parsePackagesConfig(filePath) if err != nil { return "", err @@ -118,7 +131,7 @@ func (cmdf CmdFactory) convertPackagesConfigToCsproj(filePath string, command st return "", err } - newFilename := filePath + ".csproj" + newFilename := filePath + ".nuget.debricked.csproj.temp" err = writeContentToCsprojFile(newFilename, csprojContent) if err != nil { return "", err @@ -127,6 +140,24 @@ func (cmdf CmdFactory) convertPackagesConfigToCsproj(filePath string, command st return newFilename, nil } +func (cmdf *CmdFactory) createCsprojContentWithTemplate(targetFrameworksStr string, packages []Package) (string, error) { + tmplParsed, err := template.New("csproj").Parse(cmdf.packagesConfigTemplate) + if err != nil { + return "", err + } + + var tpl bytes.Buffer + err = tmplParsed.Execute(&tpl, map[string]interface{}{ + "TargetFrameworks": targetFrameworksStr, + "Packages": packages, + }) + if err != nil { + return "", err + } + + return tpl.String(), nil +} + var ioReadAllCsproj = io.ReadAll func getDotnetVersion(command string) (string, error) { @@ -141,23 +172,16 @@ func getDotnetVersion(command string) (string, error) { return strings.TrimSpace(out.String()), nil } +// getDefaultFrameworkOfDotnetVersion returns the default target framework for a given version of .NET. func getDefaultFrameworkOfDotnetVersion(dotnetVersion string) string { - - if strings.HasPrefix(dotnetVersion, "7") { + switch { + case strings.HasPrefix(dotnetVersion, "7"): return "net7.0" - } else if strings.HasPrefix(dotnetVersion, "6") { + case strings.HasPrefix(dotnetVersion, "6"): + return "net6.0" + default: return "net6.0" - } else if strings.HasPrefix(dotnetVersion, "5") { - return "net5.0" - } else if strings.HasPrefix(dotnetVersion, "3") { - return "netcoreapp3.1" - } else if strings.HasPrefix(dotnetVersion, "2") { - return "netcoreapp2.1" - } else if strings.HasPrefix(dotnetVersion, "1") { - return "netcoreapp1.1" } - - return "net6.0" } func parsePackagesConfig(filePath string) (*Packages, error) { @@ -207,23 +231,6 @@ func collectUniqueTargetFrameworks(packages []Package, command string) (string, return strings.Join(targetFrameworks, ";"), nil } -func (cmdf CmdFactory) createCsprojContentWithTemplate(targetFrameworksStr string, packages []Package) (string, error) { - tmplParsed, err := template.New("csproj").Parse(cmdf.packagesConfigTemplate) - if err != nil { - return "", err - } - - var tpl bytes.Buffer - err = tmplParsed.Execute(&tpl, map[string]interface{}{ - "TargetFrameworks": targetFrameworksStr, - "Packages": packages, - }) - if err != nil { - return "", err - } - - return tpl.String(), nil -} var osCreateCsproj = os.Create diff --git a/internal/resolution/pm/nuget/cmd_factory_test.go b/internal/resolution/pm/nuget/cmd_factory_test.go index 4915af86..a92ba914 100644 --- a/internal/resolution/pm/nuget/cmd_factory_test.go +++ b/internal/resolution/pm/nuget/cmd_factory_test.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "log" "os" "path/filepath" "testing" @@ -13,31 +12,45 @@ import ( ) func TestMakeInstallCmd(t *testing.T) { - cmd, err := NewCmdFactory( + cmdf := NewCmdFactory( ExecPath{}, - ).MakeInstallCmd(nuget, "file") + ) + cmd, err := cmdf.MakeInstallCmd(nuget, "file") assert.NoError(t, err) assert.NotNil(t, cmd) args := cmd.Args assert.Contains(t, args, "dotnet") assert.Contains(t, args, "restore") + assert.Contains(t, args, "--use-lock-file") + assert.Contains(t, args, "--lock-file-path") + assert.Contains(t, args, "packages.lock.json") + + tmp := cmdf.GetTempoCsproj() + assert.Equal(t, "", tmp) } func TestMakeInstallCmdPackagsConfig(t *testing.T) { - cmd, err := NewCmdFactory( + cmdf := NewCmdFactory( ExecPath{}, - ).MakeInstallCmd(nuget, "testdata/valid/packages.config") + ) + cmd, err := cmdf.MakeInstallCmd(nuget, "testdata/valid/packages.config") assert.NoError(t, err) assert.NotNil(t, cmd) args := cmd.Args assert.Contains(t, args, "dotnet") assert.Contains(t, args, "restore") + assert.Contains(t, args, "--use-lock-file") + assert.Contains(t, args, "--lock-file-path") + assert.Contains(t, args, ".packages.config.nuget.debricked.lock") // Cleanup: Remove the created .csproj file - if err := os.Remove("testdata/valid/packages.config.csproj"); err != nil { + if err := os.Remove("testdata/valid/packages.config.nuget.debricked.csproj.temp"); err != nil { t.Fatalf("Failed to remove test file: %v", err) } + + tmp := cmdf.GetTempoCsproj() + assert.Equal(t, "testdata/valid/packages.config.nuget.debricked.csproj.temp", tmp) } func MockReadAll(r io.Reader) ([]byte, error) { @@ -303,10 +316,10 @@ func TestCreateCsprojContent(t *testing.T) { func TestMakeInstallCmdBadPackagesConfigRegex(t *testing.T) { - cmd, err := CmdFactory{ + cmd, err := (&CmdFactory{ execPath: ExecPath{}, packageConfgRegex: "[", - }.MakeInstallCmd(nuget, "file") + }).MakeInstallCmd(nuget, "file") assert.Error(t, err) assert.Nil(t, cmd) @@ -350,10 +363,10 @@ func (ExecPathErr) LookPath(file string) (string, error) { func TestMakeInstallCmdExecPathError(t *testing.T) { - cmd, err := CmdFactory{ + cmd, err := (&CmdFactory{ execPath: ExecPathErr{}, packageConfgRegex: PackagesConfigRegex, - }.MakeInstallCmd(nuget, "file") + }).MakeInstallCmd(nuget, "file") assert.Error(t, err) assert.Nil(t, cmd) @@ -411,7 +424,6 @@ func TestConvertPackagesConfigToCsproj(t *testing.T) { packageConfgRegex: PackagesConfigRegex, packagesConfigTemplate: tt.packagesConfigTemplate, } - log.Println(nugetCommand) _, err := cmd.convertPackagesConfigToCsproj(tt.filePath, nugetCommand) if (err != nil) != tt.wantError { t.Errorf("convertPackagesConfigToCsproj(%q) = %v, want error: %v", tt.filePath, err, tt.wantError) @@ -421,7 +433,7 @@ func TestConvertPackagesConfigToCsproj(t *testing.T) { } // Cleanup: Remove the created .csproj file - if err := os.Remove("testdata/valid/packages.config.csproj"); err != nil { + if err := os.Remove("testdata/valid/packages.config.nuget.debricked.csproj.temp"); err != nil { t.Fatalf("Failed to remove test file: %v", err) } } @@ -449,10 +461,7 @@ func TestGetDefaultFrameworkOfDotnetVersion(t *testing.T) { }{ {"7.0.100", "net7.0"}, {"6.0.100", "net6.0"}, - {"5.0.100", "net5.0"}, - {"3.1.100", "netcoreapp3.1"}, - {"2.1.100", "netcoreapp2.1"}, - {"1.1.100", "netcoreapp1.1"}, + {"5.0.100", "net6.0"}, {"0.0.0", "net6.0"}, } diff --git a/internal/resolution/pm/nuget/job.go b/internal/resolution/pm/nuget/job.go index 479d8b4b..489cd828 100644 --- a/internal/resolution/pm/nuget/job.go +++ b/internal/resolution/pm/nuget/job.go @@ -2,6 +2,7 @@ package nuget import ( "fmt" + "os" "github.com/debricked/cli/internal/resolution/job" ) @@ -47,6 +48,8 @@ func (j *Job) Run() { } +var osRemoveAll = os.RemoveAll + func (j *Job) runInstallCmd() ([]byte, error) { j.nugetCommand = nuget @@ -60,5 +63,15 @@ func (j *Job) runInstallCmd() ([]byte, error) { return installCmdOutput, j.GetExitError(err) } + // Cleanup of the temporary .csproj file (packages.config) + tempFile := j.cmdFactory.GetTempoCsproj() + if tempFile != "" { + // remove the packages.config.csproj file + err = osRemoveAll(tempFile) + if err != nil { + return installCmdOutput, j.GetExitError(err) + } + } + return installCmdOutput, nil } diff --git a/internal/resolution/pm/nuget/job_test.go b/internal/resolution/pm/nuget/job_test.go index 3ea4c409..21e8c693 100644 --- a/internal/resolution/pm/nuget/job_test.go +++ b/internal/resolution/pm/nuget/job_test.go @@ -14,7 +14,7 @@ const ( ) func TestNewJob(t *testing.T) { - j := NewJob("file", false, CmdFactory{ + j := NewJob("file", false, &CmdFactory{ execPath: ExecPath{}, }) assert.Equal(t, "file", j.GetFile()) @@ -31,6 +31,41 @@ func TestRunInstall(t *testing.T) { assert.False(t, j.Errors().HasError()) } +func TestRunInstallPackagesConfig(t *testing.T) { + cmdFactoryMock := testdata.NewEchoCmdFactory() + cmdFactoryMock.GetTempoCsprojReturn = "tempo.csproj" + j := NewJob("packages.config", false, cmdFactoryMock) + + _, err := j.runInstallCmd() + assert.NoError(t, err) + + assert.False(t, j.Errors().HasError()) +} + +func TestRunInstallPackagesConfigRemoveAllErr(t *testing.T) { + + oldOsRemoveAll := osRemoveAll + cmdErr := errors.New("os-remove-all-error") + cmdErrGt := errors.New("\n\nos-remove-all-error") + osRemoveAll = func(path string) error { + return cmdErr + } + + defer func() { + osRemoveAll = oldOsRemoveAll + }() + + cmdFactoryMock := testdata.NewEchoCmdFactory() + cmdFactoryMock.GetTempoCsprojReturn = "tempo.csproj" + j := NewJob("packages.config", true, cmdFactoryMock) + + go jobTestdata.WaitStatus(j) + j.Run() + + assert.Equal(t, j.Errors().GetAll()[0], cmdErrGt) + +} + func TestInstall(t *testing.T) { j := Job{install: true} assert.Equal(t, true, j.Install()) diff --git a/internal/resolution/pm/nuget/testdata/cmd_factory_mock.go b/internal/resolution/pm/nuget/testdata/cmd_factory_mock.go index 093ba6b2..33b43475 100644 --- a/internal/resolution/pm/nuget/testdata/cmd_factory_mock.go +++ b/internal/resolution/pm/nuget/testdata/cmd_factory_mock.go @@ -5,16 +5,22 @@ import ( ) type CmdFactoryMock struct { - InstallCmdName string - MakeInstallErr error + InstallCmdName string + MakeInstallErr error + GetTempoCsprojReturn string } func NewEchoCmdFactory() CmdFactoryMock { return CmdFactoryMock{ - InstallCmdName: "echo", + InstallCmdName: "echo", + GetTempoCsprojReturn: "", } } func (f CmdFactoryMock) MakeInstallCmd(command string, file string) (*exec.Cmd, error) { return exec.Command(f.InstallCmdName), f.MakeInstallErr } + +func (f CmdFactoryMock) GetTempoCsproj() string { + return f.GetTempoCsprojReturn +}