Merge pull request #1404 from schoenherrg/fix/with-sources-ignored

neolynx · web-flow · commit 93650efddb60 · 2024-12-11T13:01:30.000+01:00
Fix `-with-sources` not fetching differently named source packages
diff --git a/AUTHORS b/AUTHORS
@@ -67,3 +67,4 @@ List of contributors, in chronological order:
 * Christoph Fiehe (https://github.com/cfiehe)
 * Blake Kostner (https://github.com/btkostner)
 * Leigh London (https://github.com/leighlondon)
+* Gordian Schönherr (https://github.com/schoenherrg)
diff --git a/api/api.go b/api/api.go
@@ -266,8 +266,13 @@ func showPackages(c *gin.Context, reflist *deb.PackageRefList, collectionFactory
 
 		list.PrepareIndex()
 
-		list, err = list.Filter([]deb.PackageQuery{q}, withDeps,
-			nil, context.DependencyOptions(), architecturesList)
+		list, err = list.Filter(deb.FilterOptions{
+			Queries:           []deb.PackageQuery{q},
+			WithDependencies:  withDeps,
+			Source:            nil,
+			DependencyOptions: context.DependencyOptions(),
+			Architectures:     architecturesList,
+		})
 		if err != nil {
 			AbortWithJSONError(c, 500, fmt.Errorf("unable to search: %s", err))
 			return
@@ -283,8 +288,9 @@ func showPackages(c *gin.Context, reflist *deb.PackageRefList, collectionFactory
 				fmt.Println("filter packages by version, query string parse err: ", err)
 				c.AbortWithError(500, fmt.Errorf("unable to parse %s maximum version query string: %s", p.Name, err))
 			} else {
-				tmpList, err := list.Filter([]deb.PackageQuery{versionQ}, false,
-					nil, 0, []string{})
+				tmpList, err := list.Filter(deb.FilterOptions{
+					Queries: []deb.PackageQuery{versionQ},
+				})
 
 				if err == nil {
 					if tmpList.Len() > 0 {
diff --git a/api/mirror.go b/api/mirror.go
@@ -307,8 +307,12 @@ func apiMirrorsPackages(c *gin.Context) {
 
 		list.PrepareIndex()
 
-		list, err = list.Filter([]deb.PackageQuery{q}, withDeps,
-			nil, context.DependencyOptions(), architecturesList)
+		list, err = list.Filter(deb.FilterOptions{
+			Queries:           []deb.PackageQuery{q},
+			WithDependencies:  withDeps,
+			DependencyOptions: context.DependencyOptions(),
+			Architectures:     architecturesList,
+		})
 		if err != nil {
 			AbortWithJSONError(c, 500, fmt.Errorf("unable to search: %s", err))
 		}
diff --git a/api/repos.go b/api/repos.go
@@ -712,7 +712,14 @@ func apiReposCopyPackage(c *gin.Context) {
 			return &task.ProcessReturnValue{Code: http.StatusUnprocessableEntity, Value: nil}, fmt.Errorf("unable to parse query '%s': %s", fileName, err)
 		}
 
-		toProcess, err := srcList.FilterWithProgress(queries, jsonBody.WithDeps, dstList, context.DependencyOptions(), architecturesList, context.Progress())
+		toProcess, err := srcList.Filter(deb.FilterOptions{
+			Queries:           queries,
+			WithDependencies:  jsonBody.WithDeps,
+			Source:            dstList,
+			DependencyOptions: context.DependencyOptions(),
+			Architectures:     architecturesList,
+			Progress:          context.Progress(),
+		})
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("filter error: %s", err)
 		}
diff --git a/api/snapshot.go b/api/snapshot.go
@@ -748,7 +748,14 @@ func apiSnapshotsPull(c *gin.Context) {
 		}
 
 		// Filter with dependencies as requested
-		destinationPackageList, err := sourcePackageList.FilterWithProgress(queries, !noDeps, toPackageList, context.DependencyOptions(), architecturesList, context.Progress())
+		destinationPackageList, err := sourcePackageList.Filter(deb.FilterOptions{
+			Queries:           queries,
+			WithDependencies:  !noDeps,
+			Source:            toPackageList,
+			DependencyOptions: context.DependencyOptions(),
+			Architectures:     architecturesList,
+			Progress:          context.Progress(),
+		})
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, err
 		}
diff --git a/cmd/repo_move.go b/cmd/repo_move.go
@@ -116,7 +116,14 @@ func aptlyRepoMoveCopyImport(cmd *commander.Command, args []string) error {
 		}
 	}
 
-	toProcess, err := srcList.FilterWithProgress(queries, withDeps, dstList, context.DependencyOptions(), architecturesList, context.Progress())
+	toProcess, err := srcList.Filter(deb.FilterOptions{
+		Queries:           queries,
+		WithDependencies:  withDeps,
+		Source:            dstList,
+		DependencyOptions: context.DependencyOptions(),
+		Architectures:     architecturesList,
+		Progress:          context.Progress(),
+	})
 	if err != nil {
 		return fmt.Errorf("unable to %s: %s", command, err)
 	}
diff --git a/cmd/repo_remove.go b/cmd/repo_remove.go
@@ -45,7 +45,7 @@ func aptlyRepoRemove(cmd *commander.Command, args []string) error {
 	}
 
 	list.PrepareIndex()
-	toRemove, err := list.Filter(queries, false, nil, 0, nil)
+	toRemove, err := list.Filter(deb.FilterOptions{Queries: queries})
 	if err != nil {
 		return fmt.Errorf("unable to remove: %s", err)
 	}
diff --git a/cmd/snapshot_filter.go b/cmd/snapshot_filter.go
@@ -67,7 +67,14 @@ func aptlySnapshotFilter(cmd *commander.Command, args []string) error {
 	}
 
 	// Filter with dependencies as requested
-	result, err := packageList.FilterWithProgress(queries, withDeps, nil, context.DependencyOptions(), architecturesList, context.Progress())
+	result, err := packageList.Filter(deb.FilterOptions{
+		Queries:           queries,
+		WithDependencies:  withDeps,
+		Source:            nil,
+		DependencyOptions: context.DependencyOptions(),
+		Architectures:     architecturesList,
+		Progress:          context.Progress(),
+	})
 	if err != nil {
 		return fmt.Errorf("unable to filter: %s", err)
 	}
diff --git a/cmd/snapshot_pull.go b/cmd/snapshot_pull.go
@@ -97,7 +97,14 @@ func aptlySnapshotPull(cmd *commander.Command, args []string) error {
 	}
 
 	// Filter with dependencies as requested
-	result, err := sourcePackageList.FilterWithProgress(queries, !noDeps, packageList, context.DependencyOptions(), architecturesList, context.Progress())
+	result, err := sourcePackageList.Filter(deb.FilterOptions{
+		Queries:           queries,
+		WithDependencies:  !noDeps,
+		Source:            packageList,
+		DependencyOptions: context.DependencyOptions(),
+		Architectures:     architecturesList,
+		Progress:          context.Progress(),
+	})
 	if err != nil {
 		return fmt.Errorf("unable to pull: %s", err)
 	}
diff --git a/cmd/snapshot_search.go b/cmd/snapshot_search.go
@@ -103,8 +103,13 @@ func aptlySnapshotMirrorRepoSearch(cmd *commander.Command, args []string) error
 		}
 	}
 
-	result, err := list.FilterWithProgress([]deb.PackageQuery{q}, withDeps,
-		nil, context.DependencyOptions(), architecturesList, context.Progress())
+	result, err := list.Filter(deb.FilterOptions{
+		Queries:           []deb.PackageQuery{q},
+		WithDependencies:  withDeps,
+		DependencyOptions: context.DependencyOptions(),
+		Architectures:     architecturesList,
+		Progress:          context.Progress(),
+	})
 	if err != nil {
 		return fmt.Errorf("unable to search: %s", err)
 	}
diff --git a/deb/list.go b/deb/list.go
@@ -2,6 +2,7 @@ package deb
 
 import (
 	"fmt"
+	"regexp"
 	"sort"
 	"strings"
 
@@ -503,55 +504,103 @@ func (l *PackageList) Search(dep Dependency, allMatches bool, searchProvided boo
 	return
 }
 
-// Filter filters package index by specified queries (ORed together), possibly pulling dependencies
-func (l *PackageList) Filter(queries []PackageQuery, withDependencies bool, source *PackageList, dependencyOptions int, architecturesList []string) (*PackageList, error) {
-	return l.FilterWithProgress(queries, withDependencies, source, dependencyOptions, architecturesList, nil)
+// FilterOptions specifies options for Filter()
+type FilterOptions struct {
+	Queries           []PackageQuery
+	WithDependencies  bool
+	WithSources       bool // Source packages corresponding to binary packages are included
+	Source            *PackageList
+	DependencyOptions int
+	Architectures     []string
+	Progress          aptly.Progress // set to non-nil value to report progress
 }
 
-// FilterWithProgress filters package index by specified queries (ORed together), possibly pulling dependencies and displays progress
-func (l *PackageList) FilterWithProgress(queries []PackageQuery, withDependencies bool, source *PackageList, dependencyOptions int, architecturesList []string, progress aptly.Progress) (*PackageList, error) {
+// SourceRegex is a regular expression to match source package names.
+// > In a binary package control file [...], the source package name may be followed by a version number in
+// > parentheses. This version number may be omitted [...] if it has the same value as the Version field of
+// > the binary package in question.
+// > [...]
+// > Package names (both source and binary, see Package) must consist only of lower case letters (a-z),
+// > digits (0-9), plus (+) and minus (-) signs, and periods (.).
+// > They must be at least two characters long and must start with an alphanumeric character.
+// -- https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-source
+var SourceRegex = regexp.MustCompile(`^([a-z0-9][-+.a-z0-9]+)(?:\s+\(([^)]+)\))?$`)
+
+// Filter filters package index by specified queries (ORed together), possibly pulling dependencies
+func (l *PackageList) Filter(options FilterOptions) (*PackageList, error) {
 	if !l.indexed {
 		panic("list not indexed, can't filter")
 	}
 
 	result := NewPackageList()
 
-	for _, query := range queries {
-		result.Append(query.Query(l))
+	for _, query := range options.Queries {
+		_ = result.Append(query.Query(l))
+	}
+	// The above loop already finds source packages that are named equal to their binary package, but we still need
+	// to account for those that are named differently.
+	if options.WithSources {
+		sourceQueries := make([]PackageQuery, 0)
+		for _, pkg := range result.packages {
+			if pkg.Source == "" {
+				continue
+			}
+			matches := SourceRegex.FindStringSubmatch(pkg.Source)
+			if matches == nil {
+				return nil, fmt.Errorf("invalid Source field: %s", pkg.Source)
+			}
+			sourceName := matches[1]
+			if sourceName == pkg.Name {
+				continue
+			}
+			sourceVersion := pkg.Version
+			if matches[2] != "" {
+				sourceVersion = matches[2]
+			}
+			sourceQueries = append(sourceQueries, &DependencyQuery{Dependency{
+				Pkg:          sourceName,
+				Version:      sourceVersion,
+				Relation:     VersionEqual,
+				Architecture: ArchitectureSource,
+			}})
+		}
+		for _, query := range sourceQueries {
+			_ = result.Append(query.Query(l))
+		}
 	}
 
-	if withDependencies {
+	if options.WithDependencies {
 		added := result.Len()
 		result.PrepareIndex()
 
 		dependencySource := NewPackageList()
-		if source != nil {
-			dependencySource.Append(source)
+		if options.Source != nil {
+			_ = dependencySource.Append(options.Source)
 		}
-		dependencySource.Append(result)
+		_ = dependencySource.Append(result)
 		dependencySource.PrepareIndex()
 
 		// while some new dependencies were discovered
 		for added > 0 {
 			added = 0
 
 			// find missing dependencies
-			missing, err := result.VerifyDependencies(dependencyOptions, architecturesList, dependencySource, progress)
+			missing, err := result.VerifyDependencies(options.DependencyOptions, options.Architectures, dependencySource, options.Progress)
 			if err != nil {
 				return nil, err
 			}
 
 			// try to satisfy dependencies
 			for _, dep := range missing {
-				if dependencyOptions&DepFollowAllVariants == 0 {
+				if options.DependencyOptions&DepFollowAllVariants == 0 {
 					// dependency might have already been satisfied
 					// with packages already been added
 					//
 					// when follow-all-variants is enabled, we need to try to expand anyway,
 					// as even if dependency is satisfied now, there might be other ways to satisfy dependency
 					if result.Search(dep, false, true) != nil {
-						if dependencyOptions&DepVerboseResolve == DepVerboseResolve && progress != nil {
-							progress.ColoredPrintf("@{y}Already satisfied dependency@|: %s with %s", &dep, result.Search(dep, true, true))
+						if options.DependencyOptions&DepVerboseResolve == DepVerboseResolve && options.Progress != nil {
+							options.Progress.ColoredPrintf("@{y}Already satisfied dependency@|: %s with %s", &dep, result.Search(dep, true, true))
 						}
 						continue
 					}
@@ -564,19 +613,19 @@ func (l *PackageList) FilterWithProgress(queries []PackageQuery, withDependencie
 							continue
 						}
 
-						if dependencyOptions&DepVerboseResolve == DepVerboseResolve && progress != nil {
-							progress.ColoredPrintf("@{g}Injecting package@|: %s", p)
+						if options.DependencyOptions&DepVerboseResolve == DepVerboseResolve && options.Progress != nil {
+							options.Progress.ColoredPrintf("@{g}Injecting package@|: %s", p)
 						}
-						result.Add(p)
-						dependencySource.Add(p)
+						_ = result.Add(p)
+						_ = dependencySource.Add(p)
 						added++
-						if dependencyOptions&DepFollowAllVariants == 0 {
+						if options.DependencyOptions&DepFollowAllVariants == 0 {
 							break
 						}
 					}
 				} else {
-					if dependencyOptions&DepVerboseResolve == DepVerboseResolve && progress != nil {
-						progress.ColoredPrintf("@{r}Unsatisfied dependency@|: %s", dep.String())
+					if options.DependencyOptions&DepVerboseResolve == DepVerboseResolve && options.Progress != nil {
+						options.Progress.ColoredPrintf("@{r}Unsatisfied dependency@|: %s", dep.String())
 					}
 
 				}
diff --git a/deb/list_test.go b/deb/list_test.go
diff --git a/deb/package.go b/deb/package.go
diff --git a/deb/remote.go b/deb/remote.go
diff --git a/system/t04_mirror/UpdateMirror10Test_gold b/system/t04_mirror/UpdateMirror10Test_gold
diff --git a/system/t06_publish/snapshot.py b/system/t06_publish/snapshot.py

Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,14 @@ func aptlyRepoMoveCopyImport(cmd *commander.Command, args []string) error {`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`		`- toProcess, err := srcList.FilterWithProgress(queries, withDeps, dstList, context.DependencyOptions(), architecturesList, context.Progress())`
	`119`	`+ toProcess, err := srcList.Filter(deb.FilterOptions{`
	`120`	`+ Queries: queries,`
	`121`	`+ WithDependencies: withDeps,`
	`122`	`+ Source: dstList,`
	`123`	`+ DependencyOptions: context.DependencyOptions(),`
	`124`	`+ Architectures: architecturesList,`
	`125`	`+ Progress: context.Progress(),`
	`126`	`+ })`
`120`	`127`	`if err != nil {`
`121`	`128`	`return fmt.Errorf("unable to %s: %s", command, err)`
`122`	`129`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ func aptlyRepoRemove(cmd *commander.Command, args []string) error {`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`list.PrepareIndex()`
`48`		`- toRemove, err := list.Filter(queries, false, nil, 0, nil)`
	`48`	`+ toRemove, err := list.Filter(deb.FilterOptions{Queries: queries})`
`49`	`49`	`if err != nil {`
`50`	`50`	`return fmt.Errorf("unable to remove: %s", err)`
`51`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,8 +103,13 @@ func aptlySnapshotMirrorRepoSearch(cmd *commander.Command, args []string) error`
`103`	`103`	`}`
`104`	`104`	`}`
`105`	`105`
`106`		`- result, err := list.FilterWithProgress([]deb.PackageQuery{q}, withDeps,`
`107`		`- nil, context.DependencyOptions(), architecturesList, context.Progress())`
	`106`	`+ result, err := list.Filter(deb.FilterOptions{`
	`107`	`+ Queries: []deb.PackageQuery{q},`
	`108`	`+ WithDependencies: withDeps,`
	`109`	`+ DependencyOptions: context.DependencyOptions(),`
	`110`	`+ Architectures: architecturesList,`
	`111`	`+ Progress: context.Progress(),`
	`112`	`+ })`
`108`	`113`	`if err != nil {`
`109`	`114`	`return fmt.Errorf("unable to search: %s", err)`
`110`	`115`	`}`