Fix crash on first run when presets are installed read-only - #15764
Open
nevesenin wants to merge 1 commit into
Open
Fix crash on first run when presets are installed read-only#15764nevesenin wants to merge 1 commit into
nevesenin wants to merge 1 commit into
Conversation
populate_local_bundle() materialises a user-writable copy of the bundled vendor presets with fs::copy(recursive | overwrite_existing). fs::copy creates each destination directory with mkdir(dest, source_mode), so the copy inherits the permissions of the installed resources rather than the permissions a writable working copy needs. When the resources are installed without the owner-write bit -- as on Nix and Guix, where every store entry is r-xr-xr-x / r--r--r-- -- the destination directory is created 0555 and the next copy_file into it throws EACCES. Nothing catches it, so the application terminates on first run. The parent directory survives only because create_directories() applies the umask. Note that a permission fixup after fs::copy does not help: the throw happens inside fs::copy itself. Copy the tree by hand instead, so directories go through create_directories() (umask -> 0755) and files get owner-write added explicitly (-> 0644). Do the same for the .idx file, which would otherwise be copied 0444 and could not be rewritten later. On installations that already ship 0755/0644 resources this changes nothing observable; it only stops the copy from depending on the source permissions.
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
1 participant
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.
What happens
On a fresh datadir the application aborts during first run, before the main
window appears. The last line logged is
Populate vendor <repo>/<vendor>frompopulate_local_bundle(), immediately before the copy.The abort is an uncaught
boost::filesystemexception —std::terminate, not ahandled error. Narrowing it down inside that function:
fs::copy()withrecursivecreates each destination directory withmkdir(dest, source_mode),so the copy inherits the permissions of the installed resources rather than
the permissions a writable working copy needs. Where the resources carry no
owner-write bit, the destination directory is created without one too, and the
next
copy_file()into it throwsEACCES. The parent survives only becausecreate_directories()applies the umask.Note that adding a permission fixup after
fs::copy()does not help: the throwhappens inside
fs::copy()itself.Why it does not reproduce on most installs
The trigger is the source's mode bits, not whether the install is read-only.
Distribution packages under
/usr/share, and Flatpak, Snap and AppImage, allship 0755 directories — their read-only-ness is a mount property, which
fs::copynever consults — somkdir(0755)leaves owner-write and the copysucceeds. Nix and Guix are the exception: every store entry is
r-xr-xr-x/r--r--r--, because stripping the write bit is how those systems enforce storeimmutability. That is where this fires.
The fix
Copy the tree by hand instead of with
fs::copy(), so the destinationpermissions are chosen rather than inherited:
create_directories()fordirectories (umask → 0755),
copy_file()plus an explicit owner-write for files(→ 0644). Same for the
.idx, which would otherwise be copied 0444 and couldnot be rewritten later.
18 added lines in one file. On installs that already ship 0755/0644 resources
this changes nothing observable — it only stops the copy from depending on the
source permissions.
Testing
Built from this branch on NixOS — clang 21.1.8, wxWidgets 3.3.3.1 (GTK3),
system dependencies rather than the
deps/superbuild.starts, and the copied presets are writable.
When retesting, delete the datadir between runs.
populate_local_bundle()isguarded by
if (!fs::exists(dest_path / "vendor.yaml")), and a failed runleaves a partial tree, so a subsequent run skips the copy entirely and a broken
build can look fixed (or a fixed build can look untested).
Linux only. The change is platform independent, but I have not built Windows or
macOS.