Mark hunks which are present due to a rebase when diffing patch sets

The diff between two patch sets contains hunks which weren't
introduced by either patch set if a rebase occurred between those
patch sets. Previous optimizations for this case simply omitted all
files which aren't touched by either of the patch sets.

This change goes one step further: All hunks which can clearly be
identified as being introduced by a rebase are marked. In case of
doubt (e.g. they overlap with a regular hunk), they aren't marked.

To be consistent with the previous behavior, we exclude all files from
the result which only contain hunks due to rebase. In some cases (e.g.
a patch set touches 'fileA' but all identified hunks were introduced
by the rebase), this rule can be stricter than the previously mentioned
(as the previous rule would still show file 'fileA' but we exclude it
now).

Hunks which are introduced by a rebase are identified by computing
the diff between the parents of the two patch sets and transforming
the result to differences in terms of treeA of patch set A and treeB
of patch set B. This follows a suggestion which was posted as a comment
(https://gerrit-review.googlesource.com/c/33091/5//COMMIT_MSG#7) in
change I63d3a21ad4f.

As we always determine which hunks are introduced by a rebase when
two commits are explicitly specified which don't share a common
parent, we will determine those hunks even when we compute the
diff between the parents of the patch sets provided that those
parents fulfill the condition of being separated by a rebase. Those
situations should be rare and hence we refrain from adding
optimizations for this case for the moment.

Bug: Issue 217
Change-Id: If06381d506d360f0e3d24d078dcb54692698e766
This commit is contained in:
Alice Kober-Sotzek
2017-05-04 09:59:28 +02:00
parent 3227396d83
commit 2f62486149
13 changed files with 1713 additions and 91 deletions

View File

@@ -0,0 +1,81 @@
// Copyright (C) 2017 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
import com.google.gerrit.truth.ListSubject;
public class ContentEntrySubject extends Subject<ContentEntrySubject, ContentEntry> {
private static final SubjectFactory<ContentEntrySubject, ContentEntry> DIFF_INFO_SUBJECT_FACTORY =
new SubjectFactory<ContentEntrySubject, ContentEntry>() {
@Override
public ContentEntrySubject getSubject(
FailureStrategy failureStrategy, ContentEntry contentEntry) {
return new ContentEntrySubject(failureStrategy, contentEntry);
}
};
public static ContentEntrySubject assertThat(ContentEntry contentEntry) {
return assertAbout(DIFF_INFO_SUBJECT_FACTORY).that(contentEntry);
}
private ContentEntrySubject(FailureStrategy failureStrategy, ContentEntry contentEntry) {
super(failureStrategy, contentEntry);
}
public void isDueToRebase() {
isNotNull();
ContentEntry contentEntry = actual();
Truth.assertWithMessage("Entry should be marked 'dueToRebase'")
.that(contentEntry.dueToRebase)
.named("dueToRebase")
.isTrue();
}
public void isNotDueToRebase() {
isNotNull();
ContentEntry contentEntry = actual();
Truth.assertWithMessage("Entry should not be marked 'dueToRebase'")
.that(contentEntry.dueToRebase)
.named("dueToRebase")
.isNull();
}
public ListSubject<StringSubject, String> commonLines() {
isNotNull();
ContentEntry contentEntry = actual();
return ListSubject.assertThat(contentEntry.ab, Truth::assertThat).named("common lines");
}
public ListSubject<StringSubject, String> linesOfA() {
isNotNull();
ContentEntry contentEntry = actual();
return ListSubject.assertThat(contentEntry.a, Truth::assertThat).named("lines of 'a'");
}
public ListSubject<StringSubject, String> linesOfB() {
isNotNull();
ContentEntry contentEntry = actual();
return ListSubject.assertThat(contentEntry.b, Truth::assertThat).named("lines of 'b'");
}
}

View File

@@ -0,0 +1,57 @@
// Copyright (C) 2017 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.ComparableSubject;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
import com.google.gerrit.truth.ListSubject;
public class DiffInfoSubject extends Subject<DiffInfoSubject, DiffInfo> {
private static final SubjectFactory<DiffInfoSubject, DiffInfo> DIFF_INFO_SUBJECT_FACTORY =
new SubjectFactory<DiffInfoSubject, DiffInfo>() {
@Override
public DiffInfoSubject getSubject(FailureStrategy failureStrategy, DiffInfo diffInfo) {
return new DiffInfoSubject(failureStrategy, diffInfo);
}
};
public static DiffInfoSubject assertThat(DiffInfo diffInfo) {
return assertAbout(DIFF_INFO_SUBJECT_FACTORY).that(diffInfo);
}
private DiffInfoSubject(FailureStrategy failureStrategy, DiffInfo diffInfo) {
super(failureStrategy, diffInfo);
}
public ListSubject<ContentEntrySubject, ContentEntry> content() {
isNotNull();
DiffInfo diffInfo = actual();
return ListSubject.assertThat(diffInfo.content, ContentEntrySubject::assertThat)
.named("content");
}
public ComparableSubject<?, ChangeType> changeType() {
isNotNull();
DiffInfo diffInfo = actual();
return Truth.assertThat(diffInfo.changeType).named("changeType");
}
}

View File

@@ -0,0 +1,54 @@
// Copyright (C) 2017 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.IntegerSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
public class FileInfoSubject extends Subject<FileInfoSubject, FileInfo> {
private static final SubjectFactory<FileInfoSubject, FileInfo> FILE_INFO_SUBJECT_FACTORY =
new SubjectFactory<FileInfoSubject, FileInfo>() {
@Override
public FileInfoSubject getSubject(FailureStrategy failureStrategy, FileInfo fileInfo) {
return new FileInfoSubject(failureStrategy, fileInfo);
}
};
public static FileInfoSubject assertThat(FileInfo fileInfo) {
return assertAbout(FILE_INFO_SUBJECT_FACTORY).that(fileInfo);
}
private FileInfoSubject(FailureStrategy failureStrategy, FileInfo fileInfo) {
super(failureStrategy, fileInfo);
}
public IntegerSubject linesInserted() {
isNotNull();
FileInfo fileInfo = actual();
return Truth.assertThat(fileInfo.linesInserted).named("linesInserted");
}
public IntegerSubject linesDeleted() {
isNotNull();
FileInfo fileInfo = actual();
return Truth.assertThat(fileInfo.linesDeleted).named("linesDeleted");
}
}