diff --git a/Tests/Utilities/TestPatch.cpp b/Tests/Utilities/TestPatch.cpp index 9679636725..32c0901fa8 100644 --- a/Tests/Utilities/TestPatch.cpp +++ b/Tests/Utilities/TestPatch.cpp @@ -220,3 +220,44 @@ TEST_CASE(patch_adding_file_to_existing_file) EXPECT_FILE_EQ(ByteString::formatted("{}/a", s_test_dir), "a\n"sv); } + +TEST_CASE(patch_remove_file_to_empty) +{ + PatchSetup setup; + + auto patch = R"( +--- a ++++ /dev/null +@@ -1 +0,0 @@ +-1 +)"sv; + + auto file = "1\n"sv; + auto input = MUST(Core::File::open(ByteString::formatted("{}/a", s_test_dir), Core::File::OpenMode::Write)); + MUST(input->write_until_depleted(file.bytes())); + + run_patch(ExpectSuccess::Yes, {}, patch, "patching file a\n"sv); + + EXPECT(!FileSystem::exists(ByteString::formatted("{}/a", s_test_dir))); +} + +TEST_CASE(patch_remove_file_trailing_garbage) +{ + PatchSetup setup; + + auto patch = R"( +--- a ++++ /dev/null +@@ -1 +0,0 @@ +-1 +)"sv; + + auto file = "1\n2\n"sv; + auto input = MUST(Core::File::open(ByteString::formatted("{}/a", s_test_dir), Core::File::OpenMode::Write)); + MUST(input->write_until_depleted(file.bytes())); + + run_patch(ExpectSuccess::Yes, {}, patch, "patching file a\n" + "Not deleting file a as content differs from patch\n"sv); + + EXPECT_FILE_EQ(ByteString::formatted("{}/a", s_test_dir), "2\n"sv); +} diff --git a/Userland/Utilities/patch.cpp b/Userland/Utilities/patch.cpp index cebe38942b..03e8b8e714 100644 --- a/Userland/Utilities/patch.cpp +++ b/Userland/Utilities/patch.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Shannon Booth + * Copyright (c) 2023-2024, Shannon Booth * * SPDX-License-Identifier: BSD-2-Clause */ @@ -17,6 +17,11 @@ static bool is_adding_file(Diff::Patch const& patch) return patch.hunks[0].location.old_range.start_line == 0; } +static bool is_removing_file(Diff::Patch const& patch) +{ + return patch.hunks[0].location.new_range.start_line == 0; +} + static ErrorOr read_content(StringView path_of_file_to_patch, Diff::Patch const& patch) { auto file_to_patch_or_error = Core::File::open(path_of_file_to_patch, Core::File::OpenMode::Read); @@ -42,10 +47,21 @@ static ErrorOr do_patch(StringView path_of_file_to_patch, Diff::Patch cons // Apply patch to a temporary file in case one or more of the hunks fails. char tmp_output[] = "/tmp/patch.XXXXXX"; auto tmp_file = TRY(Core::File::adopt_fd(TRY(Core::System::mkstemp(tmp_output)), Core::File::OpenMode::ReadWrite)); + StringView tmp_path { tmp_output, sizeof(tmp_output) }; TRY(Diff::apply_patch(*tmp_file, lines, patch)); - return FileSystem::move_file(path_of_file_to_patch, StringView { tmp_output, sizeof(tmp_output) }); + // If the patched file ends up being empty, remove it, as the patch was a removal. + // Note that we cannot simply rely on the patch successfully applying and the patch claiming it is removing the file + // as there may be some trailing garbage at the end of file which was not inside the patch. + if (is_removing_file(patch)) { + if ((TRY(Core::System::stat(tmp_path))).st_size == 0) + return FileSystem::remove(path_of_file_to_patch, FileSystem::RecursionMode::Disallowed); + + outln("Not deleting file {} as content differs from patch", path_of_file_to_patch); + } + + return FileSystem::move_file(path_of_file_to_patch, tmp_path); } ErrorOr serenity_main(Main::Arguments arguments) @@ -73,7 +89,6 @@ ErrorOr serenity_main(Main::Arguments arguments) if (patch.header.format == Diff::Format::Unknown) break; - // FIXME: Support adding/removing a file, and asking for file to patch as fallback otherwise. StringView to_patch; if (FileSystem::is_regular_file(patch.header.old_file_path)) { to_patch = patch.header.old_file_path;