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

FSharp.Core 5.0.1 should not have FSharp.Core.xml in contentFiles #11143

Closed
johnendev opened this issue Feb 25, 2021 · 12 comments · Fixed by #11160
Closed

FSharp.Core 5.0.1 should not have FSharp.Core.xml in contentFiles #11143

johnendev opened this issue Feb 25, 2021 · 12 comments · Fixed by #11160
Labels
Area-Library Bug Impact-High
Milestone

Comments

@johnendev
Copy link

johnendev commented Feb 25, 2021

The FSharp.Core 5.0.1 package contains the FSharp.Core.xml file under contentFiles. This means Visual Studio shows that file as being part of every FSharp project with that package version installed. This was not an issue in 5.0.0.

Repro steps

  1. Create an F# project
  2. Install FSharp.Core 5.0.1
  3. Open the project in Visual Studio
  4. Notice that FSharp.Core.xml is shown as a file in the project
  5. Downgrade to 5.0.0
  6. Notice that FSharp.Core.xml doesn't show anymore

example.zip

@cartermp cartermp added Area-Library Bug Impact-High labels Feb 25, 2021
@cartermp cartermp added this to the 16.9 milestone Feb 25, 2021
@cartermp
Copy link
Contributor

cartermp commented Feb 25, 2021

Repro looks like this on my machine:

image

@cartermp cartermp modified the milestones: 16.9, 16.10 Feb 25, 2021
@cartermp
Copy link
Contributor

cartermp commented Feb 25, 2021

PR that causes this: #10579

@KevinRansom
Copy link
Member

KevinRansom commented Feb 25, 2021

This behaviour is by design. Developers of tools who want to ship FSharp.Core with it's associated msbuild file have to perform msbuild FU to deploy the FSharp.Core.xml file.

For developers who do not want to deploy the content file with their projects can use this mechanism:

  <ItemGroup>
    <PackageReference Update="FSharp.Core" Version="5.0.1">
      <ExcludeAssets>contentFiles</ExcludeAssets>
    </PackageReference>
  </ItemGroup>

image

@KevinRansom
Copy link
Member

KevinRansom commented Feb 25, 2021

@cartermp , we could modify the automagic reference, to have : <ExcludeAssets>contentFiles</ExcludeAssets>
And have a switch to include it on something like:

<PropertyGroup>
  <OutputType>Exe</OutputType>
  <TargetFramework>netcoreapp3.1</TargetFramework>
  <FSharpCoreIncludeDocFileInOutput>true</FSharpCoreIncludeDocFileInOutput>
</PropertyGroup>

@johnendev
Copy link
Author

johnendev commented Feb 25, 2021

Yes I think having it as opt-in rather than opt-out would be better

@dsyme
Copy link
Contributor

dsyme commented Feb 25, 2021

Yes I'm seeing FSharp.Core.xml actually added to any project that references FSharp.Core 5.0.1. It has to be opt-in, very few people want to distribute this

@cartermp
Copy link
Contributor

cartermp commented Feb 25, 2021

Yeah, the description from the PR is this:

This changes the xmldoc comments file for FSharp.Core into a nuget contentFile. Following this change, build and publish will deploy the FSharp.Core.xml doc comment file to the output directory, without the build author having to do any extra work.

Which seems reasonable. But should not result in XML docs being included in the solution tree at the file level. bin/obj items aren't displayed in the tree. So I think this is the wrong solution used to address the problem.

@cartermp
Copy link
Contributor

cartermp commented Feb 25, 2021

Note I kept this as severy-high because starting with 16.9, my understanding is everyone will see this, including new projects. Is that accurate?

@dsyme
Copy link
Contributor

dsyme commented Feb 25, 2021

Note I kept this as severy-high because starting with 16.9, my understanding is everyone will see this, including new projects. Is that accurate?

Yes I'm seeing it everywhere

@baronfel
Copy link
Member

baronfel commented Jun 30, 2021

EDIT: nevermind this layout seems intended.

This is still present on the released 5.0.2 package:

➜  5.0.2 tree ~/.nuget/packages/fsharp.core/5.0.2/
/Users/chethusk/.nuget/packages/fsharp.core/5.0.2/
├── Icon.png
├── contentFiles
│   └── any
│       └── netstandard2.0
│           └── FSharp.Core.xml
├── fsharp.core.5.0.2.nupkg
├── fsharp.core.5.0.2.nupkg.sha512
├── fsharp.core.nuspec
├── lib
│   └── netstandard2.0
│       ├── FSharp.Core.dll
│       ├── FSharp.Core.xml
│       ├── cs
│       │   └── FSharp.Core.resources.dll
│       ├── de
│       │   └── FSharp.Core.resources.dll
│       ├── es
│       │   └── FSharp.Core.resources.dll
│       ├── fr
│       │   └── FSharp.Core.resources.dll
│       ├── it
│       │   └── FSharp.Core.resources.dll
│       ├── ja
│       │   └── FSharp.Core.resources.dll
│       ├── ko
│       │   └── FSharp.Core.resources.dll
│       ├── pl
│       │   └── FSharp.Core.resources.dll
│       ├── pt-BR
│       │   └── FSharp.Core.resources.dll
│       ├── ru
│       │   └── FSharp.Core.resources.dll
│       ├── tr
│       │   └── FSharp.Core.resources.dll
│       ├── zh-Hans
│       │   └── FSharp.Core.resources.dll
│       └── zh-Hant
│           └── FSharp.Core.resources.dll
└── paket-installmodel.cache

18 directories, 21 files

@baronfel
Copy link
Member

baronfel commented Jun 30, 2021

If people are still seeing this, they may be using Paket, which doesn't honor the defaults set for this implicit package reference.

If you're using Paket, there is no current solution, but after this PR is merged, you can use content: none on the group, package, or individual reference level to hide content files from packages, including FSharp.Core.

@charlesroddie
Copy link
Contributor

charlesroddie commented Mar 3, 2022

I'm finding that this isn't resolved and that, if FSharp.Core is explicitly referenced, it needs

<PackageReference Include="FSharp.Core" Version="6.0.3">
  <ExcludeAssets>contentFiles</ExcludeAssets>
</PackageReference>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Library Bug Impact-High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants