diff_cover icon indicating copy to clipboard operation
diff_cover copied to clipboard

diff-cover crashes when used in a shallow repository with git >=2.28

Open gschaffner opened this issue 4 years ago • 1 comments

Git 2.28 introduced changes to how ranges are treated by git-diff.

"git diff" used to take arguments in random and nonsense range notation, e.g. "git diff A..B C", "git diff A..B C...D", etc., which has been cleaned up. --- https://lore.kernel.org/git/[email protected]/

Also see https://github.com/git/git/commit/8bfcb3a690126e6222f0d4f7012b0f68bb748018.

This can produce errors like

Traceback (most recent call last):
  File "/home/runner/venv/bin/diff-cover", line 8, in <module>
    sys.exit(main())
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/diff_cover_tool.py", line 210, in main
    percent_covered = generate_coverage_report(
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/diff_cover_tool.py", line 192, in generate_coverage_report
    reporter.generate_report(output_file)
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/report_generator.py", line 254, in generate_report
    report = template.render(self._context())
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/report_generator.py", line 303, in _context
    context = super(TemplateReportGenerator, self).report_dict()
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/report_generator.py", line 182, in report_dict
    src: self._src_path_stats(src) for src in self.src_paths()
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/report_generator.py", line 84, in src_paths
    return {src for src, summary in self._diff_violations().items()
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/report_generator.py", line 176, in _diff_violations
    for src_path in self._diff.src_paths_changed()
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/diff_reporter.py", line 147, in src_paths_changed
    diff_dict = self._git_diff()
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/diff_reporter.py", line 196, in _git_diff
    for diff_str in self._get_included_diff_results():
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/diff_reporter.py", line 169, in _get_included_diff_results
    included = [self._git_diff_tool.diff_committed(self._compare_branch)]
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/git_diff.py", line 47, in diff_committed
    return execute([
  File "/home/runner/venv/lib/python3.8/site-packages/diff_cover/command_runner.py", line 43, in execute
    raise CommandError(stderr)
diff_cover.command_runner.CommandError: fatal: origin/master...HEAD: no merge base

@dimaov reported this error in https://github.com/Bachmann1234/diff_cover/issues/152#issuecomment-669056592, but they were presumably able to fix it by installing git < 2.28.

The error occurs in shallow repositories when https://github.com/Bachmann1234/diff_cover/blob/83f00c883088bd871965a7145cc0550ca0db1dd3/diff_cover/git_diff.py#L51 calls commands like git diff origin/master...HEAD (i.e. when using diff-cover --diff-range-notation '...').

This issue does not occur if self._range_notation == "..".

Steps to replicate

Set up a shallow repository checking out the head commit of a feature branch. Also fetch origin/master so that we have a branch to compare with.

git clone --depth 1 [email protected]:ytdl-org/youtube-dl.git --branch rtmp_test
cd youtube-dl
git fetch origin master:refs/remotes/origin/master

With git 2.27 we can test

$ git --version
git version 2.27.0
$ git diff origin/master...HEAD
< output abbreviated. a diff is printed right here >
$ git diff $(git merge-base origin/master HEAD) HEAD
< the only output is a newline. exit code 0 >
$ git merge-base origin/master HEAD
< the only output is a newline. exit code 1 >

But with git 2.28 we can test

$ git --version
git version 2.28.0
$ git diff origin/master...HEAD
fatal: origin/master...HEAD: no merge base
< exit code 128 >
$ git diff $(git merge-base origin/master HEAD) HEAD
< the only output is a newline. exit code 0 >
$ git merge-base origin/master HEAD
< the only output is a newline. exit code 1 >

Why does this discrepancy occur? Because git-diff behavior changed. Notably, man git-diff (2.27 version) is incorrect. Both the 2.27 and 2.28 man pages state that

git diff [<options>] <commit>...<commit> [--] [<path>...]
    This form is to view the changes on the branch containing and up to the second <commit>, starting at a
    common ancestor of both <commit>. "git diff A...B" is equivalent to "git diff $(git merge-base A B)
    B". You can omit any one of <commit>, which has the same effect as using HEAD instead.

So the behavior and documentation properly match up when using git 2.28, but with git 2.27 the man page doesn't match the behavior.

Solutions for users

Note that this is NOT a bug with diff-cover. It is just something that diff-cover users should be aware of now that git 2.28 is out. Apologies for posting a non-issue in the issue board, but I am unaware of a better way to share this info with other users.

The fix is to not use a shallow repository.

Notably, git diff origin/master..HEAD works in both 2.27 and 2.28. If you are fine with the implications of it, you can get around this bug by using diff-cover --diff-range-notation '..'. (Read about the implications here.)

gschaffner avatar Aug 08 '20 05:08 gschaffner

Thanks for the info! I need to make some time to dig into this some.

Bachmann1234 avatar Aug 10 '20 12:08 Bachmann1234