Add namespace to load scripts

[andre-correia]

The generated load scripts don't have a namespace and this causes issues while importing assemblies that have modules defined with the same name as the assembly as that will be the default namespace.

Example:
Assembly A.dll defines module A. When this is imported via load script, I get the following error:

FS0247 - A namespace and a module named 'A' both occur in two parts of this assembly

This PR fixes this issue by adding the namespace PaketLoadScripts to all the generated load scripts.

[matthid]

But this is a breaking change, correct? Should we make that opt-in?

Load scripts will contain only #r lines, so how does adding a namespace change the behavior? Can you give an example?

[andre-correia]

As far as I am aware the load scripts contain both #r and #load lines.

With this change the load scripts will look like this:

namespace PaketLoadScripts

#r "Microsoft.CSharp" 
#r "System" 
#r "System.Configuration" 
#load (...)

@matthid ☝️

[andre-correia]

Regarding this being a breaking change, it doesn't look like.
I don't see any tests breaking due to this. I only see the same ones failing.

[enricosada]

I tried but i cannot create a clean repro of the error we get with private packages.

if we have a deps like

source https://www.nuget.org/api/v2/
framework: net462

generate_load_scripts: true

nuget Expecto.Hopac 8.4.3
nuget Expecto.FsCheck 8.4.3

and we load the scripts with

#load ".paket/load/Expecto.Hopac.fsx"
#load ".paket/load/Expecto.FsCheck.fsx"
#load ".paket/load/Expecto.fsx"

the fsi output is

[Loading E:\temp\pr3396\.paket\load\Argu.fsx
 Loading E:\temp\pr3396\.paket\load\Mono.Cecil.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.Hopac.fsx
 Loading E:\temp\pr3396\.paket\load\FsCheck.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.FsCheck.fsx
 Loading E:\temp\pr3396\fails.fsx]
namespace FSI_0002

namespace FSI_0002.Mono

namespace FSI_0002

namespace FSI_0002.Expecto

namespace FSI_0002

namespace FSI_0002.Expecto

namespace FSI_0002

because the packages with . like Expecto.FsCheck when loaded in fsi from Expecto.FsChecl.fsx generate a namespace Expecto.

this can be an issue because can be created multiple times => error

error FS0247: A namespace and a module named 'Expecto' both occur in two parts of this assembly

Renaming the file to Expecto_FsChecl.fsx fix the error too, but is a breaking change

Adding the namespace , make it like

[Loading E:\temp\pr3396\.paket\load\Argu.fsx
 Loading E:\temp\pr3396\.paket\load\Mono.Cecil.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.Hopac.fsx
 Loading E:\temp\pr3396\.paket\load\FsCheck.fsx
 Loading E:\temp\pr3396\.paket\load\Expecto.FsCheck.fsx
 Loading E:\temp\pr3396\fails.fsx]
namespace FSI_0002

fixing the previous error.

Ihmo it's not a breaking change.
These fsx are used only to #r some assemblies.
For loading assemblies, the namespace doesnt matter

So i think is just a good bugfix

[andre-correia]

@forki can you please review this? Thank you!

[matthid]

I'm just a bit confused how a namespace affects '#r' statements, should we report this to visualfsharp, it definitely feels like a bug to me?

[matthid]

Can someone show how exactly this can be reproduced and report it over there?

[andre-correia]

@matthid it looks like you are right but can we add this for the moment to paket as a workaround until it gets fixed on visualfsharp?

[matthid]

I'd prefer first opening the visualfsharp issue before merging this ;)

[enricosada]

I'll try repro the error another time in https://github.com/enricosada/load_issue_repro.
If doesnt work with pubic packages, probably can be done with redacted private packages, i'll do that.

and i'll open a bug in visualfsharp about it, let's see

[andre-correia]

@matthid can we merge this while @enricosada works on that? I am currently blocked by this and would be great to continue using this repo instead of forking it for the sake of this small fix. Thank you!

[matthid]

@andre-correia please feel free to reproduce and report yourself, you are definitely not blocked.

Historically, people don't care once stuff is merged...

[forki]

I think he meant he is blocked as in his project does not work as long as we don't merge and release the fix Matthias Dittrich <notifications@github.com> schrieb am Di., 30. Okt. 2018, 13:10:

…

@andre-correia <https://github.com/andre-correia> please feel free to reproduce and report yourself, you are definitely not blocked. Historically, people don't care once stuff is merged... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3396 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNO7krw95mHPLyauEnn3E94fguKBLks5uqEHEgaJpZM4XzXPQ> .

[andre-correia]

Exactly @forki . Unfortunately until mid of next week I don't have time to try to reproduce it with public packages but @enricosada offered to help with that. If we could go ahead witht the merge it would be much appreciated 👍

[matthid]

@forki I know, just saying how I see it ;)

[enricosada]

No luck yet with repro, but i am trying doing it.
when i have it, i'll open also a bug in vf# repo, but ihmo is a design choice of fsx, so is not something we can fix (and anyway, cannot change older fsi behaviour).

Afaik it's not a breaking change, because while we add a namespace, the code inside load script is only #r and #load so doesnt matter the namespace.
I cannot think another feature of load scripts that adding a namespace may break, because these doesnt contains code/function.