From 841b2289aaa0942253170d61141f5cc7df126455 Mon Sep 17 00:00:00 2001 From: Cristian Adam Date: Thu, 8 Oct 2026 17:36:41 +0200 Subject: [PATCH] Avoid temporary files when writing generated files if different Copy-if-different writes of autogen info files, file(GENERATE) outputs, and configure_file outputs wrote a temporary file and read it back to compare it with the destination. Opening a freshly written file is very expensive with real-time virus scanners. Compose the content in memory and compare it with the destination directly, writing only if it differs. Add cmGeneratedFileStream::WriteIfDifferent for this. --- Source/cmGeneratedFileStream.cxx | 25 ++++++++ Source/cmGeneratedFileStream.h | 15 +++++ .../cmGeneratorExpressionEvaluationFile.cxx | 47 +++++++------- Source/cmMakefile.cxx | 61 ++++++++++++------- Source/cmQtAutoGenInitializer.cxx | 12 +--- Source/cmSystemTools.cxx | 24 ++++++++ Source/cmSystemTools.h | 7 +++ 7 files changed, 137 insertions(+), 54 deletions(-) diff --git a/Source/cmGeneratedFileStream.cxx b/Source/cmGeneratedFileStream.cxx index 161dcbcdcd..08e9bb36c8 100644 --- a/Source/cmGeneratedFileStream.cxx +++ b/Source/cmGeneratedFileStream.cxx @@ -95,6 +95,31 @@ bool cmGeneratedFileStream::Close() return this->cmGeneratedFileStreamBase::Close(); } +bool cmGeneratedFileStream::WriteIfDifferent(std::string const& name, + cm::string_view content, + bool quiet, bool* written) +{ + if (written) { + *written = false; + } + if (cmSystemTools::FileHasContent(name, content)) { + return true; + } + cmGeneratedFileStream fout; + fout.Open(name, quiet, true); + if (!fout) { + return false; + } + fout.write(content.data(), static_cast(content.size())); + if (!fout.Close()) { + return false; + } + if (written) { + *written = true; + } + return true; +} + void cmGeneratedFileStream::SetCopyIfDifferent(bool copy_if_different) { this->CopyIfDifferent = copy_if_different; diff --git a/Source/cmGeneratedFileStream.h b/Source/cmGeneratedFileStream.h index b30fc64e90..9e4cb91546 100644 --- a/Source/cmGeneratedFileStream.h +++ b/Source/cmGeneratedFileStream.h @@ -6,6 +6,8 @@ #include +#include + #include "cmsys/FStream.hxx" #include "cm_codecvt_Encoding.hxx" @@ -157,4 +159,17 @@ public: * Afterward, the original encoding is restored. */ void WriteAltEncoding(std::string const& data, codecvt_Encoding encoding); + + /** + * Atomically replace the named file with the given content in binary + * mode, unless the file already has exactly this content. Unlike + * copy-if-different, this compares the content with the file directly + * instead of writing a temporary file and reading it back, which is + * expensive with real-time virus scanners. Return false if the file + * had to be written but could not be. If written is given, it is set + * to whether the file was written. + */ + static bool WriteIfDifferent(std::string const& name, + cm::string_view content, bool quiet = false, + bool* written = nullptr); }; diff --git a/Source/cmGeneratorExpressionEvaluationFile.cxx b/Source/cmGeneratorExpressionEvaluationFile.cxx index e24257990a..d74b7bacbc 100644 --- a/Source/cmGeneratorExpressionEvaluationFile.cxx +++ b/Source/cmGeneratorExpressionEvaluationFile.cxx @@ -88,34 +88,37 @@ void cmGeneratorExpressionEvaluationFile::Generate( this->Files.push_back(outputFileName); outputFiles[outputFileName] = outputContent; - bool openWithBinaryFlag = false; - if (!this->NewLineCharacter.empty()) { - openWithBinaryFlag = true; + // If no new line character is specified, use the platform's native one, + // as writing "\n" in text mode would. + std::string newLine = this->NewLineCharacter; + if (newLine.empty()) { +#ifdef _WIN32 + newLine = "\r\n"; +#else + newLine = "\n"; +#endif } - cmGeneratedFileStream fout; - fout.Open(outputFileName, false, openWithBinaryFlag); - if (!fout) { + std::string fileContent; + fileContent.reserve(outputContent.size()); + std::istringstream iss(outputContent); + std::string line; + bool hasNewLine = false; + while (cmSystemTools::GetLineFromStream(iss, line, &hasNewLine)) { + fileContent += line; + if (hasNewLine || !this->NewLineCharacter.empty()) { + fileContent += newLine; + } + } + + bool written = false; + if (!cmGeneratedFileStream::WriteIfDifferent(outputFileName, fileContent, + false, &written)) { lg->IssueMessage(MessageType::FATAL_ERROR, "Could not open file for write in copy operation " + outputFileName); return; } - fout.SetCopyIfDifferent(true); - std::istringstream iss(outputContent); - std::string line; - bool hasNewLine = false; - while (cmSystemTools::GetLineFromStream(iss, line, &hasNewLine)) { - fout << line; - if (!this->NewLineCharacter.empty()) { - fout << this->NewLineCharacter; - } else if (hasNewLine) { - // if new line character is not specified, the file will be opened in - // text mode. So, "\n" will be translated to the correct newline - // ending based on the platform. - fout << "\n"; - } - } - if (fout.Close() && perm) { + if (written && perm) { cmSystemTools::SetPermissions(outputFileName.c_str(), perm); } } diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index d22e1e0d44..0f722764b3 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3803,21 +3803,9 @@ int cmMakefile::ConfigureFile(std::string const& infile, return res; } - std::string newLineCharacters; - std::ios::openmode omode = std::ios::out | std::ios::trunc; + std::string newLineCharacters = "\n"; if (newLine.IsValid()) { newLineCharacters = newLine.GetCharacters(); - omode |= std::ios::binary; - } else { - newLineCharacters = "\n"; - } - std::string tempOutputFile = cmStrCat(soutfile, ".tmp"); - cmsys::ofstream fout(tempOutputFile.c_str(), omode); - if (!fout) { - cmSystemTools::Error("Could not open file for write in copy operation " + - tempOutputFile); - cmSystemTools::ReportLastSystemError(""); - return 0; } cmsys::ifstream fin(sinfile.c_str()); if (!fin) { @@ -3841,25 +3829,52 @@ int cmMakefile::ConfigureFile(std::string const& infile, // input file at the same time std::string inLine; std::string outLine; + std::string content; while (cmSystemTools::GetLineFromStream(fin, inLine)) { outLine.clear(); this->ConfigureString(inLine, outLine, atOnly, escapeQuotes); - fout << outLine << newLineCharacters; + content += outLine; + content += newLineCharacters; } - // close the files before attempting to copy fin.close(); - fout.close(); +#ifdef _WIN32 + if (!newLine.IsValid()) { + // Translate new lines as writing in text mode would. + cmSystemTools::ReplaceString(content, "\n", "\r\n"); + } +#endif - auto status = cmSystemTools::MoveFileIfDifferent(tempOutputFile, soutfile); + // Avoid writing a temporary file just to find out nothing changed. + if (!cmSystemTools::FileHasContent(soutfile, content)) { + std::string tempOutputFile = cmStrCat(soutfile, ".tmp"); + cmsys::ofstream fout(tempOutputFile.c_str(), + std::ios::out | std::ios::trunc | std::ios::binary); + if (!fout) { + cmSystemTools::Error("Could not open file for write in copy operation " + + tempOutputFile); + cmSystemTools::ReportLastSystemError(""); + return 0; + } + fout << content; + fout.close(); + + // The content is known to differ, so replace the output without + // comparing the files again. + cmsys::Status status; + if (!cmSystemTools::RenameFile(tempOutputFile, soutfile)) { + status = cmSystemTools::CopyFileAlways(tempOutputFile, soutfile); + cmSystemTools::RemoveFile(tempOutputFile); + } + if (!status) { + this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); + return 0; + } + } + + auto status = cmSystemTools::SetPermissions(soutfile, permissions); if (!status) { this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); res = 0; - } else { - status = cmSystemTools::SetPermissions(soutfile, permissions); - if (!status) { - this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); - res = 0; - } } return res; diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index a0289d4b49..104402e29e 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -293,21 +293,15 @@ void InfoWriter::SetConfigArray( bool InfoWriter::Save(std::string const& filename) { - cmGeneratedFileStream fileStream; - fileStream.SetCopyIfDifferent(true); - fileStream.Open(filename, false, true); - if (!fileStream) { - return false; - } - + std::ostringstream content; Json::StyledStreamWriter jsonWriter; try { - jsonWriter.write(fileStream, this->Value_); + jsonWriter.write(content, this->Value_); } catch (...) { return false; } - return fileStream.Close(); + return cmGeneratedFileStream::WriteIfDifferent(filename, content.str()); } cmQtAutoGen::ConfigStrings> generateListOptions( diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 2559d75472..af18f6cf41 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1767,6 +1767,30 @@ cmsys::Status cmSystemTools::MoveFileIfDifferent( return res; } +bool cmSystemTools::FileHasContent(std::string const& path, + cm::string_view content) +{ + // Compare sizes first to avoid opening the file when they differ. + // A missing file has length zero and is caught by opening it below. + if (FileLength(path) != content.size()) { + return false; + } + cmsys::ifstream fin(path.c_str(), std::ios::in | std::ios::binary); + if (!fin) { + return false; + } + char buffer[4096]; + while (!content.empty()) { + std::size_t const n = std::min(content.size(), sizeof(buffer)); + if (!fin.read(buffer, static_cast(n)) || + content.compare(0, n, cm::string_view(buffer, n)) != 0) { + return false; + } + content.remove_prefix(n); + } + return true; +} + void cmSystemTools::Glob(std::string const& directory, std::string const& regexp, std::vector& files) diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 3f1235b41f..689becea8e 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -229,6 +229,13 @@ public: static cmsys::Status MoveFileIfDifferent(std::string const& source, std::string const& destination); + /** + * Return true if the file exists and contains exactly the given bytes. + * Use this to avoid rewriting a file whose content would not change, + * without first writing the new content to a temporary file. + */ + static bool FileHasContent(std::string const& path, cm::string_view content); + /** * According to the CreateProcessW documentation: *