Skip to content

SharedDir: recursive vs explicit intent split + cold-discover + UI polish (#391) - #606

Merged
mrjimenez merged 6 commits into
amule-project:masterfrom
got3nks:feat/shareddir-recursive-marker
May 14, 2026
Merged

SharedDir: recursive vs explicit intent split + cold-discover + UI polish (#391)#606
mrjimenez merged 6 commits into
amule-project:masterfrom
got3nks:feat/shareddir-recursive-marker

Conversation

@got3nks

@got3nks got3nks commented May 14, 2026

Copy link
Copy Markdown
Contributor

References #391.

aMule's shareddir.dat had 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)

File Role
shareddir-recursive.dat (new) Roots the user marked recursive via UI right-click
shareddir-explicit.dat (new) Roots the user added explicitly without recursion
shareddir.dat (existing) Regenerated as the runtime union for backwards compat with older binaries and external scripts

In-memory CPreferences::shareddir_list is recomputed on every ReloadSharedFolders as shareddir_explicit_list ∪ expand(shareddir_recursive_list). Both watcher auto-add paths (RegisterNewSubdirectory and ColdDiscoverSubdirs) gate on IsRecursiveAncestor() — non-recursive shares no longer silently accumulate subdirs.

What this PR does

  1. Cold-discover subdirs created while offlineCSharedDirWatcher::ColdDiscoverSubdirs() recursively walks each recursive root's subtree at watcher startup, batch-adds previously-uncovered subdirs to shareddir_list, and triggers a debounced Reload. Symmetric with the HOT path's RegisterNewSubdirectory.

  2. Recursive/explicit data model — two new files for canonical intent, regenerated shareddir.dat for compat. ReloadSharedFolders reconciles external writes on every load.

  3. Watcher gating — both auto-add paths check CPreferences::IsRecursiveAncestor(path) before touching shareddir_list. Non-recursive shares stay strict; recursive shares pick up new subdirs as before.

  4. UI integrationPrefsUnifiedDlg loads the two intent lists into CDirectoryTreeCtrl separately, extracts them back on commit, filters descendants of recursive roots out of shareddir-explicit.dat (no orphan pinning), and rolls back all three lists on cancel.

  5. UI polish — recursive roots render in bold-italic to distinguish them from plain explicit shares and inherited descendants. HasSharedSubdirectory honors 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.

  6. 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.dat with 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-disk shareddir.dat against the expected union; entries written externally (Docker entrypoints, sysadmin edits, old-binary writes) are imported into shareddir-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). Keeps entrypoint.sh shareddir.dat workflows working unchanged.

Backwards compatibility

shareddir.dat file format unchanged (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. Caveat: 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). Documented; recoverable by re-marking in the UI on the new binary.

Verification (macOS)

End-to-end smoke test:

  • recursive-root with pre-existing subA/subB: auto-expand on first start; recursive-root/rec-hot created at runtime auto-added.
  • explicit-root 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: imported into shareddir-explicit.dat on next boot, survives subsequent regenerations.
  • UI: recursive roots render in bold-italic with red sub-shared icon; left-click on a recursive descendant shows the warning dialog and leaves state unchanged.

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).

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
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).
@got3nks got3nks changed the title SharedDirWatcher: cold-discover subdirs created while offline (#391) SharedDir: split recursive/explicit intent + cold-discover subdirs (#391) May 14, 2026
got3nks added 4 commits May 14, 2026 19:21
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 got3nks changed the title SharedDir: split recursive/explicit intent + cold-discover subdirs (#391) SharedDir: recursive vs explicit intent split + cold-discover + UI polish (#391) May 14, 2026
@got3nks
got3nks marked this pull request as ready for review May 14, 2026 19:05
@mrjimenez
mrjimenez merged commit 1c6b723 into amule-project:master May 14, 2026
12 checks passed
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.
@got3nks
got3nks deleted the feat/shareddir-recursive-marker branch May 14, 2026 19:58
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants