Skip to content

Commit

Permalink
Fix error occurred from file change due to permission bits modificati…
Browse files Browse the repository at this point in the history
…on (#356)

We use git diff to extract file infos. In particular rare cases, our
code fails to extracting relevant file infos for certain commits, due
to special extended header lines `old mode 100644` 
`new mode 100755` returned by the git diff, it throws an
AssertionError halfway during the analysis.

This is due to git, besides the file content and changes, it also
tracks the executable bit of the files' permission. As our program did
not foresee this scenario, it crashes upon the occurrence.

Let's fix this by inserting additional guard clause for this scenario
in our FileInfoExtractor.
  • Loading branch information
eugenepeh authored Oct 5, 2018
1 parent 7275036 commit 308e407
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions src/main/java/reposense/authorship/FileInfoExtractor.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public class FileInfoExtractor {
"^deleted file mode [\\d]{6}\n").asPredicate();
private static final Predicate<String> NEW_EMPTY_FILE_PREDICATE = Pattern.compile(
"^new file mode [a-zA-Z0-9\n. ]*$").asPredicate();
private static final Predicate<String> FILE_PERMISSION_CHANGED_PREDICATE = Pattern.compile(
"^old mode [a-zA-Z0-9\n. ]*$").asPredicate();

/**
* Extracts a list of relevant files given in {@code config}.
Expand Down Expand Up @@ -97,9 +99,7 @@ public static List<FileInfo> getEditedFileInfos(RepoConfiguration config, String
String[] fileDiffResultList = fullDiffResult.split(DIFF_FILE_CHUNK_SEPARATOR);

for (String fileDiffResult : fileDiffResultList) {
// file deleted, renamed, is binary file or is new and empty file, skip it
if (fileDiffResult.contains(SIMILAR_FILE_RENAMED_SYMBOL) || fileDiffResult.contains(BINARY_FILE_SYMBOL)
|| FILE_DELETED_PREDICATE.test(fileDiffResult) || NEW_EMPTY_FILE_PREDICATE.test(fileDiffResult)) {
if (!isValidFileDiff(fileDiffResult)) {
continue;
}

Expand All @@ -115,6 +115,22 @@ public static List<FileInfo> getEditedFileInfos(RepoConfiguration config, String
return fileInfos;
}

/**
* Returns false if the {@code fileDiffResult} contains either of the following:
* - file delete
* - file rename
* - file permission change
* - binary file
* - empty new file
*/
private static boolean isValidFileDiff(String fileDiffResult) {
return !(fileDiffResult.contains(SIMILAR_FILE_RENAMED_SYMBOL)
|| fileDiffResult.contains(BINARY_FILE_SYMBOL)
|| FILE_DELETED_PREDICATE.test(fileDiffResult)
|| NEW_EMPTY_FILE_PREDICATE.test(fileDiffResult)
|| FILE_PERMISSION_CHANGED_PREDICATE.test(fileDiffResult));
}

/**
* Analyzes the {@code fileDiffResult} and marks each {@code LineInfo} in {@code FileInfo} on whether they were
* inserted in between the commit range.
Expand Down

0 comments on commit 308e407

Please sign in to comment.