diff --git a/cmd/brief/dependency_names_test.go b/cmd/brief/dependency_names_test.go new file mode 100644 index 0000000..2414ff6 --- /dev/null +++ b/cmd/brief/dependency_names_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/git-pkgs/brief" +) + +func TestScanPyPIDependencyNames(t *testing.T) { + const helperEnv = "BRIEF_PYPI_NAMES_ROOT" + if root := os.Getenv(helperEnv); root != "" { + cmdScan([]string{"-json", root}) + return + } + root, err := filepath.Abs("../../testdata/pypi-names-project") + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(os.Args[0], "-test.run=^TestScanPyPIDependencyNames$") + cmd.Env = append(os.Environ(), helperEnv+"="+root, "PATH=") + out, err := cmd.Output() + if err != nil { + t.Fatalf("scan failed: %v", err) + } + var report brief.Report + if err := json.Unmarshal(out, &report); err != nil { + t.Fatalf("parsing scan output: %v\n%s", err, out) + } + if !reportHasTool(&report, "build", "Flask") || !reportHasTool(&report, "docs", "MyST-Parser") { + t.Fatalf("tools = %+v, want Flask and MyST-Parser", report.Tools) + } +} + +func TestDiffPyPIDependencyNames(t *testing.T) { + const helperEnv = "BRIEF_DIFF_PYPI_NAMES" + if os.Getenv(helperEnv) != "" { + cmdDiff([]string{"-json", "HEAD"}) + os.Exit(0) + } + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + root := t.TempDir() + initGitScanFixture(t, root) + writeScanFixture(t, root, "requirements.txt", "FLask\n") + runGitFixture(t, root, "add", "requirements.txt") + runGitFixture(t, root, "commit", "-q", "-m", "initial") + writeScanFixture(t, root, "requirements.txt", "FLask\nMYST_PARSER\n") + cmd := exec.Command(os.Args[0], "-test.run=^TestDiffPyPIDependencyNames$") + cmd.Dir = root + cmd.Env = append(os.Environ(), helperEnv+"=1") + out, err := cmd.Output() + if err != nil { + t.Fatalf("diff failed: %v", err) + } + var report brief.Report + if err := json.Unmarshal(out, &report); err != nil { + t.Fatalf("parsing diff output: %v\n%s", err, out) + } + if !reportHasTool(&report, "build", "Flask") || !reportHasTool(&report, "docs", "MyST-Parser") { + t.Fatalf("tools = %+v, want Flask and MyST-Parser", report.Tools) + } +} diff --git a/detect/dependency_names_test.go b/detect/dependency_names_test.go new file mode 100644 index 0000000..140c64c --- /dev/null +++ b/detect/dependency_names_test.go @@ -0,0 +1,103 @@ +package detect + +import ( + "testing" + "testing/fstest" +) + +func TestPyPIDependencyNamesThroughScan(t *testing.T) { + for _, tt := range []struct{ name, tool, category string }{ + {"FLask", "Flask", "build"}, + {"MYST_PARSER", "MyST-Parser", "docs"}, + } { + t.Run(tt.name, func(t *testing.T) { + engine := NewFS(loadKB(t), fstest.MapFS{"requirements.txt": {Data: []byte(tt.name + "\n")}}) + report, err := engine.Run() + if err != nil { + t.Fatal(err) + } + assertToolDetected(t, report, tt.category, tt.tool) + if len(report.Dependencies) != 1 || report.Dependencies[0].Name != tt.name { + t.Fatalf("dependency spelling changed: %+v", report.Dependencies) + } + }) + } +} + +func TestPyPIDependencyNamesRespectDevelopmentScope(t *testing.T) { + for _, tt := range []struct { + name, content string + want bool + }{ + {"runtime", "[project]\nname = 'example'\ndependencies = ['PyTest']\n", false}, + {"development", "[tool.poetry.group.test.dependencies]\nPyTest = '*'\n", true}, + } { + t.Run(tt.name, func(t *testing.T) { + base := loadKB(t) + base.ByName["pytest"].Detect.Dependencies = nil + engine := NewFS(base, fstest.MapFS{"pyproject.toml": {Data: []byte(tt.content)}}) + report, err := engine.Run() + if err != nil { + t.Fatal(err) + } + if tt.want { + assertToolDetected(t, report, "test", "pytest") + } else { + assertToolNotDetected(t, report, "test", "pytest") + } + }) + } +} + +func TestNonPyPIDependencyNamesRemainCaseSensitive(t *testing.T) { + for _, tt := range []struct { + name string + want bool + }{ + {"testthat", true}, + {"TestThat", false}, + } { + t.Run(tt.name, func(t *testing.T) { + content := "Package: example\nVersion: 1.0.0\nSuggests: " + tt.name + "\n" + engine := NewFS(loadKB(t), fstest.MapFS{"DESCRIPTION": {Data: []byte(content)}}) + report, err := engine.Run() + if err != nil { + t.Fatal(err) + } + if tt.want { + assertToolDetected(t, report, "test", "testthat") + } else { + assertToolNotDetected(t, report, "test", "testthat") + } + }) + } +} + +func TestPyPIDependencyNamesInChangedFiles(t *testing.T) { + base := loadKB(t) + engine := NewFS(base, fstest.MapFS{ + "requirements.txt": {Data: []byte("MYST_PARSER\n")}, + "app.py": {Data: []byte("print('example')\n")}, + }) + report, err := engine.Run() + if err != nil { + t.Fatal(err) + } + assertToolDetected(t, FilterByChangedFiles(report, base, []string{"requirements.txt"}), "docs", "MyST-Parser") + assertToolNotDetected(t, FilterByChangedFiles(report, base, []string{"app.py"}), "docs", "MyST-Parser") +} + +func TestPyPIDevelopmentDeclarationMatchesLockfileSpelling(t *testing.T) { + base := loadKB(t) + base.ByName["pytest"].Detect.Dependencies = nil + engine := NewFS(base, fstest.MapFS{ + "pyproject.toml": {Data: []byte("[tool.poetry.group.test.dependencies]\nPyTest = '*'\n")}, + "poetry.lock": {Data: []byte("[[package]]\nname = 'pytest'\nversion = '8.0.0'\ncategory = 'main'\n")}, + }) + report, err := engine.Run() + if err != nil { + t.Fatal(err) + } + assertToolDetected(t, report, "test", "pytest") + assertToolDetected(t, FilterByChangedFiles(report, base, []string{"poetry.lock"}), "test", "pytest") +} diff --git a/detect/detect.go b/detect/detect.go index 43e6029..426b39b 100644 --- a/detect/detect.go +++ b/detect/detect.go @@ -26,6 +26,7 @@ import ( "github.com/git-pkgs/brief/kb" "github.com/git-pkgs/licenses" "github.com/git-pkgs/manifests" + "github.com/git-pkgs/purl" "github.com/git-pkgs/roles" "go.yaml.in/yaml/v3" ) @@ -86,8 +87,8 @@ type Engine struct { fileRoles map[string]roles.Set dirCache map[string][]string depsLoaded bool - runtimeDeps map[string]bool // all runtime/unscoped dependency names - devDeps map[string]bool // development/test/build dependency names + runtimeDeps map[string]bool // all runtime/unscoped dependency keys + devDeps map[string]bool // development/test/build dependency keys allDeps map[string]bool // union of both parsedDeps []brief.DepInfo // direct dependencies with PURLs manifests []brief.ManifestInfo @@ -1064,12 +1065,13 @@ func (e *Engine) loadDeps() { }) for _, dep := range result.Dependencies { - e.allDeps[dep.Name] = true + key := dependencyKey(dep.Ecosystem, dep.Name) + e.allDeps[key] = true switch dep.Scope { case manifests.Development, manifests.Test, manifests.Build: - e.devDeps[dep.Name] = true + e.devDeps[key] = true default: - e.runtimeDeps[dep.Name] = true + e.runtimeDeps[key] = true } // Include transitive deps from lockfiles and Go manifests. // Go is special: go.mod acts as both manifest and lockfile, @@ -1455,18 +1457,27 @@ func (e *Engine) hasDependency(tool *kb.ToolDef) bool { e.loadDeps() for _, dep := range tool.Detect.Dependencies { - if e.allDeps[dep] { + if e.allDeps[dep] || e.allDeps[dependencyKey("pypi", dep)] { return true } } for _, dep := range tool.Detect.DevDependencies { - if e.devDeps[dep] { + if e.devDeps[dep] || e.devDeps[dependencyKey("pypi", dep)] { return true } } return false } +func dependencyKey(ecosystem, name string) string { + if ecosystem == "pypi" { + if key := purl.BuildPURLString(ecosystem, name, "", ""); key != "" { + return key + } + } + return name +} + // hasKey checks if a structured file (JSON, TOML) contains any of the given // dot-separated key paths (e.g. "scripts.test" in package.json). func (e *Engine) hasKey(file string, keys []string) bool { diff --git a/detect/filter.go b/detect/filter.go index c4eba6e..3b42eaf 100644 --- a/detect/filter.go +++ b/detect/filter.go @@ -159,7 +159,8 @@ func (fc *filterContext) setChangedDependencies(dependencies []brief.DepInfo, ma } manifest := manifestByPath[filepath.ToSlash(filepath.Clean(dependency.Manifest))] if manifest.Kind == "manifest" { - devDependencyManifests[dependency.Name] = append(devDependencyManifests[dependency.Name], manifest) + key := reportDependencyKey(dependency, manifest) + devDependencyManifests[key] = append(devDependencyManifests[key], manifest) } } @@ -168,15 +169,24 @@ func (fc *filterContext) setChangedDependencies(dependencies []brief.DepInfo, ma if !fc.changed[manifest] { continue } - fc.changedDependencies[dependency.Name] = true + key := reportDependencyKey(dependency, manifestByPath[manifest]) + fc.changedDependencies[key] = true if isDevelopmentScope(dependency.Scope) || lockfileHasDevelopmentDeclaration( - manifestByPath[manifest], devDependencyManifests[dependency.Name], + manifestByPath[manifest], devDependencyManifests[key], ) { - fc.changedDevDependencies[dependency.Name] = true + fc.changedDevDependencies[key] = true } } } +func reportDependencyKey(dependency brief.DepInfo, manifest brief.ManifestInfo) string { + ecosystem := manifest.Ecosystem + if strings.HasPrefix(dependency.PURL, "pkg:pypi/") { + ecosystem = "pypi" + } + return dependencyKey(ecosystem, dependency.Name) +} + func isDevelopmentScope(scope string) bool { switch brief.Scope(scope) { case brief.ScopeDevelopment, brief.ScopeTest, brief.ScopeBuild: @@ -205,12 +215,12 @@ func lockfileHasDevelopmentDeclaration(lockfile brief.ManifestInfo, manifests [] func (fc *filterContext) dependencyAffected(tool *kb.ToolDef) bool { for _, dependency := range tool.Detect.Dependencies { - if fc.changedDependencies[dependency] { + if fc.changedDependencies[dependency] || fc.changedDependencies[dependencyKey("pypi", dependency)] { return true } } for _, dependency := range tool.Detect.DevDependencies { - if fc.changedDevDependencies[dependency] { + if fc.changedDevDependencies[dependency] || fc.changedDevDependencies[dependencyKey("pypi", dependency)] { return true } } diff --git a/testdata/pypi-names-project/requirements.txt b/testdata/pypi-names-project/requirements.txt new file mode 100644 index 0000000..9dca637 --- /dev/null +++ b/testdata/pypi-names-project/requirements.txt @@ -0,0 +1,2 @@ +FLask +MYST_PARSER