Skip to content

PGO artifacts are vertical visibility by default except for dotnet-sdk-pgo-* #46063

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
jkoritzinsky merged 19 commits into from
Jan 27, 2025
Merged

PGO artifacts are vertical visibility by default except for dotnet-sdk-pgo-* #46063

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
802d827
PGO artifacts are vertical visibility by default except for dotnet-sd…
jkoritzinsky Jan 16, 2025
f1fcb12
Remove PGO exclusion
jkoritzinsky Jan 16, 2025
59cca10
Add backport as arcade->sdk flow is blocked
jkoritzinsky Jan 21, 2025
3e9416f
Fix toolset asset visibility
jkoritzinsky Jan 21, 2025
c20182d
Add patch for NuGet.Client publishing
jkoritzinsky Jan 22, 2025
a09be6a
Fix folder path
jkoritzinsky Jan 22, 2025
5ab898c
Update patch
jkoritzinsky Jan 22, 2025
0c7dfe7
Update patch from PR
jkoritzinsky Jan 22, 2025
0beba5b
Merge branch 'main' into pgo-visibility-default
ViktorHofer Jan 23, 2025
454b4b3
Remove DevVersions exclusion from join-verticals.proj
ViktorHofer Jan 23, 2025
e4f57a9
Use Artifact extension point for the FSharp packages
jkoritzinsky Jan 23, 2025
5843805
Merge branch 'main' of https://github.com/dotnet/sdk into pgo-visibil…
jkoritzinsky Jan 23, 2025
dfa98dd
Remove outdated patch
jkoritzinsky Jan 23, 2025
98a908a
Pass ArtifactVisibilitiesToPublish into PushToBuildStorage
jkoritzinsky Jan 23, 2025
45874a9
Pass the visibility as manifest artifact data, otherwise it doesn't g…
jkoritzinsky Jan 24, 2025
dbb743e
Merge branch 'main' of https://github.com/dotnet/sdk into pgo-visibil…
jkoritzinsky Jan 24, 2025
fe24e02
Change how the SDK looks things up as it publishes blobs in non-stand…
jkoritzinsky Jan 24, 2025
9f9ecf3
SDK currently classifies the PGO assets as shipping, so follow that.
jkoritzinsky Jan 27, 2025
fa14530
PR feedback
jkoritzinsky Jan 27, 2025
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
5 changes: 5 additions & 0 deletions eng/Publishing.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
<ManifestBuildData Include="AzureDevOpsBranch=$(BUILD_SOURCEBRANCH)" />
</ItemGroup>

<!-- The PGO sdk should always have External visibility, even if someone changes the default artifact visibility -->
<ItemGroup>
<Artifact Update="$(ArtifactsNonShippingPackagesDir)/dotnet-sdk-pgo-*" Visibility="External" />
</ItemGroup>

<ItemGroup>
<ToolsetAssetsToPublish Include="$(ArtifactsNonShippingPackagesDir)dotnet-toolset*.zip" />
</ItemGroup>
Expand Down
16 changes: 8 additions & 8 deletions src/SourceBuild/content/eng/join-verticals.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<PropertyGroup>
<TargetFramework>$(NetCurrent)</TargetFramework>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="$(TasksDir)Microsoft.DotNet.UnifiedBuild.Tasks\Microsoft.DotNet.UnifiedBuild.Tasks.csproj" />
</ItemGroup>
Expand All @@ -19,15 +19,15 @@
<Error Condition="'$(AzureDevOpsProject)' == ''" Text="AzureDevOpsProject is not set." />
<Error Condition="'$(MainVerticalArtifactsFolder)' == ''" Text="MainVerticalArtifactsFolder is not set." />
<Error Condition="'$(OutputFolder)' == ''" Text="OutputFolder is not set." />

<ItemGroup>
<VerticalManifest
<VerticalManifest
Include="$(VerticalManifestsPath)\*.xml"
Exclude="$(VerticalManifestsPath)\*Mono*.xml;$(VerticalManifestsPath)\*Pgo*.xml;$(VerticalManifestsPath)\*Shortstack*.xml;$(VerticalManifestsPath)\*DevVersions*.xml" />
Exclude="$(VerticalManifestsPath)\*Mono*.xml;$(VerticalManifestsPath)\*Shortstack*.xml;$(VerticalManifestsPath)\*DevVersions*.xml" />
</ItemGroup>

<!-- AzureDevOpsToken shouldn't be set when running in dnceng-public -->
<Microsoft.DotNet.UnifiedBuild.Tasks.JoinVerticals
<Microsoft.DotNet.UnifiedBuild.Tasks.JoinVerticals
VerticalManifest="@(VerticalManifest)"
MainVertical="$(MainVertical)"
BuildId="$(BuildId)"
Expand All @@ -37,5 +37,5 @@
MainVerticalArtifactsFolder="$(MainVerticalArtifactsFolder)"
OutputFolder="$(OutputFolder)" />
</Target>
</Project>

</Project>
2 changes: 2 additions & 0 deletions src/SourceBuild/content/repo-projects/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@
<BuildArgs>$(BuildArgs) /p:SourceBuiltAssetManifestsDir=$(RepoAssetManifestsDir)</BuildArgs>
<BuildArgs Condition="'$(OfficialBuildId)' != ''">$(BuildArgs) /p:OfficialBuildId=$(OfficialBuildId)</BuildArgs>
<BuildArgs Condition="'$(ForceDryRunSigning)' != ''">$(BuildArgs) /p:ForceDryRunSigning=$(ForceDryRunSigning)</BuildArgs>
<!-- PGO assets by default are "Vertical" visibilty. Each repo will enable the specific artifacts it must publish externally -->
<BuildArgs Condition="'$(PgoInstrument)' == 'true'">$(BuildArgs) /p:DefaultArtifactVisibility=Vertical</BuildArgs>
Copy link
Member

@mmitche mmitche Jan 16, 2025

Choose a reason for hiding this comment

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

@ViktorHofer @jkoritzinsky Is your thinking that default artifact visibility is something that the VMR will be responsible for passing, rather than runtime setting this under certain conditions?

Copy link
Member Author

@jkoritzinsky jkoritzinsky Jan 16, 2025

Choose a reason for hiding this comment

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

I think the default visibility should be a property of the VMR for PGO as we need to tell every repo (not just runtime) that its assets are Vertical-only.

For other scenarios (like filtering out RID-agnostic packages if we wanted to get rid of the concept of a main vertical entirely) I could see runtime controlling the default rules itself.

</PropertyGroup>

<PropertyGroup Condition="'$(DotNetBuildSourceOnly)' == 'true'">
Expand Down
Loading