Appveyor builds of a GitHub PR fails to detect target url

[distantcam]

I'm having a problem where GitLink won't correctly identify the target url when building from a PR from GitHub.

I tried reproducing this bug locally, but was unable to. The only way I can reproduce it so far is by having Appveyor build the branch.

I have set up a test repo for all of this.

• https://github.com/distantcam/AppVeyorTest
• Create readme.md distantcam/AppVeyorTest#1
• https://ci.appveyor.com/project/distantcam/appveyortest/build/1.0.24

[GeertvanHorrik]

Thanks for reporting. Does AppVeyor create a local .git directory or does it only push the actual source files to the build agent?

[distantcam]

I'm pretty sure the local .git directory is there. GitVersion works fine with AppVeyor.

[GeertvanHorrik]

Weird, it uses the same core library. Going to push some PR's to check a few things. Please keep the repository alive for a while.

[gep13]

@GeertvanHorrik @distantcam to test out what is going on, you should be able to use this:

https://www.appveyor.com/docs/how-to/rdp-to-build-worker/

To RDP onto the AppVeyor build worker, and look around at the files that it has, doesn't have.

[GeertvanHorrik]

Great additional info @gep13 , thanks.

[GeertvanHorrik]

We have some more info after adding more logging.

         Found git directory at 'C:\projects\appveyortest\.git', opening repository
         Automatically determined sha 'cb240b6ef5280f367c445251abe28be26f0f4f3c'
         Automatically determined branch ''
         Target url is missing

[GeertvanHorrik]

Going to add some more logging, we probably need to retrieve additional info to determine the branch.

[GeertvanHorrik]

"Current branch is '(no branch)', remote: '', isDetachedHead: 'True'"

[GeertvanHorrik]

I'm not at home right now, will look into this tomorrow, should be fairly easy to fix, then will release 2.4.0 stable at the end of the day.

[GeertvanHorrik]

I can reproduce locally after using these commands:

git clone -q https://github.com/distantcam/AppVeyorTest.git C:\projects\appveyortest
git fetch -q origin +refs/pull/2/merge:
git checkout -qf FETCH_HEAD

Looking for a fix now.

[GeertvanHorrik]

After some debugging, I can get to this url with some private reflection:

https://github.com/distantcam/AppVeyorTest/refs/pull/2/merge

It doesn't give me access to the actual raw files yet though. If anyone has ideas, I am happy to hear about them.

[GeertvanHorrik]

We might just ignore PR's when using GitLink?

[AArnott]

Is the problem the fact that it's a detached head? If so, the enhancement I've been working on in my fork will resolve that as it never cares about the branch name.
If the problem is that the commit ID is a (provisional) merge commit that may not exist for long, that's a more interesting problem. But then, a provisional merge commit in a PR build doesn't need a PDB that lasts too long either. So I suspect it will work out fine.

[GeertvanHorrik]

Yes, I think v3+ will work fine (as long as you pass in the url, which would otherwise automatically be determined by GitLink itself).

However, we should think about the core issue: do we want to support PR's? The PR's are mostly temporary merge actions and the source lives inside the repository of the person making the PR. I couldn't find a way to retrieve the merged result from the original repository. I think the result of a PR (the build) will hardly ever be deployed to a nuget server for people to step through the code, except for maybe very special exceptions where the reviewer wants to test a build.

In that case I think it's fair to expect from the reviewer that he/she just clones the PR and tests it locally.

[AArnott]

I suspect different git hosts will make this more or less difficult. VSTS for example makes accessing the source code behind a PR merge commit as easy as any other commit (provided you have the commit ID). If GitHub makes that harder, that might not be enough to avoid the feature altogether. But then, VSTS requires authentication to download and that breaks source servers (I think) so perhaps we have no working example at all...

As for the use case of indexing PDBs from PR builds, I'll just offer from my own team's experience, we do sometimes share builds out with others based on a PR build validation for them to sign off on. And if they (or us, during our own validation) hit issues that we want to debug into, having the PDBs index directly into the merged source code would be very helpful. Even if we had a local clone on the box, we don't have a merge commit, nor have it checked out. We'd have to carefully recreate the merge between exactly the same two commits, and check that out (oh, and stash our current work...) in order for the source code to match the PDB. But if this were all indexed, we'd just get the source code that matches automatically. :)

[GeertvanHorrik]

Ok, let's keep this open and try to fix this in v3+.

[GeertvanHorrik]

@distantcam can you try 3.0 stable and see if the issue is now resolved?