NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

[Bug]: Central Package Management does not work with F# projects #11949

Open sherryyshi opened 2 years ago

sherryyshi commented 2 years ago

NuGet Product Used

dotnet.exe, NuGet.exe

Product Version

MSBuild version 17.3.0-preview-22322-04+e504ba9f4 for .NET

Worked before?

No response

Impact

It's more difficult to complete my work

Repro Steps & Context

Repro:

  1. Create a new F# project from Visual Studio
  2. In the project root folder, create a file named Directory.Packages.props with the following content:
<Project> 
  <PropertyGroup>
    <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
  </PropertyGroup>
</Project>

Get following error: error NU1008: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion item s: FSharp.Core;System.ValueTuple.

Verbose Logs

No response

sherryyshi commented 2 years ago

We have a solution containing a few F# and mainly C# projects. Is there a way to turn off central package management for individual projects? That way we can switch the majority of our solution over.

I tried setting <ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally> in the fsproj files, but that causes it to run into other errors:

    NU1602: rpg does not provide an inclusive lower bound for dependency FsLexYacc. An approximate best match of FsLexYacc 6.0.0 was resolved.
    NU1701: Package 'FsLexYacc 6.0.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8, .NETFramework,Version=v4.8.1' instead of the project target framework '.NETCoreApp,Version=v3.1'. This package may not be fully compatible with your project.
erdembayar commented 2 years ago

@jeffkl I can repro this with VS 17.3.0 Preview 2.0, but I didn't test with latest VS internal due to other problem.

image

jeffkl commented 2 years ago

I tried fixing this a long time ago but the change was rejected: https://github.com/dotnet/fsharp/pull/6730

At this time, F# projects inject packages but don't have metadata that identifies them as implicitly defined. The only workaround is to do this yourself in Directory.Build.targets:

<PackageReference Update="FSharp.Core" IsImplicitilyDefined="true" Condition="'$(MSBuildProjectExtension)' == '.fsproj'" />
<PackageReference Update="System.ValueTuple" IsImplicitilyDefined="true" Condition="'$(MSBuildProjectExtension)' == '.fsproj'" />

Example of workaround:

https://github.com/microsoft/MSBuildSdks/blob/22cee25ad037827b54b75b6ee1c59a5bc75a744f/src/CentralPackageVersions/Sdk/Sdk.targets#L70-L76

zivkan commented 2 years ago

@jeffkl should we document this on NuGet's docs for CPM? https://docs.microsoft.com/en-us/nuget/consume-packages/central-package-management

knocte commented 2 years ago

@jeffkl sorry, how can this be "Not A Bug"?

jeffkl commented 2 years ago

@knocte Central package management detects package references that should not have versions by looking at a well known metadata value for IsImplicitlyDefined. When this attribute is set to true, CPM will allow a version to be defined on that package reference since the package reference probably came from some SDK logic that the user does not have control over. The FSharp folks rejected the idea of adding this metadata to their package references which means that CPM is going to treat these as any other package reference.

I worked around this in Microsoft.Build.CentralPackageVersions by adding this metadata for the FSharp: https://github.com/microsoft/MSBuildSdks/pull/92

However, I don't think the built-in NuGet implementation should contain workarounds or knowledge of any particular package reference. In my opinion there is a bug but it is here: https://github.com/dotnet/fsharp/issues/3678

sherryyshi commented 2 years ago

Tangentially related but after applying the workaround I am getting this message:

error NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: <URL to index.json>, C:\Program Files\dotnet\sdk\6.0.400\FSharp\library-packs

Is there a workaround for this? Thanks!

jeffkl commented 2 years ago

Tangentially related but after applying the workaround I am getting this message:

error NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: <URL to index.json>, C:\Program Files\dotnet\sdk\6.0.400\FSharp\library-packs

Is there a workaround for this? Thanks!

The only workaround at the moment is to suppress the warning with NoWarn. I'm actively working on this and it will be fixed in the next release: https://github.com/NuGet/Home/issues/11951

benjamin-hodgson commented 2 years ago

@jeffkl Thank you! I just wanted to point out a typo in your code snippet above: IsImplicitilyDefined - should be IsImplicitlyDefined, without the extra i. I (and presumably some others) tried to paste it and spent a while wondering why it didn't work 😄

jeffkl commented 2 years ago

@jeffkl Thank you! I just wanted to point out a typo in your code snippet above: IsImplicitilyDefined - should be IsImplicitlyDefined, without the extra i. I (and presumably some others) tried to paste it and spent a while wondering why it didn't work 😄

Thanks, I've updated my comment.

sherryyshi commented 2 years ago

Tangentially related but after applying the workaround I am getting this message: error NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: <URL to index.json>, C:\Program Files\dotnet\sdk\6.0.400\FSharp\library-packs Is there a workaround for this? Thanks!

The only workaround at the moment is to suppress the warning with NoWarn. I'm actively working on this and it will be fixed in the next release: #11951

Hey @jeffkl, do you have an estimate of when this fix will be available in Visual Studio? Thanks

jeffkl commented 2 years ago

Hey @jeffkl, do you have an estimate of when this fix will be available in Visual Studio? Thanks

@sherryyshi this change will be available in the next Visual Studio 17.4 Preview and the final version due out in November.

kyleratti commented 2 years ago

I tried fixing this a long time ago but the change was rejected: dotnet/fsharp#6730

At this time, F# projects inject packages but don't have metadata that identifies them as implicitly defined. The only workaround is to do this yourself in Directory.Build.targets:

<PackageReference Update="FSharp.Core" IsImplicitilyDefined="true" Condition="'$(MSBuildProjectExtension)' == '.fsproj'" />
<PackageReference Update="System.ValueTuple" IsImplicitilyDefined="true" Condition="'$(MSBuildProjectExtension)' == '.fsproj'" />

Example of workaround:

microsoft/MSBuildSdks@22cee25/src/CentralPackageVersions/Sdk/Sdk.targets#L70-L76

@jeffkl Apologies, I am not sure I am getting the intended results from following these instructions. I am working on migrating a .NET Framework solution with 94 projects (a mix of C# and F#) to CPM.

/Directory.Packages.props

<Project>
  <PropertyGroup>
    <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
  </PropertyGroup>
  <ItemGroup>
    <!-- other packages -->
    <!-- we have outstanding blocking issues preventing us from upgrading beyond F# 4 right now -->
    <PackageReference Update="FSharp.Core" Version="4.7.0" IsImplicitilyDefined="true" Condition="'$(MSBuildProjectExtension)' == '.fsproj'" />
  </ItemGroup>
</Project>

/MyProject/MyProject.fsproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net48</TargetFramework>
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
    <WarnOn>3390;$(WarnOn)</WarnOn>
    <OutputType>Library</OutputType>
    <LangVersion>4.7</LangVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="FSharp.Core" />
  </ItemGroup>
</Project>

However, msbuild.exe Project.sln /target:Restore outputs:

MyProject.fsproj : error NU1504: Duplicate 'PackageReference' items found. Remove the duplicate items or use the Update functionality to ensure a consistent restore behavior. The duplicate 'Package
Reference' items are: FSharp.Core 6.0.4, FSharp.Core .

My environment info:

$ msbuild.exe --version
MSBuild version 17.3.1+2badb37d1 for .NET Framework
17.3.1.41501

$ nuget.exe help
NuGet Version: 6.3.1.1
usage: NuGet <command> [args] [options]
stefan-schweiger commented 1 year ago

@kyleratti this worked for me:

Directory.Build.targets

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <ItemGroup Condition=" '$(EnableCentralPackageVersions)' != 'false' ">
        <!--
        Workaround the issue where FSharp SDK adds implicit PackageReference items but doesn't mark them as such
        https://github.com/NuGet/Home/issues/11949
        -->
        <PackageReference Update="FSharp.Core"
            Condition="'$(MSBuildProjectExtension)' == '.fsproj' And '$(DisableImplicitFSharpCoreReference)' != 'true' And '$(UpdateImplicitFSharpCoreReference)' != 'false'"
            IsImplicitlyDefined="true" />
    </ItemGroup>
</Project>

Directory.Packages.props

<Project>
    <PropertyGroup>
        <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
        <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
    </PropertyGroup>
    <ItemGroup>
        <PackageVersion ... />
   </itemGroup>
</Project>
stefan-schweiger commented 1 year ago

without a custom Directory.Build.targets file as described in my comment above dotnet restore still does not work without error with nuget 6.4.0.117 (dotnet nuget --version)

stefan-schweiger commented 1 year ago

@jeffkl can you confirm that this should have been resolved in nuget 6.4 and this version is also shipped with the latest .NET SDK? because as mentioned it still gives me the error

jeffkl commented 1 year ago

@stefan-schweiger the only fix available from this issue is the one about the package source mapping warning reporting local feeds. That new logic has shipped in Visual Studio 2022 17.4 and .NET SDK 7.0.100.

error NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: , C:\Program Files\dotnet\sdk\6.0.400\FSharp\library-packs

The original error is about PackageReference items having versions when they shouldn't:

error NU1008: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion item s: FSharp.Core;System.ValueTuple.

This error is because the FSharp SDK adds packages but does not indicate that they are implicitly defined. I think a recent change might help: https://github.com/dotnet/fsharp/pull/13920 But I'm not sure, that seems to disable the implicit package references so you'll have to add them yourself but at least NuGet's central package management won't give you errors. I am unsure what product version that released in. I would ask on that pull request for the most accurate information.

aaronpowell commented 10 months ago

It does not appear that this workaround is working in .NET 8

knocte commented 10 months ago

Maybe the workaround is not needed anymore? I say this because I know an F# project that is using net8 already and is using Central Package Management in master branch already: https://github.com/fsprojects/fantomas

aaronpowell commented 10 months ago

Just had a look at how fantomas does it and they've added FSharp.Core explicitly, and testing with .NET 8 that works, there's no need for the Directory.Build.targets

Lanayx commented 10 months ago

We are actively using CPM with explicit FSharp.Core nuget reference and getting

warning NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: https://api.nuget.org/v3/index.json

with .NET 6

Other than that warning everything works fine

knocte commented 9 months ago

with .NET 6

What version of .NET6? Just asking because I know CPM is only supported in .300 or newer.

Lanayx commented 9 months ago

@knocte we are just using mcr.microsoft.com/dotnet/sdk:6.0 docker image

aggieben commented 3 weeks ago

I've been experiencing this problem too, with strange behavior. If I have a project with no dependencies other than the implicit-but-not-implicit FSharp.Core, and I don't have ManagePackageVersionsCentrally set to true but have a Directory.Packages.props, it's fine for all versions of .NET 8 (.110, .306, .403). If I set ManagePackageVersionsCentrally to true, then I get the NU1008 error (for all versions): image

If I try adding an explicit reference:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally/>
    <!-- blah blah blah -->
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="Library.fs" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="FSharp.Core" />
  </ItemGroup>
</Project>

Then I get the NU1504 warning alongside the NU1008 error: image

If I try this in a project that does have dependencies, it fails to resolve those. Example, in a project that references Dapper, without ManagePackageVersionsCentrally: image

But if I set the property, then it bombs even worse:

image

What's very strange to me is that until I updated the .NET SDK to versions after 8.0.304, this was all working just fine.

Playing around with this, it seems to be mostly due to my trying to reference FSharp.Core directly, which should work 🤔