From 8023387332f7f9d03e44a52ae4dfe589aaa3a7cb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 07:07:19 +0000 Subject: [PATCH] fix: resolve real packages mis-identified as virtual in getNonVirtualPackage - Handle empty Reverse Provides by falling back to the Versions section, returning the package itself (fixes ghostscript regression, issue #218) - Handle inline "Reverse Provides: name version" header+entry format - Add regression unit tests and integration test with replay log - Address code review: clarify inline-header fallthrough comment and add edge-case test for inline header with no parseable entry --- src/cmd/apt_query/main_test.go | 9 +++ src/internal/common/apt.go | 69 ++++++++++++++++--- src/internal/common/apt_test.go | 113 ++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 8 deletions(-) diff --git a/src/cmd/apt_query/main_test.go b/src/cmd/apt_query/main_test.go index 49e040d..c2a7aca 100644 --- a/src/cmd/apt_query/main_test.go +++ b/src/cmd/apt_query/main_test.go @@ -68,3 +68,12 @@ func TestNormalizedList_VirtualPackagesExists_StdoutsConcretePackage(t *testing. result := cmdtesting.New(t, createReplayLogs).Run("normalized-list", "libvips") result.ExpectSuccessfulOut("libvips42=8.9.1-2") } + +// Regression test for https://github.com/awalsh128/cache-apt-pkgs-action/issues/218: +// a real package (ghostscript) that apt-cache show reports as purely virtual on stale apt lists +// must resolve to itself (ghostscript=), not to a mangled section-header token such as +// "Reverse=Provides:". +func TestNormalizedList_RealPackageMisidentifiedAsVirtual_StdoutsCorrectPackage(t *testing.T) { + result := cmdtesting.New(t, createReplayLogs).Run("normalized-list", "ghostscript") + result.ExpectSuccessfulOut("ghostscript=10.02.1~dfsg1-0ubuntu7.8") +} diff --git a/src/internal/common/apt.go b/src/internal/common/apt.go index bc20d19..2fa5bff 100644 --- a/src/internal/common/apt.go +++ b/src/internal/common/apt.go @@ -32,6 +32,9 @@ func isErrLine(line string) bool { } // Resolves virtual packages names to their concrete one. +// Falls back to returning the package itself (with version from the Versions section) when the +// Reverse Provides section is empty, which handles real packages that are mistakenly treated as +// virtual due to stale apt lists. func getNonVirtualPackage(executor exec.Executor, name string) (pkg *AptPackage, err error) { execution := executor.Exec("apt-cache", "showpkg", name) err = execution.Error() @@ -40,28 +43,78 @@ func getNonVirtualPackage(executor exec.Executor, name string) (pkg *AptPackage, return pkg, err } + const reverseProvides = "Reverse Provides:" + const versions = "Versions:" + + inVersions := false inReverseProvides := false + var firstVersion string + for _, line := range strings.Split(execution.CombinedOut, "\n") { trimmed := strings.TrimSpace(line) if trimmed == "" { continue } - if trimmed == "Reverse Provides:" { - inReverseProvides = true + if strings.HasPrefix(trimmed, "W: ") || isErrLine(trimmed) { continue } - if !inReverseProvides || strings.HasPrefix(trimmed, "W: ") || isErrLine(trimmed) { + if trimmed == versions { + inVersions = true + inReverseProvides = false + continue + } + if trimmed == reverseProvides { + inReverseProvides = true + inVersions = false + continue + } + // Handle "Reverse Provides: " format where the header and first entry + // appear on the same line. If the inline content is parseable (≥2 words), return it + // immediately. If it is malformed or absent (< 2 words), treat the line as a plain + // section header so that subsequent lines are still scanned for entries. + if strings.HasPrefix(trimmed, reverseProvides+" ") { + entry := strings.TrimSpace(trimmed[len(reverseProvides)+1:]) + if entry != "" { + splitLine := GetSplitLine(entry, " ", 3) + if len(splitLine.Words) >= 2 { + return &AptPackage{Name: splitLine.Words[0], Version: splitLine.Words[1]}, nil + } + } + // No parseable entry on this line; fall through to entry-scanning mode so + // subsequent lines in this section are still examined. + inReverseProvides = true + inVersions = false continue } if strings.HasSuffix(trimmed, ":") { - break - } - splitLine := GetSplitLine(trimmed, " ", 3) - if len(splitLine.Words) < 2 { + // Some other section header — reset tracking flags. + inVersions = false + inReverseProvides = false continue } - return &AptPackage{Name: splitLine.Words[0], Version: splitLine.Words[1]}, nil + // Capture the first version listed for this package (used as fallback below). + if inVersions && firstVersion == "" { + splitLine := GetSplitLine(trimmed, " ", 2) + if len(splitLine.Words) >= 1 && splitLine.Words[0] != "" { + firstVersion = splitLine.Words[0] + } + } + if inReverseProvides { + splitLine := GetSplitLine(trimmed, " ", 3) + if len(splitLine.Words) < 2 { + continue + } + return &AptPackage{Name: splitLine.Words[0], Version: splitLine.Words[1]}, nil + } } + + // If no Reverse Provides entries were found but the package itself has a known version, + // it is a real (non-virtual) package that was mistakenly treated as virtual (e.g. because + // apt-cache show was run against stale lists). Return it directly. + if firstVersion != "" { + return &AptPackage{Name: name, Version: firstVersion}, nil + } + return pkg, fmt.Errorf("unable to parse reverse provides package name and version from apt-cache showpkg output below:\n%s", execution.CombinedOut) } diff --git a/src/internal/common/apt_test.go b/src/internal/common/apt_test.go index d8aaa2c..38991dc 100644 --- a/src/internal/common/apt_test.go +++ b/src/internal/common/apt_test.go @@ -51,3 +51,116 @@ func TestGetNonVirtualPackage_WithWarningsInReverseProvides(t *testing.T) { t.Fatalf("unexpected package.\nexpected: %+v\nactual: %+v", expected, *pkg) } } + +// Regression test for https://github.com/awalsh128/cache-apt-pkgs-action/issues/218 +// A real package (ghostscript) that is mis-identified as purely virtual due to stale apt lists +// has an empty "Reverse Provides:" section in apt-cache showpkg. getNonVirtualPackage must +// fall back to the "Versions:" section and return the package itself rather than failing or +// returning the section header as the package name. +func TestGetNonVirtualPackage_WithRealPackageMisidentifiedAsVirtual(t *testing.T) { + executor := mockExecutor{ + executions: map[string]*execpkg.Execution{ + "apt-cache showpkg ghostscript": { + Cmd: "apt-cache showpkg ghostscript", + CombinedOut: strings.Join([]string{ + "Package: ghostscript", + "Versions: ", + "10.02.1~dfsg1-0ubuntu7.8 (/var/lib/dpkg/info/ghostscript.list)", + " Description Language: ", + " File: /var/lib/apt/lists/example", + " MD5: abc123", + "", + "Reverse Depends: ", + " cups,ghostscript", + "Dependencies: ", + "10.02.1~dfsg1-0ubuntu7.8 - libgs10 (5 10.02.1~dfsg1-0ubuntu7.8)", + "Provides: ", + "10.02.1~dfsg1-0ubuntu7.8 - postscript-viewer (= ) ghostscript-x (= 10.02.1~dfsg1-0ubuntu7.8)", + "Reverse Provides: ", + "", + }, "\n"), + ExitCode: 0, + }, + }, + } + + pkg, err := getNonVirtualPackage(executor, "ghostscript") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pkg == nil { + t.Fatal("expected package but got nil") + } + + expected := AptPackage{Name: "ghostscript", Version: "10.02.1~dfsg1-0ubuntu7.8"} + if *pkg != expected { + t.Fatalf("unexpected package.\nexpected: %+v\nactual: %+v", expected, *pkg) + } +} + +// Regression test: when the "Reverse Provides:" header and entry appear on the same line +// (e.g. "Reverse Provides: provider 1.0"), the entry must be parsed correctly and the +// section header must not be treated as the package name. +func TestGetNonVirtualPackage_WithInlineReverseProvides(t *testing.T) { + executor := mockExecutor{ + executions: map[string]*execpkg.Execution{ + "apt-cache showpkg somevirtual": { + Cmd: "apt-cache showpkg somevirtual", + CombinedOut: strings.Join([]string{ + "Package: somevirtual", + "Versions: ", + "", + "Reverse Provides: concrete-pkg 2.0.0 (= 2.0.0)", + "", + }, "\n"), + ExitCode: 0, + }, + }, + } + + pkg, err := getNonVirtualPackage(executor, "somevirtual") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pkg == nil { + t.Fatal("expected package but got nil") + } + + expected := AptPackage{Name: "concrete-pkg", Version: "2.0.0"} + if *pkg != expected { + t.Fatalf("unexpected package.\nexpected: %+v\nactual: %+v", expected, *pkg) + } +} + +// Edge case: "Reverse Provides: " header with no parseable inline entry falls back to +// scanning subsequent lines, then to the Versions fallback if still no entries are found. +func TestGetNonVirtualPackage_WithInlineHeaderNoEntry(t *testing.T) { + executor := mockExecutor{ + executions: map[string]*execpkg.Execution{ + "apt-cache showpkg realpkg": { + Cmd: "apt-cache showpkg realpkg", + CombinedOut: strings.Join([]string{ + "Package: realpkg", + "Versions: ", + "1.2.3 (/var/lib/dpkg/info/realpkg.list)", + "Reverse Provides: ", + "", + }, "\n"), + ExitCode: 0, + }, + }, + } + + pkg, err := getNonVirtualPackage(executor, "realpkg") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pkg == nil { + t.Fatal("expected package but got nil") + } + + expected := AptPackage{Name: "realpkg", Version: "1.2.3"} + if *pkg != expected { + t.Fatalf("unexpected package.\nexpected: %+v\nactual: %+v", expected, *pkg) + } +}