SharedDir: recursive vs explicit intent split + cold-discover + UI polish (#391) - #606
Merged
mrjimenez merged 6 commits intoMay 14, 2026
Merged
Conversation
The watcher's existing RegisterNewSubdirectory path catches new subdirectories of a shared root when they're created at runtime (HOT case) -- a wxFSW_EVENT_CREATE fires, the handler adds the path to shareddir_list, persists shareddir.dat, and starts watching the new subdir. That's the "auto-share new subdirs of watched parents" behaviour the watcher PR shipped. The COLD case had no equivalent: subdirectories that grew on disk while aMule was offline never fire a CREATE (they exist before the watcher attaches), so a /Music share whose disk gained three new albums in the interim would never see them until each album happened to gain a new file via some other path. That's the follow-up reported on amule-project#391. Add ColdDiscoverSubdirs(), called once from Enable() right after the initial RegisterAllPaths(). For each path in shareddir_list it recursively walks the subtree (CDirIterator::Dir) and collects subdirectories not yet in shareddir_list, then in a single batch: appends them to shareddir_list, calls m_watcher->Add() on each (Linux/BSD/Windows -- macOS's enclosing FSEvents stream already covers descendants), persists shareddir.dat once, and ScheduleReload()s so the in-memory shared-file list picks up files under the new subdirs without blocking Enable(). Deliberately NOT called from Refresh(): on macOS the watcher writes the symlink-resolved form ("/private/tmp/...") while CDirIterator over a /tmp-rooted shareddir.dat entry produces the unresolved form ("/tmp/..."), so a second walk after runtime RegisterNewSubdirectory wrote one form would add the other form again. shareddir.dat ended up with duplicate entries for the same subdir. Cold-discovery is genuinely a startup concern -- the HOT path handles every runtime case after Enable. Membership index is a std::set<wxString> over raw path strings, matching the comparison shape the rest of the file uses. The walker recurses through known subdirs as well as unknown ones, so a tree whose top layer is in shareddir.dat but whose deeper layers aren't still gets fully covered in a single pass. Verified on macOS: shareddir.dat with just "/tmp/cold-test-root/" plus two pre-existing album subdirs on disk -> on start, both albums get appended to shareddir.dat and the share scan picks up their files after the 5-second debounce. shareddir.dat after run contains the root plus both subdirs, no duplicates.
got3nks
marked this pull request as draft
May 14, 2026 17:39
The previous commit's cold-discovery (and the existing amule-project#591 watcher RegisterNewSubdirectory) auto-added new subdirectories to every shared dir indiscriminately. There was no way to express "share /Music but NOT its sensitive nested subfolders" -- the moment a new subdir appeared under any shared root it became part of the share. Desktop users with sensitive nested folders inside a shared root were exposed. This commit introduces a per-root recursive flag, persisted in a new shareddir-recursive.dat file. The watcher and cold-discovery now gate their auto-add behaviour on "ancestor is recursive": only roots the user has explicitly marked recursive via the UI's right- click "share subdirectories recursively" automatically include new/existing subdirectories. Non-recursive shares stay strict. Three on-disk files going forward: * shareddir-recursive.dat (new): roots the user marked recursive. * shareddir-explicit.dat (new): roots the user added explicitly, without recursion. * shareddir.dat (existing): regenerated as the runtime union for backwards compatibility -- older binaries and external scripts that read or write this file see the same effective list as before. `CPreferences::shareddir_list` (the in-memory list the share scan, watcher, etc. read as authoritative) is recomputed at every ReloadSharedFolders as `shareddir_explicit_list ∪ expand(shareddir_ recursive_list)`. The expansion walks each recursive root's subtree (CDirIterator::Dir) and adds every existing descendant directory. Migration: a pre-existing shareddir.dat with no shareddir-{explicit,recursive}.dat is loaded entirely into the explicit list. Existing users keep their path set but stop silently auto-recursing -- they opt back in per-root via the UI. Safe default for desktop users. Reconciliation runs on every Reload (startup, EC reload, UI reload, watcher-debounced reload), not just startup. Diffs the on-disk shareddir.dat against the expected union; entries written externally (Docker entrypoints, sysadmin edits, old-binary writes) are imported into shareddir_explicit_list, and entries the external writer removed are dropped from explicit (entries that came from the recursive expansion are left alone -- the user's recursive intent overrides single-entry external edits). UI side: PrefsUnifiedDlg::CommitSharedDirsWithProgress already extracted explicit vs recursive intents from the DirectoryTreeCtrl (GetSharedDirectories vs GetRecursiveSharedDirectories). The commit path now assigns each to its dedicated Preferences list instead of conflating them into shareddir_list, and the cancel rollback restores all three. Initial dialog state loads from the two intent lists rather than the runtime union (which would otherwise render auto-discovered subdirs as user-selected items). Watcher side: CSharedDirWatcher::RegisterNewSubdirectory and ::ColdDiscoverSubdirs both check CPreferences::IsRecursiveAncestor() before adding. Explicit shares no longer accumulate subdirs at runtime; recursive shares behave as before. Verified on macOS with a three-mode smoke test: * `recursive-root` (in shareddir-recursive.dat) with pre-existing subA/subB: both auto-expand into shareddir.dat at startup; recursive-root/rec-hot created at runtime is auto-added. * `explicit-root` (in shareddir-explicit.dat) with pre-existing subC: subC NOT in shareddir.dat; explicit-root/exp-hot created at runtime NOT added. * `script-added-dir` written to shareddir.dat externally (Docker-entrypoint pattern): imported into shareddir-explicit.dat on next boot, survives subsequent shareddir.dat regenerations. No file format change to shareddir.dat (still one path per line); the new files use the same format. Older binaries that ignore the new files continue to operate against shareddir.dat as the union, with the caveat that recursive intent is lost when an older binary round-trips shareddir.dat (the markers in shareddir-recursive.dat survive untouched because the old binary doesn't read or write that file).
SetSharedDirectories already calls UpdateSharedDirectories when the tree is initialised; SetRecursiveSharedDirectories didn't, so a prefs-open that loaded both lists in sequence only refreshed for the explicit set. Recursive roots loaded from shareddir-recursive.dat appeared un-bold on first paint until the user expanded the branch (at which point AddChildItem's IsRecursiveShare/IsInsideRecursiveShare check kicked in). Mirror the symmetry.
CDirectoryTreeCtrl::OnRButtonDown calls MarkChildren to keep the already-rendered subtree visually consistent with the recursive marker, and MarkChildren's CheckChanged side-effect pushes each walked descendant into m_lstShared. The result is that when GetSharedDirectories captures m_lstShared at commit time, it contains paths that the user never explicitly marked as a non-recursive share -- they're the recursive expansion that happens to share storage with the UI's "what's bold" map. Within a session the duplication is benign: the apply task's expansion deduplicates against the explicit list, and a later UI un-share of the recursive root triggers DelSharesUnder which cleans the descendants out of m_lstShared. But across sessions, if the user removes the recursive marker externally (script editing shareddir-recursive.dat) the duplicated descendants stay in shareddir-explicit.dat as orphan pinned paths -- DelSharesUnder only runs from the UI handler, not from ReloadSharedFolders. Filter explicitShares at the commit boundary: drop entries whose ancestor is in recursiveIntents. Same prefix-with-separator- boundary rule we use in CPreferences::IsRecursiveAncestor. shareddir-explicit.dat ends up containing only paths the user genuinely meant as non-recursive; the recursive expansion is strictly the responsibility of shareddir-recursive.dat.
When the user double-clicks a recursive root to un-share it, CheckChanged unbolds the root and DelRecursiveShare drops the marker. But the tree's already-rendered descendants were bolded lazily by AddChildItem checking IsInsideRecursiveShare, and that state isn't re-evaluated when the marker disappears -- the descendants stay painted bold. Result: a "ghost selection" of greyed-out-but-bold subdirs while shareddir-*.dat correctly contains nothing. CheckChanged already propagates upward via UpdateParentItems; add the symmetric downward walk via MarkChildren when we detect "item was a recursive root, now isn't, and this is the outer CheckChanged call (not a recursion from MarkChildren itself)". The MarkChildren walk also resets the SetHasSharedSubdirectory icon on the un-recursived root via its existing SetHasSharedSubdirectory(hChild, mark) call. Reported by user testing PR amule-project#606's UI flow: mark /Downloads recursive -> apply -> reopen prefs -> double-click /Downloads -> /Downloads correctly unbolds but /Downloads/Telegram Desktop and similar already-rendered descendants stayed bold + red-iconed.
Three coordinated UX changes for the recursive/explicit split: 1. Bold-italic marker on recursive roots With the recursive marker now persisted across sessions (shareddir-recursive.dat), three states paint identically as plain bold otherwise: explicit non-recursive share, recursive root, and descendant of a recursive root. The user has to right-click to learn which is which. Render recursive roots in bold-italic instead, so the actual marker pops visually. New helper GetRecursiveFont() (lazy because the tree's GetFont() isn't fully resolved until after the ctor on some backends, notably macOS), plus ApplyRecursiveMark() called from AddChildItem on initial paint and from OnRButtonDown's two branches when the user toggles the marker in-session. 2. HasSharedSubdirectory considers m_lstSharedRecursive Previously the "has shared subdirs" icon only checked m_lstShared for descendants of the candidate path. After PrefsUnifiedDlg::CommitSharedDirsWithProgress's filter strips recursive-descendants out of shareddir-explicit.dat, m_lstShared is empty under a recursive root, so the icon never appeared on roots like /Music whose subdirs live exclusively in the recursive expansion. Two new branches: the path itself is at or below a recursive root (its expansion contains its subdirs by construction), and a recursive root sits below the path (some descendant is shared via that root). 3. Click guard for recursive descendants A left-click on a descendant of a recursive root would unbold the item visually, but the apply task re-flattens the root's subtree at commit time -- so the entry reappears after Apply and the user sees a no-op. A right-click on the same item is meaningless (redundant if marking, no-op if un-marking). Both paths now early-return with a wxMessageBox explaining that the user has to modify the recursive root instead. Reported by user testing PR amule-project#606's UI: marking a recursive root and re-opening prefs showed all three states as plain bold (no way to tell roots from descendants from plain explicit shares), and a follow-up double-click on a descendant appeared to do nothing.
got3nks
marked this pull request as ready for review
May 14, 2026 19:05
mrjimenez
pushed a commit
that referenced
this pull request
May 14, 2026
When the user double-clicks a recursive root to un-share it, CheckChanged unbolds the root and DelRecursiveShare drops the marker. But the tree's already-rendered descendants were bolded lazily by AddChildItem checking IsInsideRecursiveShare, and that state isn't re-evaluated when the marker disappears -- the descendants stay painted bold. Result: a "ghost selection" of greyed-out-but-bold subdirs while shareddir-*.dat correctly contains nothing. CheckChanged already propagates upward via UpdateParentItems; add the symmetric downward walk via MarkChildren when we detect "item was a recursive root, now isn't, and this is the outer CheckChanged call (not a recursion from MarkChildren itself)". The MarkChildren walk also resets the SetHasSharedSubdirectory icon on the un-recursived root via its existing SetHasSharedSubdirectory(hChild, mark) call. Reported by user testing PR #606's UI flow: mark /Downloads recursive -> apply -> reopen prefs -> double-click /Downloads -> /Downloads correctly unbolds but /Downloads/Telegram Desktop and similar already-rendered descendants stayed bold + red-iconed.
9 tasks
3 tasks
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 26, 2026
Translated using Weblate (Spanish) Currently translated at 100.0% (247 of 247 strings) Translation: aMule/Application Man Pages Translate-URL: https://hosted.weblate.org/projects/amule/application-man-pages/es/ Translated using Weblate Translated using Weblate (Spanish) Currently translated at 100.0% (247 of 247 strings) Translation: aMule/Application Man Pages Translate-URL: https://hosted.weblate.org/projects/amule/application-man-pages/es/ Translated using Weblate Translated using Weblate (Spanish) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/es/ Translated using Weblate Translated using Weblate (Spanish) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/es/ Translated using Weblate Translated using Weblate (Spanish) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/es/ Translated using Weblate Translated using Weblate (Spanish) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/es/ Translated using Weblate Translated using Weblate (Chinese (Simplified Han script)) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/zh_Hans/ Translated using Weblate Translated using Weblate (Turkish) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/tr/ Translated using Weblate Translated using Weblate (Portuguese (Brazil)) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/pt_BR/ Translated using Weblate Translated using Weblate (Portuguese (Brazil)) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/pt_BR/ Translated using Weblate Translated using Weblate (Portuguese (Brazil)) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/pt_BR/ Translated using Weblate Translated using Weblate (Italian) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/it/ Translated using Weblate Translated using Weblate (French) Currently translated at 100.0% (1847 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/fr/ Translated using Weblate Translated using Weblate (Czech) Currently translated at 66.6% (1231 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/cs/ Translated using Weblate Translated using Weblate (Czech) Currently translated at 66.6% (1231 of 1847 strings) Translation: aMule/Application Translate-URL: https://hosted.weblate.org/projects/amule/amule-application/cs/ Co-authored-by: Got3nks <[email protected]> Co-authored-by: Guzleon <[email protected]> Co-authored-by: Hosted Weblate user 54392 <[email protected]> Co-authored-by: John Doe <[email protected]> Co-authored-by: Libre <[email protected]> Co-authored-by: Marcelo Roberto Jimenez <[email protected]> Co-authored-by: Rodrigo Belo <[email protected]> Co-authored-by: danim7 <[email protected]> Co-authored-by: mslr8 <[email protected]> Co-authored-by: nathanael <[email protected]>
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
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.
References #391.
aMule's
shareddir.dathad no way to express recursive intent: the existing watcher (#591) and the previous commit's cold-discovery would auto-add any new subdirectory under any shared root, regardless of whether the user wanted recursion. Desktop users with sensitive nested folders inside a shared root were exposed. New subdirs created while aMule was offline were also missed entirely.This PR introduces a per-root recursive flag with full UI support and gates all auto-add behaviour on it.
Storage layout (three files)
shareddir-recursive.dat(new)shareddir-explicit.dat(new)shareddir.dat(existing)In-memory
CPreferences::shareddir_listis recomputed on everyReloadSharedFoldersasshareddir_explicit_list ∪ expand(shareddir_recursive_list). Both watcher auto-add paths (RegisterNewSubdirectoryandColdDiscoverSubdirs) gate onIsRecursiveAncestor()— non-recursive shares no longer silently accumulate subdirs.What this PR does
Cold-discover subdirs created while offline —
CSharedDirWatcher::ColdDiscoverSubdirs()recursively walks each recursive root's subtree at watcher startup, batch-adds previously-uncovered subdirs toshareddir_list, and triggers a debouncedReload. Symmetric with the HOT path'sRegisterNewSubdirectory.Recursive/explicit data model — two new files for canonical intent, regenerated
shareddir.datfor compat.ReloadSharedFoldersreconciles external writes on every load.Watcher gating — both auto-add paths check
CPreferences::IsRecursiveAncestor(path)before touchingshareddir_list. Non-recursive shares stay strict; recursive shares pick up new subdirs as before.UI integration —
PrefsUnifiedDlgloads the two intent lists intoCDirectoryTreeCtrlseparately, extracts them back on commit, filters descendants of recursive roots out ofshareddir-explicit.dat(no orphan pinning), and rolls back all three lists on cancel.UI polish — recursive roots render in bold-italic to distinguish them from plain explicit shares and inherited descendants.
HasSharedSubdirectoryhonors the recursive map too, so the "has shared subdirs" icon stays on recursive roots even when the explicit list is empty. Left-click on a descendant of a recursive root (which would silently no-op via the apply-time re-expansion) now shows a message box explaining the user needs to modify the root.Un-recursive cleanup — when the user double-clicks a recursive root to un-share, descendant tree items are now walked and unbolded too (previously they stayed visually stale because their bold was set lazily via
IsInsideRecursiveShare, never re-evaluated when the marker disappeared).Migration
A pre-existing
shareddir.datwith no companion files is loaded entirely into the explicit list — existing users keep their path set but stop silently auto-recursing. They opt back in per-root via the UI right-click. Defaults toward conservative behaviour.Reconciliation
Runs on every
ReloadSharedFolders(startup, EC reload, UI reload, watcher debounced reload). Diffs the on-diskshareddir.datagainst the expected union; entries written externally (Docker entrypoints, sysadmin edits, old-binary writes) are imported intoshareddir-explicit.dat, removed entries are dropped from explicit (entries from the recursive expansion are left alone — the user's recursive intent overrides single-entry external edits). Keepsentrypoint.sh shareddir.datworkflows working unchanged.Backwards compatibility
shareddir.datfile format unchanged (one path per line); the new files use the same format. Older binaries that ignore the new files continue to operate againstshareddir.datas the union. Caveat: recursive intent is lost when an older binary round-tripsshareddir.dat(the markers inshareddir-recursive.datsurvive untouched because the old binary doesn't read or write that file). Documented; recoverable by re-marking in the UI on the new binary.Verification (macOS)
End-to-end smoke test:
recursive-rootwith pre-existingsubA/subB: auto-expand on first start;recursive-root/rec-hotcreated at runtime auto-added.explicit-rootwith pre-existingsubC:subCNOT inshareddir.dat;explicit-root/exp-hotcreated at runtime NOT added.script-added-dirwritten toshareddir.datexternally: imported intoshareddir-explicit.daton next boot, survives subsequent regenerations.Built clean on macOS (Apple Clang, wxBase OSX-Cocoa 3.3.2, Boost 1.90) and Ubuntu 26.04 ARM64 (GCC, wxBase GTK3 3.2.9, Boost 1.83).