chore(build): add signing key to AppImage and update snapcraft workflow - #529
Conversation
|
Chased the AppImage half out of curiosity and landed on a different root cause than "no signing key" — the key is there, the subcommand just reads a different variable name. In the failed run (31192564528, linux job) both secrets were injected fine:
This PR fixes it: pulling the values out of the env and passing them as flags does get the key to the CLI. One tradeoff worth naming — Actions masks secrets in log text but not in the runner's process list, and env:
TAURI_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }}
TAURI_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD }}Secret names stay as they are — only the variable the step exports changes — and the comment above the call ("reads the key/password from the env above") stays true. Your call; the current form works either way. Separately: I could not find the snapcraft failure in that run — it died at the AppImage step, so |
|
thanks for catching that, I only saw references to
the snapcraft error is found in the previous successful build |
|
Correction to my previous comment — renaming the env vars would have been going backwards. Upstream already fixed this, and the real root cause is that our lockfile is pinned to an old CLI. Root cause
Reproduced locally with a throwaway key, same v2 env vars in both runs: So 2.11.4 also carries an AppImage fixRelevant to this PR specifically:
Different problem from the bundled-libwayland one #499 fixed — this is about the bundler's own output — but it lands in the same artifact you are currently repackaging by hand, so it seems worth having. Upgrade impact, as far as I checked
Limit of what I verified: I am on macOS, so only the macOS artifact line was exercised end to end. The Windows Entirely your call whether to fold that into this PR or keep it separate — happy to open the lockfile bump as its own PR if that is easier to review. |
|
One more, and this one is the bigger picture rather than another suggestion — my first two comments were about the symptom (no key) and the direct fix (the CLI pin). This is about why this workflow ended up needing The chainSigning is a commitment to bytes, not to a filename. The moment we touch the artifact after Could we avoid touching the artifact?Three ways I looked at, none of them currently open:
What would actually put us back on the main pathlinuxdeploy — which tauri downloads and invokes — has supported exclusion all along: // linuxdeploy/src/main.cpp
args::ValueFlagList<std::string> excludeLibraryPatterns(
parser, "pattern",
"Shared library to exclude from deployment (glob pattern)",
{"exclude-library"});tauri's invocation is hardcoded, with no config field and no env var that can append to it: // tauri-bundler/src/bundle/linux/appimage/linuxdeploy.rs
cmd.args([
"--appimage-extract-and-run",
"--verbosity", log_level,
"--appdir", &app_dir_path,
"--plugin", "gtk",
]);So the whole detour comes down to one flag that cannot be passed: (For what it's worth, the libs arrive indirectly: the GTK plugin hands Three options from here:
Not our call to make — you own the release process, and the upstream ask is yours to file or not. Flagging it because right now the cost of #498 is a permanently non-standard packaging path, and that cost is not obvious from the diff. What I did not verify: I am on macOS. Everything above is read from the tauri/linuxdeploy sources and the CI logs — I have not run a Linux bundle to confirm |
|
@PathGao defined new env vars, waiting on the build to pass for now. if an upstream issue is the cause we could try and report it as such? if wrapping the linuxdeploy is simpler/more reliable than the current work around we can give it a try, but I'd like to add the respective workflow to PRs to see if it passes that specific build before merging to master |
|
One footnote on the deprecated-env-var point, because this repo has two live examples of the cost of pinning to something old. Today, in this repo. #528 — In the file this PR edits. Which is the whole of my concern about Not urgent and not blocking; |
|
Better precedent than the two I just cited, and it is one this repo already wrote down — from
Same file, same release path, same shape as the env var question: a pin that was fine on the day it was written, kept working right up until someone else's timetable ran out, and surfaced on a release. And the timetable here is already scheduled rather than hypothetical — npm updates run quarterly, grouped for Genuinely the last word from me on this. |
|
I said that was my last word on the signing step, and it was — this one is different. Re-reading my own comment, I quoted What #484 set upThree ecosystems, deliberately different cadences, because the cost of being late is not the same for each:
Each group is limited to Security advisories bypass this schedule entirely, which is why #528 showed up on its own timetable rather than waiting for October. Why it matters here
The pattern, recentlyThe reason I keep returning to this is that the last two weeks have four instances of the same shape, all found rather than chosen:
And the one that is literally this file: None of this asks you to change anything in this PR — |
|
Answering your three questions properly. Reporting upstreamWorth doing, but the framing decides whether it survives triage. Not as a feature request. "Give us an As a bug it has a real chance, because it needs no new API at all:
That has what triage looks for: a community standard being violated (the excludelist exists precisely because those libraries must come from the host — not our preference), a concrete reproducible symptom, and a fix that is one #499 already contains most of that write-up. @LargeModGames did the diagnosis, so it is his call whether he wants to be the one to file it. The wrapperI'd skip it — it is not more reliable. It depends on Testing the release path before mergingStrongly agree, and it is the actual lesson from this whole thread — (I deleted an earlier version of this comment — it led with a problem in |
|
can we include both re: snapcraft build. adding the as of latest build, AppImage completed successfully. I'll cut 2.7.2 to address #498. |
Both env var names — yes, and it's the better answerThat works, and it is strictly better than picking one. The two sets are read by different CLI generations, so defining both means the step is correct no matter which version the lockfile resolves to:
Defining both covers every cell, including the day upstream finally removes the v1 names — at which point the v1 lines become dead weight and can be deleted, but nothing breaks in the meantime. Same secrets on the right-hand side, four lines instead of two. For what it's worth I still think bumping the CLI is worth doing on its own — it drops the whole question, and 2.11.4 additionally fixes an AppImage bundler bug ( The snapcraft errorI think def get_build_snaps(self) -> set[str]:
if options.rust_channel == "none" or "rust-deps" in (options.after or []):
return set()
return {"rustup"}So the snap was being installed before your change too. What fails is the check that runs after installation: # Check if rustup is properly installed
self.validate_dependency(
dependency="rustup",
argument="dump-testament",
plugin_name="rust",
part_dependencies=part_dependencies,
)It shells out to The part doesn't need the rust plugin at all. plugin: rust
override-build: |
npm ci
npx tauri build --no-bundle
install -D -m755 src-tauri/target/release/Markpad $CRAFT_PART_INSTALL/bin/markpadNone of the plugin's build logic runs — it is overridden. All that remains of - plugin: rust
+ plugin: nil
build-packages:
- libwebkit2gtk-4.1-dev
+ - rustup # noble universe, 1.26.0-5ubuntu0.1
...
build-snaps:
- node/20/stable
- - rustup/latest/stable
override-build: |
set -e
+ rustup default stable
craftctl set version="..."
The documented alternative, if you'd rather keep One thing you may not have seenBoth steps carry
So the snap has been failing on the same line for at least the last two releases while the step showed green. Worth knowing before cutting 2.7.2 — the AppImage fix for #498 will ship regardless, but the snap channel will not update. Caveat: I'm on macOS too, so the diff above is reasoned from the craft-parts source and the noble package index, not from a build I ran. |
|
2.7.2 finished while I was writing that — the important half worked. The snap did not, and it is the same line again: That makes three consecutive releases, and there is no @PathGao has gone to bed — he'll pick this back up tomorrow, along with the CLI upgrade. Thanks for cutting 2.7.2 so quickly. |
|
I left my claude code opus 5 bot here,so you can ask questions in about 1 hour it will reply |
Changes
Strip host-coupled libraries from AppImagefails during build due to no signing key. Added keys to build step explicitlySnapcraft build failed due to
rustupdep missing.Verification
All tests in
scripts/releaseWorkflow.test.tspassed (8/8).