-
Notifications
You must be signed in to change notification settings - Fork 472
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
Move builds from TeamCity to AppVeyor and Travis CI #241
Comments
@Fir3pho3nixx how do you think we should set this up? |
Moving over the new tooling is a good idea. It gives you NuGet and AssemblyInfo built into the MsBuild which is really handy. You really don't want to break the old infrastructure whilst moving stuff across. For this I would recommend having two solutions.
Old TeamCity implementation.
Shiny new AppVeyor(Windows) + TravisCI(MacOS, Ubuntu 14 LTS) version with GH status checks. PR0: Move all the stuff in buildscripts into a deprecated folder but make sure that castle core build still passes on teamcity. This will make way for the new stuff. PR1: Get the VS2017 project system building on AppVeyor using new tooling. Build only with DesktopCLR tests. Test parallelisation will start highlighting I/O collisions with ModuleScope for net35, net40, net45. PR2: Get mono builds going on AppVeyor. This is important for packing NuGets but we might want to think about running this on Travis too. Up for grabs really. PR3: Get tests passing for dotnet core on AppVeyor. We have the option of upgrading netstandard here but it currently introduces the conundrum of moving the tests over to xunit. Might need to help the nunit guys out if we dont wan't to migrate(Or not upgrade). PR4: Get nuget publishing going from master branch only (merge to master = deploy). With GH integration for publishing releases(both draft and full). Might want to manually de-list the output for this, as it is not really a release of any kind. PR5: Get tests passing for dotnet core(and potentially Mono) on TravisCI. This will yield some fun with getting libunwind to install on Ubuntu (Trusty Tahr). Luckily a solved problem for me. PR6: Clean up and cut over. Do you think we should consider adjusting this intended PR list to include the embedding of a proper build task framework(CAKE, FAKE, RAKE)? |
I thought we'd just create a branch and start by deleting the old project files, and leaving master with TeamCity until we are ready to merge everything?
I haven't had a chance to read in detail what the problem is here, but happy to work with you to sort this one out when the problem shows up in the build.
We've only supported Mono on Linux+macOS not on Windows in the past, there was one point the Mono guys weren't even shipping a Windows build anymore. There really is little point for us to even look at Mono on Windows, no one uses it.
Not sure what you mean here, but I think we should stay supporting .NET Standard 1.3 for now to support the widest of targets. Why would we need to move to xUnit.net for 1.6?
I thought we'd drive NuGet.org publishing from tags. i.e. you tag "v1.2.3" in Git then AppVeyor builds and publishes "1.2.3" to NuGet.org and GitHub Releases tab. I'd prefer not to pollute NuGet.org with every single build from master, unlisted or not.
We can look at it if it makes things heaps easier, but it seems the new .NET tooling takes away most of the craziness we've had to do in the past, some things we currently do in the build scripts isn't needed anymore. Would like to see if we can do it with a small shell script and the .NET CLI tooling. |
Think the latest dotnet cli has trouble discovering tests for NUnit using their adapter. Moving over to xunit got tests to run easily but admittedly was the long way round. This might not be a problem, lets see how we go. As for NuGet publishing and GH Tagging, AppVeyor can drive both. For changing the workflow to use tags as opposed to commits would probably need some more cfg listed here(but totally doable): https://www.appveyor.com/docs/branches/#build-on-tags-github-gitlab-and-bitbucket-only. Get a branch going so I can start deleting the old build stuff and implement the new projects, whilst we decide how the appveyor build, test and publish workflow should work. |
I've created the branch Do you have a link to the NUnit problem in their issue tracker? |
Stumbled upon: nunit/nunit-console#10 |
There are a few issues, with embedded resources. Will fix this next. |
Ok, only 6(edited!) tests failing for net45. Almost ready to setup the netcoreapp1.1 tests. Here is my simulated PR: https://github.com/fir3pho3nixx/Core/pull/3 Notice how status checks kicked in for me once I merged all the appveyor stuff into my default branch(master)? Also noticed appveyor was having issues cloning from GitHub: It went from this: To this(with a whitespace commit): |
Very close to achieving PR1. |
@Fir3pho3nixx Awesome work! Glad to see the rapid progress! @jonorossi and @Fir3pho3nixx please let us (my teammate @alinapopa and me) know if we can help on anything. |
Made all the tests pass on appveyor for net45. Here is my simulated PR: https://github.com/fir3pho3nixx/Core/pull/3 Actual PR: @jeremymeng, @alinapopa would you be willing to start raising PR's on Castle Windsor using the same build technique I am using here in Castle Core? I will jump down from here once I have the tests passing on TravisCI and NuGets publishing using GH Tags. This would set me up nicely for the migration to dotnet core properly on Castle.Windsor. |
We are happy to. We should be able to start early next week. |
I've stuffed around for ages trying to get AppVeyor to work with this repo, however GitHub just won't fire any webhooks (after the initial creation one). Webhooks work great on my personal fork, but also don't fire on https://github.com/castleproject/core-fork-for-webhook-test. I've contacted GitHub support to try to get to the bottom of it. |
Just got a response from GitHub, below is the important snippet:
Many years ago hammett had a paid plan for this GitHub organization which should have been downgraded, I don't remember why but after GItHub changed this flag webhooks are working now, they fired on that fork repo I created and are working for a tiny commit a just made to this repo. |
Confirmed. Status checks are working: #247 |
** Edited for TL;DR ** To close out PR1, I created a branch that enables build output for all desktop clr framework monikers(net45, net451, net452 etc) and ran the tests in a single nunit console command quite a few times. Every now and then I would hit I/O collisions around 'CastleDynProxy2.dll'. The build mostly passed in my PR for appveyor but locally I was getting these failures every now and then:
I know this is a very contrived example but do you think this is anything to be worried about @jonorossi? Will skip working on PR2 and start working on PR3 next. |
I noticed that the output seems to be running the different targets in different directories, it doesn't appear the be the problem I thought you are describing:
|
What I find interesting is that even though the framework folders are different and it passes in isolation for each framework on it's own, it should be able to scale up for multiple frameworks using multiple folders. Why would net461 have I/O collisions in it' own folder? Weird right? |
It looks like the NUnit guys didn't turn on parallel test execution within the same assembly by default in NUnit 3, so it definitely is strange. Have you tried running nunit-console with |
Ran this a fair few times throughout the day on another machine, could not replicate it. Tried it again on same machine that kicked the errors out, all passed. I think what we need here is a brute force long distance test run, something that kicks out a mini dump(or similar), failing test name and some parent process info for when Castle.DynamicProxy.ModuleScope.SaveAssembly(Boolean strongNamed) tries to delete 'CastleDynProxy2.dll' in a test run and fails because of a file lock. Should we make this a separate issue and I keep chasing the new build stuff? |
Yes, a new issue definitely. It shouldn't hold up the build refactor work. |
Rather than submit a crazy PR which does not make sense, I opt for discussion. I am at a point where I can make Castle.Core tests build on dotnet core(targetting netcoreapp1.1 and net462 <- this is arbitrary btw). The good news is that it is entirely possible. The bad news is that all of the build symbols(flags) that are applied in the 'production code' (Castle.Core.dll) have to be applied to the tests(Castle.Core.Tests.dll). Can we start with a series of PR's that address this? This does mean we have to change code in tests to make this work. |
@Fir3pho3nixx what issue are you seeing? Previously the test project only defines one additional symbol. |
Thanks @jeremymeng, that does help avoid things like this: https://github.com/fir3pho3nixx/Core/commit/6942119ad991b43e16ea1284bf603e4780c9dadd#diff-9a410267b70c7f94379f9901fae787afR55 Problems are mainly around getting anything using the System.Xml.Xsl namespace to compile. Specifically things like CastleTests.Components.DictionaryAdapter.Xml.Tests.MockXPathFunction. Either I am missing a NuGet (which I hope is the case) or tests here need to start honouring the NuGets I am pulling in are here: https://github.com/fir3pho3nixx/Core/commit/6942119ad991b43e16ea1284bf603e4780c9dadd#diff-858b4452841160ed55e2d0a3a92ea51fR32 I ended up doing something really ugly to make it work which was(no future in this): Completely baffled as to how this worked for you guys before! |
I thought you had seen this in project.json for Castle Core 4.0:
https://github.com/castleproject/Core/blob/v4.0.0/src/Castle.Core.Tests/project.json#L45-L50 Rather than stuff around forever with the thousands of unit test files for DictionaryAdapter (which I want to move out of Castle Core anyway and will probably need .NET Standard 2.0 to really work well) I'd just do this for now for the .NET Standard 1.3 build. Since we have a single NLog version now you can exclude NLog too, however log4net should be restored for .NET Standard now that we are using log4net 2.0.8. |
Option 2 is most tenable for now. I am almost done with the TravisCI build (mono/netcoreapp1.1 for linux using the above mentioned method). |
@Fir3pho3nixx have you seen Rob's most recent post about NUnit support: http://www.alteridem.net/2017/05/04/test-net-core-nunit-vs2017/ |
Whoa! I have been waiting for this. Will give it a spin this weekend. Thanks @jonorossi ... 🥇 |
@jonorossi - Our VS2017 project structure now runs both netcoreapp1.1 and net461 tests equally on both windows(AppVeyor) and linux(TravisCI). This means are still good for cutting over and deleting all the old build stuff. It took quite a bit of messing around to get there. I will submit a separate PR with Robs alpha stuff (it uses NUnitLite at the mo). Shall I get the NuGets publishing on tag for AppVeyor? |
@jonorossi - Yes I have, and when it works across all platforms it is going to be amazing :) It is alpha though, and I found it threw exceptions(Win32Exception) whilst trying to spawn new processes on startup for net461 targets on Linux(netcoreapp1.1 passed though). Raised an issue here: nunit/nunit3-vs-adapter#324 Look forward to this being released! I can simplify the msbuild quite a bit, like remove the RuntimeIdentifiers(and NUnitLite) which would be great. |
Sounds good, did you want to create an issue with how you are planning to implement it. |
Happy to have the discussion here. We are nearly done. For fortress merging to master = deploy. Here we are going for a tag/release strategy. We are going to need some setup before we can do this. Look at the fortress appveyor cfg below.
This behaviour can easily be changed by using https://www.appveyor.com/docs/deployment/#deploy-on-tag-github-and-gitlab-only. I would need some help from you setting up the secure keys for this. You can do it easily by following these instructions: https://www.appveyor.com/docs/deployment/nuget/#provider-settings. Email the encrypted key to me and I will bake it in for you with the tag trigger behaviour. Will also do a review of the NuGet packages and replicate the work you and Alina did down in Castle Windsor. |
Last PR in for NuGet publish. #259 @jonorossi - Follow my instructions above and let me know how it goes! :) Before we close out this issue, can we just quickly drop a note for how we want to deal with the legacy build platform? There is significant clean up we could be doing. We just need to weigh up the pros and cons of supporting VS2017--. Mainly PR6 stuff. |
buildscripts\CommonAssemblyInfo.cs has some assembly attributes such as |
@Fir3pho3nixx @alinapopa sorry about the delay, I've been a bit busy the last couple of days. Responding to everything at the moment.
What exactly do you mean, are you concerned that users might not have VS2017 and so won't be able to build the software? I assume VS2017 Community would work in this case since this is an OSS project. |
Do we delete everything and remove the VS2017 fluff from the csproj file names? Meaning do we go 100% VS2017? |
Yes, I'm happy to go all in using the new tooling, really no point beating around the bush since we've nearly got all the old functionality across. Means we also don't have to deal with annoying csproj files that list individual files. If you are happy, I'm happy. |
If it is OK, I am going to start the cleanup process. |
Go for it. |
I have finished the clean up @ #261 CI on TravisCI and AppVeyor achieved! 👍 |
Removing APTCA as per : castleproject/Windsor#248 |
@jonorossi - Just updating the issue with what we found in the PR's. There is an issue compiling a net35 assembly found here: #261 (comment) Tracked By: dotnet/msbuild#1333 In the meantime, I am looking at work arounds to get this going. |
By falling back to msbuild I think we have a tidy way of achieving this: #262 I have added relevant comments. Also seeing correct framework monikers in the locally built NuGet packages. I ildasm'ed the net35 version and can confirm we have the correct runtime now. Another check would not hurt but I think we are good to go for releasing. |
@Fir3pho3nixx I'll have a dig through a diff tomorrow and get this merged if everything looks good. I want to get the last open pull request fixed up before release. |
/CC #266 |
All merged to master, will be in v4.1. |
Awesome! |
These are the build configurations we've currently have in TeamCity with the configuration pulled apart from the set up. It would be great if they all went to using a
build
script rather than calling msbuild/xbuild directly.I'm not sure exactly what we want to do but it might be time to move Castle Core to the new VS2017 tooling to support .NET Framework and .NET Core, this should hopefully make the NuGet packaging cleaner.
TeamCity: http://builds.castleproject.org/project.html?projectId=Core&tab=projectOverview
AppVeyor: https://ci.appveyor.com/project/castleproject/core
Travis CI: https://travis-ci.org/castleproject/Core
.NET 3.5
.NET 4.0
NET40-Release
configuration.NET 4.5
NET45-Release
configuration.NET Core
Mono (4.6.1)
Pack
A custom "Pack" build has two fields you can manually specify:
release_build
-checkbox checkedValue='true' description='Defines whether this build is intended for public release to NuGet (as either a prerelease or final version, rather than a CI build)' display='hidden' label='Release build' uncheckedValue='false'
version_suffix
-text description='Defines an optional version suffix (e.g. -beta001)' label='Version suffix' validationMode='any' display='hidden'
The text was updated successfully, but these errors were encountered: