-
Notifications
You must be signed in to change notification settings - Fork 339
Remove PackageLicenseFile preventing PackageLicenseExpression from working #4890
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
Conversation
…m working correctly
8c516c4
to
aeee3ca
Compare
@microsoft-github-policy-service agree |
src/Microsoft.TestPlatform.AdapterUtilities/Microsoft.TestPlatform.AdapterUtilities.nuspec
Outdated
Show resolved
Hide resolved
src/package/Microsoft.TestPlatform.TestHost/Microsoft.TestPlatform.TestHost.nuspec
Outdated
Show resolved
Hide resolved
src/package/Microsoft.TestPlatform.Internal.Uwp/Microsoft.TestPlatform.Internal.Uwp.nuspec
Outdated
Show resolved
Hide resolved
src/package/Microsoft.CodeCoverage/Microsoft.CodeCoverage.nuspec
Outdated
Show resolved
Hide resolved
....VsTestConsole.TranslationLayer/Microsoft.TestPlatform.VsTestConsole.TranslationLayer.nuspec
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec
Outdated
Show resolved
Hide resolved
...crosoft.TestPlatform.Extensions.TrxLogger/Microsoft.TestPlatform.Extensions.TrxLogger.nuspec
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.Build.nuspec
Outdated
Show resolved
Hide resolved
Co-authored-by: Amaury Levé <[email protected]>
With the proposed changes the package verification now fails. |
Yes because there are less files included. Let me help you here. |
All MIT packages no longer include MIT license file
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 = @{ |
There was a problem hiding this comment.
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.
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.
Correct! I did it and checked locally that build was good. -- Thanks for the contribution @lahma! |
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! |
Still working out what's the issue. |
Ok so that was just the test infra being super flaky. Thank you for the work here @lahma! |
Description
Removing the
PackageLicenseFile
allowsPackageLicenseExpression
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
After
Related issue
fixes #4816