Fails to fetch changes when pull request is made from a 'master' branch

[nathgs]

The pullrequest-resource in outputs the following error, but doesn't fail completely:

fatal: Refusing to fetch into current branch refs/heads/master of non-bare repository
Already on 'master'
Your branch is up-to-date with 'origin/master'.

This results in strange behaviour where the pullrequest-resource has cloned the git repository, but not fetched the changes from the pull request. The subsequent steps in the build plan continue to execute without including the pull request changes.

Steps to reproduce:

1. Create a pipeline with using pullrequest-resource pointing to a repository let's call it usera/repo
2. Fork the repo into another user account userb/repo
3. Make some trivial change to the master of userb/repo (it's important the change is made in master) and create a pull request from userb/repo:master to usera/repo:master
4. Check the pipeline in for the pullrequest-resource

Expected outcome:
The in of pullrequest-resource should fetch the changes from the pull request we should see the changes from userb/repo:master in Concourse.

Actual outcome:
The in of pullrequest-resource fails to fetch the changes from the pull request. It also doesn't fail in an obvious way it outputs the error but then exits with success. We see only the contents of usera/repo:master in Concourse.

This only happens when the pull request is made from the master branch of the fork. The asset/lib/in.rb seems to assume the pull request is made from a branch other than master branch_ref = pr['head']['ref'] what happens in this case is branch_ref = 'master' and then system("git fetch -q origin pull/#{id}/head:#{branch_ref} 1>&2") results in the Already on 'master' error. Quick workaround hack is to hardcode branch_ref = 'tmp', but that has other implications so not a full solution.

[jtarchie]

Let me dive into this a little more.

It was requested a while back that git fetch -q origin pull/#{id}/head:#{branch_ref} 1>&2 exist so the branch is checked out at the correct name. If it is on master then it should checkout master. I wonder if there is a way to force it.

It's interesting this is the first time that this is come up.

[jtarchie]

I see the PR that originally caused this. I've reached out to the original author for some clarification in their workflow. I don't want to break anything or make it backwards incompatible.

Please expect this to be resolved shortly.

[jtarchie]

This should have been resolved in f6210c9. The checkout branch name is being prefixed with pr-.

[jtarchie]

Please reopen this issue if you have problems with it passed v15 release.

[nathgs]

Tested with the v15 release, but got an error:

Pulling jtarchie/pr@sha256:ad59550162c6953af76795a48ae4777b048e3e8be829d8ed5091e1524c8cfe16...
sha256:ad59550162c6953af76795a48ae4777b048e3e8be829d8ed5091e1524c8cfe16: Pulling from jtarchie/pr
e110a4a17941: Pulling fs layer
036d164b66be: Pulling fs layer
f163814c49d5: Pulling fs layer
d75e7518fabf: Pulling fs layer
b913866af336: Pulling fs layer
d75e7518fabf: Waiting
b913866af336: Waiting
e110a4a17941: Verifying Checksum
e110a4a17941: Download complete
f163814c49d5: Verifying Checksum
f163814c49d5: Download complete
b913866af336: Verifying Checksum
b913866af336: Download complete
d75e7518fabf: Verifying Checksum
d75e7518fabf: Download complete
e110a4a17941: Pull complete
036d164b66be: Download complete
036d164b66be: Pull complete
f163814c49d5: Pull complete
d75e7518fabf: Pull complete
b913866af336: Pull complete
Digest: sha256:ad59550162c6953af76795a48ae4777b048e3e8be829d8ed5091e1524c8cfe16
Status: Downloaded newer image for jtarchie/pr@sha256:ad59550162c6953af76795a48ae4777b048e3e8be829d8ed5091e1524c8cfe16

Successfully pulled jtarchie/pr@sha256:ad59550162c6953af76795a48ae4777b048e3e8be829d8ed5091e1524c8cfe16.

DEPRECATION: Please note that you should update to using `version: every` on your `get` for this resource.
Cloning into '/tmp/build/get'...
fatal: Refusing to fetch into current branch refs/heads/master of non-bare repository
/opt/resource/lib/in.rb:36:in `block in <main>': git clone failed (RuntimeError)
    from /opt/resource/lib/in.rb:34:in `chdir'
    from /opt/resource/lib/in.rb:34:in `<main>'

[jtarchie]

Gah!

[jtarchie]

I don't understand. I've tested this with the reproducing instructions you provided. This is the PR I am testing with.

Do you see anything wrong with the PR?

[jtarchie]

The reproducing steps from the in step for the previous step would be.

git clone --depth 1 [email protected]:jtarchie/pullrequest-resource.git testing
cd testing
git submodule update --init --recursive
git fetch -q origin pull/34/head:pr-34

Any insight in the git repo you have would be great or minimal steps to reproduce.

Sorry the above is stream of thought.

[nathgs]

I can't reproduce the error when following those commands on my repo. It seems something different is actually happening when running the v15 of the pullrequest-resource in my pipeline.

Sorry I don't have time to investigate right now, but when I next get a chance I'll try building it myself and see if I can isolate it better for you. Might be a couple of days before I can do this.

In the meantime perhaps setup a pipeline to reproduce?

[jtarchie]

(╯°□°)╯︵ ┻━┻

I've recreated the issue with a pipeline. When I hijack the container and run the exact set of commands, it works fine. 👎

This is going to take some more debugging.

Capturing the .git/config here for later reference:

[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "origin"]
        url = https://github.com/jtarchie/pullrequest-resource
        fetch = +refs/heads/master:refs/remotes/origin/master
[branch "master"]
        remote = origin
        merge = refs/heads/master

[vito]

@jtarchie What I've observed is that when there's a PR made to master, the PR resource instead checks out master of the origin repository, rather than the fork. This probably applies to all branches, not just master, if the names match on the origin and the fork.

[jtarchie]

@vito I understand conceptually, but I don't know what to do. You think this problem would occur with people checking out PRs locally, but Google is pretty barren on this.

[jtarchie]

@nathgs, it turns out I did solve it. It's just I forgot to rebuild the docker image and push that out. Woops. Please try again.

[nathgs]

@jtarchie It's working now. Thanks for following up on this and getting it sorted so quickly!

Time to make a pipeline that automates the releasing of the pullrequest-resource docker image ;-)