Skip to content

Introduce a join point for SdkLocator and VSTemplateLocator #43723

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

Merged
merged 17 commits into from
Oct 8, 2024
Merged

Introduce a join point for SdkLocator and VSTemplateLocator #43723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ab76e2f
Introduce a join point for SdkLocator and VSTemplateLocator
Sep 26, 2024
c90d8f8
Force stringiness
Sep 26, 2024
14e4b6b
Add missing `if`
Sep 26, 2024
632fe57
Had the conditional backwards. Also, move arcade out of condition
Sep 26, 2024
c6336d8
Only build SDK once, on win-x64, depending on artifacts from arm64/x86
Sep 26, 2024
4907de7
Actually attempt to build join-point-only projects
Sep 27, 2024
4daec2a
fix condition maybe
Sep 27, 2024
0feec15
Merge remote-tracking branch 'origin/main' into 4336
Sep 30, 2024
83275fb
Merge remote-tracking branch 'origin/main' into 4336
Oct 1, 2024
50ad817
pass through build pass in dotnetbuild.props
Oct 1, 2024
ce12a28
Merge remote-tracking branch 'origin/main' into 4336
Oct 2, 2024
fef2474
oops
Oct 4, 2024
8cb27ee
Merge branch '4336' of https://github.com/directhex/sdk into 4336
Oct 4, 2024
d1a5fae
Merge remote-tracking branch 'origin/main' into 4336
Oct 4, 2024
2528512
Re-introduce exclusion but make it conditional
Oct 4, 2024
4f5f0bd
invert conditional
Oct 4, 2024
4be19ed
Merge branch 'main' into 4336
directhex Oct 4, 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/DotNetBuild.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<InnerBuildArgs>$(InnerBuildArgs) /p:IncludeAdditionalSharedFrameworks=false</InnerBuildArgs>
<InnerBuildArgs Condition="'$(SourceBuildUseMonoRuntime)' == 'true'">$(InnerBuildArgs) /p:DISABLE_CROSSGEN=true</InnerBuildArgs>
<InnerBuildArgs Condition="'$(PgoInstrument)' == 'true'">$(InnerBuildArgs) /p:PgoInstrument=true</InnerBuildArgs>
<InnerBuildArgs Condition="'$(DotNetBuildPass)' != ''">$(InnerBuildArgs) /p:DotNetBuildPass=$(DotNetBuildPass)</InnerBuildArgs>
Copy link
Member

Choose a reason for hiding this comment

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

This should be forwarded in the inner arcade command buildup: https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/SourceBuildArcadeBuild.targets#L67

You can keep it here for now though.

Copy link
Member

Choose a reason for hiding this comment

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

This property shouldn't be needed at all. Global properties are automatically passed from the outer to the inner build.

</PropertyGroup>
</Target>

Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/templates/jobs/vmr-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ jobs:
**/*.nupkg
displayName: Download Previous Build (${{ reuseBuildArtifacts }})

- ${{ eq(parameters.buildSourceOnly, true) }}:
- ${{ if eq(parameters.buildSourceOnly, true) }}:
- task: CopyFiles@2
displayName: Copy Previous Build (${{ reuseBuildArtifacts }} - Source Build artifacts)
inputs:
Expand Down
5 changes: 5 additions & 0 deletions eng/pipelines/templates/stages/vmr-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,11 @@ stages:
pool: ${{ parameters.pool_Windows }}
targetOS: windows
targetArchitecture: x64
${{ if in(parameters.scope, 'full') }}:
buildPass: 2
reuseBuildArtifactsFrom:
- Windows_x86
- Windows_arm64

- template: ../jobs/vmr-build.yml
parameters:
Expand Down
6 changes: 1 addition & 5 deletions src/Installer/redist-installer/projects/SdkResolver.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<GenerateRuntimeConfigurationFiles>false</GenerateRuntimeConfigurationFiles>
<ExcludeFromSourceOnlyBuild>true</ExcludeFromSourceOnlyBuild>
<!--
References assets from multiple architectures and can't be built inside the VMR until we have a join point job
that can pull assets from multiple legs. https://github.com/dotnet/source-build/issues/4336
-->
<ExcludeFromDotNetBuild>true</ExcludeFromDotNetBuild>
<ExcludeFromDotNetBuild Condition="'$(DotNetBuildPass)' == ''">true</ExcludeFromDotNetBuild>
Copy link
Member

Choose a reason for hiding this comment

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

Do you think there is likely to be a pass 3 (that isn't the merge pass)? If so, then I think we should be explicit about this being pass 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it depends. If we think of pass 2 as "a pass that needs artifacts from pass 1 on other verticals" and pass 3 as "a pass that needs artifacts from pass 2 on other verticals", there could be scenarios where that's needed, especially around workloads.

Do we want to constrain this to only ever work in pass 2, or simply "not in pass 1, which by definition only has one vertical and therefore won't include the required packages"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, would we expect a pass 2 job to upload all its packages (including the pass 1 stuff it already downloaded), therefore a pass 3 would have access to both pass 2 and pass 1 dependencies of a given job, therefore building these project files would work in pass 3+

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should be explicit and only produce and upload artifacts that are needed in that build pass. I will submit a PR to fine tune this.

</PropertyGroup>

<ItemGroup>
Expand Down
6 changes: 1 addition & 5 deletions src/Installer/redist-installer/projects/VSTemplateLocator.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<GenerateRuntimeConfigurationFiles>false</GenerateRuntimeConfigurationFiles>
<ExcludeFromSourceOnlyBuild>true</ExcludeFromSourceOnlyBuild>
<!--
References assets from multiple architectures and can't be built inside the VMR until we have a join point job
that can pull assets from multiple legs. https://github.com/dotnet/source-build/issues/4336
-->
<ExcludeFromDotNetBuild>true</ExcludeFromDotNetBuild>
<ExcludeFromDotNetBuild Condition="'$(DotNetBuildPass)' == ''">true</ExcludeFromDotNetBuild>
</PropertyGroup>

<ItemGroup>
Expand Down
8 changes: 2 additions & 6 deletions src/Installer/redist-installer/redist-installer.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@
<ItemGroup>
<ProjectReference Include="..\..\Layout\redist\redist.csproj" ReferenceOutputAssembly="false" />
<ProjectReference Include="$(RepoRoot)template_feed\*\*.csproj" ReferenceOutputAssembly="false" />
<!--
These two projects reference assets from multiple architectures, so they can't be built until we have a join point job that can pull assets from multiple legs.
https://github.com/dotnet/source-build/issues/4336
-->
<ProjectReference Include="projects\SdkResolver.csproj" ReferenceOutputAssembly="false" Condition="'$(DotNetBuild)' != 'true'" />
<ProjectReference Include="projects\VSTemplateLocator.csproj" ReferenceOutputAssembly="false" Condition="'$(DotNetBuild)' != 'true'" />
<ProjectReference Include="projects\SdkResolver.csproj" ReferenceOutputAssembly="false" Condition="'$(DotNetBuild)' != 'true' or '$(DotNetBuildPass)' != ''" />
<ProjectReference Include="projects\VSTemplateLocator.csproj" ReferenceOutputAssembly="false" Condition="'$(DotNetBuild)' != 'true' or '$(DotNetBuildPass)' != ''" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions src/SourceBuild/content/repo-projects/sdk.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
<BuildArgs>$(BuildArgs) /p:Architecture=$(TargetArchitecture)</BuildArgs>
<BuildArgs>$(BuildArgs) /p:DOTNET_INSTALL_DIR=$(DotNetRoot)</BuildArgs>

<!-- Pass through DotNetBuildPass due to conditional behaviour for this project on join points -->
<BuildArgs Condition="'$(DotNetBuildPass)' != ''">$(BuildArgs) /p:DotNetBuildPass=$(DotNetBuildPass)</BuildArgs>
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be hoisted out to Directory.Build.props.


<BuildArgs Condition="'$(TargetOS)' != 'windows'">$(BuildArgs) /p:AspNetCoreInstallerRid=$(TargetRid)</BuildArgs>
<!-- sdk always wants to build portable on FreeBSD -->
<BuildArgs Condition="'$(TargetOS)' == 'freebsd' and '$(DotNetBuildSourceOnly)' == 'true'">$(BuildArgs) /p:PortableBuild=true</BuildArgs>
Expand Down