Merge check hooks can not access PR target reference

[dploeger]

When a script is configured as a merge check hook, the target reference commit (to_ref as provided on STDIN) of the pull request is not available to the script.

To reproduce:

• Create a script with the following content:

  while read from_ref to_ref ref_name; do
  echo "Ref update:"
  echo " Old value: $from_ref"
  echo " New value: $to_ref"
  echo " Ref name:  $ref_name"
  echo " Diff:"
  git show $from_ref..$to_ref | sed 's/^/  /'
  done
  exit 1 # don't allow a merge just yet as we're debugging. ;-)

• Enable this script in a repository merge check hook.
• Fork the repository
• Clone, commit and push something in the repository
• Create a pull request for your change

Expected result:

• The script shows the differences in the pull request and exit with rc 1

Actual Result:

• The script exits with a bad object error

Tested on:

• Bitbucket Server 5.10.1, GIT 2.7.4
• Bitbucket Server 5.10.1, GIT 2.17.1

Problem:

Push hooks have GIT_* environment variables available. These variables, specifically GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES, combine the pull request target repository objects with the pull request source repository objects and allow to access the commits from the pull request.

These are not available when using a merge check hook. Trying to add the @SynchronousPreferred annotation didn't help.

Workaround:

The following script is a rather hacky and ugly workaround:

if [ "${PULL_REQUEST_FROM_REPO_ID}" ]
then
    # This is a pull request. Call git show from the originating repository
    cd ../${PULL_REQUEST_FROM_REPO_ID}
fi
while read from_ref to_ref ref_name; do
    echo "Ref update:"
    echo " Old value: $from_ref"
    echo " New value: $to_ref"
    echo " Ref name:  $ref_name"
    echo " Diff:"
    git show $from_ref..$to_ref | sed 's/^/  /'
done
exit 1 # Still debugging first

[seletskiy]

@dploeger: I don't quite get the case. Can you write step-by-step reproducible scenario?

BTW, little nitpick: you can check that variable is not empty in (ba)sh by simply writing

if [ "$PULL_REQUEST_FROM_REPO_ID" ]; then
...
fi

[dploeger]

@seletskiy Rewrote the whole issue and added the steps. Hopefully you can reproduce.

[seletskiy]

@dploeger: Yes I'm able to reproduce this issue, thanks for very detailed report.

I think that problem is reason for this problem is that hook is executed in the git directory of main repository and not in the git directory of forked repository (as your workaround script shows). It's pretty clear that git diff will not be able to find objects from forked repository while running in git directory of main repository.

So from my perspective it should be fixed in code of the plugin: a) we need to identify that Merge Check is running for the forked repository, probably by checking pr.getFromRef().getRepository().getId() != pr.getToRef().getRepository().getId(), b) we need to change CWD to git directory of forked repository before execution of specified script.

Can you try to modify code and see does it work for you?

[dploeger]

Why do we need a)?

[seletskiy]

@dploeger: Hmm, probably we don't need it, just re-implement cd ../{pr.getToRef().getRepository().getId()} in Java.

[dploeger]

I simply changed a line, to set the working repository to the source repository, which works. (see pr #74)

However, the method to get the repository directory on the server will be removed without alternative in 6.0, so the workflow needs to be definitely refactored at some point.