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

Get a .NET Core project to build in AppVeyor and Travis #1462

Merged
merged 14 commits into from
Sep 15, 2016
Merged

Get a .NET Core project to build in AppVeyor and Travis #1462

merged 14 commits into from
Sep 15, 2016

Conversation

mderriey
Copy link
Contributor

@mderriey mderriey commented Sep 9, 2016

First step of #1419.

It looks like AppVeyor has the dotnet CLI installed in the image so no need to download it.
Also, that's really as simple as it gets. Maybe you wanted something more ... realistic.

Made some comments in the F# code where I'm really not sure if that's The Way To Do Things.

WorkingDir = "./Octokit.Next" })

!! "./Octokit.Next/project.json"
|> Fake.DotNetCli.Build (fun p -> p)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe there's an easier way than (fun p -> p) to return the input parameter as is

@mderriey
Copy link
Contributor Author

I'm struggling a bit to gauge how simple the .NET Core has to be.

Is the intent realy only to have a .NET Core, no matter how small it is, to build on AppVeyor? Or do we want to try tackling the porting to the existing projects to .NET Core?

Thanks!
/cc @shiftkey

@shiftkey
Copy link
Member

@mderriey the reason why I suggest getting a trivial .NET Core thing building is that I fear there's lots of integration hassles waiting for us getting this running on both Travis and Appveyor.

@mderriey mderriey changed the title [WIP] - Get a .NET Core project to build in AppVeyor [WIP] - Get a .NET Core project to build in AppVeyor and Travis Sep 12, 2016
@mderriey
Copy link
Contributor Author

For the sake of simplicity, I went with a separate VS solution containing a very simple class library and a unit tests project.

It took a few trial and error tries to get it working on Travis CI.

@mderriey
Copy link
Contributor Author

Is that a good enough baseline to start with the port? How should we proceed?

dotnet: 1.0.0-preview2-003121

script:
- dotnet --info
Copy link
Member

Choose a reason for hiding this comment

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

Can we put the existing build steps in here, so they both run?

@shiftkey
Copy link
Member

shiftkey commented Sep 14, 2016

@mderriey I like where this is heading. If it's too hard to run both the original build and this new build (for example, if we hit timeouts with CI), then let's verify we can test the .NET Core project from the command line at least

@mderriey
Copy link
Contributor Author

Sure, I'll bring the existing build targets back.

If it's too hard to run both the original build and this new build (for example, if we hit timeouts with CI), then let's verify we can test the .NET Core project from the command line at least

I didn't get this part, sorry. The new build does execute tests, but I'm sure you saw that, so I must miss something.

Also, a couple of random questions:

  • for the time being, are we happy to duplicate source files in the new projects? Having both .csproj and project.json seems to be causing issues with MSBuild wanting more target frameworks in the project.json
  • if it turns out netstandard1.1 is all we need, are we happy to have it as the only target?

@shiftkey
Copy link
Member

The new build does execute tests, but I'm sure you saw that, so I must miss something.

Yes, I wasn't clear. Am I able to run the same command locally? Or are there other dependencies I need.

for the time being, are we happy to duplicate source files in the new projects? Having both .csproj and project.json seems to be causing issues with MSBuild wanting more target frameworks in the project.json

I'd rather not do this, and the MSBuild issues are similar to what I was seeing a while ago.

if it turns out netstandard1.1 is all we need, are we happy to have it as the only target?

I'm fine with targeting this initially as a new platform, as adding in extra platforms is rather straightforward. My main focus initially is ensuring we support the current platforms in project.json as well...

@mderriey
Copy link
Contributor Author

Yes, I wasn't clear. Am I able to run the same command locally? Or are there other dependencies I need.

As long as you have the dotnet CLI, then yeah, you can execute the build target to run .NET Core tests.

I'd rather not do this, and the MSBuild issues are similar to what I was seeing a while ago.

Understood. Will investigate on this a bit more by having a look at the branch you mentioned before.

I'm fine with targeting this initially as a new platform, as adding in extra platforms is rather straightforward. My main focus initially is ensuring we support the current platforms in project.json as well...

That's consistent with what you said just before.

Finally, the build timed out on Travis with both the existing and the new build. Can we kick off a new one without pushing changes to that branch just in case?

@shiftkey
Copy link
Member

As long as you have the dotnet CLI, then yeah, you can execute the build target to run .NET Core tests.

Do you think it's worth installing the dotnet CLI locally, like we do for Rx.NET?

@mderriey
Copy link
Contributor Author

Hmm.

I'd say no. Let's say you're willing to work on the port to .NET Core, you'll need to install .NET Core runtime and tooling anyway. Another argument for not installing it as part of the build is that both AppVeyor and Travis support it, which saves us from installing it ourselves on the build agents.

Does that answer your question?

@shiftkey
Copy link
Member

@mderriey all good points 👍

@mderriey
Copy link
Contributor Author

mderriey commented Sep 14, 2016

Cool.

I've just noticed another build started on Travis for this commit - use mono 4.2.3 as latest successful builds show this version was used.

Did you start this explicitly or does Travis have a retry mechanism when a build times out?

@shiftkey
Copy link
Member

@mderriey nope, this is me restarting the build :rage1:

@shiftkey shiftkey self-assigned this Sep 14, 2016
trying to get the build not to hang on Mac OS
The default Mac OS image is not compatible with .NET Core
see https://travis-ci.org/octokit/octokit.net/jobs/160036868
@shiftkey
Copy link
Member

shiftkey commented Sep 15, 2016

This seems to be an upstream problem 😞

Unhandled Exception: System.TypeInitializationException: The type initializer for 'Crypto' threw an exception. ---> System.TypeInitializationException: The type initializer for 'CryptoInitializer' threw an exception. ---> System.DllNotFoundException: Unable to load DLL 'System.Security.Cryptography.Native': The specified module could not be found.

@mderriey
Copy link
Contributor Author

Well that's weird, the last build passed but this one didn't, the only change being to bring back linux again and adding a condition around if the OS is OSX to link libs.

@shiftkey could you please restart the build for that last commit when you have some time? And just in case, could you check the last commit see if I did something wrong? My bash-fu is not very good.

@mderriey
Copy link
Contributor Author

Great, thanks! 👍
Let's just hope the build won't be too flakey.

I'll start working on moving existing platforms to project.json. If I understand this right, and if all the current platforms are supported by project.json, we could end up with only one Octokit project in the solution.

Some posts I want to keep as a reference here:

Thanks again!

@shiftkey shiftkey changed the title [WIP] - Get a .NET Core project to build in AppVeyor and Travis Get a .NET Core project to build in AppVeyor and Travis Sep 15, 2016
@shiftkey
Copy link
Member

@mderriey that's fine, we can keep an eye on it over time.

Thanks for getting started on this!

@shiftkey shiftkey merged commit c9b2c12 into octokit:master Sep 15, 2016
@mderriey
Copy link
Contributor Author

@shiftkey Thanks!

I could have squashed all these commits, it's a bit embarrassing to be honest.

@shiftkey
Copy link
Member

I could have squashed all these commits, it's a bit embarrassing to be honest.

Eh, it's fine. Onward and upward.

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.

2 participants