diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/config.cpp | 10 | ||||
| -rw-r--r-- | src/config.h | 10 | ||||
| -rw-r--r-- | src/mailsync.cpp | 55 | ||||
| -rw-r--r-- | src/mailsync.h | 28 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 27 | ||||
| -rw-r--r-- | src/mainwindow.h | 11 |
6 files changed, 139 insertions, 2 deletions
diff --git a/src/config.cpp b/src/config.cpp index cac4c51..12c5632 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -18,6 +18,8 @@ #include "config.h" +#include "mailsync.h" + #include <QFileInfo> #include <QSettings> #include <QStandardPaths> @@ -171,6 +173,14 @@ void Config::load(const QString &path) m_syncCommand.clear(); } + // Not validated for existence, unlike the command above. The log is written + // by the script when it runs, so a fresh install has no file yet, and a + // startup problem reported for that would be noise. A missing file simply + // reads as SyncOutcome::Unknown when the time comes. + m_syncLog = settings.value(QStringLiteral("sync/log")).toString().trimmed(); + if (m_syncLog.isEmpty()) + m_syncLog = MailSync::defaultLogPath(); + // Account groups are written as [account.work], [account.personal], etc. // A dot, not a slash, separates the "account" namespace from the key: // QSettings' INI backend treats "/" as its own hierarchical group diff --git a/src/config.h b/src/config.h index d3ee767..3174a03 100644 --- a/src/config.h +++ b/src/config.h @@ -90,6 +90,15 @@ public: /// Empty when unset; the caller disables the Sync button in that case. QString syncCommand() const { return m_syncCommand; } + /// The sync script's log file, read to learn the outcome of a sync this + /// process did not start (item 54). + /// + /// Never empty: an unset key falls back to where assets/mailsync.sh writes + /// by default. An empty value would make every background sync report + /// SyncOutcome::Unknown, and the pending-edit indicator would then never + /// clear on a cron sync, which is exactly the defect this exists to fix. + QString syncLog() const { return m_syncLog; } + /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } @@ -161,6 +170,7 @@ private: QList<Account> m_accounts; QList<SavedQuery> m_savedQueries; QString m_syncCommand; + QString m_syncLog; QString m_notmuchConfig; qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; diff --git a/src/mailsync.cpp b/src/mailsync.cpp index 1d2a99f..10e5ef7 100644 --- a/src/mailsync.cpp +++ b/src/mailsync.cpp @@ -19,6 +19,8 @@ #include "mailsync.h" #include <QCoreApplication> +#include <QDir> +#include <QFile> #include <QRegularExpression> namespace { @@ -240,3 +242,56 @@ void MailSync::handleError(QProcess::ProcessError error) emit outputReceived(message); emit finished(false, -1); } + +QString MailSync::defaultLogPath() +{ + // Hardcoded to match assets/mailsync.sh, which builds it the same way from + // $HOME. Deriving it from QStandardPaths::GenericStateLocation would append + // the application name and point at a file the script never writes. + return QDir::homePath() + QStringLiteral("/.local/state/mailsync.log"); +} + +SyncOutcome MailSync::lastRunOutcome(const QString &logPath) +{ + if (logPath.isEmpty()) + return SyncOutcome::Unknown; + + QFile file(logPath); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + return SyncOutcome::Unknown; + + // A run's banner is one short line, so a tail this size holds many of them + // even when a verbose mbsync run sits between two. Reading the whole file + // would be a multi-megabyte read on the UI thread every ten minutes. + constexpr qint64 kTailBytes = 64 * 1024; + const qint64 size = file.size(); + if (size > kTailBytes && !file.seek(size - kTailBytes)) + return SyncOutcome::Unknown; + + const QByteArray tail = file.readAll(); + + // Last marker wins: the log accumulates runs, and an older OK must never + // speak for a newer failure. RUN START lines are skipped rather than + // stopping the scan, since the poll that observes a lock release can land + // after the next run has already announced itself. + const QList<QByteArray> lines = tail.split('\n'); + for (auto it = lines.crbegin(); it != lines.crend(); ++it) { + const QByteArray line = it->trimmed(); + if (!line.startsWith("===== RUN END:")) + continue; + + // Matched as a token, not as a whole line: the failure banner carries + // mbsync= and notmuch= fields after the status. + if (line.contains("status=OK")) + return SyncOutcome::Ok; + if (line.contains("status=FAILED")) + return SyncOutcome::Failed; + + // A marker whose status this does not recognise. The script changed, or + // the line was truncated by the tail boundary; either way nothing was + // observed. + return SyncOutcome::Unknown; + } + + return SyncOutcome::Unknown; +} diff --git a/src/mailsync.h b/src/mailsync.h index f83b526..a826f43 100644 --- a/src/mailsync.h +++ b/src/mailsync.h @@ -65,6 +65,17 @@ private: QString m_status; }; +/// The outcome of a sync run this process did not start. +/// +/// Unknown is not a failure, it is the absence of evidence: no log, no marker, +/// an unreadable file. Callers must treat it as "nothing observed" and change +/// no state on it, exactly as SyncMonitor::State::Unknown is treated. +enum class SyncOutcome { + Unknown, + Ok, + Failed, +}; + /// Runs the configured external sync command. /// /// qtmaildir deliberately does not implement sync itself. The existing script @@ -97,6 +108,23 @@ public: QString log() const { return m_log; } + /// Where assets/mailsync.sh writes its log, unless the config overrides it. + static QString defaultLogPath(); + + /// Reads the outcome of the last COMPLETED run from \p logPath. + /// + /// This is how a sync fired by the user's cron is judged: the process that + /// ran it is gone and its exit status died with it, but the script writes + /// a "RUN END ... status=OK" line before exiting, and that line survives. + /// Deriving the outcome from mbsync's own chatter was rejected for the + /// reason given on SyncPhaseTracker: a second opinion built from loose text + /// matching eventually disagrees with the authoritative one. + /// + /// Reads a bounded tail, not the file: this runs on the UI thread every + /// time a background sync ends, against a file logrotate lets grow all day. + /// Anything unreadable, absent or unmarked is Unknown. + static SyncOutcome lastRunOutcome(const QString &logPath); + signals: void started(); void outputReceived(const QString &chunk); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f830683..96e2a79 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1902,6 +1902,33 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state) showTransientStatus( tr("Background sync completed. Press Enter in the query bar to " "refresh.")); + + // Item 54. A cron sync carries the edits to the mail store exactly as a + // local one does, so the count it cleared has to be cleared here too. + // Without this the indicator kept reporting work that had already + // shipped, and the exit prompt asked to sync for it. + // + // The outcome comes from the RUN END line the script writes, because + // the process that ran this sync is gone and its exit status with it. + // Anything other than a definite OK changes nothing: the local path's + // rule is that only a SUCCESSFUL sync may clear the count, and Unknown + // is the absence of evidence rather than evidence of success. + if (MailSync::lastRunOutcome(m_config.syncLog()) == SyncOutcome::Ok) { + m_pendingTagEdits.clear(); + m_unnettablePendingEdits = 0; + + // Cleared HERE, before flushHeldEdits() below, and the ordering is + // load-bearing for the reason spelled out on the local path at + // onSyncFinished(): the flush calls sendThreadTagChange(), which + // writes m_editedAccounts SYNCHRONOUSLY. Clearing after the flush + // would discard accounts whose edits this run did not carry, and + // those edits would then sync only when some later edit happened to + // name the same account. Running first, everything in the set at + // this moment is exactly what the finished sync carried, so the + // local path's snapshot-and-subtract collapses to a clear. + m_editedAccounts.clear(); + updatePendingIndicator(); + } } // OUTSIDE the Idle branch, deliberately. Unknown clears the busy flag above, diff --git a/src/mainwindow.h b/src/mainwindow.h index 2d41b5e..490467f 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -325,7 +325,11 @@ private: /// Sends a tag change for a set of threads without touching the undo stack. /// Both tagSelected() and ThreadTagCommand route through this. - void sendThreadTagChange(const QStringList &threadIds, + /// + /// Invokable so a test can record an edit against a known account without a + /// worker: this is where m_editedAccounts is populated, and item 54's + /// draining of it cannot be observed otherwise. + Q_INVOKABLE void sendThreadTagChange(const QStringList &threadIds, const QStringList &add, const QStringList &remove, const QString &description); @@ -543,7 +547,10 @@ private: /// The channel names for m_editedAccounts, resolved through the config. /// Empty means sync everything, which is what a fetch with nothing pending /// has to do. - QStringList pendingSyncChannels() const; + /// Invokable for the same reason as sendThreadTagChange(): it is the only + /// view onto m_editedAccounts, and a count that reaches zero while the set + /// stays full looks correct and still syncs the wrong channels. + Q_INVOKABLE QStringList pendingSyncChannels() const; }; /// Undo entry for a tag change over a set of threads. |
