Duplicate references to framework assemblies added #1333

Closed
TeaDrivenDev opened this Issue Dec 30, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@TeaDrivenDev
Contributor

TeaDrivenDev commented Dec 30, 2015

I think this is very similar to #932, if not actually exactly the same thing.

When adding a package to a project using Paket, a <Choose> section is added to the project file with references to all the assemblies in that package, as well as apparently any of the framework assemblies that are stated as dependencies in the package but not already referenced by the project. When the next package is added, those references to the framework assemblies are not found and added again in that package's <Choose> section.

An example:

FsXaml and FSharp.Desktop.UI reference the following framework assemblies:

  • PresentationCore
  • PresentationFramework
  • System.Xaml
  • System.Xml (FsXaml only)
  • WindowsBase

I now use Paket to add those to a project that already references System.Xaml, System.Xml and WindowsBase (added through the References dialog, for example).

Adding FSharp.Desktop.UI now inserts the following into the *.fsproj file:

<Choose>
  <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.5' Or $(TargetFrameworkVersion) == 'v4.5.1' Or $(  TargetFrameworkVersion) == 'v4.5.2' Or $(TargetFrameworkVersion) == 'v4.5.3' Or $(TargetFrameworkVersion) == 'v4.6' Or $(TargetFrameworkVersion) ==   'v4.6.1')">
    <ItemGroup>
      <Reference Include="FSharp.Desktop.UI">
        <HintPath>..\..\packages\FSharp.Desktop.UI\lib\net45\FSharp.Desktop.UI.dll</HintPath>
        <Private>True</Private>
        <Paket>True</Paket>
      </Reference>
      <Reference Include="PresentationCore">
        <Paket>True</Paket>
      </Reference>
      <Reference Include="PresentationFramework">
        <Paket>True</Paket>
      </Reference>
    </ItemGroup>
  </When>
</Choose>

Next I add FsXaml; System.Xaml, System.Xml and WindowsBase are detected correctly, but the other two framework assembly references are not "seen" and get included again for FsXaml:

<Choose>
  <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.5' Or $(TargetFrameworkVersion) == 'v4.5.1' Or $(  TargetFrameworkVersion) == 'v4.5.2' Or $(TargetFrameworkVersion) == 'v4.5.3' Or $(TargetFrameworkVersion) == 'v4.6' Or $(TargetFrameworkVersion) ==   'v4.6.1')">
    <ItemGroup>
      <Reference Include="FsXaml.Wpf.TypeProvider">
        <HintPath>..\..\packages\FsXaml.Wpf\lib\net45\FsXaml.Wpf.TypeProvider.dll</HintPath>
        <Private>True</Private>
        <Paket>True</Paket>
      </Reference>
      <Reference Include="FsXaml.Wpf">
        <HintPath>..\..\packages\FsXaml.Wpf\lib\net45\FsXaml.Wpf.dll</HintPath>
        <Private>True</Private>
        <Paket>True</Paket>
      </Reference>
      <Reference Include="PresentationCore">
        <Paket>True</Paket>
      </Reference>
      <Reference Include="PresentationFramework">
        <Paket>True</Paket>
      </Reference>
    </ItemGroup>
  </When>
</Choose>

The result is that the respective framework assemblies show up multiple times in Solution Explorer; however, as far as I could see, the project still compiles and runs correctly, probably because they are always references to the exact same thing and thus don't cause conflicts as seen in #932.

My naive first thought would of course be simply putting all references to framework assemblies in normal ItemGroups, but considering we're dealing with Visual Studio, MSBuild and NuGet, that sounds too easy.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Dec 30, 2015

Member

I guess we still need Choose Nodes, but need to merge all the packages...

Member

forki commented Dec 30, 2015

I guess we still need Choose Nodes, but need to merge all the packages...

@forki forki added the bug label Dec 30, 2015

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Dec 30, 2015

Member

I wonder why it installed the references at all. Usually it should not add the reference if it already finds one which doesn't have a Paket flag.

(this would also lead to a workaround)

Member

forki commented Dec 30, 2015

I wonder why it installed the references at all. Usually it should not add the reference if it already finds one which doesn't have a Paket flag.

(this would also lead to a workaround)

@mexx

This comment has been minimized.

Show comment
Hide comment
@mexx

mexx Dec 30, 2015

Member

I've also already noticed this behavior, but since it leads to no problems during the build and the result assemblies also have no problems at runtime and it's only weird display in the Solution Explorer, I haven't reported this one ;)

Currently a Choose node is generated per package and contains all assemblies, contained in the package as well as framework ones. Only the references already present without Paket flag will be removed from the list of references to include.

To deduplicate the nodes, we would need to collect framework references together with the conditions when each of it should be inserted and provide separate Choose nodes for them.
When removing packages, we would need to consider which of those references should also be removed, as not referenced anymore by any of the remaining packages.

Member

mexx commented Dec 30, 2015

I've also already noticed this behavior, but since it leads to no problems during the build and the result assemblies also have no problems at runtime and it's only weird display in the Solution Explorer, I haven't reported this one ;)

Currently a Choose node is generated per package and contains all assemblies, contained in the package as well as framework ones. Only the references already present without Paket flag will be removed from the list of references to include.

To deduplicate the nodes, we would need to collect framework references together with the conditions when each of it should be inserted and provide separate Choose nodes for them.
When removing packages, we would need to consider which of those references should also be removed, as not referenced anymore by any of the remaining packages.

@pavel-khritonenko

This comment has been minimized.

Show comment
Hide comment
@pavel-khritonenko

pavel-khritonenko Aug 10, 2016

I have the same behavior. If you add to an empty F# project nuget packages - FSharp.Data and WindowsAzure.Storage you will see System.Xml.Linq twice under References node in the Solution Explorer:

image

Although I have 3 System.Xml entries and 2 System.Xml.Linq entries in our solution it doesn't affect the build process.

Paket version 3.11.2.0

packet.lock:

  remote: https://www.nuget.org/api/v2
    FSharp.Core (4.0.0.1)
    FSharp.Data (2.3.2)
      Zlib.Portable (>= 1.11) - framework: portable-net45+sl5+win8, portable-net45+win8, portable-net45+win8+wp8+wpa81
    Microsoft.Azure.KeyVault.Core (1.0) - framework: >= net40, wpv8.0
    Microsoft.Data.Edm (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    Microsoft.Data.OData (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
      Microsoft.Data.Edm (5.7)
      System.Spatial (5.7)
    Microsoft.Data.Services.Client (5.7) - framework: >= net40
      Microsoft.Data.OData (5.7)
    Newtonsoft.Json (9.0.1) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    System.Spatial (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    WindowsAzure.Storage (7.1.2)
      Microsoft.Azure.KeyVault.Core (>= 1.0) - framework: >= net40, wpv8.0
      Microsoft.Data.OData (>= 5.6.4) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
      Microsoft.Data.Services.Client (>= 5.6.4) - framework: >= net40
      Newtonsoft.Json (>= 6.0.8) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    Zlib.Portable (1.11) - framework: portable-net45+sl5+win8, portable-net45+win8, portable-net45+win8+wp8+wpa81```

pavel-khritonenko commented Aug 10, 2016

I have the same behavior. If you add to an empty F# project nuget packages - FSharp.Data and WindowsAzure.Storage you will see System.Xml.Linq twice under References node in the Solution Explorer:

image

Although I have 3 System.Xml entries and 2 System.Xml.Linq entries in our solution it doesn't affect the build process.

Paket version 3.11.2.0

packet.lock:

  remote: https://www.nuget.org/api/v2
    FSharp.Core (4.0.0.1)
    FSharp.Data (2.3.2)
      Zlib.Portable (>= 1.11) - framework: portable-net45+sl5+win8, portable-net45+win8, portable-net45+win8+wp8+wpa81
    Microsoft.Azure.KeyVault.Core (1.0) - framework: >= net40, wpv8.0
    Microsoft.Data.Edm (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    Microsoft.Data.OData (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
      Microsoft.Data.Edm (5.7)
      System.Spatial (5.7)
    Microsoft.Data.Services.Client (5.7) - framework: >= net40
      Microsoft.Data.OData (5.7)
    Newtonsoft.Json (9.0.1) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    System.Spatial (5.7) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    WindowsAzure.Storage (7.1.2)
      Microsoft.Azure.KeyVault.Core (>= 1.0) - framework: >= net40, wpv8.0
      Microsoft.Data.OData (>= 5.6.4) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
      Microsoft.Data.Services.Client (>= 5.6.4) - framework: >= net40
      Newtonsoft.Json (>= 6.0.8) - framework: >= net40, winv4.5, wpv8.0, wpav8.1
    Zlib.Portable (1.11) - framework: portable-net45+sl5+win8, portable-net45+win8, portable-net45+win8+wp8+wpa81```
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 10, 2016

Member

can you please zip that sample project and attach it here? thanks

Member

forki commented Aug 10, 2016

can you please zip that sample project and attach it here? thanks

@pavel-khritonenko

This comment has been minimized.

Show comment
Hide comment

pavel-khritonenko commented Aug 10, 2016

Yes, it is in attachment.

ConsoleApplication1.zip

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 10, 2016

Member

Ok one System.Xml.Linq.dll is coming from FSharp.Data and one from Microsoft.WindowsAzure.Storage

we could somehow store if we already emitted the reference for a given framework and only reference it once. So it would be ommited for Microsoft.WindowsAzure.Storage.

Is it worth it?

Member

forki commented Aug 10, 2016

Ok one System.Xml.Linq.dll is coming from FSharp.Data and one from Microsoft.WindowsAzure.Storage

we could somehow store if we already emitted the reference for a given framework and only reference it once. So it would be ommited for Microsoft.WindowsAzure.Storage.

Is it worth it?

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Aug 10, 2016

Contributor

I believe this goes into the category of "should fix". It's (I believe) mostly harmless but it's something that can confuse users and is not expected.

Contributor

isaacabraham commented Aug 10, 2016

I believe this goes into the category of "should fix". It's (I believe) mostly harmless but it's something that can confuse users and is not expected.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 10, 2016

Member

so what do we do? removing the second reference?

Reordering choose nodes by grouping by target framework (see @mexx 's comment) is not what I want to do before we know how nuget is doing this in vNext.

Member

forki commented Aug 10, 2016

so what do we do? removing the second reference?

Reordering choose nodes by grouping by target framework (see @mexx 's comment) is not what I want to do before we know how nuget is doing this in vNext.

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