Fix framework pack NuGet package licenseUrl

[dagood]

For example these packages link to the license via a "floating" Core-Setup GitHub repo URL:
Microsoft.NETCore.App 2.1.13
Microsoft.NETCore.App.Ref 3.0.0
Microsoft.NETCore.App.Ref 3.1.0-preview1.19506.1
They link to https://github.com/dotnet/core-setup/blob/master/LICENSE.TXT based on licenseUrl in the nuspec.

This is awkward for repo consolidation (we have to create a probably confusing master tag to avoid breaking the link, rather than only having the master-archive tag). It's also very bad practice. NuGet packages should contain their license.

Mitigation: the NuGet packages do contain LICENSE.TXT, but the NuGet Gallery (and probably Visual Studio?) show licenseUrl, so the user would have to dig in their NuGet cache or extract the nupkg to find LICENSE.TXT.

Note that the managed library packages like Microsoft.DotNet.PlatformAbstractions 3.0.0 aren't affected because they are packed using different packaging tooling.

The pkgproj packaging tooling should support what we need to do here, but something in Core-Setup is not right or interferes. (dotnet/arcade#2313)

I believe we should definitely fix this for 3.1, and consider porting to 3.0.

/cc @MichaelSimons @dleeapho @leecow @ericstj

[dleeapho]

We should fix for 5.0 and make 3.1 backport decision then.,

[dagood]

@ericstj, it looks like CoreFX packages still have this issue, e.g. https://dotnetfeed.blob.core.windows.net/dotnet-core/flatcontainer/system.io.ports/5.0.0-alpha.1.19528.6/system.io.ports.5.0.0-alpha.1.19528.6.nupkg from https://dev.azure.com/dnceng/internal/_build/results?buildId=404850:

    <licenseUrl>https://github.com/dotnet/corefx/blob/master/LICENSE.TXT</licenseUrl>

The released Microsoft.CSharp/4.7.0-preview1.19504.10 does this too.

I got the impression from the arcade PRs that this was fixed, am I looking in the wrong place? I'd like to know what the expectation is with the CoreFX packaging tools before diving deeper myself.

---

For what it's worth, I don't know any milestone-linked driver behind this work, any info there would be useful. I do know that there's some (very understandable!) level of desire to have our packages more self-contained for the license and package icon file.

/cc @tmat

[ericstj]

dotnet/corefx@1ff8e57 removed the FWLink :(

I thought we were using the License type, at least at some point we were. I can't find any packages with it. Let's get that fixed in 3.1 if we can.

[ericstj]

https://github.com/dotnet/corefx/issues/42264

[ericstj]

Looks like the requirement is that the LicenseUrl property is not defined.

[dagood]

I tried removing the LicenseUrl override in Core-Setup and got packages with no LicenseUrl and no other license thing in the nuspec. They did contain LICENSE.TXT (as always) but my understanding is there should also be some <license type=" in the nuspec linking it up or referencing a license by name.

[ericstj]

Is $(PackageLicenseExpression) set?

[dagood]

Oh--no, I expected an Arcade default. 😄 Set to MIT, and it works:

    <license type="expression">MIT</license>
    <licenseUrl>https://licenses.nuget.org/MIT</licenseUrl>

[tmat]

License URL intentionally does not have a default - each repo must make a decision what license to use. See https://github.com/dotnet/arcade/blob/master/Documentation/ArcadeSdk.md#licensetxt

[dagood]

Yeah, I think I had wires crossed from when I assumed CoreFX was working properly and didn't see it defining one. Might be nice for Arcade to produce an error when there's no license, but that might either be up to the CoreFX tooling or interfere with it, so no big deal IMO.

[tmat]

We do validate licenses, but it turns out only if a license expression is specified:
https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/RepositoryValidation.proj#L14

That might have been temporary while not all repos had it. Feel free to follow up and make it require the expression to be specified.

[tmat]

We might have also relied on NuGet pack validating that packages have licenses. Not sure if it actually does.

[ericstj]

CoreFX was working properly and didn't see it defining one.

We had to turn it off because of a NuGet bug on pack, but never turned it back on after working around that bug.

[KalleOlaviNiemitalo]

At System.Memory 4.5.4 (the latest version) in NuGet Gallery, the License Info link is dead because of repository consolidation. Is that covered by this issue or should I file a separate one? I'm not sure because the link points to the corefx repository rather than core-setup.

[dagood]

@KalleOlaviNiemitalo that's a separate issue, it looks like corefx has a master branch and master tag, but it should only have a master tag. The tag points to the right commit that still has LICENSE.TXT: https://github.com/dotnet/corefx/tree/e99ec129cfd594d53f4390bf97d1d736cff6f860. I've reached out internally to try to figure out why this happened and if we can simply delete the branch to fix it. Thanks for pointing this out.

[dagood]

We fixed the old license link just now. This works as of writing: https://github.com/dotnet/corefx/blob/master/LICENSE.TXT. Thanks again for pointing it out!

(Note for future viewers: the main core-setup issue tracked by this thread is still open--we should switch away from the old URLs where we can.)

[NikolaMilosavljevic]

[Triage] @NikolaMilosavljevic can you check if this still repros?

[NikolaMilosavljevic]

Checked the latest Microsoft.NETCore.App.Ref package. License info is correct:

<licenseUrl>https://licenses.nuget.org/MIT</licenseUrl>

Closing.