Skip to content

Commit

Permalink
Do not merge when origin/dest. match
Browse files Browse the repository at this point in the history
Do not attempt to merge when upstream and downstream files are the same.

diff3 will not merge when the same edit happens to upstream and downstream at the same time. When the rest of the file is the same across the repos, it is safe to skip the merge.

BUG=392575734
FIXED=392575734
PiperOrigin-RevId: 723533220
Change-Id: I4bea76cdc7fb05ecd2d361c1079d3901b25e1631
  • Loading branch information
joshgoldman-google committed Feb 7, 2025
1 parent d1a2b03 commit f653206
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
10 changes: 10 additions & 0 deletions java/com/google/copybara/util/MergeImportTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
baselineFile, destinationFile);
return FileVisitResult.CONTINUE;
}
try {
// same changes to both upstream and downstream, no need to merge
if (compareFileContents(destinationFile, file)) {
return FileVisitResult.CONTINUE;
}
} catch (IOException e) {
logger.atWarning().withCause(e).log(
"Cannot read one of (%s, %s) - will not attempt to merge", file, destinationFile);
return FileVisitResult.CONTINUE;
}
filesToProcess.add(
FilePathInformation.create(file, relativeFile, baselineFile, destinationFile));

Expand Down
20 changes: 20 additions & 0 deletions javatests/com/google/copybara/util/MergeImportToolTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

Expand Down Expand Up @@ -243,6 +244,25 @@ public void testNoDestinationEdits_noMergeImport() throws Exception {
.merge(any(Path.class), any(Path.class), any(Path.class), any(Path.class));
}

@Test
public void testOriginEqualsDestination_noMergeImport() throws Exception {
commandLineDiffUtil = Mockito.mock(MergeRunner.class);
String fileName = "foo.txt";
String commonFileContents = "a\nb\nc\n";
String additionalFileContents = "foo\n";
writeFile(baselineWorkdir, fileName, commonFileContents);
writeFile(originWorkdir, fileName, commonFileContents.concat(additionalFileContents));
writeFile(destinationWorkdir, fileName, commonFileContents.concat(additionalFileContents));
underTest = new MergeImportTool(console, commandLineDiffUtil, 10, null);

var unused =
underTest.mergeImport(
originWorkdir, destinationWorkdir, baselineWorkdir, diffToolWorkdir, glob, packagePath);

verify(commandLineDiffUtil, never())
.merge(any(Path.class), any(Path.class), any(Path.class), any(Path.class));
}

private Path createDir(Path parent, String name) throws IOException {
Path path = parent.resolve(name);
Files.createDirectories(path);
Expand Down

0 comments on commit f653206

Please sign in to comment.