From 3da3a788507e030635360e204f3dd1c3285a31fc Mon Sep 17 00:00:00 2001 From: ispyisail Date: Thu, 22 Oct 2026 21:36:24 +1300 Subject: [PATCH] Fix #1579: no false "changed on disk" warning on a network share On a CIFS/SMB share every save asked "Someone else changed the file on disk", because the size and modification time read straight after saving can be the client's cached values, not the server's final ones, and the next save demanded an exact match. The size and time are still the quick check. When they differ, the file now counts as changed only if its content differs from what this QET last read or wrote (a SHA-1 kept at open and after each save). A real change by someone else still asks; the same bytes with a new time do not. tst_changedondisk: sameContentNewTimeSaves fails before and passes after; the two conflict cases now move a symbol so the content really changes. Co-Authored-By: Claude Opus 5.5 --- sources/qetproject.cpp | 37 ++++++++++++++++++++++++++---- sources/qetproject.h | 3 ++- tests/qttest/tst_changedondisk.cpp | 17 +++++++++++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index c8dd66df0..01971618f 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -504,7 +504,7 @@ QETProject::ProjectState QETProject::openFile(QFile *file) //Extract the content of the xml. The raw bytes are kept : a project //file without a persisted uuid derives its uuid from them. const QByteArray content = file->readAll(); - rememberDiskState(); + rememberDiskState(&content); //Each folio's text is kept while it is not built (readDiagramsXml()): //from a .qet only, a .qetz is a zip of parts if (!content.startsWith("PK\x03\x04")) @@ -2803,16 +2803,37 @@ QETResult QETProject::writeCopyForOlderVersions(const QString &path) } /** - @brief QETProject::rememberDiskState - Remember the size and modification time of the project file, as this QET - has just read or written it. changedOnDisk() compares against them. + @brief diskHash + @return the SHA-1 of @p content, or of the file at @p path when + @p content is null; empty if the file cannot be read */ -void QETProject::rememberDiskState() +static QByteArray diskHash(const QString &path, const QByteArray *content = nullptr) +{ + QCryptographicHash hash(QCryptographicHash::Sha1); + if (content) { + hash.addData(*content); + return hash.result(); + } + QFile file(path); + if (!file.open(QIODevice::ReadOnly) || !hash.addData(&file)) + return QByteArray(); + return hash.result(); +} + +/** + @brief QETProject::rememberDiskState + Remember the size, modification time and content hash of the project + file, as this QET has just read or written it. changedOnDisk() compares + against them. + @param content the bytes just read from the file, if they are at hand +*/ +void QETProject::rememberDiskState(const QByteArray *content) { const QFileInfo info(m_file_path); m_disk_path = m_file_path; m_disk_size = info.exists() ? info.size() : -1; m_disk_mtime = info.exists() ? info.lastModified() : QDateTime(); + m_disk_hash = info.exists() ? diskHash(m_file_path, content) : QByteArray(); } /** @@ -2822,6 +2843,10 @@ void QETProject::rememberDiskState() last read or wrote: another user, another program or another QET saved it since. A file that does not exist yet, or one this QET never read or wrote (Save As to another name), is not a change. + A different size or time alone is not enough: right after a save, a + network share (CIFS with its attribute cache, #1579) can report the size + and time from before the server settled them. The file is a change only + if its content differs from what this QET read or wrote. */ bool QETProject::changedOnDisk(QDateTime *when) const { @@ -2833,6 +2858,8 @@ bool QETProject::changedOnDisk(QDateTime *when) const return false; if (info.size() == m_disk_size && info.lastModified() == m_disk_mtime) return false; + if (!m_disk_hash.isEmpty() && diskHash(target) == m_disk_hash) + return false; if (when) *when = info.lastModified(); return true; diff --git a/sources/qetproject.h b/sources/qetproject.h index 1675f2627..049b91b5f 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -337,7 +337,7 @@ class QETProject : public QObject static QString spliceKeptFolios(QString text, const QHash &kept_folios); bool close(); QETResult write(bool overwrite_changed = false); - void rememberDiskState(); + void rememberDiskState(const QByteArray *content = nullptr); bool changedOnDisk(QDateTime *when = nullptr) const; bool isReadOnly() const; /// Opened from a .qetz of a later format (opened read-only) @@ -558,6 +558,7 @@ class QETProject : public QObject QString m_disk_path; qint64 m_disk_size = -1; QDateTime m_disk_mtime; + QByteArray m_disk_hash; /// Saving a .qetz may copy the part of a folio not built /// (toXml(), stage D4) bool m_kept_parts_trusted = false; diff --git a/tests/qttest/tst_changedondisk.cpp b/tests/qttest/tst_changedondisk.cpp index d0b31c6d4..ad84db0b5 100644 --- a/tests/qttest/tst_changedondisk.cpp +++ b/tests/qttest/tst_changedondisk.cpp @@ -12,7 +12,9 @@ // when the file on disk is no longer that one. Driven through the real // binary's --run: qet.save() writes the file behind // the open project without updating what it remembers, which is exactly "someone -// else saved"; qet.save('') is the project's own save. +// else saved"; qet.save('') is the project's own save. Only a different content +// counts: the same bytes with a new time are what a network share reports +// right after our own save (#1579). class tst_changedondisk : public QObject { Q_OBJECT @@ -71,9 +73,21 @@ private slots: QVERIFY2(out.contains(QStringLiteral("R2 true")), qPrintable(out)); } + void sameContentNewTimeSaves() + { + // the same bytes written behind the project (an unchanged project + // saves to an identical file, F124): only the time differs + const QString script = QStringLiteral( + "qet.save(%1);\nqet.log('R ' + qet.save(''));\n") + .arg(QStringLiteral("'") + m_project + QStringLiteral("'")); + const QString out = run(script, m_project); + QVERIFY2(out.contains(QStringLiteral("R true")), qPrintable(out)); + } + void changedFileIsNotOverwritten() { const QString script = QStringLiteral( + "qet.moveElement(0, qet.elementUuids(0)[0], 10, 0);\n" "qet.save(%1);\nqet.log('R ' + qet.save(''));\n") .arg(QStringLiteral("'") + m_project + QStringLiteral("'")); const QString out = run(script, m_project); @@ -84,6 +98,7 @@ private slots: void overrideOverwrites() { const QString script = QStringLiteral( + "qet.moveElement(0, qet.elementUuids(0)[0], 10, 0);\n" "qet.save(%1);\nqet.log('R ' + qet.save(''));\n") .arg(QStringLiteral("'") + m_project + QStringLiteral("'")); const QString out = run(script, m_project, true);