[Microsoft.Sbom.Targets] Generates the wrong .nupkg file name and cannot find it.

[philipp-naused]

The GenerateSbomTarget target generates the wrong path to the .nupkg file and fails. Example:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <Version>1.2.3.0</Version>
    <GenerateSBOM>true</GenerateSBOM>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Sbom.Targets" Version="3.0.1">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>
</Project>

1. delete the /bin directory
2. dotnet pack

Result: error MSB3932: Failed to unzip file "X:\ws\Test\bin\Release\Test.1.2.3.0.nupkg" because the file does not exist or is inaccessible.

The correct path would have been X:\ws\Test\bin\Release\Test.1.2.3.nupkg since the trailing 0 is trimmed.

See: https://github.com/microsoft/sbom-tool/blob/20f43605fa20db121f86de73366284970c22f0a9/src/Microsoft.Sbom.Targets/Microsoft.Sbom.Targets.targets#L43-L45

Since the unzip task is set to ErrorAndContinue, the target will then generate a new zip file called Test.1.2.3.0.nupkg that only contains the SBOM. If you run dotnet pack again without deleting the bin directory, you get a different warning:

##[warning]Error parsing NuGet component from "X:\\ws\\Test\\bin\\Release\\Test.1.2.3.0.nupkg"
System.IO.FileNotFoundException: No nuspec file was found
   at Microsoft.ComponentDetection.Detectors.NuGet.NuGetNuspecUtilities.GetNuspecBytesAsync(Stream nupkgStream)
   at Microsoft.ComponentDetection.Detectors.NuGet.NuGetComponentDetector.ProcessFileAsync(ProcessRequest processRequest)

and

##[warning]Some components or files were not detected due to parsing failures or connectivity issues.
##[warning]Please review the logs above for more detailed information.
##[warning]Components skipped for "NuGet" detector:
##[warning]- "X:\\ws\\Test\\bin\\Release\\Test.1.2.3.0.nupkg"

[KalleOlaviNiemitalo]

In NuGet.Build.Tasks.Pack.targets, the _GetOutputItemsFromPack target uses the GetPackOutputItemsTask task to get the path of the nupkg file. Because the _GetOutputItemsFromPack name starts with an underscore, the target does not seem intended to be called from elsewhere. Could Microsoft.Sbom.Targets use the GetPackOutputItemsTask task; is that considered a public stable API?

https://github.com/NuGet/NuGet.Client/blob/4ccad7689eaf96f30d41709369bebc888e4a5db3/src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets#L98-L123

https://github.com/NuGet/NuGet.Client/blob/4ccad7689eaf96f30d41709369bebc888e4a5db3/src/NuGet.Core/NuGet.Build.Tasks.Pack/GetPackOutputItemsTask.cs

[philipp-naused]

I think you can use the NuGetPackOutput item. https://github.com/NuGet/NuGet.Client/blob/4ccad7689eaf96f30d41709369bebc888e4a5db3/src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets#L192

Example:

<Target Name="FindNupgkFile" AfterTargets="Pack">
  <ItemGroup>
    <_NupgkFiles Include="@(NuGetPackOutput)" Condition="'%(Extension)' == '.nupkg'" />
  </ItemGroup>
  <Message Text="NupgkFile: @(_NupgkFiles)" Importance="high" />
</Target>

See: Provide MSBuild properties or items for resolved pack outputs or Make MSBuild Pack target return the nupkg file as output

[KalleOlaviNiemitalo]

Somewhat related https://github.com/dotnet/msbuild/issues/9881 -- I think a MSBuild v17.12.18 with BuildCheck enabled will warn that both the PackTask task (in the "GenerateNuspec" target) and the Microsoft.Build.Tasks.ZipDirectory task (in the "GenerateSbomTarget" target) output the same nupkg file. Fixing that might require telling NuGet.Build.Tasks.Pack.targets to write the nupkg file to some other directory initially and then writing only the SBOM-enriched nupkg to the final output directory. But if PackTask writes a snupkg file too, then that complicates the implementation.

[philipp-naused]

This analyzer currently only checks a few specific tasks: Csc, Vbc, Fsc, and Copy https://github.com/dotnet/msbuild/blob/90c5d64f0280e31077a0f395bd328d4a06fb1f36/src/Build/BuildCheck/Checks/DoubleWritesCheck.cs#L51

So, this shouldn't trigger the "Double Writes" warning (At least for now)

[sfoslund]

@baronfel do you have any thoughts on this issue? Is this something that needs to be addressed in this repo or one of the .NET repos?

[philipp-naused]

I think the fix needs to be made here: https://github.com/microsoft/sbom-tool/blob/20f43605fa20db121f86de73366284970c22f0a9/src/Microsoft.Sbom.Targets/Microsoft.Sbom.Targets.targets#L43-L45

[sfoslund]

Hi @philipp-naused feel free to make a PR if you would like us to consider this change, I would also love @baronfel to chime in on this before we action on it.

[baronfel]

@KalleOlaviNiemitalo is definitely on the right path here - we must be data-driven and retrieve it from the Target Outputs being discussed here.

[philipp-naused]

@baronfel, I'm not clear about which "Target Outputs" you are referring to. Do you mean the SBOM target should call the GetPackOutputItemsTask task or use the NuGetPackOutput item?

[philipp-naused]

@sfoslund Unfortunately, I cannot contribute to a public repo on this account. I might do that on a separate account later.