From dba953b655e5374ac9880629fe71b6f9a7ec822d Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Tue, 27 Aug 2024 14:07:00 -0700 Subject: [PATCH 1/7] No sense in doing this dance if we don't actually use it --- internal/backends/python/python.go | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/internal/backends/python/python.go b/internal/backends/python/python.go index 5ef2bd9f..39850632 100644 --- a/internal/backends/python/python.go +++ b/internal/backends/python/python.go @@ -275,9 +275,8 @@ func searchPypi(query string) []api.PkgInfo { return filtered } -// makePythonPoetryBackend returns a backend for invoking poetry, given an arg0 for invoking Python -// (either a full path or just a name like "python3") to use when invoking Python. -func makePythonPoetryBackend(python string) api.LanguageBackend { +// makePythonPoetryBackend returns a backend for invoking poetry +func makePythonPoetryBackend() api.LanguageBackend { return api.LanguageBackend{ Name: "python3-poetry", Alias: "python-python3-poetry", @@ -414,9 +413,8 @@ var pythonGuessRegexps = util.Regexps([]string{ `import ((?:.|\\\n)*)`, }) -// makePythonPipBackend returns a backend for invoking poetry, given an arg0 for invoking Python -// (either a full path or just a name like "python3") to use when invoking Python. -func makePythonPipBackend(python string) api.LanguageBackend { +// makePythonPipBackend returns a backend for invoking pip. +func makePythonPipBackend() api.LanguageBackend { var pipFlags []PipFlag b := api.LanguageBackend{ @@ -672,17 +670,6 @@ func listPoetrySpecfile(mergeAllGroups bool) (map[api.PkgName]api.PkgSpec, error return pkgs, nil } -// getPython3 returns either "python3" or the value of the UPM_PYTHON3 -// environment variable. -func getPython3() string { - python3 := os.Getenv("UPM_PYTHON3") - if python3 != "" { - return python3 - } else { - return "python3" - } -} - // PythonPoetryBackend is a UPM backend for Python 3 that uses Poetry. -var PythonPoetryBackend = makePythonPoetryBackend(getPython3()) -var PythonPipBackend = makePythonPipBackend(getPython3()) +var PythonPoetryBackend = makePythonPoetryBackend() +var PythonPipBackend = makePythonPipBackend() From 7604f8007150ff26341d73cca76b9eede0bc71d3 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Tue, 27 Aug 2024 14:15:12 -0700 Subject: [PATCH 2/7] Take "root" into account for isModuleLocalToRoot --- internal/backends/python/modules.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backends/python/modules.go b/internal/backends/python/modules.go index f542d643..119da730 100644 --- a/internal/backends/python/modules.go +++ b/internal/backends/python/modules.go @@ -31,7 +31,7 @@ func (state *moduleState) isModuleLocalToRoot(pkg string, root string) bool { last := components[len(components)-1] components = components[:len(components)-1] - return state.isModuleComponent(path.Join(components...), last) + return state.isModuleComponent(path.Join(append([]string{root}, components...)...), last) } // Test to see if a dotted package, `pkg`, is a python module, relative to any project root From c1672ca81086e9962abf73074e296f629adb9fea Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Tue, 27 Aug 2024 14:27:12 -0700 Subject: [PATCH 3/7] Break out common nix pasta --- internal/backends/python/python.go | 47 ++++++++++++------------------ 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/internal/backends/python/python.go b/internal/backends/python/python.go index 39850632..fbfce345 100644 --- a/internal/backends/python/python.go +++ b/internal/backends/python/python.go @@ -275,6 +275,23 @@ func searchPypi(query string) []api.PkgInfo { return filtered } +func commonInstallNixDeps(ctx context.Context, pkgs []api.PkgName, specfilePkgs map[api.PkgName]api.PkgSpec) { + //nolint:ineffassign,wastedassign,staticcheck + span, ctx := tracer.StartSpanFromContext(ctx, "python.InstallReplitNixSystemDependencies") + defer span.Finish() + ops := []nix.NixEditorOp{} + for _, pkg := range pkgs { + deps := nix.PythonNixDeps(string(pkg)) + ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) + } + + for pkg := range specfilePkgs { + deps := nix.PythonNixDeps(string(pkg)) + ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) + } + nix.RunNixEditorOps(ops) +} + // makePythonPoetryBackend returns a backend for invoking poetry func makePythonPoetryBackend() api.LanguageBackend { return api.LanguageBackend{ @@ -383,23 +400,10 @@ func makePythonPoetryBackend() api.LanguageBackend { GuessRegexps: pythonGuessRegexps, Guess: guess, InstallReplitNixSystemDependencies: func(ctx context.Context, pkgs []api.PkgName) { - //nolint:ineffassign,wastedassign,staticcheck - span, ctx := tracer.StartSpanFromContext(ctx, "python.InstallReplitNixSystemDependencies") - defer span.Finish() - ops := []nix.NixEditorOp{} - for _, pkg := range pkgs { - deps := nix.PythonNixDeps(string(pkg)) - ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) - } - // Ignore the error here, because if we can't read the specfile, // we still want to add the deps from above at least. specfilePkgs, _ := listPoetrySpecfile(true) - for pkg := range specfilePkgs { - deps := nix.PythonNixDeps(string(pkg)) - ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) - } - nix.RunNixEditorOps(ops) + commonInstallNixDeps(ctx, pkgs, specfilePkgs) }, } } @@ -584,23 +588,10 @@ func makePythonPipBackend() api.LanguageBackend { GuessRegexps: pythonGuessRegexps, Guess: guess, InstallReplitNixSystemDependencies: func(ctx context.Context, pkgs []api.PkgName) { - //nolint:ineffassign,wastedassign,staticcheck - span, ctx := tracer.StartSpanFromContext(ctx, "python.InstallReplitNixSystemDependencies") - defer span.Finish() - ops := []nix.NixEditorOp{} - for _, pkg := range pkgs { - deps := nix.PythonNixDeps(string(pkg)) - ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) - } - // Ignore the error here, because if we can't read the specfile, // we still want to add the deps from above at least. _, specfilePkgs, _ := ListRequirementsTxt("requirements.txt") - for pkg := range specfilePkgs { - deps := nix.PythonNixDeps(string(pkg)) - ops = append(ops, nix.ReplitNixAddToNixEditorOps(deps)...) - } - nix.RunNixEditorOps(ops) + commonInstallNixDeps(ctx, pkgs, specfilePkgs) }, } From 93de6ddab0aedb963e8ae15de06fad5fa802e044 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Tue, 27 Aug 2024 14:51:35 -0700 Subject: [PATCH 4/7] Break out common environment handling logic --- internal/backends/python/python.go | 39 ++++++++++++++++-------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/internal/backends/python/python.go b/internal/backends/python/python.go index fbfce345..940f945b 100644 --- a/internal/backends/python/python.go +++ b/internal/backends/python/python.go @@ -292,6 +292,21 @@ func commonInstallNixDeps(ctx context.Context, pkgs []api.PkgName, specfilePkgs nix.RunNixEditorOps(ops) } +func commonGuessPackageDir() string { + // Check if we're already inside an activated + // virtualenv. If so, just use it. + if venv := os.Getenv("VIRTUAL_ENV"); venv != "" { + return venv + } + + // Take PYTHONUSERBASE into consideration, if set + if userbase := os.Getenv("PYTHONUSERBASE"); userbase != "" { + return userbase + } + + return "" +} + // makePythonPoetryBackend returns a backend for invoking poetry func makePythonPoetryBackend() api.LanguageBackend { return api.LanguageBackend{ @@ -307,15 +322,9 @@ func makePythonPoetryBackend() api.LanguageBackend { NormalizePackageArgs: normalizePackageArgs, NormalizePackageName: normalizePackageName, GetPackageDir: func() string { - // Check if we're already inside an activated - // virtualenv. If so, just use it. - if venv := os.Getenv("VIRTUAL_ENV"); venv != "" { - return venv - } - - // Take PYTHONUSERBASE into consideration, if set - if userbase := os.Getenv("PYTHONUSERBASE"); userbase != "" { - return userbase + pkgdir := commonGuessPackageDir() + if pkgdir != "" { + return pkgdir } // Terminate early if we're running inside a repl. @@ -431,15 +440,9 @@ func makePythonPipBackend() api.LanguageBackend { NormalizePackageArgs: normalizePackageArgs, NormalizePackageName: normalizePackageName, GetPackageDir: func() string { - // Check if we're already inside an activated - // virtualenv. If so, just use it. - if venv := os.Getenv("VIRTUAL_ENV"); venv != "" { - return venv - } - - // Take PYTHONUSERBASE into consideration, if set - if userbase := os.Getenv("PYTHONUSERBASE"); userbase != "" { - return userbase + pkgdir := commonGuessPackageDir() + if pkgdir != "" { + return pkgdir } if outputB, err := util.GetCmdOutputFallible([]string{ From 7fea7fe6a48aa06147a81f307b51cb26492d5fee Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Tue, 27 Aug 2024 15:01:24 -0700 Subject: [PATCH 5/7] Reflowing python/python.go for clarity --- internal/backends/python/python.go | 256 ++++++++++++++--------------- 1 file changed, 124 insertions(+), 132 deletions(-) diff --git a/internal/backends/python/python.go b/internal/backends/python/python.go index 940f945b..5ef1e6db 100644 --- a/internal/backends/python/python.go +++ b/internal/backends/python/python.go @@ -80,16 +80,6 @@ type poetryLock struct { } `json:"package"` } -func pipIsAvailable() bool { - _, err := exec.LookPath("pip") - return err == nil -} - -func poetryIsAvailable() bool { - _, err := exec.LookPath("poetry") - return err == nil -} - // normalizeSpec returns the version string from a Poetry spec, or the // empty string. The Poetry spec may be either a string or a // map[string]interface{} with a "version" key that is a string. If @@ -208,45 +198,6 @@ func info(name api.PkgName) api.PkgInfo { return info } -func add(ctx context.Context, pkgs map[api.PkgName]api.PkgSpec, projectName string) { - //nolint:ineffassign,wastedassign,staticcheck - span, ctx := tracer.StartSpanFromContext(ctx, "poetry (init) add") - defer span.Finish() - // Initalize the specfile if it doesnt exist - if !util.Exists("pyproject.toml") { - cmd := []string{"poetry", "init", "--no-interaction"} - - if projectName != "" { - cmd = append(cmd, "--name", projectName) - } - - util.RunCmd(cmd) - } - - cmd := []string{"poetry", "add"} - for name, spec := range pkgs { - name := string(name) - if found, ok := moduleToPypiPackageAliases[name]; ok { - delete(pkgs, api.PkgName(name)) - name = found - pkgs[api.PkgName(name)] = api.PkgSpec(spec) - } - spec := string(spec) - - // NB: this doesn't work if spec has - // spaces in it, because of a bug in - // Poetry that can't be worked around. - // It looks like that bug might be - // fixed in the 1.0 release though :/ - if spec != "" { - cmd = append(cmd, name+" "+spec) - } else { - cmd = append(cmd, name) - } - } - util.RunCmd(cmd) -} - func searchPypi(query string) []api.PkgInfo { // Normalize query before looking it up in the overide map query = string(normalizePackageName(api.PkgName(query))) @@ -307,16 +258,89 @@ func commonGuessPackageDir() string { return "" } +var pythonGuessRegexps = util.Regexps([]string{ + // The (?:.|\\\n) subexpression allows us to + // match match multiple lines if + // backslash-escapes are used on the newlines. + `from (?:.|\\\n) import`, + `import ((?:.|\\\n)*) as`, + `import ((?:.|\\\n)*)`, +}) + +func readPyproject() (*pyprojectTOML, error) { + var cfg pyprojectTOML + if _, err := toml.DecodeFile("pyproject.toml", &cfg); err != nil { + return nil, err + } + return &cfg, nil +} + // makePythonPoetryBackend returns a backend for invoking poetry func makePythonPoetryBackend() api.LanguageBackend { + listPoetrySpecfile := func(mergeAllGroups bool) (map[api.PkgName]api.PkgSpec, error) { + cfg, err := readPyproject() + if err != nil { + return nil, err + } + pkgs := map[api.PkgName]api.PkgSpec{} + if cfg.Tool.Poetry == nil { + return pkgs, nil + } + for nameStr, spec := range cfg.Tool.Poetry.Dependencies { + if nameStr == "python" { + continue + } + + specStr := normalizeSpec(spec) + if specStr == "" { + continue + } + pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) + } + for nameStr, spec := range cfg.Tool.Poetry.DevDependencies { + if nameStr == "python" { + continue + } + + specStr := normalizeSpec(spec) + if specStr == "" { + continue + } + pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) + } + if mergeAllGroups && cfg.Tool.Poetry.Group != nil { + for _, group := range cfg.Tool.Poetry.Group { + for nameStr, spec := range group.Dependencies { + specStr := normalizeSpec(spec) + if specStr == "" { + continue + } + pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) + } + } + } + + return pkgs, nil + } + return api.LanguageBackend{ - Name: "python3-poetry", - Alias: "python-python3-poetry", - Specfile: "pyproject.toml", - IsSpecfileCompatible: verifyPoetrySpecfile, - Lockfile: "poetry.lock", - IsAvailable: poetryIsAvailable, - FilenamePatterns: []string{"*.py"}, + Name: "python3-poetry", + Alias: "python-python3-poetry", + Specfile: "pyproject.toml", + IsSpecfileCompatible: func(path string) (bool, error) { + cfg, err := readPyproject() + if err != nil { + return false, err + } + + return cfg.Tool.Poetry != nil, nil + }, + Lockfile: "poetry.lock", + IsAvailable: func() bool { + _, err := exec.LookPath("poetry") + return err == nil + }, + FilenamePatterns: []string{"*.py"}, Quirks: api.QuirksAddRemoveAlsoLocks | api.QuirksAddRemoveAlsoInstalls, NormalizePackageArgs: normalizePackageArgs, @@ -357,7 +381,44 @@ func makePythonPoetryBackend() api.LanguageBackend { Search: searchPypi, Info: info, - Add: add, + Add: func(ctx context.Context, pkgs map[api.PkgName]api.PkgSpec, projectName string) { + //nolint:ineffassign,wastedassign,staticcheck + span, ctx := tracer.StartSpanFromContext(ctx, "poetry (init) add") + defer span.Finish() + // Initalize the specfile if it doesnt exist + if !util.Exists("pyproject.toml") { + cmd := []string{"poetry", "init", "--no-interaction"} + + if projectName != "" { + cmd = append(cmd, "--name", projectName) + } + + util.RunCmd(cmd) + } + + cmd := []string{"poetry", "add"} + for name, spec := range pkgs { + name := string(name) + if found, ok := moduleToPypiPackageAliases[name]; ok { + delete(pkgs, api.PkgName(name)) + name = found + pkgs[api.PkgName(name)] = api.PkgSpec(spec) + } + spec := string(spec) + + // NB: this doesn't work if spec has + // spaces in it, because of a bug in + // Poetry that can't be worked around. + // It looks like that bug might be + // fixed in the 1.0 release though :/ + if spec != "" { + cmd = append(cmd, name+" "+spec) + } else { + cmd = append(cmd, name) + } + } + util.RunCmd(cmd) + }, Remove: func(ctx context.Context, pkgs map[api.PkgName]bool) { //nolint:ineffassign,wastedassign,staticcheck span, ctx := tracer.StartSpanFromContext(ctx, "poetry remove") @@ -417,23 +478,17 @@ func makePythonPoetryBackend() api.LanguageBackend { } } -var pythonGuessRegexps = util.Regexps([]string{ - // The (?:.|\\\n) subexpression allows us to - // match match multiple lines if - // backslash-escapes are used on the newlines. - `from (?:.|\\\n) import`, - `import ((?:.|\\\n)*) as`, - `import ((?:.|\\\n)*)`, -}) - // makePythonPipBackend returns a backend for invoking pip. func makePythonPipBackend() api.LanguageBackend { var pipFlags []PipFlag b := api.LanguageBackend{ - Name: "python3-pip", - Specfile: "requirements.txt", - IsAvailable: pipIsAvailable, + Name: "python3-pip", + Specfile: "requirements.txt", + IsAvailable: func() bool { + _, err := exec.LookPath("pip") + return err == nil + }, Alias: "python-python3-pip", FilenamePatterns: []string{"*.py"}, Quirks: api.QuirksAddRemoveAlsoInstalls | api.QuirksNotReproducible, @@ -601,69 +656,6 @@ func makePythonPipBackend() api.LanguageBackend { return b } -func readPyproject() (*pyprojectTOML, error) { - var cfg pyprojectTOML - if _, err := toml.DecodeFile("pyproject.toml", &cfg); err != nil { - return nil, err - } - return &cfg, nil -} - -func verifyPoetrySpecfile(path string) (bool, error) { - cfg, err := readPyproject() - if err != nil { - return false, err - } - - return cfg.Tool.Poetry != nil, nil -} - -func listPoetrySpecfile(mergeAllGroups bool) (map[api.PkgName]api.PkgSpec, error) { - cfg, err := readPyproject() - if err != nil { - return nil, err - } - pkgs := map[api.PkgName]api.PkgSpec{} - if cfg.Tool.Poetry == nil { - return pkgs, nil - } - for nameStr, spec := range cfg.Tool.Poetry.Dependencies { - if nameStr == "python" { - continue - } - - specStr := normalizeSpec(spec) - if specStr == "" { - continue - } - pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) - } - for nameStr, spec := range cfg.Tool.Poetry.DevDependencies { - if nameStr == "python" { - continue - } - - specStr := normalizeSpec(spec) - if specStr == "" { - continue - } - pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) - } - if mergeAllGroups && cfg.Tool.Poetry.Group != nil { - for _, group := range cfg.Tool.Poetry.Group { - for nameStr, spec := range group.Dependencies { - specStr := normalizeSpec(spec) - if specStr == "" { - continue - } - pkgs[api.PkgName(nameStr)] = api.PkgSpec(specStr) - } - } - } - - return pkgs, nil -} - // PythonPoetryBackend is a UPM backend for Python 3 that uses Poetry. var PythonPoetryBackend = makePythonPoetryBackend() var PythonPipBackend = makePythonPipBackend() From c90f413a50999f401c414be04319b49711ca9380 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Fri, 30 Aug 2024 11:15:45 -0700 Subject: [PATCH 6/7] Reflowing how we populate tests --- test-suite/Add_test.go | 11 ++-- test-suite/Guess_test.go | 2 +- test-suite/Install_test.go | 7 +-- test-suite/List_test.go | 8 +-- test-suite/Lock_test.go | 95 ++++++++++++++++---------------- test-suite/Remove_test.go | 14 ++--- test-suite/WhichLanguage_test.go | 10 ++-- test-suite/utils/backend_t.go | 49 ++++++++++++++++ 8 files changed, 115 insertions(+), 81 deletions(-) diff --git a/test-suite/Add_test.go b/test-suite/Add_test.go index 5e0775ab..7ee72864 100644 --- a/test-suite/Add_test.go +++ b/test-suite/Add_test.go @@ -39,23 +39,20 @@ func TestAdd(t *testing.T) { func doAdd(bt testUtils.BackendT, pkgs ...string) { for _, tmpl := range standardTemplates { - template := bt.Backend.Name + "/" + tmpl + "/" if tmpl != "no-deps" { bt.Subtest(tmpl, func(bt testUtils.BackendT) { if bt.Backend.QuirksIsReproducible() { bt.Subtest("locked", func(bt testUtils.BackendT) { bt.Subtest("each", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir(tmpl) for _, pkg := range pkgs { bt.UpmAdd(pkg) } }) bt.Subtest("all", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir(tmpl) bt.UpmAdd(pkgs...) }) }) @@ -63,14 +60,14 @@ func doAdd(bt testUtils.BackendT, pkgs ...string) { bt.Subtest("unlocked", func(bt testUtils.BackendT) { bt.Subtest("each", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir(tmpl) for _, pkg := range pkgs { bt.UpmAdd(pkg) } }) bt.Subtest("all", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir(tmpl) bt.UpmAdd(pkgs...) }) }) diff --git a/test-suite/Guess_test.go b/test-suite/Guess_test.go index ee2f6adb..bb653aaa 100644 --- a/test-suite/Guess_test.go +++ b/test-suite/Guess_test.go @@ -73,7 +73,7 @@ func TestGuess(t *testing.T) { bt.Subtest(test, func(bt testUtils.BackendT) { bt.AddTestFile(testSrcFile, test+"."+ext) - bt.AddTestFile(bt.Backend.Name+"/no-deps/"+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir("no-deps") expectFile, err := templates.FS.Open(testSrcFile + ".expect") if err != nil { diff --git a/test-suite/Install_test.go b/test-suite/Install_test.go index 65964dd3..085043ce 100644 --- a/test-suite/Install_test.go +++ b/test-suite/Install_test.go @@ -38,13 +38,8 @@ func doInstall(bt testUtils.BackendT) { continue } - template := bt.Backend.Name + "/" + tmpl + "/" bt.Subtest(tmpl, func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - if bt.Backend.QuirksIsReproducible() { - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) - } - + bt.AddTestDir(tmpl) bt.UpmInstall() packageDirPath := bt.UpmPackageDir() diff --git a/test-suite/List_test.go b/test-suite/List_test.go index c70c7ae2..30a59f34 100644 --- a/test-suite/List_test.go +++ b/test-suite/List_test.go @@ -69,12 +69,10 @@ func TestList(t *testing.T) { func doList(bt testUtils.BackendT, templatesToPackages map[string][]string) { for tmpl, expectPkgs := range templatesToPackages { - template := bt.Backend.Name + "/" + tmpl + "/" - bt.Subtest(tmpl, func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - if tmpl != "no-deps" && bt.Backend.QuirksIsReproducible() { - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir(tmpl) + if tmpl == "no-deps" && bt.Backend.QuirksIsReproducible() { + bt.RemoveTestFile(bt.Backend.Lockfile) } specs := bt.UpmListSpecFile() diff --git a/test-suite/Lock_test.go b/test-suite/Lock_test.go index 75f55e9e..23f0a16a 100644 --- a/test-suite/Lock_test.go +++ b/test-suite/Lock_test.go @@ -25,62 +25,61 @@ func TestLock(t *testing.T) { continue } - bt.Subtest(bt.Backend.Name, doLock) - } -} - -func doLock(bt testUtils.BackendT) { - // test absence of lock file - for _, tmpl := range standardTemplates { - template := bt.Backend.Name + "/" + tmpl + "/" + bt.Subtest(bt.Backend.Name, func(bt testUtils.BackendT) { + // test absence of lock file + for _, tmpl := range standardTemplates { + bt.Subtest(tmpl, func(bt testUtils.BackendT) { + bt.AddTestDir(tmpl) + bt.RemoveTestFile(bt.Backend.Lockfile) + + specDeps := bt.UpmListSpecFile() + + bt.UpmLock() + + lockDeps := bt.UpmListLockFile() + + for _, specDep := range specDeps { + found := false + for _, lockDep := range lockDeps { + if specDep.Name == lockDep.Name { + found = true + break + } + } + + if !found { + bt.Fail("expected %s in lock file", specDep.Name) + } + } + }) + } - bt.Subtest(tmpl, func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + // test absence of package dir + tmpl := "no-package-dir" + bt.Subtest(bt.Backend.Name+"/"+tmpl, func(bt testUtils.BackendT) { + bt.AddTestDir(tmpl) + bt.RemoveTestFile(bt.Backend.Lockfile) - specDeps := bt.UpmListSpecFile() + specDeps := bt.UpmListSpecFile() - bt.UpmLock() + bt.UpmLock() - lockDeps := bt.UpmListLockFile() + lockDeps := bt.UpmListLockFile() - for _, specDep := range specDeps { - found := false - for _, lockDep := range lockDeps { - if specDep.Name == lockDep.Name { - found = true - break + for _, specDep := range specDeps { + found := false + for _, lockDep := range lockDeps { + if specDep.Name == lockDep.Name { + found = true + break + } } - } - if !found { - bt.Fail("expected %s in lock file", specDep.Name) + if !found { + bt.Fail("expected %s in lock file", specDep.Name) + } } - } + }) }) } - - // test absence of package dir - bt.Subtest(bt.Backend.Name+"/no-package-dir", func(bt testUtils.BackendT) { - bt.AddTestFile(bt.Backend.Name+"/many-deps/"+bt.Backend.Specfile, bt.Backend.Specfile) - - specDeps := bt.UpmListSpecFile() - - bt.UpmLock() - - lockDeps := bt.UpmListLockFile() - - for _, specDep := range specDeps { - found := false - for _, lockDep := range lockDeps { - if specDep.Name == lockDep.Name { - found = true - break - } - } - - if !found { - bt.Fail("expected %s in lock file", specDep.Name) - } - } - }) } diff --git a/test-suite/Remove_test.go b/test-suite/Remove_test.go index 0b36c2ce..545a4cbc 100644 --- a/test-suite/Remove_test.go +++ b/test-suite/Remove_test.go @@ -51,23 +51,19 @@ func TestRemove(t *testing.T) { func doRemove(bt testUtils.BackendT, templatesToPkgsToRemove map[string][]string) { for tmpl, pkgsToRemove := range templatesToPkgsToRemove { - template := bt.Backend.Name + "/" + tmpl + "/" - if tmpl != "no-deps" { bt.Subtest(tmpl, func(bt testUtils.BackendT) { if bt.Backend.QuirksIsReproducible() { bt.Subtest("locked", func(bt testUtils.BackendT) { bt.Subtest("each", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir(tmpl) for _, pkg := range pkgsToRemove { bt.UpmRemove(pkg) } }) bt.Subtest("all", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir(tmpl) bt.UpmRemove(pkgsToRemove...) }) }) @@ -76,14 +72,16 @@ func doRemove(bt testUtils.BackendT, templatesToPkgsToRemove map[string][]string if !bt.Backend.QuirkRemoveNeedsLockfile() { bt.Subtest("unlocked", func(bt testUtils.BackendT) { bt.Subtest("each", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir(tmpl) + bt.RemoveTestFile(bt.Backend.Lockfile) for _, pkg := range pkgsToRemove { bt.UpmRemove(pkg) } }) bt.Subtest("all", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir(tmpl) + bt.RemoveTestFile(bt.Backend.Lockfile) bt.UpmRemove(pkgsToRemove...) }) }) diff --git a/test-suite/WhichLanguage_test.go b/test-suite/WhichLanguage_test.go index 22069abe..0ab0be1d 100644 --- a/test-suite/WhichLanguage_test.go +++ b/test-suite/WhichLanguage_test.go @@ -30,25 +30,23 @@ func TestWhichLanguage(t *testing.T) { continue } - template := bt.Backend.Name + "/one-dep/" - bt.Subtest(bt.Backend.Name, func(bt testUtils.BackendT) { if bt.Backend.QuirksIsReproducible() { bt.Subtest("locked", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) - bt.AddTestFile(template+bt.Backend.Lockfile, bt.Backend.Lockfile) + bt.AddTestDir("one-dep") bt.UpmWhichLanguage() }) } else { bt.Subtest("no lockfile", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir("one-dep") bt.UpmWhichLanguage() }) } if defaults[bt.Backend.Name] { bt.Subtest("default", func(bt testUtils.BackendT) { - bt.AddTestFile(template+bt.Backend.Specfile, bt.Backend.Specfile) + bt.AddTestDir("one-dep") + bt.RemoveTestFile(bt.Backend.Lockfile) bt.UpmWhichLanguage() }) } diff --git a/test-suite/utils/backend_t.go b/test-suite/utils/backend_t.go index f75b8632..4e24ecff 100644 --- a/test-suite/utils/backend_t.go +++ b/test-suite/utils/backend_t.go @@ -62,6 +62,55 @@ func (bt *BackendT) TestDir() string { return bt.testDir } +// AddTestDir relies on test-suite/templates/templates.go's embedded files, +// iterating over every bt.Backend.Name+"/"+tmpl recursively and copying them +// into the ephemeral test directory. +// +// Note, this relies on empty path segments, since the source and target path +// formats are slightly different. The source path includes the +// test classifier (no-deps, one-dep, etc), whereas the target expects every +// copied file to be copied into TestDir directly. +func (bt *BackendT) AddTestDir(tmpl string) { + var rec func(cur string) + rec = func(cur string) { + entries, _ := bt.templates.ReadDir(path.Join(bt.Backend.Name, tmpl, cur)) + for _, entry := range entries { + // Combine the current path segment with the src or dst roots + srcpath := path.Join(bt.Backend.Name, tmpl, cur, entry.Name()) + dstpath := path.Join(bt.TestDir(), cur, entry.Name()) + if entry.IsDir() { + err := os.MkdirAll(dstpath, 0o755) + if err != nil { + bt.Fail("Unable to create %s", dstpath) + } + rec(path.Join(cur, entry.Name())) + } else { + f, err := bt.templates.Open(srcpath) + if err != nil { + bt.t.Fatalf("failed to get template %s: %v", srcpath, err) + } + + dst, err := os.OpenFile(dstpath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o644) + if err != nil { + bt.t.Fatalf("failed to open or create test file %s: %v", dstpath, err) + } + + _, err = io.Copy(dst, f) + if err != nil { + bt.t.Fatalf("failed to read template %s: %v", srcpath, err) + } + } + } + } + + rec("") +} + +func (bt *BackendT) RemoveTestFile(fpath string) { + fullPath := path.Join(bt.TestDir(), fpath) + os.Remove(fullPath) +} + func (bt *BackendT) AddTestFile(template, as string) { templates := *bt.templates From c8de10990d87aa8545c1e5a7460b0290a2c93ef9 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Fri, 30 Aug 2024 11:45:58 -0700 Subject: [PATCH 7/7] Conditional, PEP440-style argument formatter --- internal/backends/python/python.go | 41 ++++++++++++------------ internal/backends/python/requirements.go | 1 + 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/internal/backends/python/python.go b/internal/backends/python/python.go index 5ef1e6db..c3363bf6 100644 --- a/internal/backends/python/python.go +++ b/internal/backends/python/python.go @@ -80,6 +80,17 @@ type poetryLock struct { } `json:"package"` } +func pep440Join(name api.PkgName, spec api.PkgSpec) string { + if spec == "" { + return string(name) + } else if matchSpecOnly.Match([]byte(spec)) { + return string(name) + string(spec) + } + // We did not match the version range separator in the spec, so we got + // something like "foo 1.2.3", we need to return "foo==1.2.3" + return string(name) + "==" + string(spec) +} + // normalizeSpec returns the version string from a Poetry spec, or the // empty string. The Poetry spec may be either a string or a // map[string]interface{} with a "version" key that is a string. If @@ -398,24 +409,18 @@ func makePythonPoetryBackend() api.LanguageBackend { cmd := []string{"poetry", "add"} for name, spec := range pkgs { - name := string(name) - if found, ok := moduleToPypiPackageAliases[name]; ok { + if found, ok := moduleToPypiPackageAliases[string(name)]; ok { delete(pkgs, api.PkgName(name)) - name = found - pkgs[api.PkgName(name)] = api.PkgSpec(spec) + name = api.PkgName(found) + pkgs[name] = api.PkgSpec(spec) } - spec := string(spec) // NB: this doesn't work if spec has // spaces in it, because of a bug in // Poetry that can't be worked around. // It looks like that bug might be // fixed in the 1.0 release though :/ - if spec != "" { - cmd = append(cmd, name+" "+spec) - } else { - cmd = append(cmd, name) - } + cmd = append(cmd, pep440Join(name, spec)) } util.RunCmd(cmd) }, @@ -523,15 +528,13 @@ func makePythonPipBackend() api.LanguageBackend { cmd = append(cmd, string(flag)) } for name, spec := range pkgs { - name := string(name) - spec := string(spec) - if found, ok := moduleToPypiPackageAliases[name]; ok { - delete(pkgs, api.PkgName(name)) - name = found - pkgs[api.PkgName(name)] = api.PkgSpec(spec) + if found, ok := moduleToPypiPackageAliases[string(name)]; ok { + delete(pkgs, name) + name = api.PkgName(found) + pkgs[name] = spec } - cmd = append(cmd, name+" "+spec) + cmd = append(cmd, pep440Join(name, spec)) } // Run install util.RunCmd(cmd) @@ -561,9 +564,7 @@ func makePythonPipBackend() api.LanguageBackend { if rawName, ok := normalizedPkgs[name]; ok { // We've meticulously maintained the pkgspec from the CLI args, if specified, // so we don't clobber it with pip freeze's output of "===" - name := string(matches[1]) - userArgSpec := string(pkgs[rawName]) - toAppend = append(toAppend, name+userArgSpec) + toAppend = append(toAppend, pep440Join(name, pkgs[rawName])) } } } diff --git a/internal/backends/python/requirements.go b/internal/backends/python/requirements.go index 4ee24a76..f04404e6 100644 --- a/internal/backends/python/requirements.go +++ b/internal/backends/python/requirements.go @@ -20,6 +20,7 @@ import ( var pep345Name = `(?:[A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])` var pep440VersionComponent = `(?:(?:~=|!=|===|==|>=|<=|>|<)\s*[^, ]+)` var pep440VersionSpec = pep440VersionComponent + `(?:\s*,\s*` + pep440VersionComponent + `)*` +var matchSpecOnly = regexp.MustCompile(`^` + pep440VersionSpec + `$`) var extrasSpec = `\[(` + pep345Name + `(?:\s*,\s*` + pep345Name + `)*)\]` var matchPackageAndSpec = regexp.MustCompile(`(?i)^\s*(` + pep345Name + `)\s*` + `((?:` + extrasSpec + `)?\s*(?:` + pep440VersionSpec + `)?)?\s*$`) var matchEggComponent = regexp.MustCompile(`(?i)\begg=(` + pep345Name + `)(?:$|[^A-Z0-9])`)