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

Jump to bottom

Add Join-Verticals task, use it at the final Join Point #19369

Closed
wants to merge 29 commits into from
Closed

Add Join-Verticals task, use it at the final Join Point #19369

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
64b685f
Add Join-Verticals task, use it at the final Join Point
dkurepa Apr 9, 2024
3b0b615
parametrize mainVertical
dkurepa Apr 9, 2024
c2ee890
small fix
dkurepa Apr 9, 2024
76855fe
Update src/SourceBuild/content/eng/tools/join-verticals.proj
dkurepa Apr 9, 2024
cc3ba36
Update src/SourceBuild/content/eng/tools/join-verticals.proj
dkurepa Apr 9, 2024
c0ec4d4
Update src/SourceBuild/content/eng/tools/join-verticals.proj
dkurepa Apr 9, 2024
44028e6
Fix bug, small improvements
dkurepa Apr 9, 2024
cb0c2f9
Address comments
dkurepa Apr 9, 2024
2d268a2
Start downloading zip as soon as it's available, use copy method
dkurepa Apr 9, 2024
69e6f1f
Merge remote-tracking branch 'source/main' into dev/dkurepa/VmrMerged…
dkurepa Apr 10, 2024
2810fb2
make some properties const
dkurepa Apr 10, 2024
645af11
Use OrdinalIgnoreCase when searching for files to copy
dkurepa Apr 10, 2024
f5b4bcc
Use build.proj to call join-verticals, based on DotNetBuildPass
dkurepa Apr 10, 2024
39bad4e
Also run final join in PRs
dkurepa Apr 10, 2024
9de8ec1
fix nullability error
dkurepa Apr 10, 2024
5d87fcd
Always run the final join stage
dkurepa Apr 10, 2024
3d35720
add the vmr-final-join in the correct PR yaml
dkurepa Apr 10, 2024
0531668
fix join-verticals.proj indentation, fix yaml parameter name
dkurepa Apr 10, 2024
6bceba3
don't set a default value for DotNetBuildPass yet
dkurepa Apr 10, 2024
0782cda
fix yaml parameters
dkurepa Apr 11, 2024
7dfafd6
improve logging
dkurepa Apr 11, 2024
749b4bf
Refactor yml, address comments
dkurepa Apr 11, 2024
4c3db41
Update src/SourceBuild/content/build.proj
dkurepa Apr 11, 2024
150cd78
Add linux support to join-verticals step, make Azdo API models records
dkurepa Apr 11, 2024
fb3b996
small fix
dkurepa Apr 11, 2024
f9936a1
Merge branch 'main' into dev/dkurepa/VmrMergedManifest
dkurepa Apr 11, 2024
586e019
fix yaml
dkurepa Apr 12, 2024
f703e79
Merge remote-tracking branch 'origin/dev/dkurepa/VmrMergedManifest' i…
dkurepa Apr 12, 2024
ebfb759
define empty variable when in public
dkurepa Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 18 additions & 6 deletions eng/pipelines/templates/jobs/vmr-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ jobs:
displayName: Publish Artifacts
condition: always()

- output: buildArtifacts
dkurepa marked this conversation as resolved.
Show resolved Hide resolved
PathtoPublish: $(sourcesPath)/artifacts/$(Agent.JobName).xml
ArtifactName: VerticalManifests
displayName: Publish Vertical Manifest

- ${{ if not(parameters.isBuiltFromVmr) }}:
- output: pipelineArtifact
displayName: Upload failed patches
Expand Down Expand Up @@ -238,7 +243,7 @@ jobs:

- ${{ if eq(parameters.targetOS, 'windows') }}:
- script: |
call $(sourcesPath)\build.cmd -ci -cleanWhileBuilding -prepareMachine /p:TargetOS=${{ parameters.targetOS }} /p:TargetArchitecture=${{ parameters.targetArchitecture }} ${{ parameters.extraProperties }}
call $(sourcesPath)\build.cmd -ci -cleanWhileBuilding -prepareMachine /p:VerticalName=$(Agent.JobName) /p:TargetOS=${{ parameters.targetOS }} /p:TargetArchitecture=${{ parameters.targetArchitecture }} ${{ parameters.extraProperties }}
displayName: Build

- ${{ if eq(parameters.runTests, 'True') }}:
Expand Down Expand Up @@ -326,6 +331,8 @@ jobs:
extraBuildProperties="$extraBuildProperties ${{ parameters.extraProperties }}"
fi

extraBuildProperties="$extraBuildProperties /p:VerticalName=$(Agent.JobName)"

buildArgs="$(additionalBuildArgs) $customBuildArgs $extraBuildProperties"

# Only use Docker when a container is specified
Expand Down Expand Up @@ -482,11 +489,8 @@ jobs:

- task: CopyFiles@2
inputs:
SourceFolder: $(sourcesPath)/artifacts
Contents: |
VerticalManifest.xml
assets/**
TargetFolder: $(Build.ArtifactStagingDirectory)/publishing
SourceFolder: $(sourcesPath)/artifacts/assets
TargetFolder: $(Build.ArtifactStagingDirectory)/publishing/assets
displayName: Copy artifacts to Artifact Staging Directory
condition: succeededOrFailed()

Expand All @@ -505,3 +509,11 @@ jobs:
displayName: Publish Artifacts
condition: succeededOrFailed()
continueOnError: true

- task: PublishBuildArtifacts@1
ViktorHofer marked this conversation as resolved.
Show resolved Hide resolved
inputs:
PathtoPublish: $(sourcesPath)/artifacts/$(Agent.JobName).xml
ArtifactName: VerticalManifests
displayName: Publish Vertical Manifest
condition: succeededOrFailed()

79 changes: 79 additions & 0 deletions eng/pipelines/templates/stages/vmr-final-join.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
parameters:
# Branch of the VMR to use (to push to for internal builds)
- name: vmrBranch
type: string
default: $(Build.SourceBranch)

- name: pool_Windows
type: object
default:
name: $(defaultPoolName)
image: $(poolImage_Windows)
demands: ImageOverride -equals $(poolImage_Windows)
os: windows

- name: primaryDependentJob
type: string
default: Windows_x64

- name: azureDevOpsOrg
type: string
default: dnceng

- name: azureDevOpsProject
type: string
default: internal

stages:
- stage: VMR_Final_Join
displayName: VMR Final Join
dependsOn: VMR_Vertical_Build
variables:
- ${{ if eq(variables['System.TeamProject'], 'internal') }}:
- group: Publish-Build-Assets
- ${{ else }}:
- name: dn-bot-dnceng-build-rw-code-rw
value: ''

- template: ../variables/vmr-build.yml
parameters:
vmrBranch: ${{ parameters.vmrBranch }}

jobs:
- job: VMR_Final_Join
pool: ${{ parameters.pool_Windows }}

templateContext:
outputs:
- output: pipelineArtifact
path: $(Build.ArtifactStagingDirectory)/artifacts
artifact: JoinedArtifacts
condition: succeededOrFailed()

steps:
- checkout: self

- task: DownloadPipelineArtifact@2
inputs:
artifactName: 'VerticalManifests'
targetPath: $(Build.ArtifactStagingDirectory)/VerticalManifests

- script: $(Build.SourcesDirectory)/build.cmd
ViktorHofer marked this conversation as resolved.
Show resolved Hide resolved
-ci
dkurepa marked this conversation as resolved.
Show resolved Hide resolved
/p:DotNetBuildPass=final
/p:VerticalManifestsPath=$(Build.ArtifactStagingDirectory)/VerticalManifests
/p:MainVertical=${{ parameters.primaryDependentJob }}
/p:BuildId=$(Build.BuildId)
/p:AzureDevOpsToken=$(dn-bot-dnceng-build-rw-code-rw)
/p:AzureDevOpsOrg=${{ parameters.azureDevOpsOrg }}
/p:AzureDevOpsProject=${{ parameters.azureDevOpsProject }}
/p:TmpFolder=$(Agent.TempDirectory)
/p:OutputFolder=$(Build.ArtifactStagingDirectory)/artifacts
displayName: 'VMR Final Join'

- ${{ if or(ne(variables['System.TeamProject'], 'internal'), eq(variables['Build.Reason'], 'PullRequest')) }}:
- publish: $(Build.ArtifactStagingDirectory)/artifacts
artifact: JoinedArtifacts
displayName: Publish Artifacts
condition: succeededOrFailed()
continueOnError: true
6 changes: 6 additions & 0 deletions eng/pipelines/vmr-build-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ stages:
${{ else }}:
scope: lite

- ${{ if eq(variables['isSourceOnlyBuild'], false) }}:
- template: templates/stages/vmr-final-join.yml
parameters:
azureDevOpsOrg: dnceng-public
azureDevOpsProject: public
Copy link
Member

Choose a reason for hiding this comment

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

Why do we import this here instead of stages/vmr-build.yml? That file lists all the jobs including the join jobs that will eventually add and is the source of truth for the list of verticals.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see this Final Join stage as a Post build stage, where we prepare for BAR publishing and eventually (as a part of the next PR) do the BAR publishing, so not putting in vmr-build made sense to me

Copy link
Member Author

Choose a reason for hiding this comment

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

I did some refactoring, renamed the stage to VMR Post Build for better clarity. I think with that name it becomes clear that it should be a separate yml. Let me know if you think differently please


# In case the VMR Build stage is temporarily disabled, the VMR synchronization step is run to validate
# that the PR can be merged and later synchronized into the VMR without problems.
- ${{ else }}:
Expand Down
7 changes: 6 additions & 1 deletion src/SourceBuild/content/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,9 @@
<PoisonUsageReportFile>$(PackageReportDir)poison-usage.xml</PoisonUsageReportFile>
</PropertyGroup>

</Project>
<PropertyGroup>
<VerticalName Condition="'$(VerticalName)' == ''">DefaultVertical</VerticalName>
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 if vertical is an appropriate term to use here. What this captures is a unique build identifier, right? cc @mmitche

Copy link
Member

Choose a reason for hiding this comment

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

Would something like ManifestName make more sense? We have the same in Arcade's Publish.proj: https://github.com/dotnet/arcade/blob/87b015b938e5400d6e57afd7650348c17a764b73/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj#L70

Copy link
Member

Choose a reason for hiding this comment

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

Most repos, like installer, that produce multiple manifests do not call each build a 'vertical', though in essence that is what they are. They also do often have a concept of a primary vertical, but there isn't really any formal term used. It's usually along the lines of: "/p:PublishNugetPackages=true". It's not consistent across repos.

You could just say "ManifestName" here. What differentiates this usage, I think, is that we're really formalizing the 'primary' vertical for join points, and we have to give that some kind of name. Primary/main vertical makes the most sense in that context, vs. "Primary Manifest" or such. Then I think it makes sense to use Vertical here too.

<MergedAssetManifestOutputPath>$(ArtifactsDir)$(VerticalName).xml</MergedAssetManifestOutputPath>
</PropertyGroup>

</Project>
dkurepa marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 9 additions & 3 deletions src/SourceBuild/content/build.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
</ProjectReference>
</ItemDefinitionGroup>

<ItemGroup>
<ItemGroup Condition="'$(DotNetBuildPass)' != 'final'">
<!-- Pre-build: Init tools-->
<ProjectReference Include="$(RepositoryEngineeringDir)init-cross-build.proj"
Condition="'$(BuildOS)' != 'windows' and '$(CrossBuild)' == 'true' and '$(ROOTFS_DIR)' == ''"
Expand All @@ -26,7 +26,12 @@
<ProjectReference Include="$(RepositoryEngineeringDir)publish.proj" />
</ItemGroup>

<Target Name="PrintInfo">
<ItemGroup Condition="'$(DotNetBuildPass)' == 'final'">
<ProjectReference Include="$(ToolsDir)join-verticals.proj" />
</ItemGroup>

<Target Name="PrintInfo"
Copy link
Member

Choose a reason for hiding this comment

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

Why not print info about the pass being the final pass?

Copy link
Member

Choose a reason for hiding this comment

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

nit: it would be good to include the build pass in the info printed.

Condition="'$(DotNetBuildPass)' != 'final'">
<PropertyGroup>
<BuildModeInfoText Condition="'$(DotNetBuildSourceOnly)' == 'true'">source-build</BuildModeInfoText>
<BuildModeInfoText Condition="'$(DotNetBuildSourceOnly)' != 'true'">non-source-build</BuildModeInfoText>
Expand All @@ -37,7 +42,8 @@
</Target>

<Target Name="LogBuildOutputFolders"
AfterTargets="Build">
AfterTargets="Build"
Condition="'$(DotNetBuildPass)' != 'final'">
<Message Importance="high" Text="Shipping packages are located in '$(ArtifactsShippingPackagesDir)'." />
<Message Importance="high" Text="Shipping assets are located in '$(ArtifactsAssetsDir)'." />
</Target>
Expand Down
3 changes: 3 additions & 0 deletions src/SourceBuild/content/eng/pipelines/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,6 @@ extends:
scope: lite
${{ else }}:
scope: full

- ${{ if eq(variables['isSourceOnlyBuild'], false) }}:
- template: /src/installer/eng/pipelines/templates/stages/vmr-final-join.yml@self
Copy link
Member

Choose a reason for hiding this comment

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

Same question as in vmr-build-pr.yml. Why don't we define this in the stages/vmr-build.yml?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did some refactoring, renamed the stage to VMR Post Build for better clarity. I think with that name it becomes clear that it should be a separate yml. Let me know if you think differently please

5 changes: 5 additions & 0 deletions src/SourceBuild/content/eng/pipelines/pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,8 @@ stages:
scope: lite
${{ else }}:
scope: full

- template: /src/installer/eng/pipelines/templates/stages/vmr-final-join.yml@self
parameters:
azureDevOpsOrg: dnceng-public
azureDevOpsProject: public
dkurepa marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 3 additions & 6 deletions src/SourceBuild/content/eng/publish.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,16 @@
<!-- Create a merge manifest from the individual repository manifest files. -->
<UsingTask TaskName="Microsoft.DotNet.UnifiedBuild.Tasks.MergeAssetManifests" AssemblyFile="$(MicrosoftDotNetUnifiedBuildTasksAssembly)" TaskFactory="TaskHostFactory" />
<Target Name="MergeAssetManifests" AfterTargets="Build">
<PropertyGroup>
<MergedAssetManifestOutputPath>$(ArtifactsDir)VerticalManifest.xml</MergedAssetManifestOutputPath>
</PropertyGroup>

<ItemGroup>
<RepoAssetManifest Include="$(AssetManifestsIntermediateDir)\**\*.xml" />
</ItemGroup>

<!-- It's OK for the VmrBuildNumber to be empty -->
<!-- It's OK for the VmrBuildNumber and VerticalName to be empty -->
<Microsoft.DotNet.UnifiedBuild.Tasks.MergeAssetManifests
AssetManifest="@(RepoAssetManifest)"
MergedAssetManifestOutputPath="$(MergedAssetManifestOutputPath)"
VmrBuildNumber="$(BUILD_BUILDNUMBER)" />
VmrBuildNumber="$(BUILD_BUILDNUMBER)"
VerticalName="$(VerticalName)" />
</Target>

</Project>
47 changes: 47 additions & 0 deletions src/SourceBuild/content/eng/tools/join-verticals.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<Project Sdk="Microsoft.Build.NoTargets">
dkurepa marked this conversation as resolved.
Show resolved Hide resolved
dkurepa marked this conversation as resolved.
Show resolved Hide resolved

<PropertyGroup>
<TargetFramework>$(NetCurrent)</TargetFramework>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="$(TasksDir)Microsoft.DotNet.UnifiedBuild.Tasks\Microsoft.DotNet.UnifiedBuild.Tasks.csproj" />
</ItemGroup>

<UsingTask TaskName="Microsoft.DotNet.UnifiedBuild.Tasks.JoinVerticals" AssemblyFile="$(MicrosoftDotNetUnifiedBuildTasksAssembly)" TaskFactory="TaskHostFactory" />
<Target Name="JoinVerticals " AfterTargets="Build">
<Error Condition="'$(MainVertical)' == ''" Text="MainVertical is not set." />
<Error Condition="'$(VerticalManifestsPath)' == ''" Text="VerticalManifestsPath is not set." />
<Error Condition="'$(BuildId)' == ''" Text="BuildId is not set." />
<Error Condition="'$(AzureDevOpsOrg)' == ''" Text="AzureDevOpsOrg is not set." />
<Error Condition="'$(AzureDevOpsProject)' == ''" Text="AzureDevOpsProject is not set." />
<Error Condition="'$(TmpFolder)' == ''" Text="TmpFolder is not set." />
<Error Condition="'$(OutputFolder)' == ''" Text="OutputFolder is not set." />

<ItemGroup>
<VerticalManifest Include="$(VerticalManifestsPath)\*.xml"/>
</ItemGroup>

<PropertyGroup>
<FlatCopy Condition="'$(DotNetBuildPass)' == 'final'">true</FlatCopy>
<FlatCopy Condition="'$(DotNetBuildPass)' != 'final'">false</FlatCopy>
</PropertyGroup>

<Message Importance="High" Text="Joining verticals $(VerticalSubSet)" Condition="'$(VerticalSubSet)' != ''"/>
<Message Importance="High" Text="VerticalSubSet not set, joining all verticals @(VerticalManifest)" Condition="'$(VerticalSubSet)' == ''"/>

<!-- AzureDevOpsToken shouldn't be set when running in dnceng-public -->
<Microsoft.DotNet.UnifiedBuild.Tasks.JoinVerticals
VerticalManifest="@(VerticalManifest)"
MainVertical="$(MainVertical)"
BuildId="$(BuildId)"
AzureDevOpsToken="$(AzureDevOpsToken)"
AzureDevOpsOrg="$(AzureDevOpsOrg)"
AzureDevOpsProject="$(AzureDevOpsProject)"
TmpFolder="$(TmpFolder)"
OutputFolder="$(OutputFolder)"
VerticalSubSet="$(VerticalSubSet)"
FlatCopy="$(FlatCopy)" />
</Target>

</Project>