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

COVERAGE: Fix/with sources ignored #1405

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,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
Expand All @@ -244,8 +249,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 {
Expand Down
8 changes: 6 additions & 2 deletions api/mirror.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,12 @@

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,
})

Check warning on line 314 in api/mirror.go

View check run for this annotation

Codecov / codecov/patch

api/mirror.go#L309-L314

Added lines #L309 - L314 were not covered by tests
if err != nil {
AbortWithJSONError(c, 500, fmt.Errorf("unable to search: %s", err))
}
Expand Down
9 changes: 8 additions & 1 deletion api/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,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)
}
Expand Down
9 changes: 8 additions & 1 deletion api/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,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
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/repo_move.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/repo_remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/snapshot_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/snapshot_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
9 changes: 7 additions & 2 deletions cmd/snapshot_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
93 changes: 71 additions & 22 deletions deb/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"fmt"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -503,55 +504,103 @@
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 correspond to binary packages are included
Source *PackageList
DependencyOptions int
Architectures []string
Progress aptly.Progress // set to non-nil 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)
}

Check warning on line 551 in deb/list.go

View check run for this annotation

Codecov / codecov/patch

deb/list.go#L550-L551

Added lines #L550 - L551 were not covered by tests
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
}
Expand All @@ -564,19 +613,19 @@
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())

Check warning on line 628 in deb/list.go

View check run for this annotation

Codecov / codecov/patch

deb/list.go#L628

Added line #L628 was not covered by tests
}

}
Expand Down
Loading
Loading