fix(deb): avoid duplicate apt source entries when .sources exists (#10011)#10019
fix(deb): avoid duplicate apt source entries when .sources exists (#10011)#10019lonexreb wants to merge 2 commits intowarpdotdev:masterfrom
Conversation
…rpdotdev#10011) Ubuntu 26.04's `apt modernize-sources` converts legacy `.list` apt source-list files to the RFC 822-style `.sources` (DEB822) format. The existing postinst.repo.template only checked for `.list` before writing, so a post-modernization Warp upgrade created a fresh `.list` alongside the existing `.sources`. The result: duplicate-source warnings on every `apt update`. Fix: - Check for both filenames (`@@REPO_NAME@@.list` legacy and `@@REPO_NAME@@.sources` DEB822) before deciding to write. - For new installs, write `.sources` (the format the issue reporter recommended; supported by every apt version Warp's packages target). - For existing installs that already have either format, do nothing. Don't migrate `.list` to `.sources` from here — `apt modernize-sources` is the canonical migration tool and overwriting a user's potentially hand-edited source file would be hostile. Behavior summary: | Existing state | Action | |----------------------------------|---------------------------------------------| | Neither file present | Write `.sources` (modern format) | | `.list` exists, `.sources` does not | Leave `.list` alone (existing setup works) | | `.sources` exists | Leave `.sources` alone (post-modernize) | | Both exist (broken state today) | Leave both alone (the bug is no longer producing additional duplicates; user's `apt modernize-sources` cleanup remains the recovery path)| The new `.sources` format is otherwise byte-equivalent to what apt would produce when modernizing the legacy `.list` from this template.
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This change prevents post-modernization upgrades from creating a duplicate legacy .list entry and switches fresh apt installs to the DEB822 .sources format.
Concerns
- Fresh installs now create
@@REPO_NAME@@.sources, but purge cleanup still removes only@@REPO_NAME@@.list, leaving the repository configuration behind after package purge. - Security pass found no additional issues.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # configured within it, install our repo. | ||
| cat > "$APT_SOURCE_LIST" <<EOF | ||
| # configured in either format, install our repo. | ||
| cat > "$APT_SOURCE_LIST_DEB822" <<EOF |
There was a problem hiding this comment.
@@REPO_NAME@@.sources, but resources/linux/debian/common/postrm.repo.template still purges only @@REPO_NAME@@.list, so purging Warp would leave this apt source entry behind; update the purge script to remove the .sources file too.
Per oz-for-oss review on PR warpdotdev#10019: the postinst now writes the modern `.sources` (DEB822) file for new installs, but postrm.repo.template still purges only the legacy `.list` file. So purging Warp on a system where the postinst wrote .sources would leave the apt source entry behind. Add a second `rm -f` for the .sources filename. `rm -f` is silent on missing files, so older installs that have only .list are unaffected (and conversely, brand-new installs that have only .sources are also cleaned correctly).
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @acarl005. Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR updates the Debian post-install repository setup to recognize both legacy .list and modern DEB822 .sources apt source files, writes .sources for fresh installs, and purges both possible source-list filenames.
Concerns
No blocking correctness or security concerns identified in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
This makes sense to me. I'm going to test this on the latest Debian and will approve once I verify that. |
2 participants
Fixes #10011.
The bug
Ubuntu 26.04's
apt modernize-sourcesconverts legacy one-line.listapt source-list files to the RFC 822-style.sources(DEB822) format. The existingresources/linux/debian/common/postinst.repo.templateonly checked for@@REPO_NAME@@.listbefore writing, so a post-modernization Warp upgrade created a fresh.listalongside the existing.sources. Result: duplicate-source warnings on everyapt update, exactly as the issue reports.The fix
Two-part change to the postinst template:
@@REPO_NAME@@.listand@@REPO_NAME@@.sources) before deciding to write a fresh source-list. If either exists, we leave the existing setup alone..sourcesfor new installs, per the reporter's explicit recommendation ("preferably .sources, as it's the modern format"). DEB822 is supported by every apt version Warp's packages target. The new.sourcescontent is byte-equivalent to whatapt modernize-sourceswould produce when migrating from the old.listtemplate.Behavior matrix
.sources(modern format).listexists,.sourcesdoes not.sourcesexists (post-modernize)apt modernize-sourcescleanup remains the recovery pathWhy not auto-migrate
.list→.sourceshere?Considered and rejected.
apt modernize-sourcesis the canonical migration tool, runs interactively, and respects user edits. Migrating from a postinst hook would overwrite potentially-hand-edited source files (the postinst even acknowledges this risk in the existing comment: "You may comment out this entry, but other modifications to the file may be lost."). The simpler, safer behavior is: don't double-write, prefer modern format for fresh installs, leave migration to the user's tooling.Test plan
warp.sourcesonly;apt updateshows zero duplicate warnings.warp.list→ upgrades Warp; postinst sees.list, no-ops;apt updatecontinues to read.list; no duplicates introduced.apt modernize-sources(now haswarp.sources, nowarp.list) → upgrades Warp; postinst sees.sources, no-ops; the bug: previously this is the case where the duplicate.listwould have been created. Now it is not..listand.sourcespresent from a prior buggy upgrade) → upgrades Warp again; postinst sees both, no-ops; user's pre-existing duplicate is preserved (not made worse) and they can clean up manually withrm /etc/apt/sources.list.d/warp.list.Scope
resources/linux/debian/common/postinst.repo.template), 27 insertions, 5 deletions.