Create and publish VerticalAssetManifest #19062

dkurepa · 2024-03-15T10:10:30Z

dotnet/source-build#4198
Adds a task that creates a VerticalAssetManifest.

Needs to be merged after #18917

...ks.PublishVerticalManifest/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest.csproj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

eng/pipelines/templates/jobs/vmr-build.yml

src/SourceBuild/content/build.proj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

…erticalManifest

src/SourceBuild/content/build.proj

...ks.PublishVerticalManifest/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest.csproj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

…ackages when in source build

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

mmitche · 2024-03-18T21:59:09Z

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

+                .Select(attribute => attribute.ToString())
+                .ToHashSet();
+
+            if (assetManifestXmls.Skip(1).Any(assetManifestXml => 


src/SourceBuild/content/build.proj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

…tribute count

mmitche · 2024-03-21T21:59:51Z

src/SourceBuild/content/repo-projects/Directory.Build.props

    -->
    <EnvironmentVariables Include="OfficialBuildId=$(OfficialBuildId)" />
+    <!-- The BUILD_BUILDNUMBER environmental variable is set by the AzDo agent. We're overwriting it below, so we need to save it in a different env variable -->
+    <EnvironmentVariables Include="VmrBuildNumber=$(BUILD_BUILDNUMBER)"/>


I think you actually want this in the root build.proj and not the repo-project build proj. since the VerticalManifest publish is not specific to a repo project. That probably means you don't have to set an env var, just use BUILD_BUILDNUMBER

Btw, I think it should be based off OfficialBuildId. That should be set at some level to BUILD_BUILDNUMBER.

I defined the OfficialBuildId property in the base Directory.Build.props. My msbuild knowledge might be wrong here, but I think it will have the value we want it to when we're calling the PublishVerticalManifest task, since the values set in repo-projects will be reset

Ahh, so this is for future use?

No we need it for the PublishVerticalManifest task, to set the correct BuildId. But it looks like it's not working as I thought it should. The OfficialBuildId is not getting set to anything after we're done with repo-projs. Think we'll have to save it in a different property before we start building

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

...ks.PublishVerticalManifest/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest.csproj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

src/SourceBuild/content/build.proj

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs

src/SourceBuild/content/build.proj

ViktorHofer · 2024-03-26T14:33:18Z

...ools/tasks/Microsoft.DotNet.SourceBuild.Tasks.MergeAssetManifests/PublishVerticalManifest.cs

 using System.Xml.Linq;

-namespace Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest
+namespace Microsoft.DotNet.SourceBuild.Tasks


This isn't a source-build task. Consider renaming this to something like Microsoft.DotNet.UnifiedBuild.Tasks.

I see that this would force you to rename everything, so I'm ok with keeping this name for now.

...ools/tasks/Microsoft.DotNet.SourceBuild.Tasks.MergeAssetManifests/PublishVerticalManifest.cs

…eBuild.Tasks.MergeAssetManifests/PublishVerticalManifest.cs

ViktorHofer · 2024-03-26T15:39:25Z

src/SourceBuild/content/build.proj

+  <Target Name="MergeAssetManifests" AfterTargets="Build">
+    <PropertyGroup>      
+      <MergedAssetManifestOutputPath>$(ArtifactsDir)VerticalManifest.xml</MergedAssetManifestOutputPath>
+      <VmrBuildNumber>$(BUILD_BUILDNUMBER)</VmrBuildNumber>


Move this property after L7 in this file and see if that changes anything.

should work there, before we go into repo-projects

Maybe a repo changes this env var?!? Super weird.

Yeah, no idea why it's not available now. Looks like we stopped publishing binlogs..

I don't see this variable mentioned anywhere else in the code.. I reverted back to the Build.props change

This reverts commit 0e0899d.

This reverts commit b1b154c.

This reverts commit 0a7d654.

ViktorHofer · 2024-03-26T19:53:49Z

src/SourceBuild/content/Directory.Build.props

+  <!-- The predefined environmental variable `BUILD_BUILDNUMBER` is getting overwritten in repo-projects,
+    so save it in a different varialbe-->
+  <PropertyGroup>
+    <VmrBuildNumber>$(BUILD_BUILDNUMBER)"</VmrBuildNumber>


Isn't the extra " wrong?

dkurepa added 2 commits March 15, 2024 11:05

Create and publish VerticalAssetManifest

aca71de

fix vertical manifest name

b4cb93d

dkurepa requested review from a team as code owners March 15, 2024 10:10

dkurepa mentioned this pull request Mar 15, 2024

Create and Publish VerticalManifests during unified builds dotnet/source-build#4198

Closed

premun reviewed Mar 15, 2024

View reviewed changes

ViktorHofer reviewed Mar 15, 2024

View reviewed changes

eng/pipelines/templates/jobs/vmr-build.yml Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 15, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

mmitche reviewed Mar 15, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

mmitche reviewed Mar 15, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

mmitche reviewed Mar 15, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

dkurepa added 3 commits March 18, 2024 13:35

Resolve some comments

18fe998

Merge remote-tracking branch 'source/main' into dkurepa/UnifiedBuildV…

289d4fe

…erticalManifest

Pass assetManifest paths to msbuild task

5b1051b

ViktorHofer reviewed Mar 18, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 18, 2024

View reviewed changes

...ks.PublishVerticalManifest/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest.csproj Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 18, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

Address more comments, add asset manifest validation, don't publish p…

5aebcd0

…ackages when in source build

mmitche reviewed Mar 18, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

mmitche reviewed Mar 18, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

mmitche reviewed Mar 18, 2024

View reviewed changes

ViktorHofer reviewed Mar 18, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 18, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

dkurepa added 5 commits March 19, 2024 14:01

Address comments, ignore AzureDevOpsBuildNumber and BuildId, check at…

439a5f1

…tribute count

fix build error

b4b1758

Also ignore IsReleaseOnlyPackageVersion attribute

02793bc

Set BuildId to Vmr build id in Vertical Manifests

cf5f6aa

define VmrBuildNumber in base Build.props

f99e11e

mmitche reviewed Mar 21, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

...ks.PublishVerticalManifest/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest.csproj Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

dkurepa added 2 commits March 26, 2024 12:56

Address comments

d8888b2

move VmrBuildNumber definition to build.proj

f2b0d0e

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

.../tasks/Microsoft.DotNet.SourceBuild.Tasks.PublishVerticalManifest/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

src/SourceBuild/content/build.proj Outdated Show resolved Hide resolved

dkurepa added 2 commits March 26, 2024 15:30

Change task name to MergeAssetManifests

aaf5d5d

fix assembly file name, change target name

0c8ed54

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

...ools/tasks/Microsoft.DotNet.SourceBuild.Tasks.MergeAssetManifests/PublishVerticalManifest.cs Outdated Show resolved Hide resolved

Update src/SourceBuild/content/eng/tools/tasks/Microsoft.DotNet.Sourc…

2142830

…eBuild.Tasks.MergeAssetManifests/PublishVerticalManifest.cs

ViktorHofer approved these changes Mar 26, 2024

View reviewed changes

Rename PublishVerticalManifest.cs to MergeAssetManifests.cs

645d902

ViktorHofer approved these changes Mar 26, 2024

View reviewed changes

ViktorHofer enabled auto-merge (squash) March 26, 2024 14:37

fix bad target name

106417c

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

dkurepa added 5 commits March 26, 2024 16:51

Define VmrBuildNumber in Directory.Build.Props

0e0899d

Revert "Define VmrBuildNumber in Directory.Build.Props"

0a7d654

This reverts commit 0e0899d.

Declare VmrBuildNumber earlier, before going into repo-projects

b1b154c

Revert "Declare VmrBuildNumber earlier, before going into repo-projects"

3a91825

This reverts commit b1b154c.

Reapply "Define VmrBuildNumber in Directory.Build.Props"

42cc390

This reverts commit 0a7d654.

ViktorHofer merged commit ca9773f into dotnet:main Mar 26, 2024

ViktorHofer reviewed Mar 26, 2024

View reviewed changes

akoeplinger mentioned this pull request Mar 27, 2024

VMR build broken: Unexpected value 'publish alias is not allowed dotnet/source-build#4264

Closed

Create and publish VerticalAssetManifest #19062

Create and publish VerticalAssetManifest #19062

Uh oh!

Conversation

dkurepa commented Mar 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants