Skip to content

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Feb 7, 2024

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

lahma commented Feb 7, 2024

@microsoft-github-policy-service agree

@lahma
Copy link
Contributor Author

lahma commented Feb 7, 2024

With the proposed changes the package verification now fails.

@Evangelink
Copy link
Member

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

@Evangelink
Copy link
Member

Example when building locally Microsoft.NET.Test.Sdk

image

All MIT packages no longer include MIT license file
@lahma
Copy link
Contributor Author

lahma commented Feb 7, 2024

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?


function Verify-Nuget-Packages {
Write-Host "Starting Verify-Nuget-Packages."
$expectedNumOfFiles = @{
Copy link
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
Copy link
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 @lahma!

@Evangelink Evangelink enabled auto-merge (squash) February 7, 2024 14:25
@Evangelink
Copy link
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
Copy link
Member

Still working out what's the issue.

@Evangelink Evangelink merged commit 5d731c3 into microsoft:main Feb 16, 2024
@lahma lahma deleted the license-expression branch February 16, 2024 09:31
@Evangelink
Copy link
Member

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

Thank you for the work here @lahma!

This was referenced Oct 20, 2025
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