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

Ref references need to not be copy local #1895

Closed
Alxandr opened this Issue Aug 27, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@Alxandr

Alxandr commented Aug 27, 2016

Description

Trying to build a netstandard application using msbuild, FAKE and paket (yes, it seems it's doable, and not even that hard), I've run into the problem that reference assemblies are being copied to the output directory, which blows up at runtime (when the reference assemblies are loaded, you get an error saying this is a reference assembly basically).

Oh, and if it's not clear, what I'm calling reference assemblies is anything found in a nuget package under the "ref" folder (as opposed to the "lib" folder).

Repro steps

Add System.Runtime as a dependency.

Expected behavior

The ref package should be added as Private: false (Copy local: false, my limited understanding of MSBuild has lead me to believe that these are the same).

<When Condition="$(TargetFrameworkIdentifier) == '.NETStandard' And $(TargetFrameworkVersion) == 'v1.5'">
  <ItemGroup>
    <Reference Include="System.Runtime">
      <HintPath>..\..\packages\System.Runtime\ref\netstandard1.5\System.Runtime.dll</HintPath>
      <Private>False</Private>
      <Paket>True</Paket>
    </Reference>
  </ItemGroup>
</When>

Actual behavior

It's added such that it's copied to the output directory.

<When Condition="$(TargetFrameworkIdentifier) == '.NETStandard' And $(TargetFrameworkVersion) == 'v1.5'">
  <ItemGroup>
    <Reference Include="System.Runtime">
      <HintPath>..\..\packages\System.Runtime\ref\netstandard1.5\System.Runtime.dll</HintPath>
      <Private>True</Private>
      <Paket>True</Paket>
    </Reference>
  </ItemGroup>
</When>

Known workarounds

Manually fix msbuild file every time you do paket install.

Related information

  • Operating system: windows
  • Branch: not sure, I've done paket.bootstrapper and paket.bootstrapper prerelease
  • .NET Runtime, CoreCLR or Mono Version: .NET 4.6
  • Performance information, links to performance testing scripts: ?

@forki forki closed this in d572b66 Aug 29, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 29, 2016

Member

it's in latest alpha. please try it out

Member

forki commented Aug 29, 2016

it's in latest alpha. please try it out

forki added a commit that referenced this issue Aug 29, 2016

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 31, 2016

Member

For now this is the correct thing to do. In the future I think we need to analyse the runtime folder (see todo in my dotnetcore PR). This is because we might need to copy other files to the output directory (especially when compiling for netcoreapp). This is what the runtime.json is for (if I understood that correctly).

It doesn't matter for now as nobody is using fsproj to compile for netcoreapp and it's not recommended either. Exception is the new FAKE which is compiling scripts for netcoreapp and therefore needs this information :(

Member

matthid commented Aug 31, 2016

For now this is the correct thing to do. In the future I think we need to analyse the runtime folder (see todo in my dotnetcore PR). This is because we might need to copy other files to the output directory (especially when compiling for netcoreapp). This is what the runtime.json is for (if I understood that correctly).

It doesn't matter for now as nobody is using fsproj to compile for netcoreapp and it's not recommended either. Exception is the new FAKE which is compiling scripts for netcoreapp and therefore needs this information :(

@Alxandr

This comment has been minimized.

Show comment
Hide comment
@Alxandr

Alxandr Sep 1, 2016

@matthid Actually, that's exactly what I was doing. I did a full day of experimenting to see if I could compile to .NETStandard using regular fsc, fake, msbuild and paket, just by slightly modifying the fsproj files. And it worked. I managed to produce dlls that were .NETStandard 1.3, meaning they should (in theory) run on .NET Core and .NET regular. The tests failed spectacularly though, cause paket was copying ref-assemblies and xunit was loading them (cause local dir has precedence over framework it would seem). Thus it blew up, and I made this issue.

As per my understanding, anything in the ref folder should be reference assemblies only. IE; it only contains metadata (method names, but no implementation). Though I don't know what runtime.json is.

Alxandr commented Sep 1, 2016

@matthid Actually, that's exactly what I was doing. I did a full day of experimenting to see if I could compile to .NETStandard using regular fsc, fake, msbuild and paket, just by slightly modifying the fsproj files. And it worked. I managed to produce dlls that were .NETStandard 1.3, meaning they should (in theory) run on .NET Core and .NET regular. The tests failed spectacularly though, cause paket was copying ref-assemblies and xunit was loading them (cause local dir has precedence over framework it would seem). Thus it blew up, and I made this issue.

As per my understanding, anything in the ref folder should be reference assemblies only. IE; it only contains metadata (method names, but no implementation). Though I don't know what runtime.json is.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Sep 1, 2016

Member

Guys we already analyze runtime folder. And I think I set copylocal to false in recent commit

Member

forki commented Sep 1, 2016

Guys we already analyze runtime folder. And I think I set copylocal to false in recent commit

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Sep 1, 2016

Member

@forki what we do right now is not enough as we need to parse runtime.json and handle the runtime tree. For example if runtime.json says that 'run-a' includes 'run-b' we need to copy files from the 'run-b' folder to the output, even when we compile for 'run-a'. the runtime.json specifies the dependency tree.

for example 'win7-x64' includes 'win-x64' includes 'win' ... but we should threat the strings as black boxes, see https://docs.microsoft.com/en-us/dotnet/articles/core/rid-catalog

@Alxandr It only matters if you want to produce and run a netcoreapp application which should run on itself... For a netstandard library and simple usage of it we don't need that...

Note that my analysis might be wrong so please correct me in that case...

Member

matthid commented Sep 1, 2016

@forki what we do right now is not enough as we need to parse runtime.json and handle the runtime tree. For example if runtime.json says that 'run-a' includes 'run-b' we need to copy files from the 'run-b' folder to the output, even when we compile for 'run-a'. the runtime.json specifies the dependency tree.

for example 'win7-x64' includes 'win-x64' includes 'win' ... but we should threat the strings as black boxes, see https://docs.microsoft.com/en-us/dotnet/articles/core/rid-catalog

@Alxandr It only matters if you want to produce and run a netcoreapp application which should run on itself... For a netstandard library and simple usage of it we don't need that...

Note that my analysis might be wrong so please correct me in that case...

@evilz

This comment has been minimized.

Show comment
Hide comment
@evilz

evilz Sep 2, 2016

Hi,

This change seems to create another issue.
In a c# project in net45 We're using Microsoft.Extensions.Options.ConfigurationExtensions that use System.ComponentModel.TypeConverter that contains a ref folder. To run it, we needs the dll in the bin folder

Moreover we can't override it using copy_local:true

https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/ProjectFile.fs#L644

let privateSettings = 
                        if relativePath.Contains @"\ref\" then "False"
                        elif copyLocal then "True" else "False"

evilz commented Sep 2, 2016

Hi,

This change seems to create another issue.
In a c# project in net45 We're using Microsoft.Extensions.Options.ConfigurationExtensions that use System.ComponentModel.TypeConverter that contains a ref folder. To run it, we needs the dll in the bin folder

Moreover we can't override it using copy_local:true

https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/ProjectFile.fs#L644

let privateSettings = 
                        if relativePath.Contains @"\ref\" then "False"
                        elif copyLocal then "True" else "False"
@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Sep 2, 2016

Member

@evilz Yeah but copying the dll from the ref folder is still wrong. We need to copy the file from the lib folder (if at all).

Member

matthid commented Sep 2, 2016

@evilz Yeah but copying the dll from the ref folder is still wrong. We need to copy the file from the lib folder (if at all).

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Sep 2, 2016

Member

Maybe always using the ref folder was not the correct thing for now (until we get that sorted out) :/

Member

matthid commented Sep 2, 2016

Maybe always using the ref folder was not the correct thing for now (until we get that sorted out) :/

@evilz

This comment has been minimized.

Show comment
Hide comment
@evilz

evilz Sep 2, 2016

@matthid It's look that the files are exactly the same

PS P:\> cd O:\System.ComponentModel.TypeConverter.4.1.0\lib\net45
PS O:\System.ComponentModel.TypeConverter.4.1.0\lib\net45> Get-FileHash -Path .\System.ComponentModel.TypeConverter.dll

Algorithm       Hash                                                                   Path
---------       ----                                                                   ----
SHA256          0AD6A7017A54D99E102C17F40EA45D312AAEED96664EA5FF6DE1755BD4F6C6FD       O:\System.ComponentModel.Type...



PS O:\System.ComponentModel.TypeConverter.4.1.0\ref\net45> Get-FileHash -Path .\System.ComponentModel.TypeConverter.dll

Algorithm       Hash                                                                   Path
---------       ----                                                                   ----
SHA256          0AD6A7017A54D99E102C17F40EA45D312AAEED96664EA5FF6DE1755BD4F6C6FD       O:\System.ComponentModel.Type...

evilz commented Sep 2, 2016

@matthid It's look that the files are exactly the same

PS P:\> cd O:\System.ComponentModel.TypeConverter.4.1.0\lib\net45
PS O:\System.ComponentModel.TypeConverter.4.1.0\lib\net45> Get-FileHash -Path .\System.ComponentModel.TypeConverter.dll

Algorithm       Hash                                                                   Path
---------       ----                                                                   ----
SHA256          0AD6A7017A54D99E102C17F40EA45D312AAEED96664EA5FF6DE1755BD4F6C6FD       O:\System.ComponentModel.Type...



PS O:\System.ComponentModel.TypeConverter.4.1.0\ref\net45> Get-FileHash -Path .\System.ComponentModel.TypeConverter.dll

Algorithm       Hash                                                                   Path
---------       ----                                                                   ----
SHA256          0AD6A7017A54D99E102C17F40EA45D312AAEED96664EA5FF6DE1755BD4F6C6FD       O:\System.ComponentModel.Type...
@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Sep 2, 2016

Member

@evilz Maybe for this package. But I don't think they need to be the same in all situations...
IMHO "ref" stands for reference assemblies and might not contain any code (only API surface). Therefore they should never be copied to the output directory.

Member

matthid commented Sep 2, 2016

@evilz Maybe for this package. But I don't think they need to be the same in all situations...
IMHO "ref" stands for reference assemblies and might not contain any code (only API surface). Therefore they should never be copied to the output directory.

@evilz

This comment has been minimized.

Show comment
Hide comment
@evilz

evilz Sep 2, 2016

Can we just force the copy_local to copy the lib from lib folder ?
Is this a good quick fix ?

evilz commented Sep 2, 2016

Can we just force the copy_local to copy the lib from lib folder ?
Is this a good quick fix ?

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Sep 2, 2016

Member

I think copying the binary from the lib folder is actually the correct fix. But it isn't a quick fix as we need to generate additional stuff into the project file...
A quick fix might be to ignore the ref folder for now. But that might break other people (for example me in FAKE)...

Member

matthid commented Sep 2, 2016

I think copying the binary from the lib folder is actually the correct fix. But it isn't a quick fix as we need to generate additional stuff into the project file...
A quick fix might be to ignore the ref folder for now. But that might break other people (for example me in FAKE)...

@matthid matthid referenced this issue Sep 3, 2016

Closed

TODOs to complete Dotnetcore Support #1909

0 of 12 tasks complete
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Sep 3, 2016

Member

@evilz I changed it so that your manual changes are used. @matthid this is probably just a temporary fix

Member

forki commented Sep 3, 2016

@evilz I changed it so that your manual changes are used. @matthid this is probably just a temporary fix

@evilz

This comment has been minimized.

Show comment
Hide comment
@evilz

evilz Sep 3, 2016

@forki thank ! I will test this next monday.

evilz commented Sep 3, 2016

@forki thank ! I will test this next monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment