Skip to content

Commit 18cffdf

Browse files
committed
perf,refactor: indexed apt lookups, parallel repo syncs, shared install helpers
Speedups: - aptcache: Lookup's last-resort path did a full O(n) scan of all entries (60k+ on Debian) on every miss — and misses are the common case during dep resolution (virtual packages). Use the existing byBareName secondary index (scanEntryByName) instead: O(1) per miss. - pacmandb.Sync / apkindex.Update: repo syncs were sequential; both now fetch in parallel (errgroup, limit 4) like the dnf/apt fetchers. apkindex parses sequentially after the parallel download phase since Index mutation is not concurrency-safe. Dedup: - pkg/signing.SignerName: shared OpenPGP identity extraction; the byte-identical copies in aptrepo/verify.go and dnfinstall/verify.go are now thin delegators. - pkg/shell.FilterEnv / HasEnvKey: shared scriptlet-environment mechanics (allow-list filter + defaults); aptinstall and dnfinstall keep their format-specific allow-lists and delegate the machinery. - project: localArtifactExists reuses signingFormatForArtifact instead of duplicating the artifact-extension switch. Deliberately NOT merged: the local fileExists helpers (files.Exists treats permission errors as "exists" — different semantics) and stripVersionConstraint (apkindex/pkgbuild can't share a home without an awkward new leaf package).
1 parent 4d6df73 commit 18cffdf

10 files changed

Lines changed: 179 additions & 119 deletions

File tree

pkg/apkindex/fetch.go

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/cavaliergopher/grab/v3"
1313
"github.com/klauspost/compress/gzip"
14+
"golang.org/x/sync/errgroup"
1415

1516
apperrors "github.com/M0Rf30/yap/v2/pkg/errors"
1617
"github.com/M0Rf30/yap/v2/pkg/httpclient"
@@ -61,26 +62,55 @@ func Update(ctx context.Context) (*Index, error) {
6162
idx := NewIndex()
6263
succeeded := 0
6364

64-
for _, repo := range repos {
65-
indexURL := repo.URL + "/" + arch + "/APKINDEX.tar.gz"
66-
cachePath := filepath.Join(apkCacheDir, "APKINDEX."+sha1Hex(indexURL)+".tar.gz")
65+
// Phase 1: download all APKINDEX tarballs in parallel (network-bound;
66+
// destinations are distinct cache files keyed by URL hash).
67+
type fetchResult struct {
68+
indexURL string
69+
cachePath string
70+
err error
71+
}
6772

68-
logger.Debug(i18n.T("logger.apkindex.debug.fetching_repo"), "url", repo.URL, "arch", arch)
73+
results := make([]fetchResult, len(repos))
6974

70-
if err := downloadFile(ctx, indexURL, cachePath, maxAPKIndexBytes); err != nil {
71-
// Log warning and continue with other repos.
72-
logger.Warn(i18n.T("logger.apkindex.warn.fetch_failed"), "url", indexURL, "error", err)
75+
g := new(errgroup.Group)
76+
g.SetLimit(4)
77+
78+
for i, repo := range repos {
79+
g.Go(func() error {
80+
indexURL := repo.URL + "/" + arch + "/APKINDEX.tar.gz"
81+
cachePath := filepath.Join(apkCacheDir, "APKINDEX."+sha1Hex(indexURL)+".tar.gz")
82+
83+
logger.Debug(i18n.T("logger.apkindex.debug.fetching_repo"), "url", repo.URL, "arch", arch)
84+
85+
results[i] = fetchResult{
86+
indexURL: indexURL,
87+
cachePath: cachePath,
88+
err: downloadFile(ctx, indexURL, cachePath, maxAPKIndexBytes),
89+
}
7390

91+
return nil
92+
})
93+
}
94+
95+
_ = g.Wait()
96+
97+
// Phase 2: parse sequentially — Index mutation is not concurrency-safe
98+
// and parsing is cheap compared to the network fetch.
99+
for i, repo := range repos {
100+
res := results[i]
101+
if res.err != nil {
102+
// Log warning and continue with other repos.
103+
logger.Warn(i18n.T("logger.apkindex.warn.fetch_failed"), "url", res.indexURL, "error", res.err)
74104
continue
75105
}
76106

77107
var sizeBytes int64
78-
if fi, statErr := os.Stat(cachePath); statErr == nil {
108+
if fi, statErr := os.Stat(res.cachePath); statErr == nil {
79109
sizeBytes = fi.Size()
80110
}
81111

82-
if err := loadIndexTarball(idx, cachePath, repo.URL); err != nil {
83-
logger.Warn(i18n.T("logger.apkindex.warn.parse_failed"), "path", cachePath, "error", err)
112+
if err := loadIndexTarball(idx, res.cachePath, repo.URL); err != nil {
113+
logger.Warn(i18n.T("logger.apkindex.warn.parse_failed"), "path", res.cachePath, "error", err)
84114

85115
continue
86116
}

pkg/aptcache/aptcache.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,14 @@ func (c *Cache) Lookup(name string) (PackageInfo, bool) {
279279
return *p, true
280280
}
281281

282-
// Last resort: scan for any entry matching name:*.
283-
// This handles edge cases where a package only exists for a foreign
284-
// architecture (e.g. a test or cross-build situation with arm64-only
285-
// packages on an amd64 host).
286-
for key, candidate := range c.entries {
287-
if bare, _, has := strings.Cut(key, ":"); has && bare == name {
288-
return *candidate, true
289-
}
282+
// Last resort: any entry matching name:* via the byBareName secondary
283+
// index — O(1) instead of a full-map scan. This handles edge cases
284+
// where a package only exists for a foreign architecture (e.g. a test
285+
// or cross-build situation with arm64-only packages on an amd64 host).
286+
// Misses are the common case during dependency resolution (virtual
287+
// packages), so this path must stay cheap.
288+
if candidate, ok := c.scanEntryByName(name); ok {
289+
return *candidate, true
290290
}
291291

292292
return PackageInfo{}, false

pkg/aptinstall/scriptlet.go

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/M0Rf30/yap/v2/pkg/errors"
1212
"github.com/M0Rf30/yap/v2/pkg/i18n"
1313
"github.com/M0Rf30/yap/v2/pkg/logger"
14+
"github.com/M0Rf30/yap/v2/pkg/shell"
1415
)
1516

1617
// scriptletEnvAllowList enumerates the environment variables we forward to
@@ -37,40 +38,18 @@ var scriptletEnvAllowList = map[string]bool{
3738
}
3839

3940
// filterScriptletEnv returns the subset of os.Environ() that's safe to
40-
// forward to a maintainer script.
41+
// forward to a maintainer script. A sane PATH is always provided so
42+
// /usr/bin lookups (debconf, dpkg-trigger, update-alternatives, ldconfig)
43+
// work even if the parent env stripped it.
4144
func filterScriptletEnv() []string {
42-
parent := os.Environ()
43-
filtered := make([]string, 0, len(parent))
44-
45-
for _, kv := range parent {
46-
k, _, ok := strings.Cut(kv, "=")
47-
if !ok {
48-
continue
49-
}
50-
51-
if scriptletEnvAllowList[k] {
52-
filtered = append(filtered, kv)
53-
}
54-
}
55-
56-
// Always provide a sane PATH so /usr/bin lookups (debconf, dpkg-trigger,
57-
// update-alternatives, ldconfig) work even if the parent env stripped it.
58-
if !hasEnvKey(filtered, "PATH") {
59-
filtered = append(filtered, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin")
60-
}
61-
62-
return filtered
45+
return shell.FilterEnv(scriptletEnvAllowList, map[string]string{
46+
"PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
47+
})
6348
}
6449

50+
// hasEnvKey reports whether key is present in env ("key=value" entries).
6551
func hasEnvKey(env []string, key string) bool {
66-
prefix := key + "="
67-
for _, kv := range env {
68-
if strings.HasPrefix(kv, prefix) {
69-
return true
70-
}
71-
}
72-
73-
return false
52+
return shell.HasEnvKey(env, key)
7453
}
7554

7655
// runScriptlet executes a maintainer scriptlet via /bin/sh as a child

pkg/aptrepo/verify.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ package aptrepo
2828
import (
2929
"bytes"
3030
"errors"
31-
"fmt"
3231
"io"
3332
"os"
3433
"path/filepath"
@@ -39,6 +38,7 @@ import (
3938
pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors"
4039

4140
yaperrors "github.com/M0Rf30/yap/v2/pkg/errors"
41+
"github.com/M0Rf30/yap/v2/pkg/signing"
4242
)
4343

4444
// defaultAptKeyringPaths is the set of files/directories apt itself trusts
@@ -259,15 +259,7 @@ func loadKeyringFile(path string) (openpgp.EntityList, error) {
259259
// Automatic Signing Key (2018) <[email protected]>") from a
260260
// verified entity, or returns the hex key ID when no UID is present.
261261
func signerName(e *openpgp.Entity) string {
262-
if e == nil {
263-
return ""
264-
}
265-
266-
for name := range e.Identities {
267-
return name
268-
}
269-
270-
return fmt.Sprintf("%X", e.PrimaryKey.KeyId)
262+
return signing.SignerName(e)
271263
}
272264

273265
// wrapPGPError translates ProtonMail's sentinel + typed errors into a

pkg/dnfinstall/scriptlet.go

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/M0Rf30/yap/v2/pkg/errors"
1515
"github.com/M0Rf30/yap/v2/pkg/i18n"
1616
"github.com/M0Rf30/yap/v2/pkg/logger"
17+
"github.com/M0Rf30/yap/v2/pkg/shell"
1718
)
1819

1920
// scriptletKind identifies which scriptlet to run.
@@ -94,33 +95,14 @@ var scriptletEnvAllowList = map[string]bool{
9495
}
9596

9697
// filterScriptletEnv returns a filtered environment suitable for scriptlet
97-
// execution. Mirrors the deb/apt approach but for RPM.
98+
// execution. Mirrors the deb/apt approach but for RPM: a sane PATH is
99+
// always provided so lookups (ldconfig, systemctl, etc.) work, and HOME
100+
// defaults to / as rpm does.
98101
func filterScriptletEnv() []string {
99-
parent := os.Environ()
100-
filtered := make([]string, 0, len(parent))
101-
102-
for _, kv := range parent {
103-
k, _, ok := strings.Cut(kv, "=")
104-
if !ok {
105-
continue
106-
}
107-
108-
if scriptletEnvAllowList[k] {
109-
filtered = append(filtered, kv)
110-
}
111-
}
112-
113-
// Always provide a sane PATH so lookups (ldconfig, systemctl, etc.) work.
114-
if !hasEnvKey(filtered, "PATH") {
115-
filtered = append(filtered, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin")
116-
}
117-
118-
// Always provide HOME.
119-
if !hasEnvKey(filtered, "HOME") {
120-
filtered = append(filtered, "HOME=/")
121-
}
122-
123-
return filtered
102+
return shell.FilterEnv(scriptletEnvAllowList, map[string]string{
103+
"PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
104+
"HOME": "/",
105+
})
124106
}
125107

126108
// looksLikeLua returns true when the body appears to be an RPM Lua scriptlet
@@ -155,16 +137,9 @@ func looksLikeLua(body string) bool {
155137
return false
156138
}
157139

158-
// hasEnvKey checks if an environment variable key exists in the list.
140+
// hasEnvKey reports whether key is present in env ("key=value" entries).
159141
func hasEnvKey(env []string, key string) bool {
160-
prefix := key + "="
161-
for _, kv := range env {
162-
if strings.HasPrefix(kv, prefix) {
163-
return true
164-
}
165-
}
166-
167-
return false
142+
return shell.HasEnvKey(env, key)
168143
}
169144

170145
// runScriptlet executes a single scriptlet from the RPM header.

pkg/dnfinstall/verify.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
yaperrors "github.com/M0Rf30/yap/v2/pkg/errors"
1818
"github.com/M0Rf30/yap/v2/pkg/i18n"
1919
"github.com/M0Rf30/yap/v2/pkg/logger"
20+
"github.com/M0Rf30/yap/v2/pkg/signing"
2021
)
2122

2223
// ErrUnsignedRPM is returned when an RPM has no signature header.
@@ -285,13 +286,5 @@ func wrapRPMSignatureError(err error) error {
285286
// <[email protected]>") from a verified entity, or returns the hex key ID
286287
// when no UID is present.
287288
func signerName(e *openpgp.Entity) string {
288-
if e == nil {
289-
return ""
290-
}
291-
292-
for name := range e.Identities {
293-
return name
294-
}
295-
296-
return fmt.Sprintf("%X", e.PrimaryKey.KeyId)
289+
return signing.SignerName(e)
297290
}

pkg/pacmandb/pacmandb.go

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import (
1212
"path/filepath"
1313
"runtime"
1414
"strings"
15+
"sync"
16+
17+
"golang.org/x/sync/errgroup"
1518

1619
"github.com/M0Rf30/yap/v2/pkg/errors"
1720
"github.com/M0Rf30/yap/v2/pkg/i18n"
@@ -40,22 +43,41 @@ func Sync(ctx context.Context) (succeeded int, err error) {
4043
logger.Info(i18n.T("logger.pacmandb.info.syncing_repos"), "repos", len(cfg.Repos),
4144
"arch", arch)
4245

43-
var firstErr error
46+
// Repos are independent (separate .db destinations) — sync them in
47+
// parallel, bounded like the other repo fetchers. Failures are
48+
// per-repo warnings; the first error is reported to the caller.
49+
var (
50+
mu sync.Mutex
51+
firstErr error
52+
)
53+
54+
g := new(errgroup.Group)
55+
g.SetLimit(4)
4456

4557
for _, repo := range cfg.Repos {
46-
if err := syncRepo(ctx, repo, arch); err != nil {
47-
if firstErr == nil {
48-
firstErr = err
49-
}
58+
g.Go(func() error {
59+
if err := syncRepo(ctx, repo, arch); err != nil {
60+
mu.Lock()
61+
if firstErr == nil {
62+
firstErr = err
63+
}
64+
mu.Unlock()
5065

51-
logger.Warn(i18n.T("logger.pacmandb.warn.repo_sync_failed"), "repo", repo.Name, "error", err)
66+
logger.Warn(i18n.T("logger.pacmandb.warn.repo_sync_failed"), "repo", repo.Name, "error", err)
5267

53-
continue
54-
}
68+
return nil // best-effort: don't abort sibling repos
69+
}
5570

56-
succeeded++
71+
mu.Lock()
72+
succeeded++
73+
mu.Unlock()
74+
75+
return nil
76+
})
5777
}
5878

79+
_ = g.Wait()
80+
5981
logger.Info(i18n.T("logger.pacmandb.info.sync_complete"), "succeeded", succeeded,
6082
"total", len(cfg.Repos))
6183

pkg/project/project.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -706,14 +706,7 @@ func (mpc *MultipleProject) localArtifactExists(pkgName string) bool {
706706
}
707707

708708
for _, m := range matches {
709-
low := strings.ToLower(m)
710-
switch {
711-
case strings.HasSuffix(low, ".deb"),
712-
strings.HasSuffix(low, ".rpm"),
713-
strings.HasSuffix(low, ".apk"),
714-
strings.HasSuffix(low, ".pkg.tar.zst"),
715-
strings.HasSuffix(low, ".pkg.tar.xz"),
716-
strings.HasSuffix(low, ".pkg.tar.gz"):
709+
if _, ok := signingFormatForArtifact(m); ok {
717710
return true
718711
}
719712
}

0 commit comments

Comments
 (0)