Skip to content

Commit 1e0a8af

Browse files
mosandlconcreativmosandlt
authored andcommitted
fix(cmd,csync): anchor --exclude patterns at the sync root regardless of filename
Summary nextcloudcmd --exclude <path> silently produced zero exclusions for every pattern in the file, unless <path> happened to be literally named "sync-exclude.lst" -- with no warning or error anywhere. Any other filename (a typo'd extension, a descriptive name, a per-tool name like myapp-exclude.lst) caused every single pattern to be matched against the wrong base directory, so none of them ever excluded anything under the sync root. Root cause ExcludedFiles::addExcludeFilePath() in src/csync/csync_exclude.cpp anchors patterns either at the sync root (_localPath) or at the exclude file's own containing directory, based on a filename check: literally "sync-exclude.lst" -> sync root, anything else -> own directory. That heuristic was introduced in 5788f35 (2020-12-09, "Skip sync exclude file from list of exclude files if it doesn't exist") to merge two previously-separate code paths: the GUI's global exclude list (always named sync-exclude.lst, lives outside any sync folder, needs sync-root anchoring) and a per-directory .sync-exclude.lst discovered during traversal (needs anchoring at its own directory). It was a side effect of that merge, not a deliberate choice -- the commit's own description and linked issue are unrelated to anchoring behavior. src/cmd/cmd.cpp passes the CLI's --exclude value into the same function unconditionally, so it inherited this GUI-specific heuristic even though CLI users can name their exclude file anything. Reported independently by multiple users: #2916, #7682, and a comment diagnosing the exact same filename dependency on #2916. Fix Add an explicit anchorToLocalPath parameter to addExcludeFilePath() (default false, preserving current behavior everywhere else) and set it for the CLI's --exclude option in cmd.cpp, so its patterns are always anchored at the sync root regardless of the file's name. Test plan - [x] New unit test: testAddExcludeFilePath_anchorToLocalPath_bypassesFilenameHeuristic - [x] Full ExcludedFilesTest suite: 25 passed, 0 failed - [x] Manual repro against a live Nextcloud account: an --exclude file not named sync-exclude.lst previously had zero effect (excluded directories were downloaded anyway); after this fix they are correctly skipped (FileIgnored) with no behavior change for GUI-managed exclude lists. Signed-off-by: Thomas Mosandl <mosandl@concreativ.de> Assisted-by: ClaudeCode:claude-sonnet-5
1 parent 25dbb90 commit 1e0a8af

4 files changed

Lines changed: 52 additions & 5 deletions

File tree

src/cmd/cmd.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,15 @@ int main(int argc, char **argv)
547547

548548
// Always try to load the user-provided exclude list if one is specified
549549
if (hasUserExcludeFile) {
550-
engine.excludedFiles().addExcludeFilePath(options.exclude);
550+
// Anchor patterns at the sync root regardless of the file's own name.
551+
// Without this, addExcludeFilePath() only anchors at _localPath when the
552+
// file is literally named "sync-exclude.lst" (a heuristic that exists to
553+
// distinguish the GUI's global exclude list from a per-directory
554+
// .sync-exclude.lst); any other --exclude filename silently anchored
555+
// patterns at the exclude file's own directory instead, so none of them
556+
// ever matched anything under the sync root. See nextcloud/desktop#2916,
557+
// #7682.
558+
engine.excludedFiles().addExcludeFilePath(options.exclude, /*anchorToLocalPath=*/true);
551559
}
552560
// Load the system list if available, or if there's no user-provided list
553561
if (!hasUserExcludeFile || QFile::exists(systemExcludeFile)) {

src/csync/csync_exclude.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,11 @@ ExcludedFiles::ExcludedFiles(const QString &localPath)
242242

243243
ExcludedFiles::~ExcludedFiles() = default;
244244

245-
void ExcludedFiles::addExcludeFilePath(const QString &path)
245+
void ExcludedFiles::addExcludeFilePath(const QString &path, bool anchorToLocalPath)
246246
{
247247
const QFileInfo excludeFileInfo(path);
248248
const auto fileName = excludeFileInfo.fileName();
249-
const auto basePath = fileName.compare(QStringLiteral("sync-exclude.lst"), Qt::CaseInsensitive) == 0
249+
const auto basePath = anchorToLocalPath || fileName.compare(QStringLiteral("sync-exclude.lst"), Qt::CaseInsensitive) == 0
250250
? _localPath
251251
: leftIncludeLast(path, QLatin1Char('/'));
252252
auto &excludeFilesLocalPath = _excludeFiles[basePath];

src/csync/csync_exclude.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,20 @@ class OCSYNC_EXPORT ExcludedFiles : public QObject
6969
* Adds a new path to a file containing exclude patterns.
7070
*
7171
* Does not load the file. Use reloadExcludeFiles() afterwards.
72+
*
73+
* By default the patterns are anchored at the directory the exclude
74+
* file itself lives in, unless that file is literally named
75+
* "sync-exclude.lst" (case-insensitive), in which case they're
76+
* anchored at the sync root (_localPath). That heuristic exists to
77+
* distinguish the GUI's global exclude list (always named
78+
* sync-exclude.lst, but stored outside any sync folder) from a
79+
* per-directory .sync-exclude.lst discovered during traversal.
80+
*
81+
* It does not fit a caller-supplied exclude file of arbitrary name,
82+
* such as nextcloudcmd's --exclude option: pass anchorToLocalPath=true
83+
* there to force sync-root anchoring regardless of the file's name.
7284
*/
73-
void addExcludeFilePath(const QString &path);
85+
void addExcludeFilePath(const QString &path, bool anchorToLocalPath = false);
7486

7587
/**
7688
* Whether conflict files shall be excluded.

test/testexcludedfiles.cpp

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,34 @@ private slots:
759759
QCOMPARE(excludedFiles->_excludeFiles[folder1].first(), folder1ExcludeList);
760760
QCOMPARE(excludedFiles->_excludeFiles[folder2].first(), folder2ExcludeList);
761761
}
762-
762+
763+
// A caller-supplied exclude file not literally named "sync-exclude.lst" -
764+
// e.g. nextcloudcmd's --exclude <path> - must still anchor its patterns at
765+
// the sync root when anchorToLocalPath is requested (nextcloud/desktop#2916,
766+
// #7682: without this, --exclude with a custom filename silently anchored
767+
// patterns at the exclude file's own directory instead, so none of them
768+
// ever matched anything under the sync root).
769+
void testAddExcludeFilePath_anchorToLocalPath_bypassesFilenameHeuristic()
770+
{
771+
const QString basePath("syncFolder/");
772+
excludedFiles.reset(new ExcludedFiles(basePath));
773+
774+
const QString customExcludeList("home/thomas/.config/my-custom-exclude.lst");
775+
776+
// Default (as used for per-directory .sync-exclude.lst discovery):
777+
// anchored at the exclude file's own directory, since its name isn't
778+
// literally "sync-exclude.lst".
779+
excludedFiles->addExcludeFilePath(customExcludeList);
780+
QCOMPARE(excludedFiles->_excludeFiles.contains(basePath), false);
781+
QCOMPARE(excludedFiles->_excludeFiles[QString("home/thomas/.config/")].first(), customExcludeList);
782+
783+
// anchorToLocalPath=true (as used for nextcloudcmd's --exclude option):
784+
// anchored at the sync root regardless of the file's name.
785+
excludedFiles.reset(new ExcludedFiles(basePath));
786+
excludedFiles->addExcludeFilePath(customExcludeList, /*anchorToLocalPath=*/true);
787+
QCOMPARE(excludedFiles->_excludeFiles[basePath].first(), customExcludeList);
788+
}
789+
763790
void testReloadExcludeFiles_fileDoesNotExist_returnTrue() {
764791
excludedFiles.reset(new ExcludedFiles());
765792
const QString nonExistingFile("directory/.sync-exclude.lst");

0 commit comments

Comments
 (0)