Skip to content

Builds: a plain NOR package a newer split one replaced leaves the index - #505

Merged
widgetii merged 2 commits into
masterfrom
firmware/retired-packages
Oct 23, 2026
Merged

widgetii merged 2 commits into
masterfrom
firmware/retired-packages

Conversation

@widgetii

Copy link
Copy Markdown
Member

The firmware index takes, for each file name, the newest retained build that published it. So a package the firmware stops publishing stays on offer for the whole retention period (90 days).

hi3516ev300 and hi3518ev300 lite and ultimate ship only the split package since OpenIPC/firmware#2656. Their last plain packages, from the nightlies of 2026-10-19 to 21, are still in the index. So the wizard gives them fixed slots, and SplitOnlyEdition, which looks for "split published, plain not", never sees them as split-only. Deleting the stale assets from latest did not change that, because the index doesn't read the release.

LoadIndex now leaves out a plain NOR package whose split package came from a newer build than the plain one's newest.

  • A board that publishes both does so in one build and keeps both (the gk7205v200 and gk7205v500 families).
  • An edition with no split package (hi3516ev300 neo) keeps its package.
  • A board whose build failed publishes neither and keeps what it had.

TestLoadIndexDropsPlainSupersededBySplit stores two firmware builds and checks each of those cases. It fails on the old LoadIndex. service/run.sh test (all packages, Postgres) passes.

After deploy, hi3516ev300 and hi3518ev300 lite and ultimate on NOR should get the split install (firmware.bin.<soc> at 0x50000), as hi3516cv6xx does.

The index takes, for each file name, the newest retained build that
published it, so a package the firmware stops publishing stays on offer
for the whole retention period. hi3516ev300 and hi3518ev300 lite and
ultimate ship only the split package since OpenIPC/firmware#2656. Their
last plain packages (nightlies of 2026-10-19 to 21) are still in the
index, so the wizard gives them fixed slots, and SplitOnlyEdition, which
looks for "split published, plain not", never sees them split-only. The
stale assets are gone from the latest release; the index did not notice,
because it does not read the release.

LoadIndex now leaves out a plain NOR package whose split package came
from a newer build than the plain one's newest. A board that publishes
both does so in one build and keeps both (the gk7205v200 and gk7205v500
families). An edition with no split package (hi3516ev300 neo) and a board
whose build failed, publishing neither, keep what they had.

Test: TestLoadIndexDropsPlainSupersededBySplit stores two firmware
builds, the 21st with the plain ev300 lite/ultimate, neo, gk7205v200 lite
and cv300 lite, and the 22nd with the ev300 split pair, neo and
gk7205v200 plain plus split. The ev300 plain pair is gone; split, neo,
gk7205v200's both, and cv300's from the 21st stay. It fails on the old
LoadIndex. service/run.sh test (all packages, Postgres) passes.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Remove plain NOR packages superseded by newer split builds

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Exclude retained plain NOR packages when a newer build publishes their split counterpart.
• Preserve packages when both variants share a build or no newer split exists.
• Add a database-backed regression test for split-only, dual-package, and failed-build cases.
Diagram

graph TD
  B[("Build tables")] --> L["LoadIndex"] --> F["NOR supersession filter"] --> I["Firmware index"] --> W["Install wizard"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Filter superseded assets in SQL
  • ➕ Could return only eligible assets from the database.
  • ➖ Adds a more complex join or anti-join to the existing newest-per-name query.
  • ➖ Makes package-name and timestamp rules harder to review.

Recommendation: Keep the post-query filter. It applies the supersession rule to the already-selected newest asset per name, while preserving the existing query and making the timestamp comparison explicit.

Files changed (2) +78 / -2

Bug fix (1) +27 / -2
source.goFilter plain NOR assets superseded by newer split builds +27/-2

Filter plain NOR assets superseded by newer split builds

• LoadIndex now reads each selected asset's build timestamp and excludes a plain NOR package when its corresponding split package has a later build timestamp. Equal timestamps and packages without a newer split counterpart remain in the index.

service/internal/builds/source.go

Tests (1) +51 / -0
builds_test.goCover retained NOR packages across two firmware builds +51/-0

Cover retained NOR packages across two firmware builds

• A database-backed regression test verifies that older plain lite and ultimate packages disappear after newer split builds. It also checks that current dual-package builds, an edition without a split package, and a package absent from the newer build remain available.

service/internal/builds/builds_test.go

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Tied build times keep stale plain packages ✓ Resolved
Description
dropSupersededPlain compares only built_at, although LoadIndex uses the build ID to decide
which build is newer when timestamps tie. If a higher-ID build publishes a split package at the same
timestamp as the retained plain package's build, the plain package remains and the installer does
not select the split-only layout.
Code

service/internal/builds/source.go[R292-293]

+			if at, split := built[firmware.SplitName(board, edition)]; split && at.After(built[a.Name]) {
+				continue
Evidence
The query treats build ID as a tie-breaker, but the new map discards it. The downstream layout check
treats an index containing both names as not split-only.

service/internal/builds/source.go[28-32]
service/internal/builds/source.go[37-48]
service/internal/builds/source.go[288-294]
service/internal/firmware/layout.go[204-213]
service/internal/db/migrations/002_builds.sql[10-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The filter loses the build-ID tie-breaker used by the asset query, so equal-timestamp builds can leave a superseded plain package in the index.
## Fix Focus Areas
- service/internal/builds/source.go[28-48]
- service/internal/builds/source.go[288-299]
## Recommended Fix
Select and retain each asset's build ID alongside its timestamp. Compare `(built_at, id)` in the same order as the SQL query, and add a test with equal timestamps and distinct build IDs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. A partial Goke build changes flash layout 🐞 Bug ≡ Correctness
Description
dropSupersededPlain removes any older plain NOR package when its split counterpart has a later
timestamp, including editions that normally publish both packages for fixed-slot installation. If a
newer Goke build publishes its split tarball but fails to publish the plain tarball, the retained
fixed-slot package disappears from the index and image construction switches that edition to the
split layout.
Code

service/internal/builds/source.go[R291-293]

+		if board, storage, edition, ok := ParseAssetName(a.Name); ok && storage == "nor" {
+			if at, split := built[firmware.SplitName(board, edition)]; split && at.After(built[a.Name]) {
+				continue
Evidence
Goke is documented and tested to use the plain package for fixed-slot images when both packages
exist. Payload validation permits a build containing only the split asset; the new filter would then
remove the retained plain asset, and layout selection would treat the edition as split-only.

service/internal/builds/source.go[288-296]
service/internal/builds/payload.go[168-181]
service/internal/catalogue/catalogue.go[38-45]
service/internal/firmware/split_test.go[153-174]
service/internal/firmware/layout.go[188-213]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newer split asset does not establish that an older plain asset was intentionally retired; partial publication can remove the fixed-slot option on boards that support both packages.
## Fix Focus Areas
- service/internal/builds/source.go[288-299]
- service/internal/builds/payload.go[168-181]
## Recommended Fix
Make retirement explicit per board and edition, whether through build metadata or an explicit transition policy, rather than inferring it for every NOR package pair from timestamps. Test a newer split-only partial publication for a Goke edition whose plain fixed-slot package should remain available.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread service/internal/builds/source.go Outdated
Comment thread service/internal/builds/source.go
From review: two builds with one built_at rank by id in LoadIndex's query;
the split check now does the same, comparing (built_at, id). Test:
TestLoadIndexSplitSupersedesOnTiedTimestamp, two builds at one time, the
later id carrying the split package. service/run.sh test passes.
@widgetii
widgetii merged commit d11e4b0 into master Oct 23, 2026
2 checks passed
@widgetii
widgetii deleted the firmware/retired-packages branch October 23, 2026 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant