Enable pipeline artifact usage for V4 publishing and rework tests (#15727)

Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>

Author: mmitche

eng/publishing/v3/publish.yml

@@ -32,7 +32,7 @@ stages:
           -DarcVersion $(DarcVersion)
 
     - task: DownloadPipelineArtifact@2
-      displayName: Download Merged Manifest
+      displayName: Download V3 Merged Manifest
       continueOnError: true
       enabled: true
       inputs:
@@ -44,11 +44,11 @@ stages:
         downloadType: 'specific'
         artifact: BlobArtifacts
         itemPattern: |
-          BlobArtifacts/MergedManifest.xml
+          **/MergedManifest.xml
         downloadPath: '$(Build.ArtifactStagingDirectory)/BlobArtifacts'
 
     - task: DownloadPipelineArtifact@2
-      displayName: Download PDB Artifacts
+      displayName: Download V4 Merged Manifest
       continueOnError: true
       enabled: true
       inputs:
@@ -58,8 +58,10 @@ stages:
         pipeline: $(AzDOPipelineId)
         buildId: $(AzDOBuildId)
         downloadType: 'specific'
-        artifact: PdbArtifacts
-        downloadPath: '$(Build.ArtifactStagingDirectory)/PdbArtifacts'
+        artifact: AssetManifests
+        itemPattern: |
+          **/MergedManifest.xml
+        downloadPath: '$(Build.ArtifactStagingDirectory)/BlobArtifacts'
 
     - task: DownloadPipelineArtifact@2
       displayName: Download Release Configs

src/Common/Microsoft.Arcade.Common.Tests/Microsoft.Arcade.Common.Tests.csproj

@@ -11,6 +11,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\Microsoft.Arcade.Common\Microsoft.Arcade.Common.csproj" />
+    <ProjectReference Include="..\..\Microsoft.DotNet.XUnitExtensions\src\Microsoft.DotNet.XUnitExtensions.csproj" />
   </ItemGroup>
 
 </Project>

src/Common/Microsoft.Arcade.Common/FileSystem.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.IO;
 
 #nullable enable
@@ -25,19 +26,32 @@ public class FileSystem : IFileSystem
 
         public string? GetExtension(string? path) => Path.GetExtension(path);
 
+        public string GetFullPath(string path) => Path.GetFullPath(path);
+
         public string PathCombine(string path1, string path2) => Path.Combine(path1, path2);
 
+        public string PathCombine(string path1, string path2, string path3) => Path.Combine(path1, path2, path3);
+
         public void WriteToFile(string path, string content)
         {
             string? dirPath = Path.GetDirectoryName(path);
             Directory.CreateDirectory(dirPath!);
             File.WriteAllText(path, content);
         }
 
-        public void CopyFile(string sourceFileName, string destFileName, bool overwrite = false) => File.Copy(sourceFileName, destFileName, overwrite);
+        public virtual void CopyFile(string sourceFileName, string destFileName, bool overwrite = false) => File.Copy(sourceFileName, destFileName, overwrite);
 
         public Stream GetFileStream(string path, FileMode mode, FileAccess access) => new FileStream(path, mode, access);
 
         public FileAttributes GetAttributes(string path) => File.GetAttributes(path);
+
+        /// <summary>
+        /// Intentionally NYI since the backing API is not supported on framework and core.
+        /// We use this in the PushToBuildStorage task under a limited set of circumstances.
+        /// </summary>
+        /// <param name="basePath">Base path</param>
+        /// <param name="targetPath">Target path that is relative to base path.</param>
+        /// <exception cref="NotImplementedException"></exception>
+        public virtual string GetRelativePath(string basePath, string targetPath) => throw new NotImplementedException("Not supported in default FileSystem implementation");
     }
 }

src/Common/Microsoft.Arcade.Common/IFileSystem.cs

@@ -26,12 +26,18 @@ public interface IFileSystem
 
         string PathCombine(string path1, string path2);
 
+        string PathCombine(string path1, string path2, string path3);
+
+        string GetFullPath(string path);
+
         void DeleteFile(string path);
 
         void CopyFile(string sourceFileName, string destFileName, bool overwrite = false);
 
         Stream GetFileStream(string path, FileMode mode, FileAccess access);
 
         FileAttributes GetAttributes(string path);
+
+        string GetRelativePath(string basePath, string targetPath);
     }
 }

src/Common/Microsoft.Arcade.Test.Common/MockFileSystem.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections.Generic;
 using System.IO;
 using Microsoft.Arcade.Common;
@@ -20,12 +21,16 @@ public class MockFileSystem : IFileSystem
 
         #endregion
 
+        public string DirectorySeparator { get; }
+
         public MockFileSystem(
             Dictionary<string, string>? files = null,
-            IEnumerable<string>? directories = null)
+            IEnumerable<string>? directories = null,
+            string directorySeparator = "/")
         {
             Directories = new(directories ?? new string[0]);
             Files = files ?? new();
+            DirectorySeparator = directorySeparator;
         }
 
         #region IFileSystem implementation
@@ -50,7 +55,9 @@ public void DeleteFile(string path)
 
         public string? GetExtension(string? path) => Path.GetExtension(path);
 
-        public string PathCombine(string path1, string path2) => path1 + "/" + path2;
+        public string PathCombine(string path1, string path2) => path1 + DirectorySeparator + path2;
+
+        public string PathCombine(string path1, string path2, string path3) => path1 + DirectorySeparator + path2 + DirectorySeparator + path3;
 
         public void WriteToFile(string path, string content) => Files[path] = content;
 
@@ -71,6 +78,18 @@ public FileAttributes GetAttributes(string path)
             return  attributes;
         }
 
+        public string GetFullPath(string path) => path;
+
+        public string GetRelativePath(string basePath, string targetPath)
+        {
+            if (targetPath.IndexOf(basePath) != 0)
+            {
+                throw new ArgumentException("targetPath is not relative to basePath");
+            }
+
+            return targetPath.Replace(basePath, "").TrimStart('/', '\\');
+        }
+
         #endregion
 
         /// <summary>

src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj

@@ -261,18 +261,64 @@
 
 			<!-- When we have <account>.visualstudio.com -->
       <AzureDevOpsAccount Condition="$(CollectionUri.IndexOf('visualstudio.com')) >= 0">$(CollectionUri.Split('.')[0].Split('/')[2])</AzureDevOpsAccount>
-      
-      <!-- Directory where pdbs pointed in `FilesToPublishToSymbolServer` are copied before publishing to AzDO artifacts. -->
-      <PDBsToPublishTempLocation>$(ArtifactsTmpDir)PDBsToPublish/</PDBsToPublishTempLocation>
-      <PdbArtifactsSourceBuiltOrTempDir Condition="'$(PushToLocalStorage)' == 'true'">$(SourceBuiltPdbArtifactsDir)</PdbArtifactsSourceBuiltOrTempDir>
-      <PdbArtifactsSourceBuiltOrTempDir Condition="'$(PushToLocalStorage)' != 'true'">$(PDBsToPublishTempLocation)</PdbArtifactsSourceBuiltOrTempDir>
+    </PropertyGroup>
+
+    <!-- Asset output locations
+         When V4 publishing is enabled, there are two primary scenarios:
+         1. The build is running as part of an inner build in the VMR, and the future artifacts location is NOT yet know.
+            Publishing is primarily used to propagate assets between repos in a vertical.
+         2. Publishing at the end of a vertical, or the end of a normal build, where the future artifacts needs to be known.
+    -->
+    <PropertyGroup>
+      <LocallyStageArtifacts>false</LocallyStageArtifacts>
+      <LocallyStageArtifacts Condition="'$(PublishingVersion)' == '4' or '$(PushToLocalStorage)' == 'true'">true</LocallyStageArtifacts>
 
       <!-- Use the SYSTEM_PHASENAME variable when available as that is guaranteed to be a unique identifier.
            For local scenarios and when the variable isn't available, use "<os>-<arch>-<buildpass>"" as the manifest name. -->
       <AssetManifestFileName Condition="'$(AssetManifestFileName)' == '' and '$(SYSTEM_PHASENAME)' != ''">$(SYSTEM_PHASENAME).xml</AssetManifestFileName>
       <AssetManifestFileName Condition="'$(AssetManifestFileName)' == ''">$(AssetManifestOS)-$(AssetManifestArch)$(AssetManifestPass).xml</AssetManifestFileName>
       <AssetManifestFilePath Condition="'$(AssetManifestFilePath)' == ''">$(ArtifactsLogDir)AssetManifest\$(AssetManifestFileName)</AssetManifestFilePath>
 
+      <!-- PDBs are special in that they require a local staging location even if LocallyStageArtifacts is false.-->
+      <PdbArtifactsLocalStorageDir Condition="'$(PdbArtifactsLocalStorageDir)' == '' and '$(LocallyStageArtifacts)' != 'true'">$(ArtifactsTmpDir)PDBsToPublish/</PdbArtifactsLocalStorageDir>
+    </PropertyGroup>
+
+    <!-- Set up local staging artifact paths if we're going to locally stage artifacts.
+         NOTE! The SourceBuilt* setups can be removed after this change has flowed to the VMR and the bootstrap.
+         arcade is updated. -->
+    <PropertyGroup Condition="'$(LocallyStageArtifacts)' == 'true'">
+      <!-- Publish staging defaults.
+           This is intended to be the location that a build will publish its pipeline artifacts. We avoid
+           using the repo source layout for better compatibility with various scanning tools.
+           Since not all CI builds are in AzDO, ensure that there is a reasonable default under the repo source layout. -->
+      <ArtifactsPublishStagingDir Condition="'$(ArtifactsPublishStagingDir)' == '' and '$(Build_ArtifactStagingDirectory)' != ''">$([MSBuild]::NormalizeDirectory('$(Build_ArtifactStagingDirectory)', 'artifacts'))</ArtifactsPublishStagingDir>
+      <ArtifactsPublishStagingDir Condition="'$(ArtifactsPublishStagingDir)' == ''">$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'staging'))</ArtifactsPublishStagingDir>
+
+      <!-- Directory where pdbs pointed in `FilesToPublishToSymbolServer` are copied during publishing. -->
+      <PdbArtifactsLocalStorageDir Condition="'$(PdbArtifactsLocalStorageDir)' == ''">$(SourceBuiltPdbArtifactsDir)</PdbArtifactsLocalStorageDir>
+      <PdbArtifactsLocalStorageDir Condition="'$(PdbArtifactsLocalStorageDir)' == ''">$(ArtifactsPublishStagingDir)pdbs</PdbArtifactsLocalStorageDir>
+
+      <!-- Directory where shipping packages are copied during publishing. -->
+      <ShippingPackagesLocalStorageDir Condition="'$(ShippingPackagesLocalStorageDir)' == ''">$(SourceBuiltShippingPackagesDir)</ShippingPackagesLocalStorageDir>
+      <ShippingPackagesLocalStorageDir Condition="'$(ShippingPackagesLocalStorageDir)' == ''">$(ArtifactsPublishStagingDir)packages/shipping</ShippingPackagesLocalStorageDir>
+
+      <!-- Directory where nonshipping packages are copied during publishing. -->
+      <NonShippingPackagesLocalStorageDir Condition="'$(NonShippingPackagesLocalStorageDir)' == ''">$(SourceBuiltShippingPackagesDir)</NonShippingPackagesLocalStorageDir>
+      <NonShippingPackagesLocalStorageDir  Condition="'$(NonShippingPackagesLocalStorageDir)' == ''">$(ArtifactsPublishStagingDir)packages/nonshipping</NonShippingPackagesLocalStorageDir>
+
+      <!-- Directory where asset manifests are copied during publishing. -->
+      <AssetManifestsLocalStorageDir Condition="'$(AssetManifestsLocalStorageDir)' == ''">$(SourceBuiltAssetManifestsDir)</AssetManifestsLocalStorageDir>
+      <AssetManifestsLocalStorageDir Condition="'$(AssetManifestsLocalStorageDir)' == ''">$(ArtifactsPublishStagingDir)manifests</AssetManifestsLocalStorageDir>
+
+      <!-- Directory where blob assets are copied during publishing. -->
+      <AssetsLocalStorageDir Condition="'$(AssetsLocalStorageDir)' == ''">$(SourceBuiltAssetsDir)</AssetsLocalStorageDir>
+      <AssetsLocalStorageDir Condition="'$(AssetsLocalStorageDir)' == ''">$(ArtifactsPublishStagingDir)assets</AssetsLocalStorageDir>
+
+      <!-- Default values for the future artifact name and publish locations if not provided.
+           VMR builds would typically specify these globally. Only used when publishing V4 is active. -->
+      <FutureArtifactName Condition="'$(PublishingVersion)' == '4' and '$(FutureArtifactName)' == '' and '$(SYSTEM_PHASENAME)' != ''">$(SYSTEM_PHASENAME)_Artifacts</FutureArtifactName>
+      <FutureArtifactName Condition="'$(PublishingVersion)' == '4' and '$(FutureArtifactName)' == ''">$(AssetManifestOS)_$(AssetManifestArch)$(AssetManifestPass)_Artifacts</FutureArtifactName>
+      <FutureArtifactPublishBasePath Condition="'$(PublishingVersion)' == '4' and '$(FutureArtifactPublishBasePath)' == ''">$(ArtifactsPublishStagingDir)</FutureArtifactPublishBasePath>
     </PropertyGroup>
 
     <!--
@@ -382,15 +428,16 @@
       AssetManifestPath="$(AssetManifestFilePath)" 
       IsReleaseOnlyPackageVersion="$(IsReleaseOnlyPackageVersion)"
       PushToLocalStorage="$(PushToLocalStorage)"
-      AssetsLocalStorageDir="$(SourceBuiltAssetsDir)"
+      AssetsLocalStorageDir="$(AssetsLocalStorageDir)"
       PreserveRepoOrigin="$(PreserveRepoOrigin)"
-      ShippingPackagesLocalStorageDir="$(SourceBuiltShippingPackagesDir)"
-      NonShippingPackagesLocalStorageDir="$(SourceBuiltNonShippingPackagesDir)"
-      AssetManifestsLocalStorageDir="$(SourceBuiltAssetManifestsDir)"
-      PdbArtifactsLocalStorageDir="$(PdbArtifactsSourceBuiltOrTempDir)"
+      ShippingPackagesLocalStorageDir="$(ShippingPackagesLocalStorageDir)"
+      NonShippingPackagesLocalStorageDir="$(NonShippingPackagesLocalStorageDir)"
+      AssetManifestsLocalStorageDir="$(AssetManifestsLocalStorageDir)"
+      PdbArtifactsLocalStorageDir="$(PdbArtifactsLocalStorageDir)"
       ArtifactVisibilitiesToPublish="@(ArtifactVisibilityToPublish)"
       UseHardlinksIfPossible="$(PublishingUseHardlinksIfPossible)"
-      PublishManifestOnly="$(PublishManifestOnly)" />
+      FutureArtifactName="$(FutureArtifactName)"
+      FutureArtifactPublishBasePath="$(FutureArtifactPublishBasePath)" />
   </Target>
 
 </Project>

src/Microsoft.DotNet.Build.Manifest/ArtifactModel.cs

@@ -72,6 +72,24 @@ public ArtifactVisibility Visibility
             }
         }
 
+        /// <summary>
+        /// Path to the artifact in the pipeline artifact specified by PipelineArtifactName.
+        /// </summary>
+        public string PipelineArtifactPath
+        {
+            get => Attributes.GetOrDefault(nameof(PipelineArtifactPath));
+            set => Attributes[nameof(PipelineArtifactPath)] = value;
+        }
+
+        /// <summary>
+        /// Name of the pipeline artifact
+        /// </summary>
+        public string PipelineArtifactName
+        {
+            get => Attributes.GetOrDefault(nameof(PipelineArtifactName));
+            set => Attributes[nameof(PipelineArtifactName)] = value;
+        }
+
         public override bool Equals(object obj)
         {
             if (obj is ArtifactModel other)

src/Microsoft.DotNet.Build.Manifest/BuildIdentity.cs

@@ -14,7 +14,7 @@ public enum PublishingInfraVersion
         V3 = 3,
         V4 = 4,
         Latest = V3,
-        Dev = 4
+        Dev = V4
     }
 
     public class BuildIdentity

src/Microsoft.DotNet.Build.Tasks.Feed.Tests/ArtifactUrlHelperTests.cs

@@ -45,6 +45,7 @@ public void BuildArtifactUrlHelper_ConstructDownloadUrl_ThrowsArgumentException(
     [Theory]
     [InlineData("https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=zip", "file.txt", "https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=file&subPath=%2Ffile.txt")]
     [InlineData("https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=zip", "subdir/file.txt", "https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=file&subPath=%2Fsubdir%2Ffile.txt")]
+    [InlineData("https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=zip", "subdir///file.txt", "https://artprodcus3.artifacts.visualstudio.com/Ab55de4ed-4b5a-4215-a8e4-0a0a5f71e7d8/7ea9116e-9fac-403d-b258-b31fcf1bb293/_apis/artifact/HASH/content?format=file&subPath=%2Fsubdir%2Ffile.txt")]
     public void PipelineArtifactDownloadHelper_ConstructDownloadUrl_ReturnsCorrectUrl(
         string baseUrl, string fileName, string expectedUrl)
     {

src/Microsoft.DotNet.Build.Tasks.Feed.Tests/PublishToSymbolServerTest.cs

@@ -107,10 +107,12 @@ public async Task NoAssetsToPublishFound()
 
             var path = TestInputs.GetFullPath("Symbol");
             var buildAsset = ReadOnlyDictionary<string, Asset>.Empty;
+            // Clear the symbol packages added by the s
+            task.BlobsByCategory.Clear();
 
             await task.HandleSymbolPublishingAsync(
                 buildInfo: buildInfo,
-                buildAssets: buildAsset,
+                assetNameToBARAssetMapping: buildAsset,
                 pdbArtifactsBasePath: path,
                 symbolPublishingExclusionsFile: "",
                 publishSpecialClrFiles: false,
@@ -163,8 +165,8 @@ await task.HandleSymbolPublishingAsync(
             // have interesting observable state.
             Assert.Contains(buildEngine.BuildMessageEvents, x => x.Message.StartsWith("Publishing Symbols to Symbol server"));
             var message = buildEngine.BuildMessageEvents.Single(x => x.Message.StartsWith("Publishing Symbols to Symbol server"));
-            Assert.Contains("Symbol package count: 1", message.Message);
-            Assert.Contains("Loose symbol file count: 1", message.Message);
+            Assert.Contains("Symbol packages (1):", message.Message);
+            Assert.Contains("Loose symbol files (1)", message.Message);
 
             Assert.Contains(buildEngine.BuildMessageEvents, x => x.Message.Contains("Creating symbol request"));
             Assert.Equal(2, buildEngine.BuildMessageEvents.Where(x => x.Message.Contains("Adding directory")).Count());
@@ -186,10 +188,13 @@ await task.HandleSymbolPublishingAsync(
 
         private static (MockBuildEngine, PublishArtifactsInManifestV3, ReadOnlyDictionary<string, Asset>, string, string, ProductConstructionService.Client.Models.Build) GetCanonicalSymbolTestAssets(SymbolPublishVisibility targetServer = SymbolPublishVisibility.Public)
         {
+            const string symbolPackageName= "test-package-a.1.0.0.symbols.nupkg";
+
             var symbolPackages = new Dictionary<string, Asset>()
             {
-                { "test-package-a.1.0.0.symbols.nupkg",  new(id: 0, buildId: 0, nonShipping: true, name: "test-package-a.1.0.0.symbols.nupkg", version: "1.0.0", locations: [])}
+                { symbolPackageName,  new(id: 0, buildId: 0, nonShipping: true, name: symbolPackageName, version: "1.0.0", locations: [])}
             }.AsReadOnly();
+            
             var exclusionFile = TestInputs.GetFullPath("Symbols/SymbolPublishingExclusionsFile.txt");
             var symbolFilesDir = TestInputs.GetFullPath("Symbols");
 
@@ -221,6 +226,13 @@ private static (MockBuildEngine, PublishArtifactsInManifestV3, ReadOnlyDictionar
                 SymbolRequestProject = "dotnettest"
             };
             task.FeedConfigs.Add(TargetFeedContentType.Symbols, feedConfigsForSymbols);
+            task.BlobsByCategory.Add(TargetFeedContentType.Symbols, new HashSet<Manifest.BlobArtifactModel>()
+            {
+                new Manifest.BlobArtifactModel()
+                {
+                    Id = symbolPackageName,
+                }
+            });
 
             ProductConstructionService.Client.Models.Build buildInfo = new(id: 4242, DateTimeOffset.Now, staleness: 0, released: false, stable: true, commit: "abcd", [], [], [], []);