chore(repo): merge upstream 2.0.11.1 - #124
Merged
Merged
Conversation
Before a write, unmountVolumes() dismounted each volume on the target disk and then called DeleteVolumeMountPointW to stop Windows from re-mounting/re-grabbing it during the clean + raw write. Nothing ever restored that mapping, and there is no SetVolumeMountPoint anywhere in the tree. On card readers that Windows treats as fixed disks (RMB=0), the Mount Manager binding is persistent, so the reader came back with no drive letter — reinserted cards no longer appeared in Explorer and the letter had to be reassigned by hand after every write. Replace the letter deletion with a lock-and-hold approach that matches what the removed mountutils dependency effectively did: lock (FSCTL_LOCK_VOLUME) and dismount each volume, then keep the handles open via a new LockedVolumes RAII holder for the duration of prepare (unmount -> clean -> open). Holding the lock keeps the volume unmounted across the critical window without touching the persistent Mount Manager binding. Once the physical drive is open (which suppresses partition re-scan), the locks are released; when the write finishes and the physical-drive handle closes, Windows re-scans and reassigns the drive letter normally. Fixes raspberrypi#1665.
87b62f8 opens the target physical drive without FILE_SHARE_WRITE so the OS volume manager can't grab it mid-write. But if a transient holder (Explorer re-scanning the just-cleaned removable disk, an AV, or the search indexer) owns the drive at open time, the previous code dropped straight to a shared open, which lets Windows mount the partition mid-write. Retry the exclusive open with geometric backoff (matching Rufus's DRIVE_ACCESS wait loop) and only fall back to shared write if exclusivity genuinely cannot be obtained. Also stop issuing FSCTL_ALLOW_EXTENDED_DASD_IO on physical-drive handles: it is only meaningful for volume handles and returned ERROR_INVALID_FUNCTION on a physical drive, logging a spurious "failed" warning on every write. Note: exclusive access reduces but does not fully eliminate the removable-reader "insert a disk" / "format the disk" shell dialogs. Those stem from Windows auto-managing the drive letter while the disk is partition-less mid-write and are only fully avoidable via mount-manager AutoMount suppression, which is deliberately not used here because of its global, crash-persistent state.
…table When writing a customised image, DeviceWrapper::sync() wrote the FAT blocks and then the MBR, and the raw-write path wrote the deferred first block (containing the MBR) after the image body — but with no device-cache flush in between. On USB card readers the bridge/card write-cache can reorder writes, so the MBR can reach the media before the filesystem is complete. Windows then briefly sees a partition with an incomplete filesystem and prompts to format it. This never reproduces on a virtual disk, which has no reordering write-cache. Flush the device cache before writing the MBR in both the customised (DeviceWrapper::sync) and non-customised (_writeComplete) paths, so the partition only becomes visible once its contents are durably on media, and flush again afterwards so the MBR itself is durable. Crash-safe: touches no global OS state. A device that does not support flush returns success from Flush(), so this is a no-op there. This is a write-ordering correctness fix; on its own it does not eliminate the clean-phase removable "insert a disk" dialog.
The fastboot gadget reuses Google's USB VID/PID (18d1:4e40), so matching
VID/PID alone cannot prove a device is a Raspberry Pi. A non-Pi device in a
colliding fastboot mode (e.g. an Android phone) could otherwise be listed as
a writable target and flashed.
Add positive identification in FastbootProtocol::isRpiFastboot():
- primary: the USB interface string descriptor the rpi-fastbootd gadget
advertises ("fastbootd-provisioner"), read at open time via the new
IUsbTransport::interfaceString() (overridden by LibusbTransport);
- fallback: the RPi-specific "block-devices" getvar, which stock Android
fastboot does not implement.
Enforce the check at discovery (drive-list poll) and again before any
destructive command in the flash thread (erase / partition / flash), so a
non-Pi 18d1:4e40 device is never enumerated or written to.
stage() has always sent the "download:" verb, not "stage:", but two tests and the header comment assumed "stage:", leaving the fastboot suite with two long-standing failures. This is correct behaviour, not a bug: rpi-fastbootd maps both "download" and "stage" to the same DownloadHandler and keeps "stage" only for backward compat with older imagers, while its restricted TCP data-plane command map accepts only download/flash/getvar. "download" is therefore the canonical, portable verb and must not be changed back to "stage". Update the two tests to expect "download:", correct the stage() documentation, and add a note at the call site so the verb is not "fixed" back. The fastboot test suite is now fully green.
_verify() runs before _customizeImage(), so the customisation files are the one part of the card nothing ever reads back. A device that acknowledges writes it never commits therefore gives a green write whose settings are simply absent on first boot, with no indication why. Record a digest and size for each file written, re-read them after the final sync, and fail on a mismatch. The read uses a fresh DeviceWrapper, as the existing block cache does not evict on sync() and would otherwise compare our own buffer against itself, and re-asserts direct I/O to bypass the page cache. The device's own cache cannot be defeated, so this yields false passes but never false failures; hence failing hard on a mismatch is safe. boot.img is content-checked only when verification is enabled, size always. Hashing goes through AcceleratedCryptographicHash, as the rest of the write path does. Reported as a new customisation_verify event. This also catches the swallowed writeFile() failure at the end of _createSecureBootFiles(), which needs no faulty hardware. Not yet exercised on real media.
Pin curl, libarchive, libusb, nghttp2, xz, zlib and zstd as git submodules under src/dependencies/vendor and build them from source via CMake (fetch-vendor.cmake + per-dependency *.cmake). This links the imager against known-good upstream versions independent of the host distribution.
Add a patch fixing Qt6 QTestSupport chrono atomics on 32-bit targets (armhf) and adjust the shared Qt build helper and armhf build notes.
Add a release pipeline that builds the desktop, CLI and embedded AppImages and wraps them into .debs for amd64, arm64 and armhf. The host architecture builds locally; foreign arches build in rootless mmdebstrap chroots, with prebuilt AppImages and Qt injected via bind mounts. Includes release.sh orchestration, chroot/mirror/keyring helpers, per-arch apt sources, Qt caching and the updated packaging metadata (rules, control, changelog, install files).
Retarget the rootless mmdebstrap chroots from trixie to bookworm and parameterise the apt source templates on CHROOT_DIST, adding the bookworm sources those templated paths resolve to. Packaging fixes on top of that pipeline: * build-source.sh collected three of the four files listed in _source.changes; the generated _source.buildinfo was destroyed with the temporary build directory, so the source upload referenced a file that was never written out. * The chroot build path never invoked linuxdeploy, so the AppImages bundled Qt and nothing else, silently resolving around twenty libraries from the host (PCRE2, zstd, brotli, GnuTLS, glib, double-conversion, liburing). Cross-architecture builds never invoked it either, since linuxdeploy cannot introspect foreign binaries, so arm64 and armhf had never been bundled correctly. Replace it with appimage_deploy_lib_closure(), which walks DT_NEEDED using readelf and so works unchanged on a foreign-architecture AppDir, and which fails the build rather than silently under-bundling. * The embedded package copies Qt from a hand-maintained list but copies the QML tree wholesale, so sixteen libraries its plugins reference were never shipped: libQt6Quick could not resolve libQt6QmlModels or libQt6OpenGL, and the QtQuick.Controls plugin could not resolve libQt6QuickControls2. Complete the closure there too, additively, so the curated list stays authoritative for payload size. * Build Qt with -no-feature-icu. No ICU-backed Qt feature is used (UTF-8 conversion only, no QCollator, timezones handled as IANA identifier strings) and it was enabled purely by configure autodetection. Saves about 12.6 MB per binary package and removes the per-release libicu soname pin that tied the packages to a single Debian release. * Prune the QML tree to the eight modules the UI imports, with one policy shared between the AppImage and embedded paths. Each unused module's plugin links a Qt library of its own, so pruning modules is what removes the libraries; libQt6QmlWorkerScript is retained because libQt6QmlMeta links it directly. * Drop the JPEG 2000 image format plugin, staged on amd64 only, whose libjasper dependency was never shipped or declared. * Declare the host-coupled libraries all three packages resolve from the system. dh_shlibdeps cannot inspect an AppImage payload, so these were previously undeclared. Verified across amd64, arm64 and armhf plus embedded: ELF machine types match their filename architecture, 60/60 checksums verify, and the dependency sets are exactly right in both directions with nothing under-declared and nothing surplus.
A singular `user:` block is merged over the distro's default_user from /etc/cloud/cloud.cfg, which upstream populates with `sudo: ["ALL=(ALL) NOPASSWD:ALL"]` for every variant including raspberry-pi-os. Omitting the key therefore inherited passwordless sudo via /etc/sudoers.d/90-cloud-init-users regardless of what the user chose. Emit `sudo: null` explicitly, which suppresses the sudoers rules while still inheriting the default user's groups, so the account keeps `sudo` group membership and is simply prompted for a password. `false` behaves the same but has been deprecated since cloud-init 22.2.
…_uring file_operations_linux.cpp uses io_uring_sqe_set_data64 and io_uring_prep_cancel64, added in liburing 2.2. Some distributions ship an older liburing that provides the library but not these helpers (Ubuntu 22.04 carries 2.1), where pkg-config succeeds and the build then fails to link. Compile-probe for the two helpers and gate both -DHAVE_LIBURING and the link against the result, warning and falling back to synchronous writes when the library is present but too old.
A package built against the host's libraries is not the package it claims to be. Building on a newer glibc than bookworm yields binaries that will not start on bookworm at all; building on an older one silently drops features -- a host with liburing < 2.2 fails the io_uring API probe and produces a functionally slower binary -- and in both cases the artifact still gets labelled for bookworm. Nothing in the artifact reveals which happened. BUILDER=auto/local existed as an escape hatch for environments that could not run sbuild. The rootless mmdebstrap chroots removed that constraint: they need no sudo, and CHROOT_AUTO_CREATE=auto creates them on demand, so there is no environment that cannot use one. Nothing in the tree selected it, and no CI referenced it. Remove choose_builder() and the BUILDER variable, the _local_build() branches in build-appimages.sh, and build-binary-local.sh, which becomes unreachable. build-embedded.sh had a separate host-arch shortcut that never consulted BUILDER at all -- invisible today only because the embedded package is arm64-only and is built on amd64 -- so make that unconditional too. Rebuilt all three architectures plus embedded: identical output, 60/60 checksums verify, ELF machine types match their filename architecture.
The embedded package shipped through a standalone path in create-embedded.sh that hand-wrote its own DEBIAN/control, so it diverged from debian/control: the Depends stanza was stale (missing the host-coupled libraries the vendored tree needs), it carried no md5sums, and its version (2.0.10) did not match the other packages. Route the packaging through debhelper instead, so debian/control is the single source of the package's dependencies and metadata: * create-embedded.sh now stages the vendored /opt tree and assembles the .deb with dh_gencontrol / dh_md5sums / dh_builddeb. Version is taken from the changelog (2.0.10-1), md5sums are generated, and Depends come from debian/control. dh_shlibdeps is deliberately not used -- on a vendored /opt tree it demands ~19 libraries from the target that are bundled precisely so a netboot micro image need not carry them -- so debian/control lists the genuinely-external libraries (C/C++ runtime, GPU stack, X11, libsystemd) explicitly. * Bake the deployed rpath in at build time (CMAKE_INSTALL_RPATH=$ORIGIN/../lib, CMAKE_BUILD_WITH_INSTALL_RPATH). The binary was copied to /opt carrying the cmake build rpath, an absolute build-host path: a privacy leak, a reproducibility hazard, and the reason dpkg-shlibdeps could not resolve the bundled libraries. * Dereference the fontconfig conf.d symlinks (cp -rL) so the bundle is self-contained instead of shipping 21 links that dangle on a target without fontconfig installed. * Remove the ICU detection and copy code. Qt is built with -no-feature-icu, so no ICU is linked or bundled; the code only ever emitted a spurious "ICU not found" warning. * CONTRIBUTING.md: document the real build (debian/release.sh embedded arm64), dropping the obsolete build-qt-embedded.sh and dpkg-buildpackage steps. Verified on the rebuilt arm64 package: version 2.0.10-1, md5sums present, Depends complete (external closure fully covered, nothing surplus), binary rpath $ORIGIN/../lib with no build-host path in any shipped ELF, no dangling symlinks, no ICU, 83/83 ELF objects aarch64.
The embedded (linuxfb netboot) target runs on a pi-gen-micro image with no session bus (SYSTEMD=0) and a five-package base. Linking QtDBus there is pure liability: it drags in libQt6DBus -> libdbus-1 -> libsystemd, none of which the image carries, so the binary could not even load. Nothing embedded actually needs DBus at runtime. The three DBus users degrade to nothing useful on that image anyway: the XDG portal file dialog (embedded already forces QML dialogs), the NetworkManager WiFi-credential backend (no NM daemon to read from), and the Pi Connect rpi-imager:// URI handler (no browser to sign in from). Drop Qt6::DBus from find_package/link for BUILD_EMBEDDED. With it unlinked Qt's QT_DBUS_LIB macro goes undefined, which compiles out the guarded DBus paths: platformquirks_linux.cpp already used that guard; extend it to the main.cpp single-instance/URI-handler blocks and to nativefiledialog_linux.cpp (whose portal implementation falls back to the QML dialog stub). Platform.cmake routes embedded to the existing CLI stubs for the WiFi-credential and suspend-inhibitor backends and drops the NetworkManager and URI-handler sources. Desktop and CLI builds are unchanged (desktop keeps DBus; CLI never had it).
Embedded was quietly using the desktop release Qt, which is built with OpenGL, DBus and (by autodetection) ICU. On the pi-gen-micro netboot image that is wrong on every count: there is no Mesa (far too large for a network-loaded image), no session bus, and the embedded build ships none of the complex-script languages that could want ICU. The result linked libEGL/libGL/libX11 and libQt6DBus and would not load on the target at all. Build a dedicated embedded Qt as its own cache variant (gcc_arm64_embedded), configured -no-opengl -qpa linuxfb (software rendering over DRM, no GL/X11), -no-dbus, and -no-feature-icu. Remove the custom from-source ICU build from build-qt-embedded.sh -- it cloned and compiled ICU only to feed a feature we no longer enable, and needs network access the chroot does not have. Wire the variant into the release pipeline: qt_embedded_path()/qt_embedded_ok() in lib.sh, and build-embedded.sh builds it on cache miss and passes it to create-embedded.sh. The cache-miss check is by file presence, not qt_embedded_ok (which runs `qmake -query`): the cached qmake is arm64 and the orchestrator runs on the host, so executing it would always fail and force a needless full Qt rebuild. Embedded is arm64 only -- the only platform the netboot installer targets.
A gitignore pattern containing no slash matches by basename at every
depth, not just where it was meant to apply. Every build product in this
file lives at the repository root, but five of the patterns were written
unanchored, so they also matched tracked source files further down the
tree:
* build-** every qt/build-qt*.sh (five scripts)
* qt-build** qt/qt-build-common.sh
* appimage-* debian/appimage-pack.sh
* timezones.txt src/timezones.txt, the fallback data the generator
falls back to; the generated file is
timezones_generated.txt and lives in the build tree
* screenshot.png tracked, and displayed by README.md
Being already tracked, those files kept working, and the breakage only
showed up on a fresh git add. debian/appimage-pack.sh is where it did:
written, silently ignored, and so never committed.
The existing !debian/build-*.sh negation was this bug being patched once
already, for one directory. Anchoring the patterns fixes it generally and
makes the negation unnecessary, so drop it. Also anchor the remaining
root-only patterns (obj-, AppDir-, debroot-embedded-, test-cli-, .debian/,
out/) which had the same latent problem, and replace the appimage-* catch
with the directory it was actually for, appimage-tools/.
Verified no tracked file is ignored any more, and that out, .debian,
AppDir-*, appimage-tools, qt-build*, qt-src, build-*, debroot-embedded-*
and *.AppImage are all still ignored.
QT_VERSION_DEFAULT in qt/qt-build-common.sh is what the build-qt*.sh scripts use, but debian/lib.sh carried its own copy of the version literal, and the release pipeline invokes those scripts with --version="$QT_VERSION". The two agreed today by coincidence; nothing made them agree tomorrow, and disagreeing would have meant the pipeline asking for one version and validating the cache against another. Read QT_VERSION_DEFAULT out of qt-build-common.sh instead. Extract it with sed rather than sourcing the file: it also sets ARCH, PLATFORM and CORES, which would clobber the scripts that source debian/lib.sh. QT_MIN_VERSION had the same problem against a different authority — it duplicated the minimum from the find_package(Qt6 ...) call in src/CMakeLists.txt, which is what actually enforces it. Read it from there, keeping the previous literal as a fallback. Environment overrides still take precedence in both cases, so a one-off build against another version works as before.
The rootless multi-architecture chroot build was essentially undocumented: debian/release.sh --help lists the commands, and everything else was recoverable only by reading roughly 2,800 lines of shell across twenty-odd scripts. Add doc/linux-build.md covering the model (every architecture, the host's own included, builds in its own bookworm mmdebstrap chroot, and why there is deliberately no host-native path), prerequisites, the commands, the on-disk layout, and each of the five stages: keyrings, chroot, Qt, AppImages and binary packages. Then the embedded package, the bundling policy, the full configuration table, troubleshooting and cleanup. Weighted towards the things that are surprising rather than the things that are merely true, since the latter can be read off the scripts: * AppDirs are built inside the target chroot but packed on the host, because linuxdeploy and appimagetool are themselves AppImages and must match the machine executing them, not the target. * appimagetool silently embeds its own host runtime when ARCH is all you give it, which is why --runtime-file is mandatory for a cross-pack and why release.sh status re-checks staged AppImages with file(1). * dpkg-buildpackage writes to $TOP/.., only $TOP is bind-mounted, so the artifacts land inside the chroot rootfs and have to be collected back out. * The AppImage and embedded exclusion predicates are deliberately different, and pruning QML means pruning modules, never libraries. No Qt version is quoted; the doc points at the script that selects it.
The Linux section of CONTRIBUTING.md still described the build as it was before the crossbuild pipeline, and had stopped working rather than merely aged: * git clone --depth 1 leaves no submodules for the vendored third-party dependencies and no tags for the git describe that produces the version string. * sudo ./qt/build-qt.sh into /opt/Qt, then a bare ./create-appimage.sh, bypasses the chroot entirely and links the result against the host's libraries — the thing the pipeline exists to prevent. Replace it with the pipeline, plus a separate short CMake loop for people iterating on the application rather than producing artifacts. The embedded section claimed the package "uses the same vendored release Qt as the desktop and CLI packages ... so there is no separate embedded Qt to build". That has not been true since the embedded Qt became its own -no-opengl -no-dbus -qpa linuxfb build in its own cache variant. State what it actually builds and why the netboot image forces it. create-embedded.sh suggested ./qt/build-qt.sh --version=6.9.1 when it could not find Qt: the wrong script, since that builds the desktop Qt this package cannot use, and a version that has not been current for some time. Point at build-qt-embedded.sh and name no version. Also drop the hardcoded Qt versions from the Windows and macOS Qt6_ROOT examples, which had drifted to three different values between them.
The four guides named a concrete Qt version on 29 lines between them, in install paths, --help transcripts and prose. They had drifted to four different values (6.8.0, 6.9.1, 6.9.3, 6.11.1), none of which was the version the scripts actually build. Replace them all with <version>, and explain it once per guide with a note pointing at QT_VERSION_DEFAULT in qt-build-common.sh — the single place the version is selected, and now the only place it is written down. Also note in the armhf guide that release builds do not need that script: the pipeline builds armhf Qt inside an armhf chroot via debian/ensure-qt.sh.
Both guides described `release.sh repo` as building "every arch", which is only true once RELEASE_ARCHES has been set. It defaults to the host architecture alone, so the command as written built one architecture and the comment next to it said otherwise. repo is also the only command that takes more than one architecture, and it reads them from RELEASE_ARCHES rather than from its arguments — worth stating, because the natural guess is that `arch` or `appimages` accepts a list. Show the inline form the script's own --help already suggests, and the release.conf form. Note that release.conf.example sets all three, which is why the same repo command behaves differently depending on whether a release.conf exists — the sort of difference that otherwise gets diagnosed as a bug. Also record that the host architecture is built first, that architectures build sequentially rather than in parallel, and that a first three-architecture run builds Qt three times, so it takes hours.
The 2.0.11 changelog entry was written as a bare 2.0.11, dropping the revision that ca98bbb's own subject line promised. Since 720b900 switched debian/source/format to 3.0 (quilt), a version without a revision is native as far as Dpkg::Version::is_native is concerned, and V3::Quilt::can_build refuses to build it: dpkg-source: error: can't build with source format '3.0 (quilt)': non-native package version does not contain a revision This went unnoticed because dpkg 1.21.1, on the Ubuntu host the releases were built on, only emits this as a warning; 1.21.22 and later turn it into a hard error, so the source build fails outright on a Debian bookworm or trixie host. Only the topmost entry is checked, so the older revision-less entries are left as they are: they date from when the package really was 3.0 (native).
This fixes the Raspberry Pi Connect stage failing to progress, by removing the direct binding of 'text' to 'value', forcing it to be re-evaluated at 'onTextChanged'.
The version parser captured exactly three numeric components, so a hotfix tag such as v2.0.11.1 parsed as 2.0.11 and the trailing .1 was dropped before it reached any template. That mattered most on Windows, where the numbers feed FILEVERSION, PRODUCTVERSION and the assembly manifest: 2.0.11.1 would have shipped reporting file version 2.0.11.0, byte-identical to the release it fixes. The install itself still succeeds, since every Source line in the Inno script carries ignoreversion, but nothing keyed on the file version -- winget, SCCM, inventory tooling, crash triage -- could tell the two builds apart. Capture an optional fourth component as IMAGER_VERSION_TWEAK, defaulting to 0, and consume it in the two Windows templates. Ordinary three-part tags are unaffected: v2.0.11 still yields 2,0,11,0. The reference to CMAKE_MATCH_5 is quoted deliberately. A regex group that does not participate leaves it unset, and an unquoted `if(VAR STREQUAL "")` then compares the literal variable name rather than its value, which would emit an empty tweak for every ordinary release. create-embedded.sh truncated the same way. It only feeds a log line, but it would have reported "numeric: 2.0.11" while building 2.0.11.1.
The version sent with download statistics was extracted with a regex capturing exactly X.Y.Z, so a hotfix build such as 2.0.11.1 reported itself as 2.0.11 -- indistinguishable from the release it fixes. Uptake is the number a hotfix exists to be measured by, so allow an optional fourth component. Every other shape is unchanged: v2.0.0, v2.0.0-rc4-60-geac7c2f0 and a bare commit hash all extract as before.
d33b6bf made both cloud-init generators emit nothing unless there is something to configure: manage_resolv_conf became a conditional prepend, and the eth0 DHCP block moved inside the wifis: block, because writing a network-config at all replaces the distro default and would otherwise take wired ethernet with it. Older fastboot gadgets failed on the spurious write that the unconditional baseline produced. That commit touched only the generator, leaving nine tests asserting the baseline it had just removed. They have failed since v2.0.7. Update them to the current contract. Eight asserted ethernets: on a netcfg generated without any Wi-Fi and now assert it is empty. The empty-settings case asserted a manage_resolv_conf-only user-data and now asserts both payloads are empty, which is the guarantee d33b6bf was actually buying -- an empty payload is what tells the fastboot and download paths to skip writing the file. No production behaviour changes here; these tests had simply stopped describing the code.
Catch2 exits with 4 when every selected test case was skipped. Because catch_discover_tests registers one CTest test per case, a case that calls SKIP() is always the only one selected, so CTest saw exit code 4 and reported a failure. Six tests that skip when optional fixtures or hardware are absent -- the pieeprom and eeprom_signer ones, which need rpi-eeprom tooling or network access -- were showing up as phantom failures because of this. Wrap catch_discover_tests to pin SKIP_RETURN_CODE and use the wrapper at every call site, so a new test binary cannot reintroduce the problem by forgetting the property.
The shared device is opened from a static constructor, so it also runs
when the binary is only listing its test cases. catch_discover_tests
parses that listing from stdout a line at a time, so the startup banner
went into the listing and CTest registered one of its lines as a test:
850 - WARNING: No test device path available (Failed)
No such case exists, so running it produced "No test cases matched" and
a permanent failure.
Send the diagnostics on this path to stderr, which discovery ignores.
They are still visible when running the binary or using
--output-on-failure. Only the pre-main and at-exit prints move; the
in-test output is untouched, since by then the listing is long done.
Two FileServer tests still described the protocol as it was before e451eab reworked file_server.cpp. Both failed; in both cases the implementation is right and the expectation was stale. "handles GetFileSize request" expected a single control transfer and saw two. The second is the zero-length acknowledgement sent for the empty filename that signals end of transfer -- an empty name is the device's done marker whatever command accompanies it, and acknowledging it matches upstream rpiboot. Assert both transfers rather than loosening the count, which covers more than the original did. "returns false on short bulk read" was misnamed: FileMessages arrive over control IN, not bulk, matching upstream's ep_read(). It expected the retry-exhaustion message, but -1 is LIBUSB_ERROR_IO, which is treated as a fatal disconnect and fails immediately without spending the retry budget -- again matching upstream, which breaks on NO_DEVICE and IO. Rename it and assert the disconnect message it actually produces. Note that "Failed to read FileMessage" is now uncovered. It remains reachable for non-fatal error codes once retries are exhausted, but exercising it needs a mock that can script a specific error code -- the mock returns -1 unconditionally on an empty queue -- and would spend about four seconds in the retry loop's sleeps.
Hotfix for the Raspberry Pi Connect customisation step, which since 2.0.11 could not be advanced past once the browser sign-in filled in the token. Carries the packaging and telemetry changes needed for a fourth version component, without which the build would report itself as 2.0.11 to Windows and to download statistics alike, and the test-suite corrections that went with them.
Both issue forms offered 2.0.10 as the newest version and as the default, so 2.0.11 reporters had no accurate option and every report has been arriving labelled with a version its author was not running. Add 2.0.11.1 as the new default and 2.0.11 behind it. Also add 2.0.1, which has been missing from both lists since it was released.
The AppImages shipped the "wayland" QPA plugin but none of the Qt plugin directories it depends on. libQt6WaylandClient looks for a shell integration under plugins/wayland-shell-integration; with libxdg-shell.so absent the plugin loads, connects to the compositor, then aborts with "Loading shell integration failed." Qt falls back to "xcb", so every session -- Wayland included -- ran through XWayland and needed libxcb-cursor0. On a target without that library (Raspberry Pi OS bookworm) both plugins fail and the application does not start at all. Neither deployment path shipped them: the manual cross-build list omitted them, and linuxdeploy-plugin-qt does not deploy them either, so the copy runs after both paths rather than inside the manual branch. These are Qt's own plugins and belong with the bundled Qt. The wayland system libraries are unaffected: libwayland-client/-cursor stay host-provided via --exclude-library and debian/control. libxdg-shell.so links only those two plus libQt6WaylandClient, which is already bundled, so no new host dependency is introduced. Verified on arm64, armhf and x86_64 against weston with libxcb-cursor0 removed, so no xcb fallback could mask the result: before, "Loading shell integration failed"; after, "Using the 'xdg-shell' shell integration" and a correctly rendered window. (cherry picked from commit 3cb178e on dev/tdewey/2.1)
|
Important Review skippedToo many files! This PR contains 109 files, which is 9 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (109)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merges upstream/main at 72ad117 (v2.0.11.1), which includes the now-landed raspberrypi#1674 that 2569eb3 had previewed. 55 upstream commits; 18 files conflicted, resolved in three groups. Round-trips - took upstream's copy, dropped ours: src/block_batcher.h, src/test/block_batcher_test.cpp - upstream merged our PR raspberrypi#1681 and took both files verbatim, changing only the UNRAID: comment marker. The files are theirs now, so the marker goes with them. Same for the BlockBatcher target in src/test/CMakeLists.txt, and for the batching comment in downloadextractthread.cpp. Five markers dropped, all accounted for: 177/58 files -> 172/56. Defects upstream still carries - kept ours: imagewriter.cpp still guarded only `fcs == 0` after ZSTD_findDecompressedSize, so ZSTD_CONTENTSIZE_UNKNOWN still set _extrLen = ULLONG_MAX and rejected a good .img.zst. fastboot_protocol.{h,cpp} still had bool isRpiFastboot(), with no TransportError and no tri-state, so a Pi whose gadget was still starting was still cached as ConfirmedNotPi. Also kept: the O_EXCL reopen degrade, the cancel re-check after _verifyCustomisation(), the performErase() cancel guards, and the DA session on the main run loop. All of these are now open upstream as raspberrypi#1701-raspberrypi#1705. downloadextractthread.cpp keeps Unraid::requireArchiveWriteSuccess over upstream's _checkResult(), which logs warning-class results and continues. libarchive's Windows backend reports a failed WriteFile() as ARCHIVE_WARN, so continuing there means a silently incomplete boot drive. Took upstream's improvement over ours: imagewriter.cpp - the custom-manifest regex now uses QRegularExpression::anchoredPattern(). PCRE2 lets '$' match before a trailing newline, so our '^..$' form accepted a URL copied out of a browser with \n attached (their issue raspberrypi#1687). HostnameCustomizationStep.qml - took trimWhitespace, kept our branded Accessible.description. src/test/CMakeLists.txt - our two fork-only test targets now use rpi_discover_tests(), which pins SKIP_RETURN_CODE=4 as upstream's helper does. Two duplicate definitions the merge produced without reporting a conflict, because each side added the same block at a different point in the file: customisationDigest() plus _verifyCustomisation() in downloadthread.cpp, and class LockedVolumes in windows/diskpart_util.h. Both copies were byte-identical apart from _verifyCustomisation()'s error routing, where ours (the _onDownloadError() route) is the copy that survives. Caught by CI, not by the conflict list - worth scanning for on the next merge. create-appimage.sh was the only real integration. Upstream moved to Ninja plus a build/pack stage split, so the branding.env sourcing now sits between cmake configure and cmake --build, and the pack stage - which runs no cmake - reads the branding.env the build stage left behind. Without that, IMAGER_EXE_NAME is empty in pack mode and the AppRun guard fires on an empty path. shellcheck clean; not otherwise exercised, as CI only runs the all-in-one mode. Submodules do not affect us: upstream vendored the third-party libraries under src/dependencies/vendor, but fetch-vendor.cmake falls back to FetchContent when the vendor tree is absent, so our checkout keeps building without recursive submodule fetch. The Debian/embedded release pipeline that most of the 55 commits build out is upstream-only; we ship no .debs.
Eli Bosley (elibosley)
force-pushed
the
chore/upstream-2.0.11.1
branch
from
August 18, 2026 16:32
fd47404 to
c7e1cef
Compare
raspberrypi#1685 is our background-eject work sent upstream, and @tdewey-rpi asked for two changes there. Applying them here too keeps the trees identical, so whenever raspberrypi#1685 lands the next upstream merge is a no-op on these lines rather than a conflict. DoneStep.qml - the EjectSucceeded case goes back to the original string, "The storage device was ejected automatically. You can now remove it safely." That exact source string is in all 27 .ts files; the reworded version reset the success case to untranslated in every language. The cost is that "automatically" is not strictly true when the user pressed Eject themselves, which Tom accepted as the cheaper of the two. imagewriter.cpp, downloadthread.cpp - report an eject as successful only on DiskResult::Success. The comment justifying the Q_OS_WIN leniency was wrong: processDriveLetter() returns Success both when a drive letter cannot be opened and when its device number does not match the target, so an unrelated volume can never contribute a Busy or an Error. Every non-Success result already comes from a volume on the drive being ejected, which made the old check pure masking - a locked volume on the target reported "safe to remove", which is exactly the claim this whole feature exists to stop being a lie.
QA failed rc.1 on Windows: formatting a stick behind a USB enclosure succeeds, then the write dies with zero bytes written. Cross-platform disk formatter succeeded in 2902 ms "Failed to open disk for rescan. Error code: 32" assignDriveLetter for disk 2 : failed ... "No volume found on disk 2." PerformanceStats: Cycle ended, state: "failed" ... dl=0 dec=0 wr=0 vfy=0 Error 32 is ERROR_SHARING_VIOLATION, and we are the ones holding the drive. DiskFormatter::FormatDrive() opens the device and never closes it; the handle lives until ~DiskFormatter destroys file_ops_. That was harmless while physical drives opened write-shared, but 87b62f8 (upstream's 2.0.11-rc2 exclusive-lock rework) now opens them FILE_SHARE_READ with no FILE_SHARE_WRITE, so no second opener may ask for write access. rescanDisk() asks for GENERIC_READ | GENERIC_WRITE, and the formatter was still in scope, so Windows refused it. The rescan is what issues IOCTL_DISK_UPDATE_PROPERTIES. Without it Windows never re-reads the partition table we just wrote, no volume is created, the extract has nothing to mount, and the run ends having written nothing. Scope the formatter so the handle is gone before we ask the OS to re-read the disk. Upstream keeps exactly this order at its own refreshDiskView() call site in DownloadThread::_onDownloadError() -- "Drop the device handle before asking the OS to refresh its view of the disk" -- but our post-format rescanDisk() call is fork-local (upstream's format flow ends at cleanDisk and never rescans), so it never inherited that discipline. Nothing upstream is affected. Not enclosure-specific in mechanism: the sharing violation fires for any physical drive. What the enclosure changes is whether it is fatal -- a plain removable stick gets a spontaneous volume-arrival from Windows and survives the failed rescan, while a drive behind a UASP/SCSI-enumerated bridge is treated as fixed, gets no spontaneous rescan, and never produces a volume. Checked the rest of the flow for the same collision: cleanDiskFast() and unmountVolumes() both open before the formatter does, and assignDriveLetter() opens the volume with access 0 and shared read/write, so none of them can hit it. This was the only one.
The Linux jobs have twice sat in "Install build dependencies" for 40+ minutes without producing a line of output, on a step that normally finishes in under two. Both times it was apt on the hosted runner, not our build: the run was killed and re-run and the same commit then built in 6-8 minutes. Once on PR #124, once on the v2.0.11.1-unraid.0-rc.2 release build. Waiting it out costs the job's whole time budget before a compiler ever starts, and it is invisible while it happens - the step just sits there, which reads identically to a slow build. So bound it instead: apt-get update gets 300s and the install 600s, three attempts, and the step as a whole gets timeout-minutes: 20 as a backstop if the retry loop itself wedges. A killed apt leaves its lock files and a half-applied dpkg state behind, so each retry clears those before trying again. The retry also moves off the mirror that stalls. GitHub's runners point at azure.archive.ubuntu.com, which is where both hangs happened, so attempt two onwards uses archive.ubuntu.com. Applied to both Linux steps - build.yml and e2e-qemu.yml run near-identical apt invocations, and the e2e one is just as exposed. $packages is deliberately unquoted so it word-splits into arguments; shellcheck's SC2086 is disabled on that line rather than silenced globally. actionlint clean.
github-project-automation
Bot
moved this to In review
in Unraid USB Creator Rewrite Board
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges upstream/main at 72ad117 (v2.0.11.1), which includes the now-landed
raspberrypi#1674 that 2569eb3 had previewed. 55 upstream commits;
18 files conflicted, resolved in three groups.
Round-trips - took upstream's copy, dropped ours:
src/block_batcher.h, src/test/block_batcher_test.cpp - upstream merged our
PR raspberrypi#1681 and took both files verbatim, changing only the UNRAID: comment
marker. The files are theirs now, so the marker goes with them. Same for the
BlockBatcher target in src/test/CMakeLists.txt, and for the batching comment
in downloadextractthread.cpp. Five markers dropped, all accounted for:
177/58 files -> 172/56.
Defects upstream still carries - kept ours:
imagewriter.cpp still guarded only
fcs == 0after ZSTD_findDecompressedSize,so ZSTD_CONTENTSIZE_UNKNOWN still set _extrLen = ULLONG_MAX and rejected a
good .img.zst. fastboot_protocol.{h,cpp} still had bool isRpiFastboot(), with
no TransportError and no tri-state, so a Pi whose gadget was still starting
was still cached as ConfirmedNotPi. Also kept: the O_EXCL reopen degrade, the
cancel re-check after _verifyCustomisation(), the performErase() cancel
guards, and the DA session on the main run loop. None of these are fixed in
upstream/main - they are going up as PRs separately.
downloadextractthread.cpp keeps Unraid::requireArchiveWriteSuccess over
upstream's _checkResult(), which logs warning-class results and continues.
libarchive's Windows backend reports a failed WriteFile() as ARCHIVE_WARN,
so continuing there means a silently incomplete boot drive.
Took upstream's improvement over ours:
imagewriter.cpp - the custom-manifest regex now uses
QRegularExpression::anchoredPattern(). PCRE2 lets '$' match before a trailing
newline, so our '^..$' form accepted a URL copied out of a browser with \n
attached (their issue raspberrypi#1687).
HostnameCustomizationStep.qml - took trimWhitespace, kept our branded
Accessible.description.
src/test/CMakeLists.txt - our two fork-only test targets now use
rpi_discover_tests(), which pins SKIP_RETURN_CODE=4 as upstream's helper does.
create-appimage.sh was the only real integration. Upstream moved to Ninja plus
a build/pack stage split, so the branding.env sourcing now sits between cmake
configure and cmake --build, and the pack stage - which runs no cmake - reads
the branding.env the build stage left behind. Without that, IMAGER_EXE_NAME is
empty in pack mode and the AppRun guard fires on an empty path. shellcheck
clean; not otherwise exercised, as CI only runs the all-in-one mode.
Submodules do not affect us: upstream vendored the third-party libraries under
src/dependencies/vendor, but fetch-vendor.cmake falls back to FetchContent when
the vendor tree is absent, so our checkout keeps building without recursive
submodule fetch. The Debian/embedded release pipeline that most of the 55
commits build out is upstream-only; we ship no .debs.
Full merge rationale is in the merge commit message (
fd47404c).Verification so far:
shellcheckclean oncreate-appimage.sh, UNRAID marker audit reconciled (177/58 files → 172/56, every dropped marker traced to the BlockBatcher round-trip). CI on this PR is the build verification.