Skip to content

Commit 090ef9a

Browse files
[VirtualOutput] Add option to only overwrite output if content is different
Add new output configuration that checks if Output.keep() will change the file content and skip overwriting when the content are the same. This avoids updating file status for the output file like timestamps.
1 parent 0c0bb5a commit 090ef9a

File tree

4 files changed

+147
-0
lines changed

4 files changed

+147
-0
lines changed

llvm/include/llvm/Support/VirtualOutputConfig.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,7 @@ HANDLE_OUTPUT_CONFIG_FLAG(CRLF, false) // OF_CRLF.
1919
HANDLE_OUTPUT_CONFIG_FLAG(DiscardOnSignal, true) // E.g., RemoveFileOnSignal.
2020
HANDLE_OUTPUT_CONFIG_FLAG(AtomicWrite, true) // E.g., use temporaries.
2121
HANDLE_OUTPUT_CONFIG_FLAG(ImplyCreateDirectories, true)
22+
// Skip atomic write if existing file content is the same
23+
HANDLE_OUTPUT_CONFIG_FLAG(OnlyIfDifferent, false)
2224

2325
#undef HANDLE_OUTPUT_CONFIG_FLAG

llvm/lib/Support/VirtualOutputBackends.cpp

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,89 @@ Error OnDiskOutputFile::initializeStream() {
337337
return Error::success();
338338
}
339339

340+
namespace {
341+
class OpenFileRAII {
342+
static const int InvalidFd = -1;
343+
344+
public:
345+
int Fd = InvalidFd;
346+
347+
~OpenFileRAII() {
348+
if (Fd != InvalidFd)
349+
llvm::sys::Process::SafelyCloseFileDescriptor(Fd);
350+
}
351+
};
352+
353+
enum class FileDifference : uint8_t {
354+
/// The source and destination paths refer to the exact same file.
355+
IdenticalFile,
356+
/// The source and destination paths refer to separate files with identical
357+
/// contents.
358+
SameContents,
359+
/// The source and destination paths refer to separate files with different
360+
/// contents.
361+
DifferentContents
362+
};
363+
} // end anonymous namespace
364+
365+
static Expected<FileDifference>
366+
areFilesDifferent(const llvm::Twine &Source, const llvm::Twine &Destination) {
367+
if (sys::fs::equivalent(Source, Destination))
368+
return FileDifference::IdenticalFile;
369+
370+
OpenFileRAII SourceFile;
371+
sys::fs::file_status SourceStatus;
372+
// If we can't open the source file, fail.
373+
if (std::error_code EC = sys::fs::openFileForRead(Source, SourceFile.Fd))
374+
return convertToOutputError(Source, EC);
375+
376+
// If we can't stat the source file, fail.
377+
if (std::error_code EC = sys::fs::status(SourceFile.Fd, SourceStatus))
378+
return convertToOutputError(Source, EC);
379+
380+
OpenFileRAII DestFile;
381+
sys::fs::file_status DestStatus;
382+
// If we can't open the destination file, report different.
383+
if (std::error_code Error =
384+
sys::fs::openFileForRead(Destination, DestFile.Fd))
385+
return FileDifference::DifferentContents;
386+
387+
// If we can't open the destination file, report different.
388+
if (std::error_code Error = sys::fs::status(DestFile.Fd, DestStatus))
389+
return FileDifference::DifferentContents;
390+
391+
// If the files are different sizes, they must be different.
392+
uint64_t Size = SourceStatus.getSize();
393+
if (Size != DestStatus.getSize())
394+
return FileDifference::DifferentContents;
395+
396+
// If both files are zero size, they must be the same.
397+
if (Size == 0)
398+
return FileDifference::SameContents;
399+
400+
// The two files match in size, so we have to compare the bytes to determine
401+
// if they're the same.
402+
std::error_code SourceRegionErr;
403+
sys::fs::mapped_file_region SourceRegion(
404+
sys::fs::convertFDToNativeFile(SourceFile.Fd),
405+
sys::fs::mapped_file_region::readonly, Size, 0, SourceRegionErr);
406+
if (SourceRegionErr)
407+
return convertToOutputError(Source, SourceRegionErr);
408+
409+
std::error_code DestRegionErr;
410+
sys::fs::mapped_file_region DestRegion(
411+
sys::fs::convertFDToNativeFile(DestFile.Fd),
412+
sys::fs::mapped_file_region::readonly, Size, 0, DestRegionErr);
413+
414+
if (DestRegionErr)
415+
return FileDifference::DifferentContents;
416+
417+
if (memcmp(SourceRegion.const_data(), DestRegion.const_data(), Size) != 0)
418+
return FileDifference::DifferentContents;
419+
420+
return FileDifference::SameContents;
421+
}
422+
340423
Error OnDiskOutputFile::keep() {
341424
// Destroy the streams to flush them.
342425
BufferOS.reset();
@@ -351,6 +434,25 @@ Error OnDiskOutputFile::keep() {
351434
if (!TempPath)
352435
return Error::success();
353436

437+
if (Config.getOnlyIfDifferent()) {
438+
auto Result = areFilesDifferent(*TempPath, OutputPath);
439+
if (!Result)
440+
return Result.takeError();
441+
switch (*Result) {
442+
case FileDifference::IdenticalFile:
443+
// Do nothing for a self-move.
444+
return Error::success();
445+
446+
case FileDifference::SameContents:
447+
// Files are identical; remove the source file.
448+
(void) sys::fs::remove(*TempPath);
449+
return Error::success();
450+
451+
case FileDifference::DifferentContents:
452+
break; // Rename the file.
453+
}
454+
}
455+
354456
// Move temporary to the final output path and remove it if that fails.
355457
std::error_code RenameEC = sys::fs::rename(*TempPath, OutputPath);
356458
if (!RenameEC)

llvm/unittests/Support/VirtualOutputBackendsTest.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,4 +765,45 @@ TEST(VirtualOutputBackendAdaptors, makeMirroringOutputBackendCreateError) {
765765
FailedWithMessage(Error1->Msg));
766766
}
767767

768+
TEST(OnDiskBackendTest, OnlyIfDifferent) {
769+
OnDiskOutputBackendProvider Provider;
770+
auto Backend = Provider.createBackend();
771+
std::string FilePath = Provider.getFilePathToCreate();
772+
StringRef Data = "some data";
773+
OutputConfig Config = OutputConfig().setOnlyIfDifferent();
774+
775+
OutputFile O1, O2, O3;
776+
sys::fs::file_status Status1, Status2, Status3;
777+
// Write first file.
778+
EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O1),
779+
Succeeded());
780+
O1 << Data;
781+
EXPECT_THAT_ERROR(O1.keep(), Succeeded());
782+
EXPECT_FALSE(O1.isOpen());
783+
EXPECT_FALSE(sys::fs::status(FilePath, Status1, /*follow=*/false));
784+
785+
// Write second with same content.
786+
EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O2),
787+
Succeeded());
788+
O2 << Data;
789+
EXPECT_THAT_ERROR(O2.keep(), Succeeded());
790+
EXPECT_FALSE(O2.isOpen());
791+
EXPECT_FALSE(sys::fs::status(FilePath, Status2, /*follow=*/false));
792+
793+
// Make sure the output path file is not modified with same content.
794+
EXPECT_EQ(Status1.getUniqueID(), Status2.getUniqueID());
795+
796+
// Write third with different content.
797+
EXPECT_THAT_ERROR(Backend->createFile(FilePath, Config).moveInto(O3),
798+
Succeeded());
799+
O3 << Data << "\n";
800+
EXPECT_THAT_ERROR(O3.keep(), Succeeded());
801+
EXPECT_FALSE(O3.isOpen());
802+
EXPECT_FALSE(sys::fs::status(FilePath, Status3, /*follow=*/false));
803+
804+
// This should overwrite the file and create a different UniqueID.
805+
EXPECT_NE(Status1.getUniqueID(), Status3.getUniqueID());
806+
}
807+
808+
768809
} // end namespace

llvm/unittests/Support/VirtualOutputConfigTest.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ TEST(VirtualOutputConfigTest, construct) {
2222
EXPECT_TRUE(OutputConfig().getDiscardOnSignal());
2323
EXPECT_TRUE(OutputConfig().getAtomicWrite());
2424
EXPECT_TRUE(OutputConfig().getImplyCreateDirectories());
25+
EXPECT_FALSE(OutputConfig().getOnlyIfDifferent());
2526

2627
// Test inverted defaults.
2728
EXPECT_TRUE(OutputConfig().getNoText());
2829
EXPECT_TRUE(OutputConfig().getNoCRLF());
2930
EXPECT_FALSE(OutputConfig().getNoDiscardOnSignal());
3031
EXPECT_FALSE(OutputConfig().getNoAtomicWrite());
3132
EXPECT_FALSE(OutputConfig().getNoImplyCreateDirectories());
33+
EXPECT_TRUE(OutputConfig().getNoOnlyIfDifferent());
3234
}
3335

3436
TEST(VirtualOutputConfigTest, set) {

0 commit comments

Comments
 (0)