From caaae11d98339d63fa43802eff9d10f859ecae94 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 26 Sep 2026 14:52:44 -0400 Subject: [PATCH] Say the folder dialogue is the portal, and what the Flatpak has not proved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit distribution.md §4, outstanding.md's FR-PLAT-LIN-3 entry, the Flatpak manifest's comment and the README all said a library was chosen by typing a path and that nothing in the tree called the FileChooser portal. Since 6683c14 every folder the desktop asks for is chosen through rfd's xdg-portal backend, so those sentences were false. What they now say instead is narrower than "it works in the sandbox": no Flatpak has been built here, so whether the portal's path opens a library, holds across a restart and takes a sidecar is unobserved, and volumes() still cannot see a host card. The chooser also landed in ui/ rather than behind the dr-plat seam distribution.md had proposed, and both documents say so. FR-PLAT-LIN-3 gets a status note to the same effect. --- README.md | 11 +- docs/dev/distribution.md | 102 ++++++++++-------- docs/dev/outstanding.md | 22 ++-- docs/dev/requirements.md | 6 ++ packaging/flatpak/paris.tourolle.darkroom.yml | 24 +++-- 5 files changed, 98 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 610e3f4..df4fa04 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ texture directly — no readback between the GPU and the screen. | Arch Linux | [`packaging/PKGBUILD`](packaging/PKGBUILD) — `makepkg -si` | Built from every release | | Android | The APK from each CI run, or `./docker/android/package.sh --install` | Runs on a tablet; F-Droid not yet submitted | | Windows | `DarkRoom--x86_64-setup.exe`, cross-built by CI ([windows.md](docs/dev/windows.md)) | Verified under Wine only; unsigned | -| Flatpak | [`packaging/flatpak/`](packaging/flatpak/) | Manifest in tree; choosing a library does not yet work in the sandbox | +| Flatpak | [`packaging/flatpak/`](packaging/flatpak/) | Manifest in tree; folders are chosen through the portal, but no Flatpak has been built to prove it | Or build it. Git LFS is required for the model weights, and the toolchain pins itself to 1.92.0: @@ -95,10 +95,11 @@ the rest are written down rather than merely absent. survey culling, AI denoise, tiled rendering, HDR merge and focus stacking, importing a Lightroom or darktable catalog, translations beyond the launch screen, most of the Android platform integration beyond -running, and the Flatpak's library chooser. The performance targets are half -verified: the per-commit benchmark suite §8 requires exists for everything -that does not need a frame — the catalog, the scan, the thumbnails — and -not yet for the render path, so a regression there fails nothing. +running, and a Flatpak actually built and run in its sandbox. The +performance targets are half verified: the per-commit benchmark suite §8 +requires exists for everything that does not need a frame — the catalog, +the scan, the thumbnails — and not yet for the render path, so a regression +there fails nothing. [outstanding.md](docs/dev/outstanding.md) is the list, with the reasoning for each. diff --git a/docs/dev/distribution.md b/docs/dev/distribution.md index 8cce5b3..070ef57 100644 --- a/docs/dev/distribution.md +++ b/docs/dev/distribution.md @@ -18,7 +18,7 @@ permission to a package rather than after. | Platform | Channel | State | What it constrains | |---|---|---|---| | Linux | Arch source package — [`packaging/PKGBUILD`](../../packaging/PKGBUILD) | Built, in tree | Nothing. Full filesystem access, system Vulkan, system secret daemon | -| Linux | Flatpak — [`packaging/flatpak/`](../../packaging/flatpak/) | Manifest in tree, **library selection does not work** (§4) | Portals only. No `--filesystem=`, no host mount table, no typed paths | +| Linux | Flatpak — [`packaging/flatpak/`](../../packaging/flatpak/) | Manifest in tree; folders chosen through the FileChooser portal since 0.17.0, **never built or run here** (§4) | Portals only. No `--filesystem=`, no host mount table, no typed paths | | Linux | AppImage | v1 channel, **recipe not yet written** (§5) | Oldest supported glibc, and no sandbox at all | | Android | F-Droid | v1 channel, not yet submitted | GPLv3-clean build, reproducible, no proprietary blobs | | Android | Play Store | **Not v1** (§6) | Would make ARCH §6.9 binding as policy rather than as engineering | @@ -78,7 +78,7 @@ The Arch package and an AppImage both hand the application the same unrestricted process the developer runs it in, so neither can discover that a design assumed unrestricted access. Flatpak takes that assumption away, and FR-PLAT-LIN-3 exists to make the discovery happen deliberately rather than in a -bug report. §4 is what it discovered. +bug report. §4 is what it discovered, and what has been done about it. The same argument runs the other way on Android, where SAF has been the only option since before the first line was written (ARCH §6.9) and `SourceRef` @@ -120,31 +120,55 @@ advance: --- -## 4. What does not work: choosing a library +## 4. Choosing a library: built, not yet proved in the sandbox -**FR-PLAT-LIN-3 is not satisfied today, and the manifest does not pretend -otherwise.** +**FR-PLAT-LIN-3 was not satisfied up to 0.16.0, and is still not shown to +be.** What changed in 0.17.0 is the code; what has not changed is that no +Flatpak has been built here, so nothing below has been observed inside one. -A folder library is chosen by typing an absolute path. `dr-sync-folder`'s -provider declares `SignIn::EndpointOnly` with the placeholder -`/home/you/Pictures`, and `normalise_endpoint` expands `~`, requires the path -to be absolute, and checks it with `std::fs`. Nothing in the tree calls the -FileChooser portal — there is no `ashpd`, no `rfd`, and no toolkit file dialog -anywhere in `ui/`, `platform/` or `core/`. +Up to 0.16.0 a folder library was chosen by typing an absolute path, and +nothing in the tree called the FileChooser portal. Inside a sandbox with no +`--filesystem=`, `$HOME` still resolves to the real home *path* but that +directory holds only the application's own `.var/app/…` tree, so a typed +`~/Pictures` failed the `exists()` check and the launch screen said so — a +truthful message about a situation the user could not fix from inside the +application. -Inside a sandbox with no `--filesystem=`, `$HOME` still resolves to the real -home *path* but that directory holds only the application's own -`.var/app/…` tree. So a typed `~/Pictures` fails the `exists()` check and the -launch screen says `No folder at /home/you/Pictures.` — a truthful message -about a situation the user cannot fix from inside the application. +**Every folder the desktop asks for is now chosen in the platform's +dialogue** (FR-EXP-6): the library folder on the launch screen (`Choose +folder…`), an import's source and second copy, a folder or file of +Lightroom presets, and an album's folder on this device. +`ui/dr-ui/src/folder_dialog.rs` asks through `rfd` with its `xdg-portal` +backend — `org.freedesktop.portal.FileChooser` over D-Bus, which Flatpak +always permits without a `--talk-name` — and the common item dialogue on +Windows. No path is typed anywhere on the desktop any more; Android keeps +its fields (`Pickers.local-paths`), because SAF returns document trees rather +than paths. -Import is blocked one step earlier. `dr_plat::volumes()` finds a camera card by -reading `/proc/self/mountinfo` and the `removable` flag under `/sys`. A +What the portal hands back inside a sandbox is a path under +`/run/user/$UID/doc/` that the document portal has exported, and the chosen +library goes through the same `normalise_endpoint` a typed one did, which +checks it with `std::fs`. That is the route this section used to ask for — +`rfd` drives `ashpd` underneath, the crate it named. Two things differ from +the plan, and both are stated rather than smoothed over: + +- **It is not behind a platform seam.** The plan put the chooser beside + `LocalStorage::grant` in `dr-plat`, the one place a `Path` enters the + application. It is in `ui/`, and the path it returns reaches the folder + connector as a string, as a typed one did. +- **Nothing has confirmed the sandbox half.** Whether the exported path + still resolves after a restart — a library is remembered across launches, + so it has to — and whether the export is writable, so a sidecar can be + written beside a photograph, are the first two things a Flatpak build has + to check. + +Import is still blocked one step earlier. `dr_plat::volumes()` finds a camera +card by reading `/proc/self/mountinfo` and the `removable` flag under `/sys`. A sandboxed process is in its own mount namespace, so the table it reads describes the sandbox; a card mounted at `/run/media/…` on the host is not in it. `volumes()` correctly returns an empty list, which the interface presents -as "no card found" — right for the code, wrong for the user, who is looking at -a card. +as "no card found" — but the import page now offers `Browse…` beside that +message, and the dialogue it opens is the portal's, which can reach the card. ### The permission that would hide this, and why it is not in the manifest @@ -152,41 +176,31 @@ a card. names as the alternative to portals. Granting it would mean the sandboxed build never exercises the sandbox, which removes the entire reason for shipping one (§2). `--filesystem=xdg-pictures` is narrower and would be tempting, but it is -still a static grant that lets a typed path resolve — it makes the same design -work by not testing it, only in a smaller directory. +still a static grant that lets a path resolve without the portal — it makes +the same design work by not testing it, only in a smaller directory. -So the manifest grants no filesystem access at all. The consequence is stated -plainly: **a Flatpak built from this manifest can open photographs handed to it -and cannot yet be pointed at a library.** +So the manifest grants no filesystem access at all, and a Flatpak built from +it reaches the user's photographs only through what the portal hands it. ### What closes it -Two changes, in this order: - -1. **A portal file chooser behind a platform seam.** `ashpd`'s - `OpenFileRequest` with `directory(true)` returns a URI the document portal - has exported, which the sandbox can read and which stays valid across - restarts. It resolves to a real path under `/run/user/$UID/doc/`, so - `normalise_endpoint` accepts it as it stands — `canonicalize()` on a fuse - path returns the path itself. The seam matters more than the crate: this - belongs beside `LocalStorage::grant` in `dr-plat`, which is already the one - place a `Path` enters the application, and must not become a second way for - `ui/` to learn about paths. -2. **Removable volumes through the same door.** There is no portal for "list - the mounted cards". The honest answer is that under a sandbox - `imports_supported()` should report the same `false` it reports on Android, - for the same reason it gives there — the operation cannot be performed - however hard the user tries — and the import flow should offer the folder - chooser instead of a volume list. +1. **Build it and run it.** `flatpak-builder` is not installed on the machine + this is developed on, so the manifest has never produced a package. +2. **Removable volumes.** There is no portal for "list the mounted cards". + Under a sandbox `imports_supported()` should report the same `false` it + reports on Android, for the same reason — the volume list cannot be right + however hard the user tries — and leave `Browse…` as the way to a card. **Done when:** a Flatpak built from [`packaging/flatpak/paris.tourolle.darkroom.yml`](../../packaging/flatpak/paris.tourolle.darkroom.yml), with its `finish-args` unchanged and no `flatpak override` applied, can select a -library root, scan it, and write a sidecar back into it. +library root, scan it, write a sidecar back into it, and open it again after a +restart. ### Running a Flatpak build before then -For testing the rest of the application inside the sandbox, grant the access +If the portal's path turns out not to hold across a restart, the rest of the +application can still be tested inside the sandbox by granting the access per-installation rather than in the manifest, so the file that describes the application keeps telling the truth: diff --git a/docs/dev/outstanding.md b/docs/dev/outstanding.md index 058b063..9a1e408 100644 --- a/docs/dev/outstanding.md +++ b/docs/dev/outstanding.md @@ -275,14 +275,20 @@ Intent read over JNI, and an `ExportProvider` rooted at `getFilesDir()` rather t been exercised on a device** — the tests read the manifest and the Java through `include_str!`, which catches a deleted filter but not a class loader that cannot find the class. -**FR-PLAT-LIN-3 — packaged, not satisfied.** There is a Flatpak manifest now, granting no -filesystem permission of any kind, plus AppStream metainfo and `docs/distribution.md`. The -requirement is still not met, and cannot be met by packaging: a folder library is chosen by typing -an absolute path, nothing in the tree calls the FileChooser portal, and inside the sandbox `$HOME` -holds only `.var/app/...`. `dr_plat::volumes()` reads `/proc/self/mountinfo`, so a card mounted on -the host is invisible to a sandboxed process as well. The fix is an `ashpd` directory picker beside -`LocalStorage::grant`, not a change to the manifest. No Flatpak has been built here — -`flatpak-builder` is not installed — so the permission set is reasoned, not observed. +**FR-PLAT-LIN-3 — packaged and wired, not yet proved.** There is a Flatpak manifest, granting no +filesystem permission of any kind, plus AppStream metainfo and `docs/distribution.md`. Until 0.17.0 +the requirement could not be met by packaging: a folder library was chosen by typing an absolute +path, nothing called the FileChooser portal, and inside the sandbox `$HOME` holds only +`.var/app/...`. Since 0.17.0 every folder the desktop asks for — the library, an import's source +and second copy, Lightroom presets, an album's folder — is chosen in the platform's dialogue +(`ui/dr-ui/src/folder_dialog.rs`, `rfd` over the XDG portal), which is what a sandbox needs. Two +things stop this entry being struck. No Flatpak has been built here — `flatpak-builder` is not +installed — so the portal path has never been seen to open a library, reopen it after a restart, +or take a sidecar inside the sandbox; [distribution.md §4](distribution.md) says what has to be +checked. And +`dr_plat::volumes()` still reads `/proc/self/mountinfo`, so a card mounted on the host is invisible +to a sandboxed process; the import page's `Browse…` reaches one through the portal instead. The +chooser landed in `ui/` rather than behind the `dr-plat` seam distribution.md had proposed. **NFR-COMPAT-2 — distribution channels. Stated, which is all this requirement asks.** The paragraph above cites [distribution.md](distribution.md) and it is the same document that answers this: §1 diff --git a/docs/dev/requirements.md b/docs/dev/requirements.md index 98cde24..10ac849 100644 --- a/docs/dev/requirements.md +++ b/docs/dev/requirements.md @@ -1187,6 +1187,12 @@ protocol is unavailable, FR-DSP-8's stated fallback applies. portals and credential storage uses the Secret Service portal, both verified to satisfy FR-NC-2 and FR-CAT-1 within the sandbox. +*Status (2026-09-26).* Not verified, because no Flatpak has been built. Since 0.17.0 every folder the +desktop asks for is chosen through the FileChooser portal (FR-EXP-6), so the filesystem half has the +code it needs; whether the path the portal returns opens, persists and takes a sidecar inside the +sandbox is unobserved ([distribution.md §4](distribution.md)). Credentials reach the session's secret +daemon through a talk hole rather than the Secret Service portal, for the reason the manifest gives. + #### Windows Specified in [windows.md](windows.md); a stated channel under NFR-COMPAT-2, not a v1 one. diff --git a/packaging/flatpak/paris.tourolle.darkroom.yml b/packaging/flatpak/paris.tourolle.darkroom.yml index 4df2803..1c9455b 100644 --- a/packaging/flatpak/paris.tourolle.darkroom.yml +++ b/packaging/flatpak/paris.tourolle.darkroom.yml @@ -90,17 +90,21 @@ finish-args: # path in argv and opens. That path is genuinely portal-mediated and needs no # code change. # - # What that leaves broken: choosing a *library root*. The folder connector - # takes a typed absolute path (`SignIn::EndpointOnly`, placeholder - # `/home/you/Pictures`) and checks it with `std::fs`, and nothing in the tree - # calls the FileChooser portal — there is no ashpd, no rfd, no toolkit dialog. - # A path typed into that field does not exist in this sandbox, so the launch - # screen refuses it with "that folder does not exist", which is at least an - # honest error. + # What that leaves to the portal: choosing a *library root*, an import's + # source and every other folder the desktop asks for. Since 0.17.0 they are + # chosen in `ui/dr-ui/src/folder_dialog.rs`, which asks the FileChooser + # portal through rfd (no talk hole needed: portals are always reachable), + # and a folder chosen there arrives as a document-portal path under + # /run/user/$UID/doc. That is the design; no Flatpak has been built from + # this manifest yet, so it has not been seen to work in the sandbox. + # Removable cards are still invisible to `dr_plat::volumes()`, which reads + # the sandbox's own mount table; the import page's Browse… reaches one + # through the same portal. # - # `--filesystem=host` would make that work today and is exactly what the - # requirement forbids, so it is not here. docs/dev/distribution.md §4 records what - # closes the gap and how to run a Flatpak build in the meantime. + # `--filesystem=host` would make all of it work without the portal and is + # exactly what the requirement forbids, so it is not here. + # docs/dev/distribution.md §4 records what is left to prove and how to run a + # Flatpak build in the meantime. modules: - name: darkroom