Fix mismatch between GetPackOutputItemsTask and PackTask generated filenames

[gekka]

Bug

Fixes: Nuget/Home#14711
Fixes: Nuget/Home#12644

Description

This pull request fixes a mismatch between the file name that GetPackOutputItemsTask expects for a generated .nupkg and the actual file name produced by PackTask when a project uses NuspecFile or NuspecProperties (including dynamically generated versions).

This PR also fixes an issue with the OutputFileNamesWithoutVersion property, ensuring Pack respects that setting when computing and emitting the final .nupkg file name so downstream targets receive the expected version‑less output.

PR Checklist

• Meaningful title, helpful description and a linked NuGet/Home issue
• Added tests
• Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

Note

One of this tests invoke msbuild to compare the file name computed by GetPackOutputItemsTask with the actual file produced by PackTask. As a result, the NuGet.Build.Tasks.Pack.Test project acquires a new dependency.
I’m not certain whether this test implementation and approach align with the repository’s conventions, so feedback or guidance would be appreciated, especially if anything is incorrect.

[zivkan]

@gekka I'm conceptually fine with what this PR claims to fix. But I see the CI failures are related to changes made by this PR, so I'm not going to do a full review until we get a green CI.

No problem adding the project reference to the test project.

[gekka]

This PR encountered a few failing tests. A portion of them were due to my code, but there were also intermittent failures that don’t seem to be related.
Since the final rerun passed without issues, it seems that some earlier failures were caused by test instability.

This flakiness timeout failures seem to be coming from NuGet.Protocol.Tests.PackageUpdateResourceTests class.

There’s an existing issue that appears to mention the same type of flakiness.
It’s not the exact same function, but it’s within the same part of the codebase and might be related.

[Copilot]

Pull request overview

This PR addresses inconsistencies in how pack output filenames are computed vs. produced when packing with a .nuspec and/or NuspecProperties, and ensures OutputFileNamesWithoutVersion is respected consistently so downstream MSBuild targets see the expected outputs.

Changes:

• Update _GetOutputItemsFromPack to pass NuspecFile, NuspecProperties, and OutputFileNamesWithoutVersion into GetPackOutputItemsTask.
• Align PackTaskLogic/GetPackOutputItemsTask handling of NuspecProperties (notably version) and apply excludeVersion when generating filenames.
• Add new tests/fixtures (including an MSBuild invocation test) to validate that computed output item names match the files actually produced.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/PackageNameTests.cs	Adds theory-based coverage comparing GetPackOutputItemsTask output vs. produced pack artifacts.
test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/NuGet.Build.Tasks.Pack.Test.csproj	Adds a project reference needed for the new MSBuild-based test setup.
test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/BuildFixture.cs	Introduces a fixture to locate patched dotnet/MSBuild and built task/targets artifacts for integration-style tests.
src/NuGet.Core/NuGet.Build.Tasks/NuGet.Build.Tasks.Pack.targets	Passes nuspec inputs and versionless-output option into _GetOutputItemsFromPack’s GetPackOutputItemsTask.
src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs	Factors nuspec property parsing into a helper and normalizes/validates version.
src/NuGet.Core/NuGet.Build.Tasks.Pack/GetPackOutputItemsTask.cs	Uses nuspec inputs to compute output names consistently and respects OutputFileNamesWithoutVersion.

[gekka]

@nkolev92
I’ve addressed all the review comments, and it has been idle for about a month.
When you have a moment, I’d appreciate another review. Thanks!

[nkolev92]

Sorry for the delay @gekka.

I'll take another look.

[nkolev92]

Apologies for the delay.

I have some suggestions that improve the readiability of the test cases and simplify the actual bootstrapping logic.

[nkolev92]

This may be a regression.

In particular, I think nuspecProperties are supposed to be used for substitution but not authoritatively override.

[gekka]

PackTask and GetPackOutputItemsTask were producing different outputs, so this change updates GetPackOutputItemsTask to match file names from PackTask .
PackTask is the authoritative behavior and is too large to modify.
There is also an existing test that has meta name for id and version in nuspec file, and to keep that test unchanged while aligning the outputs.

The processing order in this PR is required.

NuGet.Client/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs

Lines 1480 to 1505 in 4b68681

	[PlatformTheory(Platform.Windows)]
	// Command line : /p:NuspecProperties=\"id=MyPackage;version=1.2.3;tags=tag1;description="hello world"\"
	[InlineData("/p:NuspecProperties=\\\"id=MyPackage;version=1.2.3;tags=tag1;description=\"hello world\"\\\"", "MyPackage",
	"1.2.3", "hello world", "tag1")]
	// Command line : /p:NuspecProperties=\"id=MyPackage;version=1.2.3;tasg=tag1,tag2;description=""\"
	[InlineData("/p:NuspecProperties=\\\"id=MyPackage;version=1.2.3;tags=tag1,tag2;description=\"hello world\"\\\"", "MyPackage",
	"1.2.3", "hello world", "tag1,tag2")]
	// Command line : /p:NuspecProperties=\"id=MyPackage;version=1.2.3;tags=;description="hello = world"\"
	[InlineData("/p:NuspecProperties=\\\"id=MyPackage;version=1.2.3;tags=;description=\"hello = world\"\\\"", "MyPackage",
	"1.2.3", "hello = world", "")]
	// Command line : /p:NuspecProperties=\"id=MyPackage;version=1.2.3;tags="";description="hello = world with a %3B"\"
	[InlineData("/p:NuspecProperties=\\\"id=MyPackage;version=1.2.3;tags=\"\";description=\"hello = world with a %3B\"\\\"",
	"MyPackage", "1.2.3", "hello = world with a ;", "")]
	public void PackCommand_PackProject_PacksFromNuspecWithTokenSubstitution(
	string nuspecProperties,
	string expectedId,
	string expectedVersion,
	string expectedDescription,
	string expectedTags
	)
	{
	var nuspecFileContent = @"<?xml version=""1.0""?>
	<package>
	<metadata>
	<id>$id$</id>
	<version>$version$</version>

[nkolev92]

Let's try this:

pr7137-id-divergence-test.patch

There should be a test there more precisely showing the scenario I am referring to.

[nkolev92]

Also, consider rebasing so that you pull in new test cases as well.

[dotnet-policy-service]

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 30 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

[nkolev92]

I'll look again.

[dotnet-policy-service]

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 30 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

[dotnet-policy-service]

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 30 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.