Skip to content
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

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

Open
sherryyshi opened this issue Jul 7, 2022 · 24 comments
Open

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

sherryyshi opened this issue Jul 7, 2022 · 24 comments
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Product:VS.Client Resolution:NotABug This issue appears to not be a bug Type:Bug

Comments

@sherryyshi
Copy link

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
Copy link
Author

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 erdembayar added Area:RestoreCPM Central package management Product:VS.Client and removed Triage:Untriaged labels Jul 7, 2022
@erdembayar
Copy link
Contributor

erdembayar commented Jul 7, 2022

@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

@zivkan zivkan added the Priority:2 Issues for the current backlog. label Jul 14, 2022
@jeffkl
Copy link
Contributor

jeffkl commented Jul 14, 2022

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:

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

@jeffkl jeffkl added the Resolution:NotABug This issue appears to not be a bug label Jul 14, 2022
@zivkan
Copy link
Member

zivkan commented Jul 14, 2022

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

@sherryyshi sherryyshi changed the title [Bug]: Central Package Management does not with F# projects [Bug]: Central Package Management does not work with F# projects Jul 15, 2022
@knocte
Copy link

knocte commented Jul 26, 2022

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

@jeffkl
Copy link
Contributor

jeffkl commented Jul 29, 2022

@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: microsoft/MSBuildSdks#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: dotnet/fsharp#3678

@sherryyshi
Copy link
Author

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
Copy link
Contributor

jeffkl commented Aug 11, 2022

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

@benjamin-hodgson
Copy link

@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
Copy link
Contributor

jeffkl commented Sep 6, 2022

@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
Copy link
Author

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
Copy link
Contributor

jeffkl commented Sep 19, 2022

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
Copy link

kyleratti commented Oct 19, 2022

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
Copy link

@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
Copy link

stefan-schweiger commented Nov 16, 2022

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
Copy link

@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
Copy link
Contributor

jeffkl commented Nov 17, 2022

@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: dotnet/fsharp#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
Copy link

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

@knocte
Copy link

knocte commented Dec 18, 2023

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
Copy link

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
Copy link

Lanayx commented Dec 19, 2023

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
Copy link

knocte commented Feb 2, 2024

with .NET 6

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

@Lanayx
Copy link

Lanayx commented Feb 2, 2024

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

@jeffkl jeffkl added Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. and removed Priority:2 Issues for the current backlog. labels May 8, 2024
@jeffkl jeffkl removed their assignment May 8, 2024
@aggieben
Copy link

aggieben commented Oct 10, 2024

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 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Product:VS.Client Resolution:NotABug This issue appears to not be a bug Type:Bug
Projects
None yet
Development

No branches or pull requests