aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-19 11:10:51 +0200
committerDanilo M. <danix@danix.xyz>2026-08-19 11:10:51 +0200
commit41d0b94dcd2b4208e5d3e7483c88ce9b664ce565 (patch)
tree61aceb2bd653d4c0df463955cc610942f928f46e /src
parenta36fff5617f16ac1d17c0c52f2112a20c4fa9336 (diff)
downloadqtmaildir-41d0b94dcd2b4208e5d3e7483c88ce9b664ce565.tar.gz
qtmaildir-41d0b94dcd2b4208e5d3e7483c88ce9b664ce565.zip
fix(worker): give a moved message a fresh maildir name
mbsync's manual, under "the more efficient default UID mapping scheme": "it is important that the MUA renames files when moving them between Maildir folders", and "the general expectation is that a completely new filename is generated as if the message was new". qtmaildir is that MUA and did not rename. moveMessages() kept QFileInfo(from).fileName() verbatim, `,U=<n>` included. That infix is mbsync's per-folder IMAP UID, so carrying it across a folder boundary makes it a claim about a folder the file is no longer in; moving a message out and back then reinserts a UID the server has since reassigned. Reported by the user as `Maildir error: duplicate UID 1`, and measured on the real Maildir: four collisions in one folder, eight distinct messages, none lost. freshMaildirName() regenerates the unique part and keeps ONLY the `:2,<flags>` suffix. Keeping the flags is not a contradiction of "as if the message was new": they record seen, flagged and replied, and maildir.synchronize_flags is true, so dropping them would mark every deleted message unread and lose Important on the way to the trash. Two things fell out of the change and both were defects waiting to happen. The already-in-the-destination guard compared full PATHS, which worked only because the name was carried across; with a fresh name it can never be true, so a message already in the destination would be renamed on every move. It compares directories now. And test_mainwindow's folderHasMessageFile() matched on the filename stem, so all fifty-odd assertions using it began reporting "the file is not there" about files that were there. It reads the Message-ID out of each file instead, which is what those assertions always meant. Three mutations fail: the old name carried across, the flags dropped, and the uniqueness counter frozen so two messages moved in one batch collide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
-rw-r--r--src/notmuchworker.cpp77
1 files changed, 73 insertions, 4 deletions
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp
index 2c03226..66b408c 100644
--- a/src/notmuchworker.cpp
+++ b/src/notmuchworker.cpp
@@ -20,9 +20,12 @@
#include <notmuch.h>
+#include <QCoreApplication>
+#include <QDateTime>
#include <QDir>
#include <QDirIterator>
#include <QFileInfo>
+#include <QHostInfo>
#include <QSet>
#include <cstdlib>
@@ -662,6 +665,63 @@ void NotmuchWorker::applyTags(const TagChange &change)
emit tagsApplied(change);
}
+namespace {
+
+/// A fresh Maildir filename for a message being moved between folders,
+/// preserving only its `:2,<flags>` suffix.
+///
+/// mbsync's manual is explicit about why this exists, under "the more
+/// efficient default UID mapping scheme": "it is important that the MUA
+/// renames files when moving them between Maildir folders", and "the general
+/// expectation is that a completely new filename is generated as if the
+/// message was new".
+///
+/// The `,U=<n>` infix mbsync writes is its per-folder IMAP UID. Carrying it
+/// into another folder makes it a claim about a folder the file is no longer
+/// in; moving a message out and back then reinserts a UID the server has
+/// since reassigned, and mbsync refuses the folder with `Maildir error:
+/// duplicate UID`. Measured on real mail, four collisions in one folder from
+/// a single move-and-restore.
+///
+/// The FLAGS are kept, deliberately, and that is not a contradiction of
+/// "as if the message was new". They record seen, flagged and replied, and
+/// `maildir.synchronize_flags` is true, so notmuch reads them back as tags:
+/// dropping them would mark every deleted message unread and lose Important
+/// on the way to the trash. Only the unique part is regenerated.
+QString freshMaildirName(const QString &oldName)
+{
+ // The `:2,` suffix, when there is one. `info` is everything from the
+ // separator on, so an empty-flag `:2,` is preserved as faithfully as
+ // `:2,FS`.
+ QString info;
+ const int sep = oldName.indexOf(QStringLiteral(":2,"));
+ if (sep >= 0)
+ info = oldName.mid(sep);
+
+ // The conventional left-to-right unique part: time, a per-process counter,
+ // the pid, the host. The counter is what makes two messages moved in the
+ // same second distinct, which a timestamp alone does not guarantee.
+ static quint64 counter = 0;
+ const qint64 now = QDateTime::currentSecsSinceEpoch();
+ const QString host = QHostInfo::localHostName().isEmpty()
+ ? QStringLiteral("localhost")
+ : QHostInfo::localHostName();
+
+ return QStringLiteral("%1.M%2P%3Q%4.%5%6")
+ .arg(now)
+ .arg(QDateTime::currentMSecsSinceEpoch() % 1000)
+ .arg(QCoreApplication::applicationPid())
+ .arg(++counter)
+ // A `/` or a `:` in a hostname would break the path or the flag
+ // separator. Neither is legal in a hostname, so this is belt and
+ // braces rather than a known case.
+ .arg(QString(host).replace(QLatin1Char('/'), QLatin1Char('_'))
+ .replace(QLatin1Char(':'), QLatin1Char('_')))
+ .arg(info);
+}
+
+} // namespace
+
void NotmuchWorker::moveMessages(const QStringList &messageIds,
const QString &destFolder)
{
@@ -728,15 +788,24 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds,
continue;
}
- const QString to = destDir + QLatin1Char('/') + QFileInfo(from).fileName();
- if (from == to) {
- // Already where it was asked to go. Reported as moved, since the
- // caller's request is satisfied.
+ // Already where it was asked to go, compared on the DIRECTORY rather
+ // than on the full path. It used to compare paths, which worked only
+ // because the filename was carried across unchanged; with a fresh name
+ // that test can never be true, so a message already in the destination
+ // would be renamed on every move for no reason, and every rename is a
+ // new filename mbsync has to reconcile.
+ if (QFileInfo(from).absolutePath() == QFileInfo(destDir).absoluteFilePath()) {
moved.append(id);
origins.insert(id, origin);
continue;
}
+ // A FRESH name, never the old one. See freshMaildirName(): carrying
+ // the `,U=` infix across a folder boundary is what produced
+ // `Maildir error: duplicate UID` on real mail.
+ const QString to = destDir + QLatin1Char('/')
+ + freshMaildirName(QFileInfo(from).fileName());
+
if (!QFile::rename(from, to)) {
emit errorOccurred(QStringLiteral("Cannot move %1 to %2")
.arg(QFileInfo(from).fileName(), destFolder));