Skip to content

Conversation

@jtschuster
Copy link
Member

@jtschuster jtschuster commented Feb 22, 2024

Adds a task to validate that the final sdk archive has the same files as the daily installer builds. It will download the latest daily build from the same aka.ms links found in the installer table, so it's not a perfect version match, but it should be accurate enough. It also will only report any differences as messages, it won't fail the build.

Part of the fix for dotnet/source-build#4119

@jtschuster jtschuster requested a review from a team as a code owner February 22, 2024 16:31
@jtschuster jtschuster requested review from ViktorHofer and removed request for a team February 22, 2024 16:33
jtschuster and others added 3 commits February 22, 2024 14:44
…eBuild.Tasks.SdkArchiveDiff/Microsoft.DotNet.SourceBuild.Tasks.SdkArchiveDiff.csproj

Co-authored-by: Viktor Hofer <[email protected]>
…eBuild.Tasks.SdkArchiveDiff/Microsoft.DotNet.SourceBuild.Tasks.SdkArchiveDiff.csproj

Co-authored-by: Viktor Hofer <[email protected]>
@jtschuster
Copy link
Member Author

This isn't currently fully running in any CI pipelines since the sdk repo is still excluded in non-sourcebuild VMR builds and sourcebuild only runs on the centos.8-x64 rid. Is there a pipeline that runs the VMR in source-build mode with a portable RID?

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 26, 2024

This isn't currently fully running in any CI pipelines since the sdk repo is still excluded in non-sourcebuild VMR builds and sourcebuild only runs on the centos.8-x64 rid. Is there a pipeline that runs the VMR in source-build mode with a portable RID?

No, source-build is non-portable by definition. The sdk repository is already enabled in the unified-build. Do you mean the installer repo? That should soon be running as part of unified-build: #18632

@ViktorHofer
Copy link
Member

Installer now builds live in the unified-build pipeline.

@jtschuster
Copy link
Member Author

Looks like it found a few differences in the ubuntu release, adding a .exe extension to one, and missing a windows(?) dll.

I'll fix the / at the end of the file path.

  Difference in sdk archive: Removed: sdk/{VERSION}/Containers/containerize/containerize/
  Difference in sdk archive: Added: sdk/{VERSION}/Containers/containerize/containerize.exe/
  Difference in sdk archive: Removed: sdk/{VERSION}/DotnetTools/dotnet-watch/{VERSION}/tools/net9.0/any/runtimes/win/lib/net9.0/System.Diagnostics.EventLog.Messages.dll/

@mmitche
Copy link
Member

mmitche commented Feb 29, 2024

Interesting! /cc @ViktorHofer

<Error Text="Multiple valid dotnet-sdk archives found."
Condition="'@(_BuiltSdkArchivePath->Count())' != '1'" />

<GetClosestOfficialSdk BuiltArchivePath="@(_BuiltSdkArchivePath)"
Copy link
Member

Choose a reason for hiding this comment

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

I would be surprised if this fails to find a matching SDK...does this happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you referring to the Condition="'@(_BuiltSdkArchivePath)' != ''" or something else? I think we error in the tasks before we'd hit the conditions for GetClosestOfficialSdk and FindArchiveDiffs, so I'll remove the conditions.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha, yeah that should never happen, we'll error out in the task first. I removed those conditions.

@jtschuster jtschuster requested a review from a team as a code owner March 6, 2024 19:41
</_sdkFilesDiff>
<_sdkFilesDiff Include="@(_ContentDifferences)" Condition="'%(_contentDifferences.Kind)' == 'Unchanged'" >
<DiffIndicator> </DiffIndicator>
</_sdkFilesDiff>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure having the Unchanged files is really valuable in the diff, it just makes the file larger than necessary and ideally if there's no diff the file would be empty/non-existing.

</_sdkFilesDiff>
<_sdkFilesDiff Include="@(_ContentDifferences)" Condition="'%(_contentDifferences.Kind)' == 'Removed'" >
<DiffIndicator>-</DiffIndicator>
</_sdkFilesDiff>
Copy link
Member

Choose a reason for hiding this comment

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

Checking the Ubuntu run I see:

+ templates/{VERSION}/microsoft.dotnet.common.itemtemplates.{VERSION}.nupkg
+ sdk/{VERSION}/Containers/containerize/containerize.exe
- templates/{VERSION}/microsoft.dotnet.common.itemtemplates.{VERSION}.nupkg
- sdk/{VERSION}/Containers/containerize/containerize
- sdk/{VERSION}/DotnetTools/dotnet-watch/{VERSION}/tools/net9.0/any/runtimes/win/lib/net9.0/System.Diagnostics.EventLog.Messages.dll

The "templates" one doesn't make sense to me since it seems to be the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that one is weird. I wonder if it's related to having two Versions in the path? I'll look into it.

</ItemGroup>

<PropertyGroup>
<SdkArchiveDiffsReport>$(ArtifactsLogDir)SdkArchiveContent.diff</SdkArchiveDiffsReport>
Copy link
Member

Choose a reason for hiding this comment

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

instead of .diff what if we generate xunit/mstest xml reports instead? those could be picked up by the AzDO test tab so we don't have to build custom UI.

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.

4 participants