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

AppVeyor #1508

Closed
Closed

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Dec 27, 2017

I have enabled AppVeyor on NHibernate, but only for branches having the Yaml file.

This PR relies on three commits I have separately PR-ed (#1505, #1506, #1507). (Now merged and rebased.)

This AppVeyor testes all databases tested on TeamCity excepted Oracle, which is not available on AppVeyor images, and for which scripting an installation is a bit challenging.

It also generates the build artifacts within its release job.

Build times are generally shorter than on TeamCity, but with greater variability. Sometimes a job takes more than double the time than usual. The way AppVeyor works, each job uses a clean VM created just for the job, so it will not have issues like those sometimes occurring on the TeamCity build agent due to things like Windows Update.

Currently all the work is scripted in the Yaml file, but it could be split a bit if wished by putting powershell scripts in dedicated files.

We could moreover add a 'beta release' job which would publish pre-release NuGet from merges on master, allowing easier early testing.

TeamCity only supplies the software: the build agent has to be provided. Switching to AppVeyor would avoid having to supply it.

About the Oracle case, the first difficulty is to get an installer for it on the build machine. It should be downloaded from somewhere, but Oracle, as far as I know, does not provide a public accessible URI for such a download.

@hazzik has suggested me three options:

  • Get this URL as a secure AppVeyor variable

    Meaning putting an installer "somewhere", without disclosing publicly its url thanks to AppVeyor encryption. Using the AppVeyor cache feature, we could then even suppress it once it has been cached for all jobs of the matrix (cache is isolated by matrix jobs).

  • Use docker image for Oracle (it will require running docker for ubuntu-on-windows)

    Surely cleaner than the first option. And I guess it would also solve the second likely issue, getting Oracle installed by scripts. Now about Docker, my knowledge is currently limited to knowing that it exists. Well, AppVeyor also seems to have some built-in support for it, included in the image used in this PR.

  • Or we can leave Oracle on the TeamCity.

    Why not, it would at least allow to reduce its "size", and maybe wait for a native support by AppVeyor.

So this PR currently takes the third path.

- db: SqlServer2012

matrix:
exclude:
Copy link
Member Author

@fredericDelaporte fredericDelaporte Dec 27, 2017

Choose a reason for hiding this comment

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

Debug configuration is tested with all databases, release is done only with SQL 2012. (On Teamcity side, this is similar: release is done with SQL 2008, but why not modernizing a bit?)

- git config --global core.autocrlf true

cache:
- C:\firebird\Firebird-3.0.2.32703-0_x64.zip
Copy link
Member Author

Choose a reason for hiding this comment

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

It is downloaded only for Firebird database, and so it is cached only for it. (Cache is not put to on PR by default, so this caching will be in effect only after a build has been done directly from a repository branch.)
This caching improve the reliability of the Firebird installation process: I am not sure the SourceForge URI I have used is permanent. This URI will no more be actually used once the above file is cached.

before_build:
- ps: >-
Invoke-Command -ScriptBlock {
# Package sources if this is a release build.
Copy link
Member Author

Choose a reason for hiding this comment

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

Doing this before building avoids filtering out build artifacts.

mkdir $nugetPath
dotnet msbuild (Join-Path $env:APPVEYOR_BUILD_FOLDER 'src\NHibernate.sln') /verbosity:minimal `
'/p:Platform="Any CPU"' /p:GeneratePackageOnBuild=True /p:IncludeSymbols=True `
/p:IncludeSource=True "/p:PackageOutputPath=$nugetPath"
Copy link
Member Author

Choose a reason for hiding this comment

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

NuGet are generated by each job, but will be published as artifacts only on the release one.
It appears dotnet yields warning on the error output, causing them to appears in red in the AppVeyor console output. But this does not fail the build. It is just a bit annoying.

$configDir = (Join-Path $env:APPVEYOR_BUILD_FOLDER 'current-test-configuration')

#Configuration matrix
$allSettings = @{
Copy link
Member Author

Choose a reason for hiding this comment

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

Here lies all the settings for testing the databases.

switch ($env:db) {
'Firebird' {
# Install Firebird
# Firebird zip is cached, check if we have it, otherwise download it
Copy link
Member Author

@fredericDelaporte fredericDelaporte Dec 27, 2017

Choose a reason for hiding this comment

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

There is a choco package for Firebird too. I have not tested it, and it currently has a failing status. This manual install is not complex, so I have not estimated it was worth trying the choco package.

Add-Content -Path '.\SYSDBA.sql' -Value "create user SYSDBA password 'masterkey';`r`ncommit;`r`nquit;"
.\isql -user sysdba employee -input SYSDBA.sql
# Start Firebird
.\firebird -a
Copy link
Member Author

Choose a reason for hiding this comment

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

Not even installed as a service, just directly run.

@@ -0,0 +1,335 @@
version: 5.1.0.{build}
image: Visual Studio 2017
Copy link
Member Author

Choose a reason for hiding this comment

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

Required for having the dotnet command.

Start-Service 'postgresql-x64-10'
}
'SqlServer2008' { Start-Service 'MSSQL$SQL2017' }
'SqlServer2012' { Start-Service 'MSSQL$SQL2017' }
Copy link
Member Author

@fredericDelaporte fredericDelaporte Dec 27, 2017

Choose a reason for hiding this comment

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

The Visual Studio 2017 image does not have SQL 2008 and SQL 2012, it has 2014, 2016 and 2017. So I have picked up one of them.

$html = (Join-Path $refPackage 'html')
mkdir $html
cd $html
java -classpath $saxonClassPath com.icl.saxon.StyleSheet $masterFile (Join-Path $refFolder 'styles/html_chunk.xsl')
Copy link
Member Author

Choose a reason for hiding this comment

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

These java commands does also yield non-error messages in the error output, which is a bit annoying when watching the log output on AppVeyor.

@fredericDelaporte fredericDelaporte changed the title App veyor AppVeyor Dec 28, 2017
@ngbrown
Copy link
Contributor

ngbrown commented Dec 29, 2017

When I developed an AppVeyor build, I did it with the intent to also be able to run in a Linux docker container so the builds and tests for .Net core could run on Linux with Circle-CI (see fd78581). The same would apply for testing under macOS.

I used Cake so I could use only the .NET Core SDK and didn't need Mono configured.

With no new releases of NAnt, does it still work well with newer versions of Mono?

@fredericDelaporte
Copy link
Member Author

With no new releases of NAnt, does it still work well with newer versions of Mono?

No idea. But NAnt is not used by the AppVeyor build here. All steps have been coded as scripts in appveyor.yaml (mostly powershell scripts), without any dependency on some other scripts in NHibernate repository.

I have not written that AppVeyor build for .Net Core, but as a way to migrate away from TeamCity.

@hazzik
Copy link
Member

hazzik commented Dec 29, 2017

The idea of CI is that it uses the same tools and means as you would use on your machine. Otherwise we will need to rewrite scripts for different CI's. So, the AppVeyor, TC, TravisCI, etc would need to invoke the scripts.

The ultimate goal was to switch away from NAnt to MSBuild because NAnt is not well maintained.

I'm strongly against introducing new unnecessary tools like Cake (I fear that it would share the destiny of NAnt) or PowerShell (for the reasons of duplication).

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Dec 29, 2017

The idea of CI is that it uses the same tools and means as you would use on your machine.

I do not have this view. I never use the same tools because one is a fully automated process while the other is a process with a human in the loop. On my machine, I build and test from an IDE most of the time, not from command line. I am mostly never building doc or packages from my machine, because that is the build server job. Powershell script can still be run on my machine if I need to debug them locally.

Otherwise we will need to rewrite scripts for different CI's

That will always be the case. They will always have some variations requiring some dedicated scripting, like having to start the services we need, installing additional software, adjusting configuration to the way the build agent is setup, ...

PowerShell (for the reasons of duplication)

All the "duplicated as ps script" NAnt stuff could be removed, and replaced by the powershell scripts with some more configuration variables. (Off course, only once TeamCity is dropped.)

But what should be considered duplicated? The teamcity NAnt tasks for tests are not really "machine agnostic", doing a bunch of assumptions on the way databases are installed. Most of them cannot run on my machine due to this. We have the build menu for running tests with the "current test database" configuration instead, which could already be considered as a kind of duplicate of the teamcity NAnt tasks for tests.

@hazzik
Copy link
Member

hazzik commented Dec 30, 2017

(Off course, only once TeamCity is dropped.)

I don't understand how this is related.

@gliljas
Copy link
Member

gliljas commented Dec 31, 2017

I agree with @fredericDelaporte, and I think that aiming for MSBuild only is wrong. There's a reason why so many builds are moving away from declarative "scripts".

Cake is great. It has quite a few rough edges, but in general I find it to be much more convenient than Powershell.

@hazzik hazzik self-requested a review January 1, 2018 01:22
@fredericDelaporte
Copy link
Member Author

(Off course, only once TeamCity is dropped.)

I don't understand how this is related.

It is related because I am not considering migrating the TeamCity build away from NAnt if we consider moving away from TeamCity.

About Cake vs PowerShell, I never tried Cake, so I cannot judge of its advantages. For what we have to do, doing it "only" with PowerShell does not seem over complicated to me (335 lines in the Yaml, which includes all the PS scripts currently). And PowerShell for sure has more backing than Cake, so I am not convinced it is worth introducing Cake here.
Gunnar, maybe you could PR a Cake version on my AppVeyor branch if you wish, to show how things would be changed.

Happy new year to you all by the way.

@ngbrown
Copy link
Contributor

ngbrown commented Jan 3, 2018

@fredericDelaporte This isn't really PowerShell that can be used or tested anywhere else, but PowerShell code embedded in a .yml file. If it is meant to replace the NAnt builds, then there should be .ps1 files that can be ran and tested from outside of the CI server.

Microsoft has released PowerShell to both Linux and macOS, so it is an option, but again the .ps1 files need to exist, and using something like the psake modules would make for a better build experience.

Cake is an alternative to having to install PowerShell and use psake. It works on all platforms that dotnet runs on and can be used in both CI and on developer machines. Of course, even with Cake, there are differences between CI environments, but that is confined to only dealing with the differences instead of having the entirety of the build in a CI specific format.

If you are open to the option, I could expand my current Cake file to do everything that you are trying to do, but for now, it builds and tests on AppVeyor (Windows), Circle-CI (Linux and theoretically macOS), and the command-line. Building documentation wasn't going to be possible on Linux/macOS anyways: fd78581#diff-4d3634747586b964df9f7712bc3ec669

As for using msbuild for providing a single unifying build pipeline for all required tasks on all systems; maybe it's possible, but it certainly seems very obtuse to do. Even Microsoft, who should know the most in how to build entirely with msbuild, uses a lot of .ps1 files for builds: https://github.com/Microsoft/ChakraCore/tree/master/Build/scripts and https://github.com/aspnet/EntityFrameworkCore/blob/dev/run.ps1 (which uses https://github.com/aspnet/BuildTools/tree/dev/files/KoreBuild)

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Jan 3, 2018

This PR is an attempt at moving away from TeamCity, for no more having to provide a build agent. It is not meant to supply a general build solution.

Although in my opinion the current NHibernate NAnt builds are also far from supplying a general build solution too, it is closer to it, having some parts re-used by the ShowBuildMenu.bat. I was not considering this partial sharing of build script as an important asset. But it seems it is, looking at the comments here.

So in this regard this PR is wrong. It cannot simply move away from TeamCity. First a truly general build logic has to be put in place, very likely as a dedicated PR not changing the CI platform at the same time.

I am not much knowledgeable in build systems overall, and moreover, I am not very interested in investing time investigating solutions like Cake, psake or whatever. So it looks like this subject will have to wait for someone else to propose a PR for such a general build solution first.

Update:

In fact I do not believe that a general build solution is the way to go. This AppVeyor build was doable with just a few ps lines of code because it is custom made for AppVeyor.

General build solution needs more complex logic to accommodate for a wide range of environments, thus the need for additional tooling like Cake or psake (or NAnt). And as seen with current NAnt solution, it can be supposedly general but ends up not working for the cases later envisioned. It at least still needs some minimal additional integration with new build platforms. This work may even be heavier to do than writing a full specific build for the new build platform, as the current situation seems to demonstrate. (As far as I know, no attempt at using current NAnt logic elsewhere than on TeamCity has been as far as this current PR about migrating the features (tests + artifacts) of the current TeamCity build.) Moreover, "minimal additional integration" is also a trouble in my view, since this usually implies relying the less possible on the platform specific features, reducing the benefits each platform can bring.

That is why I am on the side of making build logic specific to the build platform. NHibernate currently needs only one CI platform. With .Net Standard compatibility, adding a Linux based build platform would be better for ascertaining its compatibility. But maintaining two build solutions could be still easier than managing a general build solution.

But I realize my view on the subject is not shared, thus the closing of this PR and the conclusion that we need someone to redesign a general build solution first, before attempting to use other CI environments.

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

Successfully merging this pull request may close these issues.

4 participants