feat/phase7-cleanup #336
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
thwap/qpkg!336
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/phase7-cleanup"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PR Review: feat/phase7-cleanup
Overview: Major refactoring — new C-based
qpkg.fsmodule replacing shell-out patterns, build pipeline overhaul with env propagation,pkgdef_output/pkgdef_no_symlinksupport, newinit/infocommands, colored output, and many new pkgdefs. Clean, well-structured changes overall.All findings are non-blocking. Issues filed for each.
Suggestions
lib/qpkg/build.lua:175–table.concat(commands, " ")joins stage commands with spaces. Old code ran each command individually. If a pkgdef had multiple entries in a stage, they'd become a single command with extra args instead of sequential execution. Currently no pkgdef exercises this, but the API contract changed silently.lib/qpkg/cli.lua:22-44–initandinfocommands exist but aren't listed in help output. Users can't discover them.Makefile:8-9–LUA_INC/LUA_LIBhardcoded to/home/fuzzy/.thwap.d/lua/linux/amd64/...– embeds username, arch, OS. Considerpkg-configor env var overrides.lib/qpkg/cli.lua:497– Build failure ininstall_rawdoesn't showlog.path()to user. Hard to debug failures without knowing log file location.lib/qpkg/fs.so– Compiled binary checked into git. Should be in.gitignore+ built viamake build.tests/test.lua– No coverage for:fsC module,init/infocommands,pkgdef_outputprocessing, registry output persistence,pkgdef_no_symlinkbehavior, env propagation in build stages.lib/qpkg/cli.lua:975-1001– Dynamic dispatch viacommands[cmd]passesnilfor missing positional args. No per-command arg validation.initandinfocommands #338The prior change used table.concat to join command elements with spaces, breaking the API contract where each element runs as its own os.execute call. No pkgdef relied on multi-element stages for sequential commands (split-argument patterns like `{"configure", "--prefix=$pkgdir"}` were always meant to be single commands), but the silent change was a ticking time bomb. - Restore iterative per-element execution in build.run_stage - Collapse all 24 split-argument pkgdefs into single-string commands - Each element is now one os.execute call; sequential commands within a stage should use && in a single element Closes #337Re-review after push (
cd40f31c). Commands concatenation issue is resolved —build.luanow preserves per-command execution, and all existing multi-element pkgdef command arrays have been collapsed to single strings. No remaining blocking issues. Non-blocking suggestions already filed as #337–#343.New commits pushed, approval review dismissed automatically according to repository settings
0fdff8b363d79aa29795d79aa297951d720cce29profile defaults to one entry per package (the active version). profile -a shows all installed versions (old behavior). list delegates to profile("-a").f6402183f9844611d38fSolid cleanup. Key wins:
initcommand for shell integration snippetNo blocking issues.