Core clr support. #57

matthid · 2016-07-01T22:59:05Z

Rebased Version of #47

Additionally I added a special case for a "First" parameter if it is "Mandatory" in that case it can be assigned an empty name (which basically allows it to be used as positional argument).

Example is added as Unit Test. Let me know if you want this to be split to an extra PR.

eiriktsarpalis · 2016-07-02T10:29:58Z

Hi Matthias,

Personally, I'd prefer to steer clear from CoreCLR as long as it remains in
flux. It seems like an unnecessary burden having to cater to
soon-to-be-obsoleted stuff like project.json and all the API
incompatibilities. Why not keep this in a separate 'coreclr' branch for now
and have it merged as soon as the dust has settled?

I agree that the new feature should be made as a separate PR for better
review.

Thanks,
Eirik

On Sat, 2 Jul 2016 at 01:59, Matthias Dittrich [email protected]
wrote:

Rebased Version of #47 #47

Additionally I added a special case for a "First" parameter if it is
"Mandatory" in that case it can be assigned an empty name (which basically
allows it to be used as positional argument).

Example is added as Unit Test. Let me know if you want this to be split to

an extra PR.

You can view, comment on, or merge this pull request online at:

#57
Commit Summary

exec dotnet new --lang f# to create the .net cli project

remove generated main

it's a library, so remove emitEntryPoint from project.json

add source files

remove netcore deps

use framework net46 instead of netcore (dnxcore50)

add framework Assemblies

add nuget packages

ignore project.lock.son

dotnet new the test project and use net46

reference the project

add Argu specific smoke test

add .NET core target

[netcore] use xmldocument package

[netcore] fix reflection

[netcore] it's System.Xml.XDocument the right package

add dnxcore50 framework to test project

build .net cli in appveyor

upgrade from dnxcore50 to netstandard1.5

fix appveyor

fix the build.

add test to parse FAKE args

increase version

add special case for a first argument which is mandatory and allow
empty name.

make it compile again

File Changes

M .gitignore
https://github.com/fsprojects/Argu/pull/57/files#diff-0 (3)

M appveyor.yml
https://github.com/fsprojects/Argu/pull/57/files#diff-1 (19)

A src/Argu.Dotnetcli.Tests/NuGet.Config
https://github.com/fsprojects/Argu/pull/57/files#diff-2 (10)

A src/Argu.Dotnetcli.Tests/Program.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-3 (59)

A src/Argu.Dotnetcli.Tests/project.json
https://github.com/fsprojects/Argu/pull/57/files#diff-4 (32)

M src/Argu/ArgumentParser.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-5 (15)

M src/Argu/ConfigReaders.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-6 (14)

A src/Argu/NuGet.Config
https://github.com/fsprojects/Argu/pull/57/files#diff-7 (10)

M src/Argu/Parsers/Cli.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-8 (14)

M src/Argu/PreCompute.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-9 (27)

M src/Argu/UnParsers.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-10 (8)

M src/Argu/UnionArgInfo.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-11 (5)

M src/Argu/Utils.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-12 (41)

A src/Argu/project.json
https://github.com/fsprojects/Argu/pull/57/files#diff-13 (68)

M tests/Argu.Tests/Tests.fs
https://github.com/fsprojects/Argu/pull/57/files#diff-14 (54)

Patch Links:

https://github.com/fsprojects/Argu/pull/57.patch

https://github.com/fsprojects/Argu/pull/57.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#57, or mute the thread
https://github.com/notifications/unsubscribe/ACrtszKDvTVQxu0B3Zx-FJCACRi3C-LFks5qRZu5gaJpZM4JDiRV
.

matthid · 2016-07-02T10:33:50Z

I'm fine with that. However consider that a lot of tools in the F# ecosystem are depending on Argu, so it should be one of the first projects to move. In my (limited) tests its working fine with dotnetcore and it has no dependencies, so it easy to keep up2date even if a lot of stuff is still changing.

Will try to move the feature :)

enricosada · 2016-07-05T09:32:34Z

src/Argu/project.json

+            "buildOptions": {
+                "define": [
+                "NETSTANDARD1_5",
+                "NETSTANDARD1_6"


NETSTANDARD1_6 is not needed, it's already defined by default for netstandard1.6, like netcoreapp1.0 define NETCOREAPP1_0

Yeah maybe being explicit is a good idea anyway (as we saw in the 1.5 -> 1.6 move)...?

enricosada · 2016-07-05T09:59:44Z

@eiriktsarpalis the api incompatibilities are not going to be fixed really too soon (getType etc), now that 1.0 is rtm, and are only an #ifdef

also project.json is going to stay until the msbuild is ready, but before be ready is going in beta, so not usable without issues, so. project.json atm works ok.

you can just use .net core sdk and project json to build the .net core assemblies (netstandard1.6) and add that to the already build package by fake (like suave and others do). it's two more tasks inside build.fsx to run dotnet restore, dotnet pack -c Release and dotnet mergenupkg.

i'd like to use Argu in the console app projects i have, it's a good improvement.

But i understand it suck to be an early adoperter, but .net core (coreclr/bcl) is 1.0 rtm.
Tooling may change a bit, but from preview1 to preview2 it's not changed a lot for example, and it's not going to change until the msbuild replacement is ready and working (and there is going to be a migration path project.json -> msbuild because all csproj are project.json atm)

enricosada · 2016-07-05T10:01:26Z

If you need help, just ping me, in slack (there is #dotnetcore channel with other people like @7sharp9) twitter.
i am trying to write some docs and tutorial, but atm the workflow is pretty stable. It's just another framework profile

matthid · 2016-07-05T19:20:33Z

@enricosada I don't know when I have time to integrate the changes. For now my only goal was to get a package such that I can continue the work on Fake/Paket :). Once I'm happy with that I will go the reverse and cleanup.

Of course if someone want's to take it from here that's fine as well.

eiriktsarpalis · 2016-07-06T08:43:54Z

@enricosada your arguments are valid, my primary concern however is one of maintainability. It looks to me that we will need to be maintaining two separate build configurations. ~~Given that the dev tools I'm using (VS and XS) do not seem to offer support for project.json at the moment~~, how do I make sure that changes I'm making do not break the CoreCLR build? Is it possible to somehow target netstandard from msbuild?

EDIT: now installing VS 2015 CoreCLR tooling preview. See how that works.

enricosada · 2016-07-06T10:12:49Z

@eiriktsarpalis i am trying to learn the best (or good enough) workflow to have .net core version of existing libraries, i really like your feedback.
The goal is: add .net core support, with minimal project.json maintenance (i cannot auto generate it atm, packages are not the same, msbuild is hard when custom, etc).
i care less about project.json, but that's currently the only way to do .net core (or at least the most documented), time ago that was the way to go (fsproj => project.json), now it's not (wasted effort to do that), but it's needed.

i really need help of f# maintainers, to say what's too annoying, so we can find workarounds/improve.

for example:

two packages? => merge it, using github.com/enricosada/dotnet-mergenupkg (paket maybe soon)
travis/appveyor => samples
fake?rake?paket? => samples or helpers
editor support => ionide atm.

@eiriktsarpalis for example vs with xproj should work, i'll publish the templates soon, but you can just copy the c# xproj in the same directory, and add the project in the solution. That's enough for build, but no intellisense.

7sharp9 · 2016-07-06T10:35:28Z

Ive been waiting for Argu to support dot net core, all these updates although a pain are necessary to get F# viable on the dotnet core platform.

eiriktsarpalis · 2016-07-06T10:44:50Z

Would it make sense for now to keep the CoreCLR stuff in a separate branch and publish as a separate nuget package?

enricosada · 2016-07-06T10:46:58Z

@eiriktsarpalis no, two package are really really bad, in my experience with other projects. the .net core is a simple new target framework (like portable), should be working the same (that's not too hard, but we can improve fixing pain points).
And is worse for you to maintain, branch/build/publish/version alignment

I'll start an meta issue about "add .net core support to existing libraries, without going crazy", we can improve a lot.
So for example:

use paket instead of dotnet mergenupkg (if it's already used, so less tool to learn, easier workflow)
fake/rake helpers or at least samples.
fsproj => project.json generator (at least a source files and project type), now is stable

eiriktsarpalis · 2016-07-06T10:48:24Z

Could the F# tooling be made to target CoreCLR just like any other platform, without requiring a separate project.json to do this?

enricosada · 2016-07-06T11:21:02Z

The dotnet build call the fsc ( try `dotnet --verbose build). But it's not enough , some stuff are managed by sdk ( thanks about that otherwise was a lot harder for f#).
I can create an msbuild, but like that is harder for you to run/test it. And may not be aligned.

You can wait some months for msbuild support instead of project.json. That's an option.
But sdk now is feature rich, works xplat, and vf# team is not going to create an alternative format, just wait for c# and core sdk. But That's months from now ( plus bugs )

enricosada · 2016-07-06T12:56:46Z

@eiriktsarpalis i am not say'n a custom msbuild target (include in the fsproj) to produce only netcore cannot be the solution, it's an idea.
But require changes to fsproj too (conditional ref), so i fear issues with tooling (i dont want to create issue with the main fsproj), parallel project.json is safer i think atm

eiriktsarpalis · 2016-07-21T09:47:10Z

build.fsx

+
+let isDotnetSDKInstalled = try Shell.Exec("dotnet", "--version") = 0 with _ -> false
+
+Target "Nuget.AddNetCore" (fun _ ->


why do the packages need to be merged after creation? Can this not be handled by paket.template?

eventually it will

@forki can't we just manually specify the CoreCLR bits in the files section? Or is there more to it?

I guess we can, but lets use merge for now and do that when paket is ready.
it's all a bit of bootstrapping problem. Argu needs FAKE/Paket featrues and paket needs argu...

not atm, afaik, it's up to paket team, i'd like an helper cmd too.
That's the minimal flow (maintenance, easy to test) afaik to add support for .net core.
Also like that you dont need to redefine the dependencies inside paket.template (paket doesnt manage project.json), and are always aligned with project.json (used for build)

Are merged only:

lib/netstandard1.6 (the .net core assemblies)

the dependencies for .netstandard1.6, used only for that profile (no issue with others)

not the metadata like authors, etc, that's defined in paket.template already.

@eiriktsarpalis @forki files is ok, problem are dependencies becasue doesnt support deps by conditional fw ( ref fsprojects/Paket#913 ).

propose by me workaround fsprojects/Paket#1539
and .net core discussion (the one we are having there) fsprojects/Paket#1793

as note: i have written the dotnet mergenupkg tool for project without paket, just to make it easier the integration, but works where paket is used too. i see that as a temporary solution to an issue (until project.json => msbuild )

I'm already creating a dnc helper in fake. Will send pull request to Enricos fork of argu soon

@forki can we use argu or better chessie as the example for fake new dnc helpers?

In another pr if possibile, so we can iterate the design.

Atm this workflow work really well, checklist of steps, i'd like to make people use that atm, and update to fake helpers later.
i think that can speedup a little .net core adoption.

Also i think current one is not too bad, because make easy to see dotnet command, so can be run locally, no magic (atm people see dnc sdk as magic)

eiriktsarpalis · 2016-07-21T10:03:30Z

I'm happy to merge as it is. Certain issues still exist, but I think the most urgent one is this.

enricosada · 2016-07-21T10:11:31Z

@eiriktsarpalis i have little time today (webcast live tonight about .net core and f# 😄 )

I'll ask tomorrow in dotnet/cli dotnet/corefx dotnet/coreclr gitter chat about that (good source of help)

forki · 2016-07-21T15:30:04Z

I just sent a PR to @matthid which uses FAKE's new helpers: matthid#2

Use FAKE's DotNet CLI helpers

forki · 2016-07-21T15:58:39Z

ok @matthid merged it. So this PR is up-to-date

eiriktsarpalis · 2016-07-22T09:59:43Z

Travis seems to be failing with this error

matthid · 2016-07-22T19:44:10Z

@forki https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/DotNetCLIHelper.fs#L45-L52

eiriktsarpalis · 2016-07-27T21:06:51Z

(bump)

Any progress on this?

matthid · 2016-07-27T21:16:31Z

Yeah sorry we have one more fake update which should remove the error and then I guess we are good to go (expect the change soon on the weekend from me, or faster if someone else does it in the mean time)

enricosada · 2016-07-27T23:49:17Z

@matthid well if .net core is installed, no error 😄

you got pr matthid#3 for this pr

That add osx, ubuntu

i tried running build.sh Nuget.AddNetCore on travis, and that should works (and run build/test/mergenupkg) but the .net 3.5 build give me error on travis

error FS0078: Unable to find the file 'FSharp.Core.dll' in any of� /usr/lib/mono/2.0-api� /home/travis/build/enricosada/Argu/src/Argu� /usr/lib/cli/fsharp

add .netcore sdk to travis

matthid · 2016-08-25T10:11:33Z

@enricosada Sorry for taking ages to merge ...

Actually I now have some time to revisit this. Once we can push all dependencies to nuget we can use the new Fake version to clean everything up :).
For the time being things will be a bit dirty/hacky (at least for all projects FAKE depends on)

matthid · 2016-08-25T10:21:18Z

@enricosada I think the build is failing because ancient FSharp.Core 2.3.0.0 is no longer distributed with mono?

matthid · 2016-08-25T10:47:42Z

@eiriktsarpalis I think this can be fixed by switching to the FSharp.Core nuget package...

Delete https://github.com/fsprojects/Argu/blob/master/src/Argu/Argu.fsproj#L81-L83 and run paket install

eiriktsarpalis · 2016-08-26T14:20:38Z

@matthid I'm fine with adding the nuget FSharp.Core as a dependency

eiriktsarpalis · 2016-08-29T17:38:38Z

Merged to master. Thanks guys.

matthid · 2016-08-29T18:45:31Z

@eiriktsarpalis Nice! Please let me know when a nuget is released with dnc support, so I can remove the self build binaries :)

eiriktsarpalis · 2016-08-30T23:42:45Z

@matthid dotnet core unit tests are failing on my machine with the following error message:

No executable found matching command "dotnet-test-"
Running build failed.
Error:
System.Exception: Test failed on "test" "C:\Users\eirik.tsarpalis\devel\public\Argu\tests\Argu.NetCore.Tests\project.json" --configuration Release
   at [email protected](String message) in C:\code\FAKE\src\app\FakeLib\DotNetCLIHelper.fs:line 308
   at Fake.DotNet.Test(FSharpFunc`2 setTestParams, IEnumerable`1 projects) in C:\code\FAKE\src\app\FakeLib\DotNetCLIHelper.fs:line 308
   at [email protected](Unit _arg3) in C:\Users\eirik.tsarpalis\devel\public\Argu\build.fsx:line 192
   at Fake.TargetHelper.runSingleTarget(TargetTemplate`1 target) in C:\code\FAKE\src\app\FakeLib\TargetHelper.fs:line 492

I notice that the tests project is lacking a project.json instance. Could that be causing the problem?

enricosada · 2016-08-31T08:06:03Z

no, the problem is fake is running dotnet test with a project.json ( tests/Argu.NetCore.Tests/project.json ) who is just a console app (netcoreapp1.0).
A dotnet test project is a library project with a testRunner property (and try to run dotnet-test-{testRunner} => dotnet-test- ).
fake should just dotnet run instead of dotnet test.
I think after unquote support netcore (i already sent the pr, waiting for feedback+merge), we can remove it and just use the tests 😄

@eiriktsarpalis i'll send a pr to fix it, let me check why ci was green instead of red, sry for issue.

matthid · 2016-08-31T08:24:22Z

I think problems like these will slowly go away once all core packages are released and fsprojects/FAKE#1281 can be merged. We than have an automatic way to install and use the new dotnet sdk and we don't need to disable the "dotnet sdk"-build when it's not installed. It hopefully will be less error prone after that...

enricosada · 2016-08-31T08:58:54Z

@matthid maybe i written that badly (back from vacation, i am a bit sleepy). fake is doing ok, it's just this build.fsx with this particular test project, and appveyor/travis configuration in Argu.

appveyor doesnt run fake, just msbuild the solution and discover/run tests with appveyor. that should be fixed ihmo to run fake.
travis doesnt run netcore, i'll check that, but the right version is installed ( dotnet --info is ok from log), i'll check why

I'll add the project.json the other xunit test project, and reference the .netcore test source file, so is ready when unquote is released (we need to just add the current test file to sources)

matthid · 2016-08-31T09:11:26Z

@enricosada My answer wasn't specific to this particular problem. What I mean is that the way we do things now is quite error prone (editing travis and appveyor config files to install dotnet sdk) and we might need to edit them all the time to update/upgrade. On the other side if MS decides to change the download url's all our projects break and we need to fix it everywhere...

What I'm trying to say is that we should just make it run to be able to bootstrap and release. We can figure things out along the way. It is very likely that things will change after the FAKE upgrade anyway.
So it might not all that important to be able to run tests for dotnetcore right now (as long as the regular test suite is running OK).

If you still manage to make it run that would of course be even better 👍 .
That appveyor is not running fake is indeed disturbing :)

enricosada · 2016-08-31T16:16:14Z

atm install of .net core sdk preview2 is ok, it's ugly but that's ms recommended way (waiting for osx brew and ubuntu pkg, or better support in travis with language c# ). i'd really like to remove that.

continue in #62 ( use fake script in appveyor/travis, fix error )

matthid force-pushed the core_clr branch from 7ac5232 to 8c27155 Compare July 2, 2016 10:44

This was referenced Jul 2, 2016

First mandatory special case #58

Closed

FAKE 5 fsprojects/FAKE#1281

Closed

Dotnetcore build for Paket.Core fsprojects/Paket#1785

Merged

fix the build. enricosada/Argu#1

Closed

enricosada reviewed Jul 5, 2016
View reviewed changes

matthid force-pushed the core_clr branch from b3711ef to e2bc44b Compare July 7, 2016 16:02

enricosada added 5 commits July 9, 2016 16:13

exec dotnet new --lang f# to create the .net cli project

db5dfee

remove generated main

70dfbb9

it's a library, so remove emitEntryPoint from project.json

aa1d1e8

add source files

355d853

remove netcore deps

28f94cd

eiriktsarpalis reviewed Jul 21, 2016
View reviewed changes

forki added 2 commits July 21, 2016 17:27

Use FAKE dotnet CLI helpers

4ff2ab8

Use proper FSharp.Core package

19a4b8e

Merge pull request #2 from forki/core_clr

aa1b190

Use FAKE's DotNet CLI helpers

matthid mentioned this pull request Jul 23, 2016

fix exception when dotnet cli is not installed. fsprojects/FAKE#1332

Merged

add .netcore sdk to travis

0c0c8c1

Merge pull request #3 from enricosada/add_netcoresdk_to_travis

a1a7dc1

add .netcore sdk to travis

eiriktsarpalis merged commit aac0303 into fsprojects:master Aug 29, 2016


		let isDotnetSDKInstalled = try Shell.Exec("dotnet", "--version") = 0 with _ -> false

		Target "Nuget.AddNetCore" (fun _ ->

Core clr support. #57

Core clr support. #57

Conversation

matthid commented Jul 1, 2016

eiriktsarpalis commented Jul 2, 2016

an extra PR.

matthid commented Jul 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enricosada commented Jul 5, 2016

enricosada commented Jul 5, 2016

matthid commented Jul 5, 2016

eiriktsarpalis commented Jul 6, 2016 • edited Loading

enricosada commented Jul 6, 2016

7sharp9 commented Jul 6, 2016

eiriktsarpalis commented Jul 6, 2016

enricosada commented Jul 6, 2016 • edited Loading

eiriktsarpalis commented Jul 6, 2016 • edited Loading

enricosada commented Jul 6, 2016

enricosada commented Jul 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eiriktsarpalis commented Jul 21, 2016

enricosada commented Jul 21, 2016

forki commented Jul 21, 2016

forki commented Jul 21, 2016

eiriktsarpalis commented Jul 22, 2016

matthid commented Jul 22, 2016

eiriktsarpalis commented Jul 27, 2016

matthid commented Jul 27, 2016

enricosada commented Jul 27, 2016 • edited Loading

matthid commented Aug 25, 2016

matthid commented Aug 25, 2016

matthid commented Aug 25, 2016

eiriktsarpalis commented Aug 26, 2016

eiriktsarpalis commented Aug 29, 2016

matthid commented Aug 29, 2016

eiriktsarpalis commented Aug 30, 2016

enricosada commented Aug 31, 2016

matthid commented Aug 31, 2016

enricosada commented Aug 31, 2016

matthid commented Aug 31, 2016

enricosada commented Aug 31, 2016

eiriktsarpalis commented Jul 6, 2016 •

edited

Loading

enricosada commented Jul 6, 2016 •

edited

Loading

eiriktsarpalis commented Jul 6, 2016 •

edited

Loading

enricosada commented Jul 27, 2016 •

edited

Loading