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

Full netcore #3183

Merged
merged 35 commits into from Sep 21, 2018

Conversation

Projects
None yet
5 participants
@enricosada
Copy link
Collaborator

enricosada commented May 3, 2018

make paket and paket.bootstrapper as .net sdk tools.

idea is users can maintain the paket.bootstrapper (github download, etc) flow with minimal noise (one text file .paket\paket.bootstrapper.proj and installed as dotnet restore .paket).

but because paket is a sdk tool too, can be installed normally as dotnet tool install.
The bootstrapper is used to download locally the nupkg (needed by sdk) and install it locally as native executable (so bypassing nuget client as much as possible)

more info in https://github.com/enricosada/paket-netcore-testing-as-tool

TODO

  • make the #ifdef just normal args, if make sense
  • back appsetting if possibile with system.configuration like argu does
  • reenable cache (now is doing bootstrapper -f)
    • disabled the cache by default, not yet implemented
  • enable test from #3208
@matthid

This comment has been minimized.

Copy link
Member

matthid commented May 4, 2018

  • I'd like to see the build-script changed in a way such that we push the next version including this with the netcore version (as I'm still a bit skeptical that this works)
  • As mentioned in slack, I'd like to see a dotnet-paket cli tool (mainly for experimenting with fake)
@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented May 4, 2018

I'd like to see the build-script changed in a way such that we push the next version including this with the netcore version (as I'm still a bit skeptical that this works)

yes, i'll do that (i want this PR green :D).
mostly mean use .net core sdk 2.1.300-preview2 and pass /p:PackAsTool=true to dotnet pack

As mentioned in slack, I'd like to see a dotnet-paket cli tool (mainly for experimenting with fake)

I'll do that, but i want to choose if do a dotnet-paket.cmd/.sh or a .net exe.
In my idea, that will do just a wrapper to execute the real .paket/paket inside repo:

  • find in the working directory hierarchy for .paket/paket.
  • run that passing original arguments.

plus a paketg as global tool, for stuff like init (no need for dotnet paketg, just paketg will be enough)

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented May 4, 2018

current status:

  • sdk integration works
  • dockerized app (as best i know to reuse layer) till the alpine

i have an issue with sdk 2.1.300-preview2 because packages dir is not used by references, but i think is related to sdk preview, not this netcore PR.
i'll open another PR to check the behaviour, maybe was changed from current stable 2.1.105

@enricosada enricosada force-pushed the enricosada:full_netcore branch from 50841b6 to 18d2216 May 17, 2018

@enricosada enricosada force-pushed the enricosada:full_netcore branch from 18d2216 to a7fa645 May 18, 2018

@ctaggart

This comment has been minimized.

Copy link
Contributor

ctaggart commented Jun 15, 2018

@enricosada, how is this going?

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Jul 7, 2018

I schedule some time Monday and Tuesday for it. Hope will be enough to finish it

@matthid

This comment has been minimized.

Copy link
Member

matthid commented Aug 4, 2018

@enricosada What's missing here and can I somehow help? I just started to update some of the old stuff here: #3324
After I'm finished it should be easy to create a dotnet tool, such that the following workflow would work:

  1. fake build
  2. the build script executes dotnet tool install --... paket
  3. We have to hook the path into the targets file:
    <PaketExePath Condition=" '$(PaketExePath)' == '' AND Exists('$(PaketRootPath)paket.exe')">$(PaketRootPath)paket.exe</PaketExePath>

Note that this doesn't need updates to paket itself (besides the netcoredistribution) and is therefore a "simpler" approach than this.
Do we want direkt support for dotnet tool in paket for versioning? I guess the answer is yes, but we can add this later and make the above workflow easier.

Alternative to dotnet tool install would be to just add the nuget package to your paket.dependencies and then use dotnet <path to paket.dll> in the paket targets. We could have paket.sh and paket.cmd bundled in the package. ./packages/build/paket/paket.sh --help is a bit worse than .paket/paket.exe --help but at least everything is versioned in your dependencies file.

What do you think?

@nojaf

This comment has been minimized.

Copy link

nojaf commented Aug 16, 2018

Hi @enricosada, I've looked at your repo.

I like method 5:

dotnet tool install paket --version "[1.2.5-netcore]" --tool-path ".paket" --add-source https://www.myget.org/F/paket-netcore-as-tool/api/v2

and use paket as we always do.
How do updates fit in this scenario?

@enricosada enricosada force-pushed the enricosada:full_netcore branch from a7fa645 to 7e48b99 Sep 5, 2018

@forki

This comment has been minimized.

Copy link
Member

forki commented Sep 8, 2018

Is this ready?

@enricosada enricosada force-pushed the enricosada:full_netcore branch from 7e48b99 to 4dddf66 Sep 12, 2018

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 12, 2018

@forki nearly done, working on it.
at first, will be making the current paket and paket.bootstrapper nupkg valid dotnet tool.
Current packages layout with paket.exe/paket.bootstrapper in tools doesnt change, so is backward compatibile.

in a following PR i'll add the boostrapper script.

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 12, 2018

@nojaf the new bootstrapper script will upgrade paket too, to choosen version

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 14, 2018

ready to review (/cc @matthid )

i built a local package and pushed to myget, and i updated https://github.com/enricosada/paket-netcore-testing-as-tool/ with it, so can be tested easier (before merge).

there are some minor issues (see the https://github.com/enricosada/paket-netcore-testing-as-tool#known-bugs ) but can be fixed later, having on github helps to test github download too

@enricosada enricosada force-pushed the enricosada:full_netcore branch from c9254be to 275989c Sep 17, 2018

@enricosada enricosada changed the title [WIP] Full netcore Full netcore Sep 17, 2018

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 17, 2018

latest fix unix/osx support

@enricosada enricosada requested a review from matthid Sep 18, 2018

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 18, 2018

disabled the cache by default

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 18, 2018

added also the upload of Paket nupkg to github release.
That package will be used when download strategy is github

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 18, 2018

test failures are unrelated, already exists in master.

As for now, i am satified with this (it's no more wip), the only thing missing are:

  • proxy support (fails in @vasily-kirichenko environment)
  • cache support. atm is disabled by code, but that's just an enhancement

While paket can be installed as cli tool to some dir with a command, ihmo that should be not the reccomended way.
The new proposed flow for repo is in https://github.com/enricosada/paket-netcore-testing-as-tool/ with a .paket\paket.bootstrapper.proj who will pratically replace the magic mode paket.exe or the commit of paket.bootstrapper.exe

That support:

  • dotnet build et similar works from clean clone (it will automatically bootstrap, and do restore as usual)
  • .paket\paket.bootstrapper.exe.config for customization (like pin the paket version, or feeds)
  • dotnet restore .paket to just bootstrap paket as .paket/paket or .paket\paket.exe

as a note, flow currently do:

  1. install paket.bootstrapper as cli tool in .paket dir (support only nuget feed as source)
  2. run paket.bootstrapper to install paket as cli tool in .paket dir (support github/nuget sources)
  3. from here, everything is as usual, just .paket/paket (or .paket\paket.exe) is a native binary instead of a .net assembly

we can discuss later if we want to just install paket as cli too, but we lose github releases and possibly cache (but maybe the global nuget cache it's used by dotnet tool -i)

/cc @forki @matthid

enricosada added some commits May 3, 2018

paket full .net core, as .net core tool
both paket and paket.bootstrapper

convert Paket.Bootstrapper to .net sdk:

- appsettings not supported on .net core
- system web proxy not supported on .net core
- ssl3 is deprecated on .net core
workaround msbuild bug Microsoft/msbuild#3268
fix nuget feed urls if needed (msbuild pass `http:/` instead of `http://`)

@enricosada enricosada force-pushed the enricosada:full_netcore branch from 10a2adf to 3e429ad Sep 19, 2018

@forki forki merged commit 8c0ef3e into fsprojects:master Sep 21, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@matthid

This comment has been minimized.

Copy link
Member

matthid commented Sep 22, 2018

@enricosada Can we downgrade FSharp.Core for the Paket.Core library? I'm fine with updating Paket to 4.5.2 but Paket.Core gives me a headache right now in Fake :(

Also we have a problem in the dependencies:

image

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 22, 2018

@matthid good catch, i'll check.

also with v4.5 i am a bit scared of issue with AsyncPrimitives not found

@matthid

This comment has been minimized.

Copy link
Member

matthid commented Sep 22, 2018

Just for the record this is what I saw:

image

Took me quite a while to realize that this was due to the paket.core upgrade.
I think what happens is:

  • The Runtime is unifying dependencies to the highest (4.5.0.0)
  • SDK thought 4.3 if fine (due to dependencies fulfilled

-> error at runtime with a completely unrelated stacktrace

@matthid

This comment has been minimized.

Copy link
Member

matthid commented Sep 22, 2018

Even worse with an older (2.1.2) runtime all you see is:
image

matthid added a commit to fsharp/FAKE that referenced this pull request Sep 22, 2018

@enricosada

This comment has been minimized.

Copy link
Collaborator Author

enricosada commented Sep 22, 2018

@matthid sent PR #3373

Doesnt fix the nuspec for netstandard, but at least doesnt use fsharp.core v4.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.