Skip to content

Remove PackageLicenseFile preventing PackageLicenseExpression from working - #4890

Merged
Amaury Levé (Evangelink) merged 3 commits into
microsoft:mainfrom
lahma:license-expression
Feb 16, 2024
Merged

Remove PackageLicenseFile preventing PackageLicenseExpression from working#4890
Amaury Levé (Evangelink) merged 3 commits into
microsoft:mainfrom
lahma:license-expression

Conversation

@lahma

@lahma Marko Lahma (lahma) commented Feb 7, 2024

Copy link
Copy Markdown
Contributor

Description

Removing the PackageLicenseFile allows PackageLicenseExpression to work correctly. Adding explicit packaging instructions to nuspec to keep the MIT license as part of packages.

Checking artifacts from .\build.cmd -pack

Before

image

After

image

Related issue

fixes #4816

  • I have ensured that there is a previously discussed and approved issue.

@lahma

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Comment thread Directory.Build.props Outdated
Comment thread Directory.Build.props Outdated
Comment thread src/package/Microsoft.CodeCoverage/Microsoft.CodeCoverage.nuspec Outdated
Comment thread src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec Outdated
Comment thread src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.Build.nuspec Outdated
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
@lahma

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Contributor Author

With the proposed changes the package verification now fails.

@Evangelink

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Member

Yes because there are less files included. Let me help you here.

@Evangelink

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Member

Example when building locally Microsoft.NET.Test.Sdk

image

All MIT packages no longer include MIT license file
@lahma

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Contributor Author

My assumption was that the license file should part of the package even though license expression would be MIT, this is why added explicit packaging. So should the checks just be changed to have one file less as expected count?

Comment thread eng/verify-nupkgs.ps1
@@ -17,18 +17,18 @@ Add-Type -AssemblyName System.IO.Compression.FileSystem
function Verify-Nuget-Packages {
Write-Host "Starting Verify-Nuget-Packages."
$expectedNumOfFiles = @{

ghost Feb 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all packages using MIT license, reduce count by 1 as license file is no longer present.

@Evangelink

ghost commented Feb 7, 2024

Copy link
Copy Markdown
Member

My assumption was that the license file should part of the package even though license expression would be MIT, this is why added explicit packaging.

File was added explicitly because VSTest was only moved to our common build infra recently and there are still reminicense of the past that we missed.

So should the checks just be changed to have one file less as expected count?

Correct! I did it and checked locally that build was good.

--

Thanks for the contribution Marko Lahma (@lahma)!

@Evangelink

ghost commented Feb 13, 2024

Copy link
Copy Markdown
Member

Hum that's weird. I am pretty busy until Thursday so I will only be able to investigate and ensure this gets merged then.

Sorry for the delay!

@Evangelink

ghost commented Feb 15, 2024

Copy link
Copy Markdown
Member

Still working out what's the issue.

@Evangelink
Amaury Levé (Evangelink) merged commit 5d731c3 into microsoft:main Feb 16, 2024
@lahma
Marko Lahma (lahma) deleted the license-expression branch February 16, 2024 09:31
@Evangelink

ghost commented Feb 16, 2024

Copy link
Copy Markdown
Member

Ok so that was just the test infra being super flaky.

Thank you for the work here Marko Lahma (@lahma)!

This was referenced Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't combine PackageLicenseExpression and PackageLicenseFile (only use PackageLicenseExpression for MIT licensed packages)

2 participants