[WIP]Adding paket package resolution to fsi and VF# editor #2483

forki · 2017-02-24T12:53:31Z

This is "experimental concept development" for fsharp/fslang-suggestions#542

@dsyme says: We are investigating techniques to implement integrated package management into the F# scripting model. This is experimental and won't be integrated in its current form, but is useful as a proof-of-concept.

The topic is discussed on the thread above.

smoothdeveloper · 2017-02-24T13:08:19Z

@forki I wonder to resolve the paket.dependencies / .paket folder, if we could instead crowl up the directory structure and only resort to temporary place when no paket.dependencies / .paket folder were found?

dsyme · 2017-02-24T13:15:57Z

src/fsharp/fsi/fsi.fs

+
+    let resolvePaket ctok istate errorLogger m =
+        if not needsPaketInstall then istate else
+        let paketExePath = @"D:\temp\fsi\.paket\paket.exe"


Could there be a directive to specify this location before anything else? Perhaps the default could be the local directory + .paket/paket.exe, or search directories upwards to look for a paket.exe

Yep this will need to evolve into bootstrapper layer

forki · 2017-02-24T13:17:02Z

Yes that would ensure that you get all the deps that are already in the deps file ootb.
We just need something like "#r paket:" to activate the install switch

dsyme · 2017-02-24T13:39:23Z

Yep this will need to evolve into bootstrapper layer

I'm not sure we would need a bootstrapper layer in the first case (if that means hot-updating paket.exe?) - editor integration for this logic plus a good error message saying "put paket.exe in a parent .paket directory " would be enough. I suppose if paket.bootstrapper.exe exists we could run it, maybe that's what you mean by a bootstrapper.

vasily-kirichenko · 2017-02-24T14:20:44Z

src/fsharp/fsi/fsi.fs

@@ -1859,20 +1861,57 @@ type internal FsiInteractionProcessor
            stopProcessingRecovery e range0    
            None

+    let tempDir = @"D:\temp\fsi"
+    let paketPrefix = "paket: "


[<Literal>]?

vasily-kirichenko · 2017-02-24T14:23:18Z

src/fsharp/fsi/fsi.fs

@@ -1859,20 +1861,57 @@ type internal FsiInteractionProcessor
            stopProcessingRecovery e range0    
            None

+    let tempDir = @"D:\temp\fsi"


System.Environment.ExpandEnvironmentVariables "%TEMP%\fsi"?

vasily-kirichenko · 2017-02-24T14:24:14Z

src/fsharp/fsi/fsi.fs

@@ -1859,20 +1861,57 @@ type internal FsiInteractionProcessor
            stopProcessingRecovery e range0    
            None

+    let tempDir = @"D:\temp\fsi"
+    let paketPrefix = "paket: "
+    let paketExePath = @"D:\temp\fsi\.paket\paket.exe"


Path.Combine(tempDir, ".paket", "paket.exe")

vasily-kirichenko · 2017-02-24T14:24:53Z

src/fsharp/fsi/fsi.fs

+        let p = Process.Start(startInfo)
+        p.WaitForExit()
+        if p.ExitCode <> 0 then
+            failwithf "Paket restore failed."


We need more info here.

Yes. Though it's just a hack right now :) I'm going to iterate on it to make it activate at design-time

isaacabraham · 2017-02-24T16:39:47Z

Any reason why we just don't go for a #paket node rather than something explicitly coupled to #r? I suppose I could see #load being used in the same way (for Paket to read from GitHub etc.) but maybe putting it right at the top level could open up possibilities for adding features to F# scripts aside from just reference and load?

cartermp · 2017-02-24T17:53:33Z

(Side note: should we split out an issue to discuss this in parallel with this PR?)

There is also the .NET Core vector to consider here. Note that one of the problems @KevinRansom is looking into affects this. For example, assume that FSI loads System.Runtime v4.1.0. Somewhere in the script, a paket reference pulls in System.Runtime v4.1.1 via a transitive dependency. This could cause any number of problems. We already have this issue today with directly referencing an assembly via #r. There are a number of ways to approach this:

Fail to reference the paket reference (or assembly reference), citing a version number issue in the error message.
Restart the FSI process and load the higher version of the assembly
Attempt to re-calculate assemblies to reference on the fly

This poses a problem given how assemblies are laid out on disk in a .NET Core world, and assumptions in this area for the current FSI implementation need to be teased out and rewritten. It's going to be quite a bit of work and we're not confident we know the correct solution here. My question is:

Does Paket affect this beyond possibly pulling in a different assembly version than what is currently loaded in an FSI process? If we're to take this change (I support taking it, BTW), then we'll have to make sure we can still solve the current problem we have in a .NET Core 2.0 timeframe. Otherwise, we'll be shipping an FSI which has an additional vector with which you can have issues with assembly references.

forki · 2017-02-24T17:58:47Z

This is just WIP for the language design issue fsharp/fslang-suggestions#542 - It's not ready for the real deep engineering questions. Please discuss in the language design issue.

forki · 2017-02-24T18:01:54Z

@enricosada it seems .NET core doesn't like

Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData)

what do we use instead?

matthid · 2017-02-24T22:35:36Z

@forki I used an environment variable in my netcore ports (fake/paket). this is probably one of the apis comming back with netstandard 2.

forki · 2017-02-24T22:55:47Z

Can you please send a pull request to my fork here or just link to the line in paket? Really appreciate it. Didn't find it yet

dsyme · 2017-02-25T01:01:35Z

@cartermp - Yes, this is just an experiment. Don’t even bother looking at it. We’re mucking about to see what integrated package management might look like. We’re not seriously going to even suggest pulling that PR in anything remotely like its current form. I'm just working with @forki to hack through a demonstrator.

My suggestion is that we just label the PR as “Totally Experimental” and you can safely ignore it.

It's my opinion that whatever we do here must work on both .NET Framework and .NET Core. We shouldn’t commit to anything in this space until .NET Core scripting is done, and we have a solution that works for both .NET Core and .NET Framework.

dsyme · 2017-02-25T01:03:45Z

@forki To be honest we should probably close this PR and work from your branch until we're done with mucking about.

dsyme · 2017-02-25T01:40:54Z

@isaacabraham

Any reason why we just don't go for a #paket node rather than something explicitly coupled to #r?

Let's discuss on the fslang-suggestion.

dsyme · 2017-02-25T02:10:22Z

src/fsharp/CompileOps.fs

@@ -4624,8 +4624,7 @@ let RequireDLL (ctok, tcImports:TcImports, tcEnv, thisAssemblyName, m, file) =
    let tcEnv = (tcEnv, asms) ||> List.fold (fun tcEnv asm -> AddCcuToTcEnv(g,amap,m,tcEnv,thisAssemblyName,asm.FSharpViewOfMetadata,asm.AssemblyAutoOpenAttributes,asm.AssemblyInternalsVisibleToAttributes)) 
    tcEnv,(dllinfos,asms)

-
-let paketPrefix = "paket: "


what was this change about?

I mean, we only want things starting with paket: to be included, right? And we want #r "paket" to be a normal DLL reference?

Yeah I should revert that part. The thing I wanted to support was basically a switch to just activate the paket references from the ambient paket.dependencies that is already existing in the repo. Basically I thought

#r "paket: "

Looks weird, so let's make it

#r "paket"

But that's something for the language design part. I will use the first version for now.

dsyme · 2017-02-25T02:16:36Z

src/fsharp/CompileOps.fs

+                Some loadScript
+
+            | _ ->
+                File.WriteAllLines(paketDepsFile.FullName, packageManagerTextLines)


This seems to overwrite the existing paket.dependencies

Or was the idea that simply adding #r "paket" would get you the packages listed there? But then what if there are additional packages specified in the script?

Yes the idea was to use the existing file and append to it with additional deps from the script.

forki · 2017-02-25T08:32:14Z

@cartermp and @dsyme - I'd like to keep it on this fork since it's the only with CI. And CI already pointed out problems.

forki · 2017-02-25T10:41:20Z

@dsyme I remove the bootstrapper.exe call. With Paket4 we will always use magic mode. https://fsprojects.github.io/Paket/bootstrapper.html#Magic-mode

tldr: we renamed the bootstrapper to paket.exe and it will magically restore the real paket.exe to some appfolder. So you commit 40KB to your repo (as paket.exe) and never worry again. It's magic!

ErikSchierboom · 2017-07-04T13:57:23Z

This is great! My primary use case for F# at work is writing scripts with it, and this would be very helpful!

matthid · 2017-07-08T22:09:27Z

I suggest to add #r "paket: group NetCore" to reference a group of an existing paket.dependencies of the current solution.
This would in best case add all references as specified in the lockfile and in worst case do a install/update to write the lockfile first.

Will probably update FAKE 5 to use this PR and then we can see how it works out in practice.

forki · 2017-07-09T08:45:40Z

Yes I think currently we always load main group. That would be weird in FAKE settings. Regarding fake: it's a high risk to add this right now to fake since tooling will not support it and therefore ionide and vs will always show squigglies. Currently it would basically be a fork of the language. I'm not sure if that's a good thing. Unfortunately we don't how long it will take to get it in.
On the other hand most C# builds forked the c# language and people still think it is valuable. I have no good answer here

matthid · 2017-07-25T11:31:54Z

@dsyme If we can "approve in principle" we could go ahead and test this in the FAKE 5 alpha.
@forki is against a potential fork if we just go ahead and integrate this PR.

This could potentially solve some problems we have in the netcore world in FAKE 5. But obviously we can only be sure if we try to implement it there. Also it would be nice to tell us how this relates to #2667

Maybe it would make more sense for the visualfsharp team to accept this if we can proof that it works in practice with netcore and FAKE 5?

Thanks!

forki · 2017-08-26T07:38:03Z

Ping.
We would really like to see this going in or at least e approved so that we can use it in FAKE.
It should not be merged together with .NET core decisions. This is already super useful as is.

0x53A · 2017-08-26T14:51:42Z

I know that Microsoft as a Corporation probably has relative strict support and backwards-compatibility requirements, but I would really love if this could be "preview-released" NOW, even if it will be broken later.

Otherwise I fear that we will repeat this discussion in a year or two from now.

I think we will trip over so many issues, that at least one breaking-change release will be required anyway.

My suggestion would be to either enable this behind an extra flag (fsi --enable-experimental-package-support) or print a (hidable with a different flag) warning if the parser encounters a usage.

This would make it absolutely clear that there are no support guarantees, but still enable us to use this and gather experience.

KevinRansom · 2017-08-28T21:29:11Z

I am so very, very sorry about this. it's on the list.

realvictorprm · 2017-11-15T12:47:18Z

Ping :-)

dsyme · 2017-12-01T16:38:45Z

Closing in favour of #4042

(@KevinRansom @forki is this correct?)

forki · 2017-12-01T17:00:05Z

I'm in favor of having the paket stuff in the box but that's probably just me

dsyme · 2017-12-01T18:41:09Z

@forki Either way we'd do that as a separate PR over #4042 right?

KevinRansom · 2017-12-01T23:41:53Z

@forki I agree that paket should be in the box. But it will certainly be a separate PR. Thinking about it I expect the nuget package manager will be a separate PR too. This PR should probably handle the extensibility and the wiring, and the actual implementations of specific packagemanagement should follow!

KevinRansom · 2017-12-01T23:46:26Z

I would like to keep this open for reference pls.

cartermp · 2018-11-01T20:20:15Z

Closing in favor of #5850 which uses the same core mechanism

msftclas added the cla-already-signed label Feb 24, 2017

forki mentioned this pull request Feb 24, 2017

Package references in F# scripting fsharp/fslang-suggestions#542

Closed

forki force-pushed the paket branch 2 times, most recently from 6604c36 to 0f5194a Compare February 24, 2017 12:55

forki changed the title ~~[WIP] Addin "#r paket: " support to fsi~~ [WIP] Adding "#r paket: " support to fsi Feb 24, 2017

dsyme reviewed Feb 24, 2017

View reviewed changes

vasily-kirichenko reviewed Feb 24, 2017

View reviewed changes

dsyme changed the title ~~[WIP] Adding "#r paket: " support to fsi~~ [Totally Experimental] Adding "#r paket: " support to fsi Feb 25, 2017

dsyme reviewed Feb 25, 2017

View reviewed changes

forki force-pushed the paket branch 3 times, most recently from fb8c3ab to 66e5ed5 Compare February 25, 2017 10:38

Give warning if we can not load an extension

f195442

forki closed this Jul 4, 2017

forki reopened this Jul 4, 2017

msftclas added the cla-already-signed label Jul 4, 2017

dsyme mentioned this pull request Jul 8, 2017

support .NET Standard 2.0 in F# Interactive and type providers #3307

Closed

dsyme mentioned this pull request Sep 1, 2017

Full FSI support on .NET Core 2.0 #2407

Closed

msftgits removed the cla-already-signed label Sep 26, 2017

matthid mentioned this pull request Oct 11, 2017

Proposal: Type provider design-time DLLs should be chosen more appropriately #3736

Closed

KevinRansom mentioned this pull request Dec 1, 2017

[WIP] - Build packagemanagement based on Steffen's Package manager design. #4042

Closed

dsyme closed this Dec 1, 2017

KevinRansom reopened this Dec 1, 2017

KevinRansom removed the Candidate for F# 4.3 label Dec 7, 2017

KevinRansom changed the title ~~Adding paket package resolution to fsi and VF# editor~~ [WIP]Adding paket package resolution to fsi and VF# editor Dec 7, 2017

enricosada mentioned this pull request Jan 24, 2018

Improve Paket.Bootstrapper.exe and Paket.exe for use in F# Scripting fsprojects/Paket#1759

Closed

KevinRansom mentioned this pull request Oct 31, 2018

Package Management for FSI #5850

Merged

cartermp closed this Nov 1, 2018

[WIP]Adding paket package resolution to fsi and VF# editor #2483

[WIP]Adding paket package resolution to fsi and VF# editor #2483

Conversation

forki commented Feb 24, 2017 • edited by dsyme Loading

smoothdeveloper commented Feb 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forki commented Feb 24, 2017

dsyme commented Feb 24, 2017 • edited Loading

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

isaacabraham commented Feb 24, 2017

cartermp commented Feb 24, 2017 • edited Loading

forki commented Feb 24, 2017

forki commented Feb 24, 2017

matthid commented Feb 24, 2017

forki commented Feb 24, 2017

dsyme commented Feb 25, 2017 • edited Loading

dsyme commented Feb 25, 2017

dsyme commented Feb 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsyme Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forki commented Feb 25, 2017

forki commented Feb 25, 2017

ErikSchierboom commented Jul 4, 2017

matthid commented Jul 8, 2017

forki commented Jul 9, 2017

matthid commented Jul 25, 2017

forki commented Aug 26, 2017

0x53A commented Aug 26, 2017 • edited Loading

KevinRansom commented Aug 28, 2017

realvictorprm commented Nov 15, 2017

dsyme commented Dec 1, 2017

forki commented Dec 1, 2017 via email • edited by dsyme Loading

dsyme commented Dec 1, 2017 • edited Loading

KevinRansom commented Dec 1, 2017

KevinRansom commented Dec 1, 2017

cartermp commented Nov 1, 2018

forki commented Feb 24, 2017 •

edited by dsyme

Loading

dsyme commented Feb 24, 2017 •

edited

Loading

cartermp commented Feb 24, 2017 •

edited

Loading

dsyme commented Feb 25, 2017 •

edited

Loading

dsyme Feb 25, 2017 •

edited

Loading

0x53A commented Aug 26, 2017 •

edited

Loading

forki commented Dec 1, 2017 via email •

edited by dsyme

Loading

dsyme commented Dec 1, 2017 •

edited

Loading