WIP: global dotnet cli-tool #1932

kblohm · 2018-05-16T20:54:20Z

Hi,
since dotnet core 2.1 is in RC now, i created a global FAKE cli tool, based on the 'old' cli-tool.
I build fake itself with the tool, so it should be working fine (at least on my windows machine).
There are still some things we need to decide/do:

As far as i know a global tool can not be installed as a DotNetCliToolReference. So should there be 2 tools now? I guess the DotNetCliToolReference makes it easier for CI-Server so thats probably good. So i vote for 2 tools :)
The name of the project. I named it Fake-Cli for now. Could also be dotnet-fake-global or fake.cli or whatever. The name of the project does not have to influence the name of the command, which is just fake.
I did not update the docs yet.
Your bootstrapping-repo should probably be updated afterwards.

matthid · 2018-05-16T21:00:26Z

build.fsx

@@ -224,7 +224,16 @@ Trace.setBuildNumber nugetVersion
 //if current |> Seq.contains CoreTracing.defaultConsoleTraceListener |> not then
 //    CoreTracing.setTraceListeners (CoreTracing.defaultConsoleTraceListener :: current)

-let dotnetSdk = lazy DotNet.install DotNet.Release_2_1_4
+
+let dotnetSdk = lazy DotNet.install (fun p -> 


Should we add that version to Fake.DotNet.Cli as well?

I would have, but i was not sure about the build version. We could just add '2.1.300-rc1' without the build version and just update that 'silently' if it changes. But i would not pollute the module with several 2.1.300-rc1-versions.

matthid · 2018-05-16T21:03:55Z

yes yes yes. Thanks for looking into this :)
Yes finding a reasonable name is the hardest thing here:

fake-bin
fake-cli
fake-global-tool
dotnet-fake-global
... or with . instead of -?

Honestly I don't really know (which is probably the reason why I have not looked into this yet)

kblohm · 2018-05-16T21:10:04Z

just 'fake' would be nice ;).
Could also be fake-runner.
Shouldn't travis use the version that was just downloaded?

matthid · 2018-05-16T21:10:16Z

Seems like slack likes fake-cli, which is actually quite nice

kblohm · 2018-05-16T21:11:32Z

fake-cli <- all lowercase?

matthid · 2018-05-16T21:12:39Z

I would write it all lower case, but I don't think casing actually matters when used on the command line later on.

matthid · 2018-05-16T21:13:21Z

Shouldn't travis use the version that was just downloaded?

Yes it does, but it seems like environment variables are set such that the new version uses the old sdk-files....

matthid · 2018-05-16T21:14:51Z

I think we need something similar to what we have in the msbuild helper:

https://github.com/fsharp/FAKE/blob/d099facbe19ed1676e66c74fef295baa43a73c66/src/app/Fake.DotNet.MSBuild/MSBuild.fs#L231-L233

kblohm · 2018-05-16T21:17:59Z

Probably not, but its also consistent with dotnet-fake. I will change it to lower.

Exactly the same lines are in the DotNet module. Thats probably a bug.

matthid · 2018-05-16T21:21:28Z

Yes either a bug in dotnet cli or just another environment variable we need to look at. Also we might need to ensure that "our" newly installed dotnet-sdk is the first in PATH...

kblohm · 2018-05-16T21:22:55Z

Are you already merging or can i still force-push?

matthid · 2018-05-16T21:25:57Z

You can still force-push, I just changed to base because this was on the wish list of rc13 :)

kblohm · 2018-05-16T21:33:17Z

At least for me, PATH just points to the root dotnet folder and not a specific version. So i guess its the travis.yml that sets the dotnet environment variable to 2.1.4.

kblohm · 2018-05-16T21:36:15Z

Do u prefer updating the travis.yml or removing 'dotnet' from the environment in addition to MSBuild stuff (I have MSBUILD and MSBUILD_PATH on my machine and none of the ones listed. So they are missing aswell maybe?)

matthid · 2018-05-16T21:43:23Z

Does that mean you have found the root cause? I'm not sure I understand. I feel like we should find the root cause but chaning travis.yml is a workaround if we can't find it.

kblohm · 2018-05-16T22:00:47Z

So build.fsx changes the workingDirectory to the directory of the just downloaded sdk. It does not change the dotnet executable though. That one is still the one that DotNet.exec finds. As in home/.dotnet i suppose. Do u know if the installer script both installs to home/.dotnet/sdk/2.1.300-rc1-008673 AND updates the executable that is in home/.dotnet/ ? Because it does not look like it does that on linux.

kblohm · 2018-05-16T22:05:06Z

Ok, i have changed the casing and i added the RC to the DotNet module.
I also changed the travis version. But just to see if that even matters. I will look into a proper fix later.

matthid · 2018-05-17T21:17:00Z

src/app/Fake.DotNet.Cli/DotNet.fs

@@ -10,30 +10,30 @@ module DotNet =

    // NOTE: The #if can be removed once we have a working release with the "new" API
    // Currently we #load this file in build.fsx
-    #if NO_DOTNETCORE_BOOTSTRAP
+    // #if NO_DOTNETCORE_BOOTSTRAP


Maybe just remove. I think you are correct and we don't need this anymore.

matthid · 2018-05-17T21:17:46Z

Do we need fsprojects/Paket#3208 or how did you make it work?

matthid · 2018-05-18T19:01:57Z

paket.dependencies

@@ -95,7 +95,7 @@ group Build
    nuget SourceLink.Fake
    nuget ILRepack
    nuget RoslynTools.ReferenceAssemblies
-
+    nuget Newtonsoft.Json


Where does this come from?

I #load the DotNet module directly so i can use DotNet.Release_2_1_300_RC1. If i dont add the Newtonsoft.Json the Bootstrap test fails. I can remove it and just add the new version directly and update that later.

kblohm · 2018-05-18T19:15:36Z

Did u read the reason why travis build is probably using the wrong version? Do u have any opinions on how to fix that? I could

set the right path in the build script
add a new method to the DotNet module that can be used to try and find a specific version of Dotnet
add a new property to the Dotnet options that specifies the prefered version. Since the current toolpath is set in the Options.Create() i would have to search for the tool twice if the property is set, so that might be kinda strange.

matthid · 2018-05-18T19:23:31Z

Sadly I cannot see the build anymore due to rebasing, the current build seems to have triggered a dotnet-cli bug :/

But IIRC the previous build did use the correct version (ie it started he correct version installed within the build) - instead the problem was the dotnet-cli used the msbuild sdk files from the global existing installation.

I would consider this a bug in the Fake.Dotnet.Cli module, which probably can be debugged by just printing all environment variables before starting dotnet-cli

kblohm · 2018-05-18T19:28:19Z

It did use the right version, but probably only because i changed the travis.yml. I wanted to see if that helped.
The build script still just sets the working directory and not the actual dotnet executable to the downloaded version.
So i dont think that is a bug. I am pretty sure Fake.Dotnet.Cli just uses the version it finds, and that is probably the one travis puts there. But i do not really know what happens exactly in travis when u set a dotnet version in travis.yml.

matthid · 2018-05-18T19:37:29Z

So i dont think that is a bug. I am pretty sure Fake.Dotnet.Cli just uses the version it finds

Maybe we talk through each other but if you use DotNet.install according to the docs:

https://fake.build/dotnet-cli.html

Ie combine the DotNet.install and DotNet.<exec> calls then it uses exactly the installed binary (this is ensured by the function returned by DotNet.install and afaik that works).

You can see it in the logs /usr/bin/dotnet is the one installed by travis. In the previous log it started /home/<something>/dotnet

matthid · 2018-05-19T09:49:42Z

Ok circle-ci added an option recently to build pull requests, so next push should have an circleci build (to ease it a bit)

matthid · 2018-05-19T09:51:19Z

On the arguments part: there are like 5 different approaches to do process arguments in FAKE. Maybe we should look into unifying them somehow? I am using the Process.xxxParam functions now. The string-module is very inconsistent.

Yes I agree, basically all but one are broken for some kind of input. We probably should all mark as obsolete.

matthid · 2018-05-19T09:53:42Z

Added an issue #1940, will look at it eventually

kblohm · 2018-05-19T09:55:15Z

I am not using Args.toWindowsCommandLine right now. At least for travis the Process.quote is working fine (in this case at so far at least).

matthid · 2018-05-19T09:58:59Z

Ok I think we probably should filter

  'MSBuildLoadMicrosoftTargetsReadOnly' -> 'true'
  'MSBuildSDKsPath' -> '/usr/share/dotnet/sdk/2.1.4/Sdks'

in addition to MSBuildExtensionsPath and MSBUILD_EXE_PATH. I'm pretty sure it will work then :)
Can you add that to the msbuild and dotnet-cli modules?

matthid · 2018-05-19T10:00:12Z

And there is 'DOTNET_HOST_PATH' -> '/usr/share/dotnet/dotnet' which at least looks suspicious

matthid · 2018-05-19T10:02:09Z

Google says it might be a good idea to unset that as well (at least when running the dotnet-cli)

kblohm · 2018-05-19T10:05:02Z

Google never lies! I added them.

kblohm · 2018-05-19T10:55:45Z

Should we remove all of the environment vars by default or just in the installer case? Somebody might set those intentionally.

matthid · 2018-05-19T13:11:52Z

Actually it should be fine, I designed the setup in a way that if you set those manually it will "just work"

matthid · 2018-05-19T13:13:31Z

Not sure why circleci was failing, retried the build.

matthid · 2018-05-19T13:21:05Z

Should I merge this on staging so we can play around and look at the result? Please just merge/rebase on top of release/rc thanks

matthid · 2018-05-19T14:00:22Z

I think the current circleci error goes away after merge/rebase.

kblohm · 2018-05-19T14:10:29Z

Ahh you are too fast ;). Should i let these run or rebase again?

kblohm · 2018-05-19T14:13:02Z

The "UnskipAndRevertAssemblyInfo" target. Could that be a final target instead? This will not solve all the problems, as i sometimes just skip a build if a realise i forgot something, but at least for the cases where it just fails it should be in a valid state afterwards.

matthid · 2018-05-19T14:18:30Z

Yes good idea. maybe we could detect ctrl+c and run final targets anyway no idea...

kblohm · 2018-05-19T15:06:15Z

It would probably be a good idea if you could test the global tool on your machine. I am still kinda sceptical because of

Do we need fsprojects/Paket#3208 or how did you make it work?

As i said, its working for me, but there must be a reason why he needed to make the changes to paket.

matthid · 2018-05-19T16:18:51Z

OK lets try this out on our new shiny staging instance :)

matthid · 2018-05-19T16:19:57Z

Our new tool

kblohm · 2018-05-19T16:30:00Z

Nice :). Ping me if something is not working.

matthid · 2018-05-20T13:07:30Z

Just for reference: The failure we saw on travis was a bug in travis, because it didn't actually install all dotnet sdk requirements. I reported it here: travis-ci/travis-ci#9644

matthid · 2018-05-21T00:48:13Z

It's released :)

matthid reviewed May 16, 2018

View reviewed changes

matthid changed the base branch from master to release/rc_13 May 16, 2018 21:16

kblohm force-pushed the globalDotnetTool branch from 9ed1aa0 to ee6a727 Compare May 16, 2018 22:03

kblohm force-pushed the globalDotnetTool branch 4 times, most recently from 8198eca to f66af87 Compare May 17, 2018 19:23

matthid reviewed May 17, 2018

View reviewed changes

matthid reviewed May 18, 2018

View reviewed changes

matthid mentioned this pull request May 19, 2018

Mark Process.quote functionality as Obsolete and point to Args.toWindowsCommandLine #1940

Closed

kblohm force-pushed the globalDotnetTool branch from 7265992 to 79b92cb Compare May 19, 2018 10:04

matthid changed the base branch from release/rc_13 to release/rc May 19, 2018 13:20

global dotnet cli-tool

cc999dc

kblohm force-pushed the globalDotnetTool branch from 79b92cb to cc999dc Compare May 19, 2018 14:04

Merge branch 'release/rc' into globalDotnetTool

448e6ed

matthid merged commit 7303149 into fsprojects:release/rc May 19, 2018

kblohm deleted the globalDotnetTool branch May 19, 2018 16:30

WIP: global dotnet cli-tool #1932

WIP: global dotnet cli-tool #1932

Conversation

kblohm commented May 16, 2018

matthid May 16, 2018

Choose a reason for hiding this comment

kblohm May 16, 2018

Choose a reason for hiding this comment

matthid commented May 16, 2018

kblohm commented May 16, 2018

matthid commented May 16, 2018

kblohm commented May 16, 2018

matthid commented May 16, 2018

matthid commented May 16, 2018

matthid commented May 16, 2018

kblohm commented May 16, 2018

matthid commented May 16, 2018

kblohm commented May 16, 2018

matthid commented May 16, 2018

kblohm commented May 16, 2018

kblohm commented May 16, 2018

matthid commented May 16, 2018

kblohm commented May 16, 2018

kblohm commented May 16, 2018

matthid May 17, 2018

Choose a reason for hiding this comment

matthid commented May 17, 2018

matthid May 18, 2018

Choose a reason for hiding this comment

kblohm May 18, 2018 • edited Loading

Choose a reason for hiding this comment

kblohm commented May 18, 2018 • edited Loading

matthid commented May 18, 2018

kblohm commented May 18, 2018

matthid commented May 18, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

kblohm commented May 19, 2018 • edited Loading

matthid commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

kblohm commented May 19, 2018

kblohm commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

kblohm commented May 19, 2018

kblohm commented May 19, 2018

matthid commented May 19, 2018

kblohm commented May 19, 2018

matthid commented May 19, 2018

matthid commented May 19, 2018

kblohm commented May 19, 2018

matthid commented May 20, 2018

matthid commented May 21, 2018

kblohm May 18, 2018 •

edited

Loading

kblohm commented May 18, 2018 •

edited

Loading

kblohm commented May 19, 2018 •

edited

Loading