diff --git a/presubmit_diff.py b/presubmit_diff.py index 67d722bd7..f60cac072 100755 --- a/presubmit_diff.py +++ b/presubmit_diff.py @@ -45,7 +45,7 @@ def fetch_content(host: str, repo: str, ref: str, file: str) -> bytes: return base64.b64decode(response.read()) -def git_diff(src: str | None, dest: str | None, unified: int | None) -> str: +def git_diff(src: str | None, dest: str | None) -> str: """Returns the result of `git diff --no-index` between two paths. If a path is not specified, the diff is against /dev/null. At least one of @@ -54,23 +54,13 @@ def git_diff(src: str | None, dest: str | None, unified: int | None) -> str: Args: src: Source path. dest: Destination path. - unified: Number of lines of context. If None, git diff uses 3 as - the default value. Returns: A string containing the git diff. """ - args = ["git", "diff", "--no-index"] - if unified is not None: - # git diff doesn't error out even if it's given a negative value. - # e.g., --unified=-3323, -U-3 - # - # It just ignores the value and treats it as 0. - # hence, this script doesn't bother validating the value. - args.append(f"-U{unified}") - - args.extend(["--", src or DEV_NULL, dest or DEV_NULL]) - return subprocess2.capture(args).decode("utf-8") + return subprocess2.capture( + ["git", "diff", "--no-index", "--", src or DEV_NULL, dest + or DEV_NULL]).decode("utf-8") def _process_diff(diff: str, src_root: str, dst_root: str) -> str: @@ -109,8 +99,7 @@ def _process_diff(diff: str, src_root: str, dst_root: str) -> str: return header -def _create_diff(host: str, repo: str, ref: str, root: str, file: str, - unified: int | None) -> str: +def _create_diff(host: str, repo: str, ref: str, root: str, file: str) -> str: new_file = os.path.join(root, file) if not os.path.exists(new_file): new_file = None @@ -128,12 +117,12 @@ def _create_diff(host: str, repo: str, ref: str, root: str, file: str, raise RuntimeError(f"Could not access file {file} from {root} " f"or from {host}/{repo}:{ref}.") - diff = git_diff(old_file, new_file, unified) + diff = git_diff(old_file, new_file) return _process_diff(diff, tmp_root, root) -def create_diffs(host: str, repo: str, ref: str, root: str, files: list[str], - unified: int | None) -> dict[str, str]: +def create_diffs(host: str, repo: str, ref: str, root: str, + files: list[str]) -> dict[str, str]: """Calculates diffs of files in a directory against a commit. Args: @@ -142,8 +131,6 @@ def create_diffs(host: str, repo: str, ref: str, root: str, files: list[str], ref: Gerrit commit. root: Path of local directory containing modified files. files: List of file paths relative to root. - unified: Number of lines of context. If None, git diff uses 3 as - the default value. Returns: A dict mapping file paths to diffs. @@ -155,8 +142,7 @@ def create_diffs(host: str, repo: str, ref: str, root: str, files: list[str], with concurrent.futures.ThreadPoolExecutor( max_workers=MAX_CONCURRENT_CONNECTION) as executor: futures_to_file = { - executor.submit(_create_diff, host, repo, ref, root, file, unified): - file + executor.submit(_create_diff, host, repo, ref, root, file): file for file in files } for future in concurrent.futures.as_completed(futures_to_file): @@ -179,20 +165,15 @@ def main(argv): parser.add_argument("--root", required=True, help="Folder containing modified files.") - parser.add_argument("-U", - "--unified", - required=False, - type=int, - help="generate diffs with lines context", - metavar='') parser.add_argument( "files", nargs="+", help="List of changed files. Paths are relative to the repo root.", ) options = parser.parse_args(argv) + diffs = create_diffs(options.host, options.repo, options.ref, options.root, - options.files, options.unified) + options.files) unified_diff = "\n".join([d for d in diffs.values() if d]) if options.output: diff --git a/tests/presubmit_diff_test.py b/tests/presubmit_diff_test.py index b78811d12..c6f5f2bfc 100755 --- a/tests/presubmit_diff_test.py +++ b/tests/presubmit_diff_test.py @@ -60,7 +60,7 @@ class PresubmitDiffTest(unittest.TestCase): def _test_create_diffs(self, files: List[str], expected: Dict[str, str]): actual = presubmit_diff.create_diffs("host", "repo", "ref", self.root, - files, None) + files) self.assertEqual(actual.keys(), expected.keys()) # Manually check each line in the diffs except the "index" line because @@ -81,7 +81,6 @@ class PresubmitDiffTest(unittest.TestCase): "ref", self.root, ["doesnotexist.txt"], - None, ) def test_create_diffs_with_unchanged_file(self): @@ -90,22 +89,6 @@ class PresubmitDiffTest(unittest.TestCase): {"unchanged.txt": ""}, ) - @mock.patch('subprocess2.capture', return_value="".encode("utf-8")) - def test_create_diffs_executes_git_diff_with_unified(self, capture): - create_diffs = presubmit_diff.create_diffs - # None => no -U - create_diffs("host", "repo", "ref", self.root, ["unchanged.txt"], None) - capture.assert_called_with( - ["git", "diff", "--no-index", "--", mock.ANY, mock.ANY]) - # 0 => -U0 - create_diffs("host", "repo", "ref", self.root, ["unchanged.txt"], 0) - capture.assert_called_with( - ["git", "diff", "--no-index", "-U0", "--", mock.ANY, mock.ANY]) - # 3 => -U3 - create_diffs("host", "repo", "ref", self.root, ["unchanged.txt"], 3) - capture.assert_called_with( - ["git", "diff", "--no-index", "-U3", "--", mock.ANY, mock.ANY]) - def test_create_diffs_with_added_file(self): expected_diff = """diff --git a/added.txt b/added.txt new file mode 100644