Skip to content

Commit 01c6791

Browse files
mosandlconcreativmosandlt
authored andcommitted
fix(cmd): add --exclude-anchored, keep --exclude backwards-compatible
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 Added an explicit anchorToLocalPath parameter (now the ExcludedFiles::ExcludeFileAnchor enum) to addExcludeFilePath() in csync_exclude.cpp/.h (default FileDirectory, preserving current behavior everywhere else). Per @mgallien's review, --exclude's own anchoring behavior is left unchanged (still FileDirectory / the historic filename heuristic) so existing setups relying on it, however unintentionally, don't silently change behavior. Instead cmd.cpp gets a new, separate --exclude-anchored [file] option that always anchors at the sync root (ExcludeFileAnchor::SyncRoot) regardless of the file's name -- use it instead of --exclude when patterns aren't matching because the exclude file isn't literally named "sync-exclude.lst". Test plan - [x] New unit test: testAddExcludeFilePath_syncRootAnchor_bypassesFilenameHeuristic (covers both ExcludeFileAnchor values directly on ExcludedFiles) - [x] Full ExcludedFilesTest suite: 25 passed, 0 failed - [x] Manual repro against a live Nextcloud account: an exclude file not named sync-exclude.lst had zero effect via --exclude (unchanged, by design); passed via --exclude-anchored instead, its patterns are correctly applied (FileIgnored) with no behavior change for --exclude or GUI-managed exclude lists. Signed-off-by: Thomas Mosandl <mosandl@concreativ.de> Assisted-by: ClaudeCode:claude-sonnet-5
1 parent fca13e6 commit 01c6791

4 files changed

Lines changed: 76 additions & 8 deletions

File tree

src/cmd/cmd.cpp

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ struct CmdOptions
6767
bool interactive = false;
6868
bool ignoreHiddenFiles = false;
6969
QString exclude;
70+
QString excludeAnchored;
7071
QString unsyncedfolders;
7172
int restartTimes = 0;
7273
int downlimit = 0;
@@ -171,6 +172,10 @@ void help()
171172
std::cout << " Proxy is http://server:port" << std::endl;
172173
std::cout << " --trust Trust the SSL certification." << std::endl;
173174
std::cout << " --exclude [file] Exclude list file" << std::endl;
175+
std::cout << " --exclude-anchored [file] Exclude list file, always anchored at the" << std::endl;
176+
std::cout << " sync root regardless of the file's own name" << std::endl;
177+
std::cout << " (use this if --exclude patterns aren't matching," << std::endl;
178+
std::cout << " see nextcloud/desktop#2916, #7682)" << std::endl;
174179
std::cout << " --unsyncedfolders [file] File containing the list of unsynced remote folders (selective sync)" << std::endl;
175180
std::cout << " --user, -u [name] Use [name] as the login name" << std::endl;
176181
std::cout << " --password, -p [pass] Use [pass] as password" << std::endl;
@@ -248,6 +253,8 @@ void parseOptions(const QStringList &app_args, CmdOptions *options)
248253
options->password = it.next();
249254
} else if (option == "--exclude" && !it.peekNext().startsWith("-")) {
250255
options->exclude = it.next();
256+
} else if (option == "--exclude-anchored" && !it.peekNext().startsWith("-")) {
257+
options->excludeAnchored = it.next();
251258
} else if (option == "--unsyncedfolders" && !it.peekNext().startsWith("-")) {
252259
options->unsyncedfolders = it.next();
253260
} else if (option == "--max-sync-retries" && !it.peekNext().startsWith("-")) {
@@ -542,11 +549,11 @@ int main(int argc, char **argv)
542549

543550
// Exclude lists
544551

545-
bool hasUserExcludeFile = !options.exclude.isEmpty();
552+
bool hasUserExcludeFile = !options.exclude.isEmpty() || !options.excludeAnchored.isEmpty();
546553
QString systemExcludeFile = ConfigFile::excludeFileFromSystem();
547554

548-
// Always try to load the user-provided exclude list if one is specified
549-
if (hasUserExcludeFile) {
555+
// Always try to load the user-provided exclude list(s) if specified
556+
if (!options.exclude.isEmpty()) {
550557
if (!QFile::exists(options.exclude)) {
551558
// A user-supplied --exclude path that can't be found is a
552559
// configuration error, not something to silently ignore:
@@ -556,15 +563,29 @@ int main(int argc, char **argv)
556563
qFatal("Exclude list file supplied via --exclude does not exist: %s", qUtf8Printable(options.exclude));
557564
return EXIT_FAILURE;
558565
}
566+
// Keeps addExcludeFilePath()'s historic filename-based anchoring
567+
// heuristic for --exclude, so existing setups that rely on it
568+
// (however unintentionally) don't change behavior. Use
569+
// --exclude-anchored instead if patterns aren't matching because
570+
// the file isn't literally named "sync-exclude.lst".
559571
engine.excludedFiles().addExcludeFilePath(options.exclude);
560572
}
573+
if (!options.excludeAnchored.isEmpty()) {
574+
if (!QFile::exists(options.excludeAnchored)) {
575+
qFatal("Exclude list file supplied via --exclude-anchored does not exist: %s", qUtf8Printable(options.excludeAnchored));
576+
return EXIT_FAILURE;
577+
}
578+
// Always anchor patterns at the sync root regardless of the file's
579+
// own name, see ExcludedFiles::addExcludeFilePath().
580+
engine.excludedFiles().addExcludeFilePath(options.excludeAnchored, ExcludedFiles::ExcludeFileAnchor::SyncRoot);
581+
}
561582
// Load the system list if available, or if there's no user-provided list
562583
if (!hasUserExcludeFile || QFile::exists(systemExcludeFile)) {
563584
engine.excludedFiles().addExcludeFilePath(systemExcludeFile);
564585
}
565586

566587
if (!engine.excludedFiles().reloadExcludeFiles()) {
567-
qFatal("Cannot load system exclude list or list supplied via --exclude");
588+
qFatal("Cannot load system exclude list or list supplied via --exclude/--exclude-anchored");
568589
return EXIT_FAILURE;
569590
}
570591

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, ExcludeFileAnchor anchor)
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 = anchor == ExcludeFileAnchor::SyncRoot || 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: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,38 @@ class OCSYNC_EXPORT ExcludedFiles : public QObject
6262
public:
6363
using Version = std::tuple<int, int, int>;
6464

65+
/**
66+
* Where a caller-supplied exclude file's patterns are anchored, see
67+
* addExcludeFilePath().
68+
*/
69+
enum class ExcludeFileAnchor {
70+
FileDirectory, // anchor at the exclude file's own containing directory
71+
SyncRoot // anchor at the sync root (_localPath), regardless of the file's name
72+
};
73+
6574
explicit ExcludedFiles(const QString &localPath = QStringLiteral("/"));
6675
~ExcludedFiles() override;
6776

6877
/**
6978
* Adds a new path to a file containing exclude patterns.
7079
*
7180
* Does not load the file. Use reloadExcludeFiles() afterwards.
81+
*
82+
* By default (ExcludeFileAnchor::FileDirectory) the patterns are
83+
* anchored at the directory the exclude file itself lives in, unless
84+
* that file is literally named "sync-exclude.lst" (case-insensitive),
85+
* in which case they're anchored at the sync root (_localPath)
86+
* regardless. That heuristic exists to distinguish the GUI's global
87+
* exclude list (always named sync-exclude.lst, but stored outside any
88+
* sync folder) from a per-directory .sync-exclude.lst discovered
89+
* during traversal.
90+
*
91+
* It does not fit a caller-supplied exclude file of arbitrary name,
92+
* such as nextcloudcmd's --exclude option: pass
93+
* ExcludeFileAnchor::SyncRoot there to force sync-root anchoring
94+
* regardless of the file's name.
7295
*/
73-
void addExcludeFilePath(const QString &path);
96+
void addExcludeFilePath(const QString &path, ExcludeFileAnchor anchor = ExcludeFileAnchor::FileDirectory);
7497

7598
/**
7699
* Whether conflict files shall be excluded.

test/testexcludedfiles.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,31 @@ 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+
// must still anchor its patterns at the sync root when
765+
// ExcludeFileAnchor::SyncRoot is requested.
766+
void testAddExcludeFilePath_syncRootAnchor_bypassesFilenameHeuristic()
767+
{
768+
const QString basePath("syncFolder/");
769+
excludedFiles.reset(new ExcludedFiles(basePath));
770+
771+
const QString customExcludeList("home/thomas/.config/my-custom-exclude.lst");
772+
773+
// Default (as used for per-directory .sync-exclude.lst discovery):
774+
// anchored at the exclude file's own directory, since its name isn't
775+
// literally "sync-exclude.lst".
776+
excludedFiles->addExcludeFilePath(customExcludeList);
777+
QCOMPARE(excludedFiles->_excludeFiles.contains(basePath), false);
778+
QCOMPARE(excludedFiles->_excludeFiles[QString("home/thomas/.config/")].first(), customExcludeList);
779+
780+
// ExcludeFileAnchor::SyncRoot (as used for nextcloudcmd's --exclude
781+
// option): anchored at the sync root regardless of the file's name.
782+
excludedFiles.reset(new ExcludedFiles(basePath));
783+
excludedFiles->addExcludeFilePath(customExcludeList, ExcludedFiles::ExcludeFileAnchor::SyncRoot);
784+
QCOMPARE(excludedFiles->_excludeFiles[basePath].first(), customExcludeList);
785+
}
786+
763787
void testReloadExcludeFiles_fileDoesNotExist_returnTrue() {
764788
excludedFiles.reset(new ExcludedFiles());
765789
const QString nonExistingFile("directory/.sync-exclude.lst");

0 commit comments

Comments
 (0)