Skip to content

Support for Visual Studio Online Build vNext - VSO Build Step #562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 21 commits into from

Conversation

clairernovotny
Copy link

This is an implementation for #561. It should be done and has been tested with CI builds, manual builds and pull request builds.

Needs code review.

@clairernovotny
Copy link
Author

Not sure about that build failure- - it's an access denied error on the copy of GitVersion.exe as part of the build?

Anyway, @JakeGinnivan @pascalberger, I think this is ready for you to take a look at. I could not get a git fetch working from within GitVersion due to not knowing the right temp session creds, but I was able to powershell my way around it, I think...

I've tested this with regular CI builds, pull requests and manual queue builds. You'll need to use the tfx tools to upload the build\GitVersionVsoTaskBuild directory to your VSO instance until the VSO gallery is available. We can also ask the VSO team if this can get published another way somehow before that.

@clairernovotny
Copy link
Author

The docs/video on how to install TFX are here: https://github.com/Microsoft/tfs-cli/blob/master/docs/buildtasks.md. Basically npm install tfx-cli -g, then you can do tfx login and then tfx upload and point to the dir.

Delete doesn't seem to work well due to agent caching of the tasks, so I've just incremented the patch version every time I make a change and re-upload it. That won't be necessary for the real package, just for dev.

@pascalberger
Copy link
Member

Thanks!

Why all the handling of git branches in the PowerShell script? As mentioned by @JakeGinnivan in this comment I understand this should already happen inside gitversion.exe.

Do you have deliberately added all the changes from #560 again? Doesn't make much sense IMHO. I would keep this separated (#560 with the necessary changes in gitversion and this PR with only the VSO build task)

## Basic Usage
In [Visual Studio Online](https://www.visualstudio.com/) build vNext (the web based build system) you can add a build step as follows:

* **Build Step:** Command Line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why command line when this PR is about a custom build step?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's old and needs to change. That refers to using GitVersion.exe as a standalone. That won't get past the detached head issues that the VSO build step does. I'll update the docs.

@clairernovotny
Copy link
Author

So the reason all of the changes are there is that it's a branch off of a branch. I needed my fixes there to continue my work here. This is the only PR that should be needed; the other one can die.

The reason I had to do it all in PowerShell is that the C# code relies on authentication and VSO needs per-session tokens. I've pinged the team but have not gotten any response yet if any usable username/password is even available for use by the task to do the fetch/pull. This powershell seems to work so the fetch isn't needed. VSO does a full clone but does the checkout of a commit leaving the detached head. My script recreates the local branches and then sets the selected branch at the designated commit.

* **Tool:** `GitVersion.exe`
* **Arguments:** `/output buildserver /updateassemblyinfo true`

Then in your build parameters simply [add a placeholder](#nuget-in-teamcity) of the GitVersion variables you would like to use.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like copy/past from TeamCity documentation and not accurate for this PR

@pascalberger
Copy link
Member

OK, I see the point about the authentication. But this would also mean that the NuGet package cannot be used successfully with VSO? Maybe this would also be useful information to add to the docs.

@clairernovotny
Copy link
Author

Correct.... for now, this VSO build step is the only way to get it working.... unless someone else runs a similar script before running the NuGet package version.

The GitVersionTask would work too, long as this build step runs first as it'll prep the local clone.

@pascalberger
Copy link
Member

Just thinking if it maybe would make sense to split the task into two tasks:

  • One for the branch handling
  • One for running gitversion.exe

Like this for NuGet or GitVersionTask scenarios only the branch handling task can be included. Not that executing gitversion twice will do much harm, but it might clutter the logs.

What do you think?

@clairernovotny
Copy link
Author

Maybe... but then people would need to remember to add two tasks/build steps to their build definition. There's something to be said by keeping it simple. Just drop this one step in and it'll do whatever it needs to just work.

@pascalberger
Copy link
Member

OK, thats a point :)

@clairernovotny clairernovotny changed the title Vso build step Support for Visual Studio Online Build vNext - VSO Build Step Aug 11, 2015
@johnkattenhorn
Copy link
Contributor

This is awesome work guys, I dock my hat to you all! What's the word Jake ? Is this good to go :-)

@JakeGinnivan
Copy link
Contributor

Nearly, have a branch at https://github.com/JakeGinnivan/GitVersion/commits/vso-build-step which builds on this PR.

I want to figure out how I can get PR's detected properly in GitVersions branch normalisation. Tomorrow. But for now I am heading to bed :)

@johnkattenhorn
Copy link
Contributor

Cool, I can't wait, I'll be a test subject if you want @JakeGinnivan when your ready as I'm working on CI over the next couple of days.

@JakeGinnivan
Copy link
Contributor

Feel free to pull down my branch and give it a spin.

I reckon I'll get this merged tomorrow and released on the weekend. Was great work by Oren to get it to this point so don't want to delay it.

Sent from my Windows Phone


From: John Kattenhornmailto:[email protected]
Sent: ‎13/‎08/‎2015 10:33 PM
To: GitTools/GitVersionmailto:[email protected]
Cc: Jake Ginnivanmailto:[email protected]
Subject: Re: [GitVersion] Support for Visual Studio Online Build vNext - VSO Build Step (#562)

Cool, I can't wait, I'll be a test subject if you want @JakeGinnivanhttps://github.com/JakeGinnivan when your ready as I'm working on CI over the next couple of days.


Reply to this email directly or view it on GitHubhttps://github.com//pull/562#issuecomment-130697439.

@JakeGinnivan
Copy link
Contributor

Ok, so there is a slight issue with the integration.

VSO does not force merge commits. Meaning the reason pull requests do not detect is because the branch is fast forwarded so the HEAD which is building matches an actual branch, so we never go searching for a pull request ref.

The fact that VSO doesn't force merge commits might have some other issues (multiple heads pointing at the same commit etc). I need to investigate. I have also reached out to some MS people to see if it's planned to allow forcing merge commits on branches.
I will still merge/release this and just put some notes up for things to look out for. I might even try and change the logic in normalisation to cater for fast forward merges in PR merge targets.

@dazinator
Copy link
Member

Could we add some logic to check for multiple heads tips at the same commit, in order to check for these fast forward merges? It would be nice to handle fast forward merges - for the instances where people (or VSO) don't use no-ff

@JakeGinnivan
Copy link
Contributor

I do have some logic, but it is still non deterministic. There is enough logged that when it blows up, it gives us enough context of the scenario that we can figure how to improve it.

Sent from my Windows Phone


From: Darrellmailto:[email protected]
Sent: ‎14/‎08/‎2015 6:57 PM
To: GitTools/GitVersionmailto:[email protected]
Cc: Jake Ginnivanmailto:[email protected]
Subject: Re: [GitVersion] Support for Visual Studio Online Build vNext - VSO Build Step (#562)

Could we add some logic to check for multiple heads pointing to the same commit, in order to check for these fast forward merges? It would be nice to handle fast forward merges - for the instances where people (or VSO) don't use no-ff


Reply to this email directly or view it on GitHubhttps://github.com//pull/562#issuecomment-131075412.

@clairernovotny
Copy link
Author

I think I have a solution but it's VSO specific. VSO passes the branch name in BUILD_SOURCEBRANCH and the commit id in BUILD_SOURCEVERSION.

The source branch is passed in the form refs/heads/foo, refs/heads/features/bar, etc for normal branches. For PR's it's refs/pull/<id>/merge. It's not a real/committed branch.

The workaround is to see if the specified source branch matches any actual branches and create it if not. I did have that part in my script but it could be done in code too. Whether the branch exists or not depends on if there are multiple merge builds and whether "clean" is set in the VSO repo options. Clean for Git nukes the .git directory, not-clean preserves it but nukes all the files and just re-checks-out. In the case there was already a branch, I did a reset --merge to the commit id to ensure it was at the right place after checkout.

Maybe the BuildServerBase class and IBuildServer interface could have a PrepForNormalization() method where agent-specific pre-normalization code like this could run? The data can be pulled from the environment variables and local branches created for PR's based on the commit before the regular normalization happens?

@JakeGinnivan
Copy link
Contributor

See #571

@johnkattenhorn
Copy link
Contributor

So not off a great start, I can't even get tfx-cli to run at all. I assume the I needed npm installed in order to grab the tfx-cli package as I found the installation instructions https://github.com/Microsoft/tfs-cli.

I've tried running npm install from command prompt as both administrator and non-administrator and the install runs but then running tfx after the install gets me 'tfx' is not recognised.

I'll log an issue at the tfs-cli repo and see what I've missed.

@pascalberger
Copy link
Member

@johnkattenhorn Have you installed tfx-cli globally: npm install -g tfx-cli?

@johnkattenhorn
Copy link
Contributor

Yep, tried global, non-global all the same from Windows 10 Command Prompt and Admin, Non-Admin, Powershell.

I had this type of problem before with node and npm but don't know enough about npm to debug. I can see a node_module in my roaming profile but nothing in the path and no .exe / .cmd etc.

@johnkattenhorn
Copy link
Contributor

Here is a screenshot.

image

@johnkattenhorn
Copy link
Contributor

Someone helping me on tfx-cli, I'll report back shortly. I can pull the latest straight from master now can't I as the latest bits are merged right ?

@pascalberger
Copy link
Member

Strange. Works fine for me on Windows 10. After global install of the tfx-cli module you should see the module installed in \npm\node_modules\tfx-cli\ (it contains no exe or cmd)

You can download the latest version from the master branch from the AppVeyor build.

@johnkattenhorn
Copy link
Contributor

Bingo, seems that the installation of node from a full VS2015 install doesn't add the NPM directory to the path.

Doing a
set PATH=%PATH%;C:\Users\johnk\AppData\Roaming\npm

Does the trick, thanks @pascalberger, i'll pull from there.

@johnkattenhorn
Copy link
Contributor

Investigating now, I think I remember @JakeGinnivan post something about the libraries.

image

@pascalberger
Copy link
Member

Maybe this is related to #573. I have only tested yet with earlier versions of the VSO task where I haven't experienced this error.

@johnkattenhorn
Copy link
Contributor

Looks like it could be @pascalberger, good find! @SimonCropp, @JakeGinnivan what do you think ? Should we be modifying the VSO BuildTask to include the external dependencies ?

@johnkattenhorn
Copy link
Contributor

Yep, if I rebuild from source then I get a Nativebinaries directory under the GitVersionVsoTaskBuild so I'm going to attempt to upload this custom task over the top of the existing.

@johnkattenhorn
Copy link
Contributor

Yep that seems to be working fine, now to put together a custom build step for Octopus Packing ....

@SimonCropp
Copy link
Contributor

@johnkattenhorn yeah i think i fixed that on a later commit. the native binaries should now be shipped with all the packaged things. let me know if any cant find the dependencies

@clairernovotny clairernovotny deleted the vso-build-step branch August 19, 2015 01:47
@pascalberger
Copy link
Member

@SimonCropp I get the same exception with current head version. I created #589 for this.

@SimonCropp
Copy link
Contributor

@pascalberger not the same exception as in 589. the one here is about not being bale to find the libgit native files. 589 is about git.exe

@SimonCropp
Copy link
Contributor

@pascalberger sorry i stand corrected. i see u have two errors there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants