fsprojects / Paket

A dependency manager for .NET with support for NuGet packages and Git repositories.
https://fsprojects.github.io/Paket/
MIT License
2.02k stars 525 forks source link

--generate-load-scripts for .NET Core appears to create incorrect #r directives #2156

Open filipw opened 7 years ago

filipw commented 7 years ago

Description

--generate-load-scripts for .NET Core appears to create incorrect #r directives as it's including GAC references.

Repro steps

paket.dependencies

source https://nuget.org/api/v2
nuget Newtonsoft.Json

Run:

.paket\paket.exe install --generate-load-scripts load-script-framework netstandard1.6

Expected behavior

The produced CSX file contains only references to physical DLLs from packages folder.

Actual behavior

The produced CSX file contains GAC references too.

#r "System.Xml.Linq" 
#r "System.Xml" 
#r "mscorlib" 
#r "System.Core" 
#r "System" 
#r "System.Runtime.Serialization" 
#r "System.ComponentModel.Composition" 
#r "Microsoft.CSharp" 
#r "../../../packages/Microsoft.CSharp/lib/netstandard1.3/Microsoft.CSharp.dll" 
#r "../../../packages/Newtonsoft.Json/lib/netstandard1.0/Newtonsoft.Json.dll" 
#r "../../../packages/System.Dynamic.Runtime/lib/netstandard1.3/System.Dynamic.Runtime.dll" 
#r "../../../packages/System.IO.FileSystem.Primitives/lib/netstandard1.3/System.IO.FileSystem.Primitives.dll" 
#r "../../../packages/System.Linq/lib/netstandard1.6/System.Linq.dll" 
#r "../../../packages/System.Linq.Expressions/lib/netstandard1.6/System.Linq.Expressions.dll" 
#r "../../../packages/System.ObjectModel/lib/netstandard1.3/System.ObjectModel.dll" 
#r "../../../packages/System.Reflection.Emit/lib/netstandard1.3/System.Reflection.Emit.dll" 
#r "../../../packages/System.Reflection.Emit.ILGeneration/lib/netstandard1.3/System.Reflection.Emit.ILGeneration.dll" 
#r "../../../packages/System.Reflection.Emit.Lightweight/lib/netstandard1.3/System.Reflection.Emit.Lightweight.dll" 
#r "../../../packages/System.Reflection.TypeExtensions/lib/netstandard1.5/System.Reflection.TypeExtensions.dll" 
#r "../../../packages/System.Runtime.Handles/ref/netstandard1.3/System.Runtime.Handles.dll" 
#r "../../../packages/System.Runtime.Serialization.Primitives/lib/netstandard1.3/System.Runtime.Serialization.Primitives.dll" 
#r "../../../packages/System.Text.RegularExpressions/lib/netstandard1.6/System.Text.RegularExpressions.dll" 
#r "../../../packages/System.Threading/lib/netstandard1.3/System.Threading.dll" 
#r "../../../packages/System.Threading.Tasks.Extensions/lib/netstandard1.0/System.Threading.Tasks.Extensions.dll" 
#r "../../../packages/System.Xml.ReaderWriter/lib/netstandard1.3/System.Xml.ReaderWriter.dll" 
#r "../../../packages/System.Xml.XDocument/lib/netstandard1.3/System.Xml.XDocument.dll" 
#r "../../../packages/System.IO.FileSystem/ref/netstandard1.3/System.IO.FileSystem.dll" 

Known workarounds

When processing the file, it's possible to manually strip away the offending references.

filipw commented 7 years ago

Note that this issue is C# specific as F# scripting is all net461

forki commented 7 years ago

/cc @smoothdeveloper

smoothdeveloper commented 7 years ago

How can we get the list of assemblies to exclude? There is already hardcoded exclusion in generation but we can improve it

On 26-Feb-2017 15:19, "Steffen Forkmann" notifications@github.com wrote:

/cc @smoothdeveloper https://github.com/smoothdeveloper

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fsprojects/Paket/issues/2156#issuecomment-282558976, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFXiCeJ9810hN4LcCzipssY_IWsncHZks5rgYnmgaJpZM4MMZW_ .

smoothdeveloper commented 7 years ago

@filipw you can check this, with similar logic to exclude nuget package: https://github.com/fsprojects/Paket/blob/fc6683aac367b32d2ea0c00725f2ec27eb6a322b/src/Paket.Core/ScriptGeneration.fs#L102

If you have a list of assemblies / nuget to excude for dotnetcore, we can add similar and pass the framework script is generated for.

Can you precise what is the issue with loading those assemblies? what error message, is it with all C# scripting environments or only some?

filipw commented 7 years ago

I don't think there is a list, however for .NET Core / .NET Standard targets, the GAC references should never be there (i.e. #r "System.Xml.Linq"). They are only valid for Desktop CLR (net46 and so on).

These references are visible in NuGet packages as "Framework Assembly References". I.e. here at the bottom:

screenshot 2017-02-26 15 48 43

In other words, there is no point in generating a #r that doesn't point to a physical file for .NET Core.

smoothdeveloper commented 7 years ago

@filipw see generateCSharpScript in that paket sourcecode above, at frameworkRefLines location, would excluding all those if platform is netcore be correct?

matthid commented 7 years ago

@smoothdeveloper Yes I think that should work

smoothdeveloper commented 7 years ago

I need to work on few other things at this time, @matthid or @filipw would you give it a shot?

smoothdeveloper commented 7 years ago

I'm looking at nuspec files, I see exclude="Compile" stuff for example in Microsoft.CSharp.nuspec stuff.

@filipw I hope for now you can find a hack around to make use of the scripts, I'll need some help from paket guys to better understand how to surface that information to the script gen code.

I don't think going with hardcoded list is going to help us (ORLY?...) and that exclude="Compile" might be what we need to rely on, but I can't find anything in paket right now which holds this information, some help to get this in the install model information (framework assembly description is a bit anemic in retained information it seems).

forki commented 7 years ago

can you show what information you are looking for exactly? screenshot from the package?

filipw commented 7 years ago

Given the following nuspec:

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
  <metadata minClientVersion="2.12">
    <id>System.Xml.XDocument</id>
    <version>4.3.0</version>
    <title>System.Xml.XDocument</title>
    <authors>Microsoft</authors>
    <owners>microsoft,dotnetframework</owners>
    <requireLicenseAcceptance>true</requireLicenseAcceptance>
    <licenseUrl>http://go.microsoft.com/fwlink/?LinkId=329770</licenseUrl>
    <projectUrl>https://dot.net/</projectUrl>
    <iconUrl>http://go.microsoft.com/fwlink/?LinkID=288859</iconUrl>
    <description>Provides the classes for Language-Integrated Query (LINQ) to Extensible Markup Language (XML). LINQ to XML is an in-memory XML programming interface that enables you to modify XML documents efficiently and easily.

Commonly Used Types:
System.Xml.Linq.XElement
System.Xml.Linq.XAttribute
System.Xml.Linq.XDocument
System.Xml.Linq.XText
System.Xml.Linq.XNode
System.Xml.Linq.XContainer
System.Xml.Linq.XComment
System.Xml.Linq.XObject
System.Xml.Linq.XProcessingInstruction
System.Xml.Linq.XDocumentType

When using NuGet 3.x this package requires at least version 3.4.</description>
    <releaseNotes>https://go.microsoft.com/fwlink/?LinkID=799421</releaseNotes>
    <copyright>© Microsoft Corporation.  All rights reserved.</copyright>
    <serviceable>true</serviceable>
    <dependencies>
      <group targetFramework="MonoAndroid1.0" />
      <group targetFramework="MonoTouch1.0" />
      <group targetFramework=".NETFramework4.5" />
      <group targetFramework=".NETCore5.0">
        <dependency id="System.Collections" version="4.3.0" exclude="Compile" />
        <dependency id="System.Diagnostics.Debug" version="4.3.0" exclude="Compile" />
        <dependency id="System.Diagnostics.Tools" version="4.3.0" exclude="Compile" />
        <dependency id="System.Globalization" version="4.3.0" exclude="Compile" />
        <dependency id="System.IO" version="4.3.0" />
        <dependency id="System.Reflection" version="4.3.0" exclude="Compile" />
        <dependency id="System.Resources.ResourceManager" version="4.3.0" exclude="Compile" />
        <dependency id="System.Runtime" version="4.3.0" />
        <dependency id="System.Runtime.Extensions" version="4.3.0" exclude="Compile" />
        <dependency id="System.Text.Encoding" version="4.3.0" exclude="Compile" />
        <dependency id="System.Threading" version="4.3.0" exclude="Compile" />
        <dependency id="System.Xml.ReaderWriter" version="4.3.0" />
      </group>
      <group targetFramework=".NETStandard1.0">
        <dependency id="System.IO" version="4.3.0" />
        <dependency id="System.Runtime" version="4.3.0" />
        <dependency id="System.Xml.ReaderWriter" version="4.3.0" />
      </group>
      <group targetFramework=".NETStandard1.3">
        <dependency id="System.Collections" version="4.3.0" exclude="Compile" />
        <dependency id="System.Diagnostics.Debug" version="4.3.0" exclude="Compile" />
        <dependency id="System.Diagnostics.Tools" version="4.3.0" exclude="Compile" />
        <dependency id="System.Globalization" version="4.3.0" exclude="Compile" />
        <dependency id="System.IO" version="4.3.0" />
        <dependency id="System.Reflection" version="4.3.0" exclude="Compile" />
        <dependency id="System.Resources.ResourceManager" version="4.3.0" exclude="Compile" />
        <dependency id="System.Runtime" version="4.3.0" />
        <dependency id="System.Runtime.Extensions" version="4.3.0" exclude="Compile" />
        <dependency id="System.Text.Encoding" version="4.3.0" exclude="Compile" />
        <dependency id="System.Threading" version="4.3.0" exclude="Compile" />
        <dependency id="System.Xml.ReaderWriter" version="4.3.0" />
      </group>
      <group targetFramework=".NETPortable0.0-Profile259" />
      <group targetFramework="Windows8.0" />
      <group targetFramework="WindowsPhone8.0" />
      <group targetFramework="WindowsPhoneApp8.1" />
      <group targetFramework="Xamarin.iOS1.0" />
      <group targetFramework="Xamarin.Mac2.0" />
      <group targetFramework="Xamarin.TVOS1.0" />
      <group targetFramework="Xamarin.WatchOS1.0" />
    </dependencies>
    <frameworkAssemblies>
      <frameworkAssembly assemblyName="System.Xml.Linq" targetFramework=".NETFramework4.5" />
    </frameworkAssemblies>
  </metadata>
</package>

Anything from frameworkAssemblies doesn't apply in .NET Core (plus it has a targetFramework which doesn't seem to be respected at the moment).

forki commented 7 years ago
filipw commented 7 years ago

which targetframework is missing?

I just meant that even without explicitly excluding frameworkAssembly entries for .NET Core, <frameworkAssembly assemblyName="System.Xml.Linq" targetFramework=".NETFramework4.5" /> should have never been included there in the first place since it's targeting ".NETFramework4.5" rather than NET Standard / NET Core so that validation never seemed to have happened. Anyway, they should be excluded regardless.

smoothdeveloper commented 7 years ago

sorry guys, I'll try to finish the changes to exclude frameworkAssemblies tonight.

smoothdeveloper commented 7 years ago

@filipw I've made a PR, could you check that comment: https://github.com/fsprojects/Paket/pull/2162#issuecomment-283190489 if that matches your expectation?