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

Enable publishing in VMR #7233

Merged
merged 4 commits into from
Mar 14, 2024
+12 −6
Merged

Enable publishing in VMR #7233

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
67bbe8a
Enable publishing in VMR
NikolaMilosavljevic Mar 7, 2024
f62d4c4
Trim publishing content to minimum required
NikolaMilosavljevic Mar 11, 2024
109356e
Add reference to tracking issue
NikolaMilosavljevic Mar 11, 2024
f8fa09b
Trim publishing.props file
NikolaMilosavljevic Mar 14, 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
32 changes: 27 additions & 5 deletions eng/Publishing.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,29 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<PropertyGroup>
<PublishingVersion>3</PublishingVersion>
<ProducesDotNetReleaseShippingAssets>true</ProducesDotNetReleaseShippingAssets>
</PropertyGroup>
</Project>
<PropertyGroup>
<PublishingVersion>3</PublishingVersion>
<ProducesDotNetReleaseShippingAssets>true</ProducesDotNetReleaseShippingAssets>
<PublishDependsOnTargets>$(PublishDependsOnTargets);_PublishPackages</PublishDependsOnTargets>
</PropertyGroup>

<ItemGroup>
<_PackagesToPublish Remove="@(_PackagesToPublish)" />
<_PackagesToPublish Include="$(ArtifactsPackagesDir)**\*.nupkg" UploadPathSegment="Roslyn-analyzers" Condition="'$(DotNetBuildRepo)' == 'true'" />
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 skeptical any of this is needed. In addition, UploadPathSegment won't do anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is very similar to the fsharp case: dotnet/fsharp#16838 (comment). Arcade infra won't collect all of the packages as the sub-directories under artifacts/Packages are different than what arcade expects.

Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't collect everything I think. This will include duplicates of packages (with different branding).

Copy link
Member Author

@NikolaMilosavljevic NikolaMilosavljevic Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Old model of harvesting artifacts (RepoManifest.xml) was collecting all of these. Here's are the packages:

artifacts/packages/Release/PreRelease/Microsoft.CodeAnalysis.Analyzers.3.11.0-beta1.final.nupkg
artifacts/packages/Release/PreRelease/Microsoft.CodeAnalysis.AnalyzerUtilities.3.11.0-beta1.final.nupkg
artifacts/packages/Release/PreRelease/Microsoft.CodeAnalysis.NetAnalyzers.9.0.0-preview.final.nupkg
artifacts/packages/Release/PreRelease/Roslyn.Diagnostics.Analyzers.3.11.0-beta1.final.nupkg
artifacts/packages/Release/Release/Microsoft.CodeAnalysis.Analyzers.3.11.0.nupkg
artifacts/packages/Release/Release/Microsoft.CodeAnalysis.AnalyzerUtilities.3.11.0.nupkg
artifacts/packages/Release/Release/Microsoft.CodeAnalysis.NetAnalyzers.9.0.0.nupkg
artifacts/packages/Release/Release/Roslyn.Diagnostics.Analyzers.3.11.0.nupkg
artifacts/packages/Release/Shipping/Microsoft.CodeAnalysis.Analyzers.3.11.0-beta1.24128.1.nupkg
artifacts/packages/Release/Shipping/Microsoft.CodeAnalysis.AnalyzerUtilities.3.11.0-beta1.24128.1.nupkg
artifacts/packages/Release/Shipping/Microsoft.CodeAnalysis.NetAnalyzers.9.0.0-preview.24128.1.nupkg
artifacts/packages/Release/Shipping/Roslyn.Diagnostics.Analyzers.3.11.0-beta1.24128.1.nupkg

No duplication of package names, but I'm unsure of contents and if we need all of them. Linux VMR build did require more than what was harvested by default arcade publishing infra.

I'm doing a local test to see if we can avoid changes in this PR altogether. Same with fsharp changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK - it appears that none of the other packages are needed from this repo - I'm going to close this PR once I'm done with verification.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried to remove all of these packages and 2nd-stage Linux build failed with missing dependency (package downgrade):

##vso[task.logissue type=error;sourcepath=/src/git/dotnet/src/sdk/src/BuiltInTools/dotnet-watch/dotnet-watch.csproj;linenumber=0;columnnumber=0;code=NU1605;](NETCORE_ENGINEERING_TELEMETRY=Restore) Warning As Error: Detected package downgrade: Microsoft.CodeAnalysis.AnalyzerUtilities from 3.11.0 to 3.11.0-beta1.24128.1. Reference the package directly from the project to select a different version. %0A dotnet-watch -> Microsoft.CodeAnalysis.CSharp.Features 4.10.0-3.24129.7 -> Microsoft.CodeAnalysis.Features 4.10.0-3.24129.7 -> Microsoft.CodeAnalysis.AnalyzerUtilities (>= 3.11.0) %0A dotnet-watch -> Microsoft.CodeAnalysis.CSharp.Features 4.10.0-3.24129.7 -> Microsoft.CodeAnalysis.AnalyzerUtilities (>= 3.11.0-beta1.24128.1)

I am now testing a change to include the following packages, and only in source-only build:

artifacts/packages/Release/Release/Microsoft.CodeAnalysis.Analyzers.3.11.0.nupkg
artifacts/packages/Release/Release/Microsoft.CodeAnalysis.AnalyzerUtilities.3.11.0.nupkg
artifacts/packages/Release/Release/Microsoft.CodeAnalysis.NetAnalyzers.9.0.0.nupkg
artifacts/packages/Release/Release/Roslyn.Diagnostics.Analyzers.3.11.0.nupkg

Copy link
Member Author

Choose a reason for hiding this comment

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

Trimmed the publishing to include minimum required packages - f62d4c4

Without these changes, 2nd stage Linux build is failing. This can be evaluated at later time to determine if there is a way to get rid of these additional packages. Linux source-build was requiring all of these packages and more. This is an improvement.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a reference to tracking issue with 109356e

</ItemGroup>

<Target Name="_PublishPackages">
<ItemGroup>
<!-- Do not push .nupkg files from Linux and macOS builds. They'll be packed up separately and signed on Windows.
Do not remove if post build sign is true, as we avoid the xplat codesign jobs, and need to have
the nupkgs pushed. Do not do this if building from source, since we want the source build intermediate package
to be published. Use DotNetBuildRepo as DotNetBuildFromSource is only set in the internal source build,
and Build.proj is invoked from the wrapper build. -->
<ItemsToPushToBlobFeed Remove="@(ItemsToPushToBlobFeed)" Condition="'$(OS)' != 'Windows_NT' and '$(PostBuildSign)' != 'true' and '$(DotNetBuildRepo)' != 'true'" />

<ItemsToPushToBlobFeed Include="@(_PackagesToPublish)">
<IsShipping>true</IsShipping>
</ItemsToPushToBlobFeed>
</ItemGroup>
</Target>

</Project>