Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple Python packages post-linter #764

Merged
merged 10 commits into from
Oct 24, 2023
Merged
1 change: 1 addition & 0 deletions pkg/linter/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var DefaultLinters = []string{
"dev",
"empty",
"opt",
"pythonmultiple",
"srv",
"setuidgid",
"strip",
Expand Down
79 changes: 77 additions & 2 deletions pkg/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ var postLinterMap = map[string]postLinter{
FailOnError: false,
Explain: "Verify that this package is supposed to be empty; if it is, disable this linter; otherwise check the build",
},
"pythonmultiple": postLinter{
LinterFunc: pythonMultiplePackagesPostLinter,
FailOnError: false,
Explain: "Split this package up into multiple packages",
},
}

var isDevRegex = regexp.MustCompile("^dev/")
Expand All @@ -119,13 +124,13 @@ var isUsrLocalRegex = regexp.MustCompile("^usr/local/")
var isVarEmptyRegex = regexp.MustCompile("^var/empty/")
var isCompatPackageRegex = regexp.MustCompile("-compat$")
var isObjectFileRegex = regexp.MustCompile(`\.(a|so|dylib)(\..*)?`)
var isSbomPath = regexp.MustCompile("^var/lib/db/sbom/")
var isSbomPathRegex = regexp.MustCompile("^var/lib/db/sbom/")

// Determine if a path should be ignored by a linter
// NOTE(Elizafox): This should be called from each linter, in case we want to
// lint the SBOM someday.
func isIgnoredPath(path string) bool {
return isSbomPath.MatchString(path)
return isSbomPathRegex.MatchString(path)
}

func devLinter(_ LinterContext, path string, _ fs.DirEntry) error {
Expand Down Expand Up @@ -319,6 +324,76 @@ func emptyPostLinter(_ LinterContext, fsys fs.FS) error {
return fmt.Errorf("Package is empty but no-provides is not set")
}

func pythonMultiplePackagesPostLinter(_ LinterContext, fsys fs.FS) error {
pythondirs, err := fs.Glob(fsys, filepath.Join("usr", "lib", "python3.*"))
if err != nil {
// Shouldn't get here, per the Go docs.
return fmt.Errorf("Error checking for Python site directories: %w", err)
}

if len(pythondirs) == 0 {
// Nothing to do
return nil
} else if len(pythondirs) > 1 {
return fmt.Errorf("More than one Python version detected: %d found", len(pythondirs))
}

matches, err := fs.Glob(fsys, filepath.Join(pythondirs[0], "site-packages", "*"))
if err != nil {
// Shouldn't get here as well.
return fmt.Errorf("Error checking for Python packages: %w", err)
}

// Filter matches and ignore duplicates (.so vs directories for example)
pmatches := map[string]struct{}{}
for _, m := range matches {
base := filepath.Base(m)

if strings.HasPrefix(base, "_") {
// Ignore __pycache__ and internal packages
continue
}

if base == "test" || base == "tests" {
// Exclude tests
continue
}

if base == "doc" || base == "docs" {
// Exclude docs
continue
}

ext := filepath.Ext(base)
if ext == ".egg-info" || ext == ".dist-info" || ext == ".pth" {
// Exclude various metadata files and .so files
continue
}

if len(ext) > 0 {
base = base[:len(ext)]
if base == "" {
// No empty strings
continue
}
}
pmatches[fmt.Sprintf("%q", base)] = struct{}{}
}

if len(pmatches) > 1 {
i := 0
slmatches := make([]string, len(pmatches))
for k := range pmatches {
slmatches[i] = k
i++
}
smatches := strings.Join(slmatches, ", ")
return fmt.Errorf("Multiple Python packages detected: %d found (%s)", len(slmatches), smatches)
}

return nil
}

// Checks if the linters in the given slice are known linters
// Returns an empty slice if all linters are known, otherwise a slice with all the bad linters
func CheckValidLinters(check []string) []string {
Expand Down
108 changes: 99 additions & 9 deletions pkg/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func Test_emptyLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"empty"},
Disabled: []string{"dev", "opt", "setuidgid", "srv", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"dev", "opt", "pythonmultiple", "setuidgid", "srv", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -67,7 +67,7 @@ func Test_usrLocalLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"usrlocal"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "srv", "tempdir", "varempty", "worldwrite"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "setuidgid", "strip", "srv", "tempdir", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -100,7 +100,7 @@ func Test_varEmptyLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"varempty"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "worldwrite"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -134,7 +134,7 @@ func Test_devLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"dev"},
Disabled: []string{"empty", "opt", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"empty", "opt", "pythonmultiple", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -168,7 +168,7 @@ func Test_optLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"opt"},
Disabled: []string{"dev", "empty", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"dev", "empty", "pythonmultiple", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand All @@ -190,6 +190,96 @@ func Test_optLinter(t *testing.T) {
assert.True(t, called)
}

func Test_pythonMultiplePackagesLinter(t *testing.T) {
dir, err := os.MkdirTemp("", "melange.XXXXX")
defer os.RemoveAll(dir)
assert.NoError(t, err)

cfg := config.Configuration{
Package: config.Package{
Name: "testpythonmultiplepackages",
Version: "4.2.0",
Epoch: 0,
Checks: config.Checks{
Elizafox marked this conversation as resolved.
Show resolved Hide resolved
Enabled: []string{"pythonmultiple"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}

// Base dir
pythonPathdir := filepath.Join(dir, "usr", "lib", "python3.14", "site-packages")

linters := cfg.Package.Checks.GetLinters()
assert.Equal(t, linters, []string{"pythonmultiple"})

fsys := os.DirFS(dir)
lctx := NewLinterContext(cfg.Package.Name, fsys)

// Make one "package"
packagedir := filepath.Join(pythonPathdir, "foo")
err = os.MkdirAll(packagedir, 0700)
assert.NoError(t, err)

// One package should not trip it
called := false
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// Egg info files should not count
_, err = os.Create(filepath.Join(pythonPathdir, "fooegg-0.1-py3.14.egg-info"))
assert.NoError(t, err)
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// dist info files should not count
_, err = os.Create(filepath.Join(pythonPathdir, "foodist-0.1-py3.14.dist-info"))
assert.NoError(t, err)
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// pth files should not count
_, err = os.Create(filepath.Join(pythonPathdir, "foopth-0.1-py3.14.pth"))
assert.NoError(t, err)
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// .so files duplicate with a dir should not count
_, err = os.Create(filepath.Join(pythonPathdir, "foo.so"))
assert.NoError(t, err)
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// __pycache__ dirs should not count
err = os.MkdirAll(filepath.Join(pythonPathdir, "__pycache__"), 0700)
assert.NoError(t, err)
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.False(t, called)

// Make another "package" (at this point we should have 2)
packagedir = filepath.Join(pythonPathdir, "bar")
err = os.MkdirAll(packagedir, 0700)
assert.NoError(t, err)

// Two should trip it
assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) {
called = true
}, linters))
assert.True(t, called)
}

func Test_srvLinter(t *testing.T) {
dir, err := os.MkdirTemp("", "melange.XXXXX")
defer os.RemoveAll(dir)
Expand All @@ -202,7 +292,7 @@ func Test_srvLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"srv"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "setuidgid", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -236,7 +326,7 @@ func Test_tempDirLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"tempdir"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "srv", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "setuidgid", "strip", "srv", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -301,7 +391,7 @@ func Test_setUidGidLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"setuidgid"},
Disabled: []string{"dev", "empty", "opt", "srv", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "srv", "strip", "tempdir", "usrlocal", "varempty", "worldwrite"},
},
},
}
Expand Down Expand Up @@ -341,7 +431,7 @@ func Test_worldWriteLinter(t *testing.T) {
Epoch: 0,
Checks: config.Checks{
Enabled: []string{"worldwrite"},
Disabled: []string{"dev", "empty", "opt", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty"},
Disabled: []string{"dev", "empty", "opt", "pythonmultiple", "setuidgid", "strip", "srv", "tempdir", "usrlocal", "varempty"},
},
},
}
Expand Down
Loading