git merge-base error when using pre-commit hook

[phitoduck]

Hi! Thank you for this incredible tool.

✅ release or Git commit for the version of Darker you're using: 1.4.1
✅ Python, black and isort versions: black==22.1.0, isort==5.10.1
✅ your command line: iTerm2 on OSX
✅ file to be formatted as an attachment, if possible – also great if squeezed down to a minimal example: see code blocks
✅ resulting output or error message
✅ expected output: "PASSED" on the pre-commit hook or "FAILED" when there are violations in git --diff PR_BRANCH trunk

A friend and I are struggling to get the pre-commit hook working. We use pre-commit in two places:

1. on contributors' machines (when they go to commit changes)
2. in CI to fail the build if there are linting violations in changes relative to our trunk/main/master branch

We're seeing this error when we run pre-commit run --all-files in CI.

darker...................................................................Failed
- hook id: darker
- exit code: 1

Traceback (most recent call last):
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/bin/darker", line 8, in <module>
    sys.exit(main_with_error_handling())
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/lib/python3.9/site-packages/darker/__main__.py", line 416, in main_with_error_handling
    return main()
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/lib/python3.9/site-packages/darker/__main__.py", line 335, in main
    revrange = RevisionRange.parse_with_common_ancestor(args.revision, root)
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/lib/python3.9/site-packages/darker/git.py", line 128, in parse_with_common_ancestor
    return cls._with_common_ancestor(rev1, rev2, cwd)
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/lib/python3.9/site-packages/darker/git.py", line 167, in _with_common_ancestor
    common_ancestor = _git_check_output_lines(merge_base_cmd, cwd)[0]
  File "/home/runner/.cache/pre-commit/repofr9qzy56/py_env-python3/lib/python3.9/site-packages/darker/git.py", line 212, in _git_check_output_lines
    return check_output(  # nosec
  File "/opt/hostedtoolcache/Python/3.9.10/x64/lib/python3.9/subprocess.py", line 424, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/opt/hostedtoolcache/Python/3.9.10/x64/lib/python3.9/subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['git', 'merge-base', 'origin/trunk', 'HEAD']' returned non-zero exit status 1.

And here is our hook:

- repo: https://github.com/jabracadabrah/darker
      rev: 1.4.1 # version of darker
      hooks:
        # fail if black, pylint, flake8, isort, or pydocstyle find errors in the 'git --diff'
        # between this branch and latest commit on 'trunk'; this is great because it does not require
        # contributors to make changes to parts of the codebase they didn't change. Said otherwise:
        # if you submit a PR, the build will only fail if the code *you* wrote/changed does not
        # satisfy these quality check tools, but if there were already issues in the codebase before
        # you got there, the build will still pass and your PR can go through.
        - id: darker
          args:
            - --revision trunk
            - --isort
            - -L flake8
            - -L pylint
            - -L pydocstyle
            - --verbose
          additional_dependencies:
          - isort~=5.9
          - flake8~=4.0
          - pylint~=2.12
          - pydocstyle~=6.1

We're running this using GitHub Actions in this repo.

Could it be that the --revision flag was never meant to be used in a pre-commit hook? I read somewhere that git merge-base ... results in an error if there is a merge conflict with the target branch. We've verified that this isn't the case.

This question may be an example of clinging to a particular solution when we should be focusing on the original problem. Is there a recommended way to use darker with pre-commit to only fail on violations in the git --diff with main/master/trunk?

Thanks!

[phitoduck]

Got this resolved. The default entrypoint for the darker pre-commit hook is darker --revision :PRE-COMMIT:. We just needed to override that in this file with darker --revision trunk. Simple as that :)

[akaihola]

Hi @phitoduck, thanks for your issue report and also thanks for taking the effort to post your solution!

As documentation for -r/--revision states:

With the magic value :PRE-COMMIT:, Darker works in pre-commit compatible mode. Darker expects the revision range from the PRE_COMMIT_FROM_REF and PRE_COMMIT_TO_REF environment variables. If those are not found, Darker works against HEAD.

It's a bit unclear to me why that default mode of operation doesn't work for you. It would be nice to figure that out so we can make sure the defaults make sense and that the documentation is sufficient. I'd be grateful if you can point us to where the documentation might have misguided you.

Be sure to check out the extensive discussion on this feature in #113. Maybe @rossbar, @tkolleh or @philipgian have insight into what's happening in your particular use case.

But glad you got it sorted out! And thanks for your kind words – I should also acknowledge all the contributions from other people who have joined the effort for this tool along the way.

You may want to look into using --revision=trunk... instead – that will work correctly even if there have been commits both in trunk and your feature branch – the comparison will be to the branch point (i.e. latest common commit in the history of the two branches; see Git documentation for Triple dot). If you just compare to trunk, Darker will also reformat lines which have been changed in trunk!