Skip to content
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.

Sign up for GitHub

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

Add an implicit reference to the Generate SBOM task #43151

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+12 −0
Open

Add an implicit reference to the Generate SBOM task #43151

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0d60446
Add an implicit reference to the SBOM Generation Task
gustavoaca1997 Aug 29, 2024
bec035e
fix Target syntax and introduce version for package that dependabot c…
baronfel Aug 30, 2024
9a4177c
Merge pull request #2 from baronfel/user/gustavoca/sbom-generation-poc
gustavoaca1997 Aug 30, 2024
b1bb846
Stamp the version of the SBOM tools package at SDK construction time.
baronfel Aug 30, 2024
92344ec
Merge pull request #3 from baronfel/stamp-sbom-version-into-built-sdk
gustavoaca1997 Aug 30, 2024
5559b7a
Update Generate SBOM branch with main
gustavoaca1997 Aug 31, 2024
4508093
Udpate baseline version
baronfel Sep 10, 2024
d794c78
Update Versions.props for SBOM targets
baronfel Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<MicrosoftWindowsCsWin32PackageVersion>0.3.49-beta</MicrosoftWindowsCsWin32PackageVersion>
<!-- This is the version of the zip archive for the WiX toolset and is different from the NuGet package version format. -->
<WixVersion>3.14.1.8722</WixVersion>
<MicrosoftSbomTargetsPackageVersion>0.0.0-preview.0</MicrosoftSbomTargetsPackageVersion>
Copy link
Author

@gustavoaca1997 gustavoaca1997 Aug 31, 2024 •
edited by baronfel
Loading

Choose a reason for hiding this comment

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

This will be updated once we publish an official Nuget package.

  • Update this with the official version.

</PropertyGroup>
<PropertyGroup Label="NUnit3.DotNetNew.Template version">
<!-- NUnit3.DotNetNew.Template versions do not 'flow in' -->
Expand Down
5 changes: 5 additions & 0 deletions src/Installer/redist-installer/targets/GenerateBundledVersions.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@

<_NETCoreSdkBeingBuiltIsPreview Condition=" '$(DotNetFinalVersionKind)' != 'release' ">true</_NETCoreSdkBeingBuiltIsPreview>
<_NETCoreSdkBeingBuiltIsPreview Condition=" '$(DotNetFinalVersionKind)' == 'release' ">false</_NETCoreSdkBeingBuiltIsPreview>

<_MicrosoftSbomTargetsPackageVersion>$(MicrosoftSbomTargetsPackageVersion)</_MicrosoftSbomTargetsPackageVersion>
</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -535,7 +537,10 @@ Copyright (c) .NET Foundation. All rights reserved.
<NETCoreSdkRuntimeIdentifier>$(ProductMonikerRid)</NETCoreSdkRuntimeIdentifier>
<NETCoreSdkPortableRuntimeIdentifier>$(PortableProductMonikerRid)</NETCoreSdkPortableRuntimeIdentifier>
<_NETCoreSdkIsPreview>$(_NETCoreSdkBeingBuiltIsPreview)</_NETCoreSdkIsPreview>
<!-- SBOM Tools Versions -->
<MicrosoftSbomTargetsPackageVersion Condition="'%24(MicrosoftSbomTargetsPackageVersion)' == ''">$(_MicrosoftSbomTargetsPackageVersion)</MicrosoftSbomTargetsPackageVersion>
joeloff marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<ItemGroup>
@(ImplicitPackageVariable->'<ImplicitPackageReferenceVersion Include="%(Identity)" TargetFrameworkVersion="%(TargetFrameworkVersion)" DefaultVersion="%(DefaultVersion)" LatestVersion="%(LatestVersion)" />', '
')
Expand Down
6 changes: 6 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ Copyright (c) .NET Foundation. All rights reserved.
Condition="'@(PackageReference->AnyHaveMetadataValue('Identity', 'Microsoft.Net.Compilers.Toolset.Framework'))' == 'true'" />
</Target>

<Target Name="_AddGenerateSbomTask" BeforeTargets="CollectPackageReferences">
<ItemGroup Condition=" '$(GenerateSbom)' == 'true' ">
<PackageReference Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" IsImplicitlyDefined="true" />
Copy link
Member

Choose a reason for hiding this comment

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

Ok, so this will be a problem when users have opted into NuGet Central Package Version Management, because CPM requires that users separate the declaration of PackageReferences and their versions.

This is a good opportunity to develop a pattern. Perhaps something like the following:

<ItemGroup Condition=" '$(GenerateSbom)' == 'true' ">
  <PackageReference Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" IsImplicitlyDefined="true" Condition="'$(ManagePackageVersionsCentrally)' != 'true' " />
  <!-- If using CPM create the PackageReference and PackageVersion -->
  <PackageReference Include="Microsoft.Sbom.Targets"  IsImplicitlyDefined="true" Condition="'$(ManagePackageVersionsCentrally)' == 'true' " />
  <PackageVersion Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" Condition="'$(ManagePackageVersionsCentrally)' == 'true' "/>
</ItemGroup>

cc @JonDouglas / @nkolev92 for feedback on how we should manage SDK-delivered CPM-able dependencies. This will be a Big Deal when we turn on SBOM generation by default for 10.

Copy link

Choose a reason for hiding this comment

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

gonna defer to @jeffkl on this one. @zivkan may have ideas too. @nkolev92 may when he returns.

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to handle CPM. IsImplicitlyDefined="true" will tell NuGet that this package is not managed by the customer, and therefore is expected to explicitly have the version set on the PackageReference.

It also means that in VS we won't show it available for updates, and I think that maybe we disable the install/upgrade button even if you search for the package in the browse tab. I'm not yet aware of partners engaging with us about how to allow customers to upgrade implicitly defined packages, hence there's no feature for that at this time.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. What about users using lock files? We currently have a really bad story there with implicit packages and the SDK, and this is one more instance of that pain.

Copy link
Member

Choose a reason for hiding this comment

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

🤷

lock files basically have two purposes:

  • to ensure that the same package version is restored
  • the package contents (hash) are the same.

If different versions of the SDK pull in different versions of the package, yes, it'll be a bad time, but it's working precisely as intended. One way customers can mitigate this is by using a global.json to pin to a specific .NET SDK, and that generally necessitates using dotnet-install.[ps1|sh] to install the pinned SDK, rather than using the CI agent installed.

Perhaps our teams need to get together and talk more about lock file experiences and how we can make implicit packages work better. But at the same time lock files are used by customers who want deterministic builds, and different versions of the SDK brining in different package versions conflicts with that goal.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, let's set something up soon to talk about it - maybe a Monday partner sync Monday after next or something? cc @dsplaisted

</ItemGroup>
</Target>

<Target Name="_CheckMicrosoftNetSdkCompilersToolsetPackageExists"
Condition="'$(_NeedToDownloadMicrosoftNetSdkCompilersToolsetPackage)' == 'true' and '$(DesignTimeBuild)' != 'true'"
BeforeTargets="CoreCompile">
Expand Down
Loading