fix: Add PackageLicenseExpression for /src

[bartelink]

Applies a PackageLicenseExpression to projects under /src tree, in order that this information pass through to the emitted .nupkg files.

The field is used by automated security vulnerability scanning tools to avoid having to parse a potentially ambiguous license file. I believe this to be standard practice in recent years:

• exact same line in FSharp.Control.TaskSeq https://github.com/fsprojects/FSharp.Control.TaskSeq/blob/349124b7e53dd5b37e87921e6bdc5b3e8a0e26e1/src/FSharp.Control.TaskSeq/FSharp.Control.TaskSeq.fsproj#L20
• Example project that uses it: https://www.nuget.org/packages/serilog
• Example package before adding it: https://www.nuget.org/packages/FsCodec/3.0.0-rc.13
• example package after adding it: https://www.nuget.org/packages/FsCodec/3.0.0-rc.14

[bartelink]

Key differences this makes:

1. it prevents triggering Unknown License Identified alerts in commercial vulnerability scanners
2. nuget provides a direct link (i.e. the third link is only available for the FsCodec version with the expression included)

[TheAngryByrd]

Thank you! I didn't realize PackageLicenseFile wasn't enough.

[TheAngryByrd]

Hmm, looks like this doesn't mix well.

Error: /usr/share/dotnet/sdk/7.0.404/Sdks/NuGet.Build.Tasks.Pack/buildCrossTargeting/NuGet.Build.Tasks.Pack.targets(221,5): error NU5033: Invalid metadata. Cannot specify both a PackageLicenseExpression and a PackageLicenseFile.

[bartelink]

Ah, doh! I'll research how to resolve it later...

(I actually made the change locally in a clone but neglected to try to build it, 🤦)

[bartelink]

• It seems the correct resolution is to include the file in the package, but switch to PackageLicenseExpression.
• It also seems adding a Copyright (which will flow through to the nuget) is generally the done thing - but should that specify demystifp or something else (is there a CLA?). FSharp.Control.TaskSeq uses <Copyright>Copyright 2023</Copyright>, but the license file names Abel (and uses 2022)

Will have a deeper read to understand the nuance though. e.g. microsoft/MSBuildSdks#312

I'll work on the assumption you want to preserve the inclusion of the license file in the nupkg (though FSharp.Control.TaskSeq does not do that)

cc @abelbraaksma did you have a particular reasoning for not including the license in the package and/or are you aware of a convention? Or, if you want me to do a PR to align FS.C.TS with this (i.e. include the MIT license file in the nupkg?)

[TheAngryByrd]

I'll work on the assumption you want to preserve the inclusion of the license file in the nupkg

It doesn't matter that much to me but it's more surprising, that since this is a typical scenario of including the file, the scanner you're using can't support it.

[bartelink]

I believe the TL;DR of the issue is that the NuGet PackageLicenseExpression assertion is more complete; it's saying "no need to parse the text of any license - the author of the package has, by specifying it this way, made an assertion that all licensing end to end is in compliance with the definition of that license", thus adding the PLE means that the link from FsCodec's nuget page is to https://licenses.nuget.org/Apache-2.0, not to the license text.

That means that the scanner software and/or people leaning on the conclusions it draws are "not on the hook for" and/or having to having to lean on the heuristics such as those used in the rendering of https://github.com/demystifyfp/FsToolkit.ErrorHandling/blob/master/License which presumably does some form of textual comparison that's tolerant of whitespace diffs and a copyright clause at the top (though IIRC it ignores stuff after too, which would probably give Lawyers kittens).

The PLEs are references to https://spdx.org/licenses/

FWIW the scanner in question is Black Duck, but I have no reason to believe that it's unique in following this approach.

[TheAngryByrd]

I get it's a lot easier to just look for a key in a xml file than doing all the extra work, it's just strange to me that tool lacks some ability for this since there's 45k results in a naive search across github with people using PackageLicenseFile.

https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu5033
Use one or the other. If you are using a well known license that's part of the standard license list, consider using an expression. Otherwise please use a license file.

Yeah just remove the license file from the packing.

[bartelink]

Yeah just remove the license file from the packing.

Thanks, Will do (later)

I get it's a lot easier to just look for a key in a xml file than doing all the extra work, it's just strange to me that tool lacks some ability for this since there's 45k results in a naive search across github with people using PackageLicenseFile.

I suspect the reason for that it is primarily that it's a relatively recent addition to NuGet's tooling.

And I definitely agree that scanners can be assess, and there needs to be reasonable pushback e.g. serilog/serilog-sinks-console#145 (comment)

But it is pretty clear that this is the correct path for packages on mainstream licenses: https://learn.microsoft.com/en-us/dotnet/standard/library-guidance/nuget#important-nuget-package-metadata

[bartelink]

OK, applied https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices
Pretty much every one of these settings/policies are applied consistently on active github.com/jet projects

[bartelink]

https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices suggests comma separation

[bartelink]

These settings can be consolidated via DotNet.ReproducibleBuilds which @baronfel introduced me to, e.g. https://github.com/jet/equinox/blob/master/Directory.Build.props#L19C39-L19C57

But that's a step too far for this PR/batch of changes in this space

[bartelink]

was mangled, see https://nuget.info/packages/FsToolkit.ErrorHandling/4.11.0

[bartelink]

OK, I think I'm happy with this as it is now
Ping me when it releases and I'll do a validation of
a) the "shut up the scanner" objective being achieved
b) everything else looking correct on nuget

Note there's no rush on merging and/or releasing - we'll feed it through as and when it happens

And finally, thanks for the stewardship of the project; as a newcomer to the project in recent times I've found the docs do a good job in general

[TheAngryByrd]

Thank you for fixing up all those other places too!

[bartelink]

Thanks for the release - I can confirm the scan is now clear, and the NuGet listing looks like everything has percolated correctly