Skip to content
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

Build scripts for Packaging and Publishing GitVersion #1485

Merged
merged 13 commits into from
Oct 10, 2018

Conversation

arturcic
Copy link
Member

This is a PR for the issue #1445.

Some details about the scripts - inspired by the work done in the cake-build project.
There is a powershell core script - bootstrap that runs on all platforms - run.ps1
Added support for building on Azure Pipelines for all 3 platforms
The run.cake script is the one covering the build/testing/packaging and publishing.

The unit tests run on each of the platforms both dotnet test as well as using NUnit.

The Build/ Test and Packaging run on each build on each platform so that we could actually publish from any build server.

The Publish is currently enabled from AppVeyor.

There are 2 type of releases - from GitTools/GitVersion repository master branch. If it is tagged - the release is a stable release. If it's not tagged it's a pre-release.

The pre-release artifacts are published only to AppVeyor/Azure Pipelines. For the Stable release it's also published from AppVeyor to DockerHub, Nuget.org, Chocolatey.org, rubygems.org, and from Travis the Docker images. I decided to enable the Docker build and publish only if it's a stable or pre-release as it takes a lot of time to build them on windows - ~ 30 mins

I have tested pushing for all packages (using self hosted proget server) except for TFS.

There are 4 Docker images -
2 Windows images - one based on nano with dotnet core runtime image, one based on windowsservercore with full framework 4.7.2
2 Linux images - one based on dotnet core runtime image, one based on mono.

There are several required environment variables that we need to update with corresponding values:

GITHUB_TOKEN
DOCKER_USERNAME
DOCKER_PASSWORD
NUGET_API_KEY
NUGET_API_URL
CHOCOLATEY_API_KEY
CHOCOLATEY_API_URL
TFX_TOKEN
RUBY_GEM_API_KEY

There are also some optional env variables that act as flags to disable some parts of the build script:
DISABLE_UNIT_TESTS
DISABLE_PUBLISH_GEM
DISABLE_PUBLISH_TFS
DISABLE_PUBLISH_NUGET
DISABLE_PUBLISH_CHOCOLATEY
DISABLE_PUBLISH_DOCKER

Currently the README.md file has a badge for Azure pipelines build status that needs to be updated with the GitTools team's Azure pipelines url when it's set up.

I copied the nuspec files to the root of the repo as well as I have moved the code for ruby gems to a separate folder.

I kept the old build scripts so that you can compare them side by side with the new one.

I wasn't able to test the push of TFS package so I have included the code but it's not reachable at the moment. I'll need some assistance on that.

Please have a look and review it. Let me know if it covers all the scenarios and all the comments are welcomed.

@JakeGinnivan @dazinator @asbjornu @gep13

Copy link
Member

@dazinator dazinator left a comment

Choose a reason for hiding this comment

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

Great stuff.

.travis.yml Outdated Show resolved Hide resolved
run.ps1 Show resolved Hide resolved
run.ps1 Show resolved Hide resolved
@arturcic
Copy link
Member Author

I'll have to check why the Gem packaging is failing when it's a pull request - it's somehow related to the version generated when it's a pull request

@asbjornu
Copy link
Member

This looks like a huge improvement over the current build. Would it be possible to have prereleases auto-published to NuGet? Otherwise I think this looks great. Thanks for the work you put into this, @arturcic! Is there anything I can do to help?

@arturcic
Copy link
Member Author

arturcic commented Oct 1, 2018

Actually you can help - please review which publish tasks should be included when pre-release and when stable release, so that the appropriate criteria is applied. And would be great if you or who has access to AppVeyor account set the correct values for the required environment variables so that we can start testing/fixing the publish tasks.

Would be great if you can also setup a GitTools team account on Azure Pipelines so that the build happens on that CI as well.

@asbjornu
Copy link
Member

asbjornu commented Oct 1, 2018

@arturcic: Excellent. I'd like every package repository that supports unstable/pre-releases to be included when we do pre-releases. I don't know much about other package repositories than NuGet and Homebrew, so I can't say authoritatively which repositories should and shouldn't be included other than NuGet, since it supports it. Homebrew does not, so it should be excluded.

I have access to AppVeyor, but I don't have access to Docker, NuGet, Chocolatey, TFX (what's that?) or RubyGems, so I can't generate new credentials. I'm also not entirely sure how to generate a GitHub auth token properly. What access does AppVeyor need and who should own the token?

I've created an Azure DevOps account for GitTools and started setting up a pipeline, but stopped at the template step as I'm unsure whether the suggested ".NET Desktop" template would be a good fit. Perhaps we could hash this out on Gitter to speed up the interaction?

@JakeGinnivan
Copy link
Contributor

If you DM me on twitter I can probably grant access to anything. This is awesome, thanks @arturcic

We should push to docker as a tagged image, and only update latest on stable

@arturcic
Copy link
Member Author

arturcic commented Oct 1, 2018

I have access to AppVeyor, but I don't have access to Docker, NuGet, Chocolatey, TFX (what's that?) or RubyGems, so I can't generate new credentials. I'm also not entirely sure how to generate a GitHub auth token properly. What access does AppVeyor need and who should own the token?

tfx-cli is the tool used for packaging/publishing to Visual Studio marketplace, https://github.com/Microsoft/tfs-cli, so basically it's a task that can be used in TFS or Azure DevOps

@arturcic: Excellent. I'd like every package repository that supports unstable/pre-releases to be included when we do pre-releases.

As far as I know, Nuget, Chocolatey, RubyGems support pre-release, not sure about VS marketplace.
Docker has tags that we can use for pre-releases as well. Regarding Homebrew, we don't support that yet, I was planning to add that after this is merged

@arturcic
Copy link
Member Author

arturcic commented Oct 1, 2018

We should push to docker as a tagged image, and only update latest on stable

Agree. I was also thinking to add the multi-arch manifest creation for Docker, but unfortunatelly it is not supported on any of the build servers.

@kaylangan
Copy link

I'm a Program Manager on Azure Pipelines. Let me know if you have any questions or suggestions. I'm happy to help!

@arturcic
Copy link
Member Author

arturcic commented Oct 1, 2018

I'm a Program Manager on Azure Pipelines. Let me know if you have any questions or suggestions. I'm happy to help!

Good to know, thanks

@gep13
Copy link
Member

gep13 commented Oct 1, 2018

@arturcic this is an AWESOME pull request. I have only had a very quick look through the changes, but everything seems legit to me.

@JakeGinnivan were you able to setup access to everything that is required? If not, let me know what is still outstanding, and I can take a look.

tools/packages.config Show resolved Hide resolved
build/utils.cake Show resolved Hide resolved
build/utils.cake Show resolved Hide resolved
nuspec/chocolateyInstall.ps1 Outdated Show resolved Hide resolved
nuspec/chocolateyUninstall.ps1 Outdated Show resolved Hide resolved
run.cake Outdated Show resolved Hide resolved
@arturcic arturcic force-pushed the feature/build-scripts branch from 6343193 to e73b7ca Compare October 2, 2018 16:56
@arturcic arturcic force-pushed the feature/build-scripts branch 2 times, most recently from 982ed33 to 122add2 Compare October 3, 2018 18:30
…ranch and not PR, reviewed the versioning of artifacts
@arturcic arturcic force-pushed the feature/build-scripts branch from 122add2 to fd0c83a Compare October 3, 2018 18:53
@gep13
Copy link
Member

gep13 commented Oct 4, 2018

@arturcic thanks for making those changes!

@gep13
Copy link
Member

gep13 commented Oct 4, 2018

So, all in all, this looks good to me, an EPIC PR from @arturcic.

@asbjornu @dazinator @JakeGinnivan you guys ok with this getting pulled in?

@arturcic
Copy link
Member Author

arturcic commented Oct 4, 2018

Quick update on this

  • fixed the versioning schema for artifacts, it remains to fix the tfs versioning as it does not acccept pre-releases
  • tested the publish to Docker (you can check the dockerhub of gittools). After the merge of this PR I'll update the descriptions of the docker images on dockerhub to point to dockerfiles from github
  • tested the publish to my own nuget/choco server
  • tested publish to rubygem (https://rubygems.org/gems/gitversion there is already published)

Right now I'm finilizing the TFS publishing so could you wait with the merge till I finish with the changes @gep13? Hopefully today I'll have everything done.

@gep13
Copy link
Member

gep13 commented Oct 4, 2018

@arturcic said...
Right now I'm finilizing the TFS publishing so could you wait with the merge till I finish with the changes @gep13? Hopefully today I'll have everything done.

Sounds good to me!

@asbjornu
Copy link
Member

asbjornu commented Oct 4, 2018

Big 👍 from me!

@arturcic
Copy link
Member Author

arturcic commented Oct 4, 2018

I have tested the changes, the only thing that is not working - Publish-TFS task - is because of the personal access token expired when trying to publish to visual studion marketplace. There is a flag on appveyor that allows disabling the publishing for now for TFS till we get the new personal access token in place, so that we can merge the PR and see how the new scripts work. I have tested the Publish-TFS with a different name of the extension for testing purposes and it was working.

So basically the PR is ready for merge.

kll pushed a commit to kll/GitVersion that referenced this pull request Oct 4, 2018
It is easy to forget to keep it updated. Case in point the version it was pinned to is already EOL from MS. It also makes it harder for developers to build because they have to go manually install an old version of the SDK.
It is pinned in better ways on the build servers starting with PR GitTools#1485
@dazinator
Copy link
Member

@arturcic does this change sit ok with you: 2cf0fb1

@arturcic
Copy link
Member Author

arturcic commented Oct 4, 2018

Yeah I guess it is ok for the time we merge that branch. We'll just add the version of the sdk directly in the bootstrap script.

@kll
Copy link
Contributor

kll commented Oct 4, 2018

I went and actually read the docs now and I think we can just specify the major.minor version to pin us to the 2.1 series which is LTS and probably a reasonable thing to do without getting tied to a specific release that quickly gets out of date. If it works like I think is that an agreeable middle ground?

@dazinator
Copy link
Member

@kll that sounds good.

kll added a commit to kll/GitVersion that referenced this pull request Oct 4, 2018
@kll
Copy link
Contributor

kll commented Oct 4, 2018

Never mind. It definitely did not work like I thought it did. I also didn't realize this PR was reading in the contents of the file as part of the new build process so I've just added it back with a matching version number. Even with a specific version number it looks like it rolls forward on the patch version automatically which prevents needing to go find an old exact version number to make it build. Hopefully it's kept up to date because I rarely have old versions of the SDK installed. :)

@jmezach
Copy link

jmezach commented Oct 10, 2018

Is this still waiting for something in particular, or is this ready to merge?

@asbjornu asbjornu merged commit e32ff6c into GitTools:master Oct 10, 2018
@asbjornu
Copy link
Member

@jmezach: This is such a giant leap in the right direction that regardless of what's missing, I'm going ahead and merging this. Thanks for the poke!

@asbjornu asbjornu mentioned this pull request Oct 10, 2018
14 tasks
@arturcic
Copy link
Member Author

Thanks for merging the PR, I'll check if the build for the merged code is publishing correctly and fix the issues. Still the TFS publishing is not working because we need a valid personal access token (PAT) for publishing to visual studio marketplace. Hope @JakeGinnivan will have time to have a look and provide a valid PAT

@asbjornu
Copy link
Member

@arturcic: Please poke @JakeGinnivan on Gitter as he doesn’t have capacity to stay on top of GitHub issue notifications. 🙏 😄

Copy link
Contributor

@kll kll left a comment

Choose a reason for hiding this comment

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

I know there was a good reason to keep the old build scripts in place for reference but it is a bit confusing. I vote to clean them up because the old ones can always be viewed in the git history.

run.ps1 Show resolved Hide resolved
run.ps1 Show resolved Hide resolved
@arturcic
Copy link
Member Author

I think I'll remove or probably rename back to build.cake and build.ps1 after the #1445 is closed

@arturcic arturcic deleted the feature/build-scripts branch October 13, 2018 06:43
@asbjornu asbjornu mentioned this pull request Jan 22, 2019
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.

8 participants