Skip to content

Commit

Permalink
fix missing targetframework in packages.config
Browse files Browse the repository at this point in the history 
and improve lock file generation logic by deleting temp csproj files and using special naming for this custom lock file
  • Loading branch information
@emilwareus
emilwareus committed Sep 29, 2023
1 parent 09c872e commit 5bdcb2e
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 55 deletions.
77 changes: 42 additions & 35 deletions internal/resolution/pm/nuget/cmd_factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

type ICmdFactory interface {
MakeInstallCmd(command string, file string) (*exec.Cmd, error)
GetTempoCsproj() string
}

type IExecPath interface {
Expand Down Expand Up @@ -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)

Expand All @@ -70,19 +77,25 @@ 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)

return &exec.Cmd{
Path: path,
Args: []string{command, "restore",
file,
"--use-lock-file",
"--lock-file-path",
fileLockName,
},
Dir: fileDir,
}, err
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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

Expand Down
41 changes: 25 additions & 16 deletions internal/resolution/pm/nuget/cmd_factory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"io"
"log"
"os"
"path/filepath"
"testing"
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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"},
}

Expand Down
13 changes: 13 additions & 0 deletions internal/resolution/pm/nuget/job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nuget

import (
"fmt"
"os"

"github.com/debricked/cli/internal/resolution/job"
)
Expand Down Expand Up @@ -47,6 +48,8 @@ func (j *Job) Run() {

}

var osRemoveAll = os.RemoveAll

func (j *Job) runInstallCmd() ([]byte, error) {

j.nugetCommand = nuget
Expand All @@ -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
}
37 changes: 36 additions & 1 deletion internal/resolution/pm/nuget/job_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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())
Expand Down
12 changes: 9 additions & 3 deletions internal/resolution/pm/nuget/testdata/cmd_factory_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 5bdcb2e

Please sign in to comment.