Fails when source branch has been merged in the target branch before

[rmelotte]

(The following applies to the GitRetriever).
In case the source branch has already been merged into the target branch before, dco-check silently fails:

dco-check --verbose
Options:
        check_merge_commits: False
        default_branch: master
        default_branch_from_remote: False
        default_remote: origin
        quiet: False
        verbose: True

Detected: git (default)
        using default branch 'master'
error: 

This is because the git merge-base --forkpoint master command returns nothing (and exits with 1):

dco-check --verbose
Options:
        check_merge_commits: False
        default_branch: master
        default_branch_from_remote: False
        default_remote: origin
        quiet: False
        verbose: True

Detected: git (default)
        using default branch 'master'
['git', 'merge-base', '--fork-point', 'master']
error: 

$ git merge-base --fork-point master
$

It's relatively easy to reproduce with an empty repo:

mkdir -p /tmp/test-repo
cd /tmp/test-repo
git init
touch init
git add init
git commit -m init
git checkout -b topic
touch topic
git add topic
git commit -m topic
git checkout master
git merge --no-ff topic -m "Merge branch topic into stable"
git checkout topic
touch another_file
git add another_file
git commit -m another_file
dco-check --verbose

Note that the no-ff flag is needed, otherwise a fast-forward merge is done and dco-check doesn't fail in that case.

Could it be related to this note in the manual of git-merge-base?

   Also, the remote-tracking branch you use the --fork-point mode with must be the one your topic forked from its tip. If you forked from an older commit than the tip, this mode would not find the fork point (imagine in the
   above sample history B0 did not exist, origin/master started at B1, moved to B2 and then B, and you forked your topic at origin/master^ when origin/master was B1; the shape of the history would be the same as above, without
   B0, and the parent of B1 is what git merge-base origin/master topic correctly finds, but the --fork-point mode will not, because it is not one of the commits that used to be at the tip of origin/master).

I'm note sure what dco-check could do in this case...
Maybe if merge-base --fork-point failed, we should fallback to using git merge-base master topic?

EDIT: it looks like there could be multiple reason why --forkpoint would fail: https://public-inbox.org/git/xmqq7ewckbpk.fsf@gitster.mtv.corp.google.com/t/

[christophebedard]

Thanks for reporting this, it's a pretty specific use-case!

In case the source branch has already been merged into the target branch before, dco-check silently fails:

Detected: git (default)
       using default branch 'master'
error: 

The error message could definitely be improved as a starting point. It should at least mention what/where the error happened and include the command error message/stderr output, if any.

Maybe if merge-base --fork-point failed, we should fallback to using git merge-base master topic?

I think this is reasonable. If you could open a PR with this fix that would be great, otherwise I'll get to it at some point!