From b0578a43914a2d710c5dd231e2d7a44dec2d65d6 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Mon, 16 Dec 2024 15:40:40 -0800
Subject: [PATCH 1/9] Add support for including ArtifactVisibility in manifest
 data automatically (and respect Build visibility)

---
 .../tools/Publish.proj                            | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
index d3646f578ef..e316755396a 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
@@ -104,6 +104,13 @@
     </ItemGroup>
 
     <!-- Respect Artifact item repo extension point for packages -->
+    <ItemGroup>
+      <Artifact>
+        <!-- Build visibility means don't publish outside of this build in any way. -->
+        <SkipPublish Condition="'%(Artifact.SkipPublish)' == '' and '%(Artifact.ArtifactVisibility)' == 'Build'">true</SkipPublish>
+      </Artifact>
+    </ItemGroup>
+
     <ItemGroup Condition="'@(Artifact)' != ''">
       <ExistingSymbolPackages Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and $([System.String]::Copy('%(Filename)%(Extension)').EndsWith('.symbols.nupkg'))" />
       <PackagesToPublish Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Extension)' == '.nupkg'" />
@@ -213,7 +220,7 @@
 
     <!-- Add non-package Artifact items (repo extension point) as package already got added in the BeforePublish target. -->
     <ItemGroup>
-      <ItemsToPushToBlobFeed Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Extension)' != '.nupkg'" />
+      <ItemsToPushToBlobFeed Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Artifact.ArtifactVisibility)' != 'Build' and '%(Extension)' != '.nupkg'" />
     </ItemGroup>
 
     <ItemGroup>
@@ -223,6 +230,12 @@
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
+    <ItemGroup>
+      <ItemsToPushToBlobFeed>
+        <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.ArtifactVisibility)' != ''">%(ItemsToPushToBlobFeed.ManifestArtifactData);ArtifactVisibility=%(ItemsToPushToBlobFeed.ArtifactVisibility)</ManifestArtifactData>
+      </ItemsToPushToBlobFeed>
+    </ItemGroup>
+
     <!--
       The user can set `PublishingVersion` via eng\Publishing.props
     -->

From 7cf105dae22c5bf05846a3fc7ccc555bf0831db3 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Tue, 17 Dec 2024 11:52:52 -0800
Subject: [PATCH 2/9] Remove Build visibility, make Vertical visibility illegal
 outside of vertical builds.

Don't publish artifacts with non-External visibility
---
 .../tools/Publish.proj                        | 30 ++++++++++-----
 .../tools/Sign.props                          |  1 +
 .../src/PublishArtifactsInManifestBase.cs     | 20 +++++++---
 .../BuildManifest/ManifestModelTests.cs       | 38 +++++++++++++++++++
 .../BuildManifest/Model/ArtifactVisibility.cs | 30 +++++++++++++++
 .../BuildManifest/Model/BlobArtifactModel.cs  | 24 ++++++++++++
 .../Model/PackageArtifactModel.cs             | 24 ++++++++++++
 7 files changed, 152 insertions(+), 15 deletions(-)
 create mode 100644 src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/ArtifactVisibility.cs

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
index e316755396a..d272b1b45c9 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
@@ -104,13 +104,6 @@
     </ItemGroup>
 
     <!-- Respect Artifact item repo extension point for packages -->
-    <ItemGroup>
-      <Artifact>
-        <!-- Build visibility means don't publish outside of this build in any way. -->
-        <SkipPublish Condition="'%(Artifact.SkipPublish)' == '' and '%(Artifact.ArtifactVisibility)' == 'Build'">true</SkipPublish>
-      </Artifact>
-    </ItemGroup>
-
     <ItemGroup Condition="'@(Artifact)' != ''">
       <ExistingSymbolPackages Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and $([System.String]::Copy('%(Filename)%(Extension)').EndsWith('.symbols.nupkg'))" />
       <PackagesToPublish Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Extension)' == '.nupkg'" />
@@ -220,9 +213,21 @@
 
     <!-- Add non-package Artifact items (repo extension point) as package already got added in the BeforePublish target. -->
     <ItemGroup>
-      <ItemsToPushToBlobFeed Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Artifact.ArtifactVisibility)' != 'Build' and '%(Extension)' != '.nupkg'" />
+      <ItemsToPushToBlobFeed Include="@(Artifact)" Condition="'%(Artifact.SkipPublish)' != 'true' and '%(Extension)' != '.nupkg'" />
     </ItemGroup>
 
+    <ItemGroup>
+      <ItemsToPushToBlobFeed>
+        <!-- Default artifact visibility is External -->
+        <Visibility Condition="'%(ItemsToPushToBlobFeed.Visibility)' == ''">External</Visibility>
+        <!-- Default to IsShipping=true -->
+        <IsShipping Condition="'%(ItemsToPushToBlobFeed.IsShipping)' == ''">true</IsShipping>
+      </ItemsToPushToBlobFeed>
+    </ItemGroup>
+
+    <Error Condition="@(ItemsToPublishToBlobFeed->AnyWithMetadataValue('Visibility','Vertical')) and '$(DotNetBuildOrchestrator)' != 'true'"
+           Text="Visibility 'Vertical' is only supported in vertical builds." />
+
     <ItemGroup>
       <ItemsToPushToBlobFeed>
         <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.IsShipping)' == 'false'">%(ItemsToPushToBlobFeed.ManifestArtifactData);NonShipping=true</ManifestArtifactData>
@@ -230,9 +235,16 @@
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
+    <Error Condition="@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Vertical'))"
+           Text="Visibility 'Vertical' is not supported for shipping artifacts." />
+    <Error Condition="@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Internal'))"
+           Text="Visibility 'Internal' is not supported for shipping artifacts." />
+    <Error Condition="@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical')) and '$(DotNetBuildOrchestrator)' != 'true'"
+           Text="Visibility 'Vertical' is only supported in vertical builds." />
+
     <ItemGroup>
       <ItemsToPushToBlobFeed>
-        <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.ArtifactVisibility)' != ''">%(ItemsToPushToBlobFeed.ManifestArtifactData);ArtifactVisibility=%(ItemsToPushToBlobFeed.ArtifactVisibility)</ManifestArtifactData>
+        <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.Visibility)' != ''">%(ItemsToPushToBlobFeed.ManifestArtifactData);Visibility=%(ItemsToPushToBlobFeed.Visibility)</ManifestArtifactData>
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
index 1169cb81e7b..5889e5bacf8 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
@@ -11,6 +11,7 @@
     <Artifact>
       <PublishFlatContainer>true</PublishFlatContainer>
       <IsShipping>true</IsShipping>
+      <Visibility>External</Visibility>
     </Artifact>
   </ItemDefinitionGroup>
 
diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
index 8a73efec7fb..2b1c2370970 100644
--- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
+++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
@@ -831,11 +831,15 @@ protected async Task HandlePackagePublishingAsync(ReadOnlyDictionary<string, Ass
 
         private HashSet<PackageArtifactModel> FilterPackages(HashSet<PackageArtifactModel> packages, TargetFeedConfig feedConfig)
         {
+            IEnumerable<PackageArtifactModel> filteredPackages = new HashSet<PackageArtifactModel>(packages);
+            // We only publish externally visible packages to any feeds.
+            filteredPackages = filteredPackages.Where(p => p.Visibility == ArtifactVisibility.External);
+
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => packages,
-                AssetSelection.NonShippingOnly => packages.Where(p => p.NonShipping).ToHashSet(),
-                AssetSelection.ShippingOnly => packages.Where(p => !p.NonShipping).ToHashSet(),
+                AssetSelection.All => filteredPackages.ToHashSet(),
+                AssetSelection.NonShippingOnly => filteredPackages.Where(p => p.NonShipping).ToHashSet(),
+                AssetSelection.ShippingOnly => filteredPackages.Where(p => !p.NonShipping).ToHashSet(),
 
                 // Throw NIE here instead of logging an error because error would have already been logged in the
                 // parser for the user.
@@ -1055,11 +1059,15 @@ protected async Task HandleBlobPublishingAsync(ReadOnlyDictionary<string, Asset>
         /// <returns></returns>
         private HashSet<BlobArtifactModel> FilterBlobs(HashSet<BlobArtifactModel> blobs, TargetFeedConfig feedConfig)
         {
+            IEnumerable<BlobArtifactModel> filteredBlobs = new HashSet<BlobArtifactModel>(blobs);
+            // We only publish externally visible blobs to any feeds.
+            filteredBlobs = filteredBlobs.Where(p => p.Visibility == ArtifactVisibility.External);
+
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => blobs,
-                AssetSelection.NonShippingOnly => blobs.Where(p => p.NonShipping).ToHashSet(),
-                AssetSelection.ShippingOnly => blobs.Where(p => !p.NonShipping).ToHashSet(),
+                AssetSelection.All => filteredBlobs.ToHashSet(),
+                AssetSelection.NonShippingOnly => filteredBlobs.Where(p => p.NonShipping).ToHashSet(),
+                AssetSelection.ShippingOnly => filteredBlobs.Where(p => !p.NonShipping).ToHashSet(),
 
                 // Throw NYI here instead of logging an error because error would have already been logged in the
                 // parser for the user.
diff --git a/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs b/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
index f81f0dec032..91d82e8acce 100644
--- a/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
+++ b/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
@@ -569,6 +569,25 @@ public void PackageArtifactModelEquals_ReturnsFalseWhenObjectsAreDifferentTypes(
             Assert.False(packageArtifact.Equals("thisIsNotAPackageArtifact!"));
         }
 
+        [InlineData("Vertical")]
+        [InlineData("External")]
+        [InlineData("Internal")]
+        [InlineData("")]
+        [Theory]
+        public void PackageArtifactModel_Visibility(string visibility)
+        {
+            PackageArtifactModel packageArtifact = new PackageArtifactModel
+            {
+                Id = "AssetName",
+                Attributes = new Dictionary<string, string>
+                {
+                    { "Visibility", visibility }
+                }
+            };
+
+            Assert.Equal((ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), packageArtifact.Visibility);
+        }
+
         [Fact]
         public void BlobArtifactModelEquals_ReturnsTrueWhenTwoObjectsHaveMatchingAttributes()
         {
@@ -656,6 +675,25 @@ public void BlobArtifactModelEquals_ReturnsFalseWhenObjectsAreDifferentTypes()
             Assert.False(blobArtifact.Equals("thisIsNotABlobArtifact!"));
         }
 
+        [InlineData("Vertical")]
+        [InlineData("External")]
+        [InlineData("Internal")]
+        [InlineData("")]
+        [Theory]
+        public void BlobArtifactModel_Visibility(string visibility)
+        {
+            BlobArtifactModel blobArtifact = new BlobArtifactModel
+            {
+                Id = "AssetName",
+                Attributes = new Dictionary<string, string>
+                {
+                    { "Visibility", visibility }
+                }
+            };
+
+            Assert.Equal((ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), blobArtifact.Visibility);
+        }
+
         private BuildModel CreatePackageOnlyBuildManifestModel()
         {
             return new BuildModel(new BuildIdentity { Name = "SimpleBuildManifest", BuildId = "123" })
diff --git a/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/ArtifactVisibility.cs b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/ArtifactVisibility.cs
new file mode 100644
index 00000000000..b2e37182556
--- /dev/null
+++ b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/ArtifactVisibility.cs
@@ -0,0 +1,30 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.DotNet.VersionTools.BuildManifest.Model
+{
+    /// <summary>
+    /// Description of how an artifact should be published between different builds in various different scenarios.
+    /// </summary>
+    public enum ArtifactVisibility
+    {
+        /// <summary>
+        /// The artifact should be published for external usage, whether as a shipping or non-shipping package.
+        /// </summary>
+        External,
+        /// <summary>
+        /// The artifact is used by different jobs within the same overall build, vertical or not.
+        /// The artifact should be uploaded to build artifacts, but should not be published to any NuGet/blob feeds.
+        /// </summary>
+        /// <remarks>
+        /// This visibility should be used for artifacts that must be flowed to jobs in a later build pass but not published.
+        /// </remarks>
+        Internal,
+        /// <summary>
+        /// The artifact is used by other repositories targeting the same target RID/platform.
+        /// In vertical builds, it should be published to the "on-disk" locations, but not to any NuGet/blob feeds.
+        /// In non-vertical builds, it should be treated as a non-shipping package with <see cref="External"/> visibility.
+        /// </summary>
+        Vertical,
+    }
+}
diff --git a/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/BlobArtifactModel.cs b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/BlobArtifactModel.cs
index 67ee68522bf..9f56e7cb282 100644
--- a/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/BlobArtifactModel.cs
+++ b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/BlobArtifactModel.cs
@@ -43,6 +43,30 @@ public string RepoOrigin
             set => Attributes[nameof(RepoOrigin)] = value;
         }
 
+        public ArtifactVisibility Visibility
+        {
+            get
+            {
+                string val = Attributes.GetOrDefault(nameof(Visibility));
+                if (string.IsNullOrEmpty(val))
+                {
+                    return ArtifactVisibility.External;
+                }
+                else if (Enum.TryParse(val, out ArtifactVisibility visibility))
+                {
+                    return visibility;
+                }
+                else
+                {
+                    throw new ArgumentException($"Invalid value for {nameof(Visibility)}: {val}");
+                }
+            }
+            set
+            {
+                Attributes[nameof(Visibility)] = value.ToString();
+            }
+        }
+
         public override string ToString() => $"Blob {Id}";
 
         public override bool Equals(object obj)
diff --git a/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/PackageArtifactModel.cs b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/PackageArtifactModel.cs
index d135f054f26..5e0bc452b03 100644
--- a/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/PackageArtifactModel.cs
+++ b/src/VersionTools/Microsoft.DotNet.VersionTools/BuildManifest/Model/PackageArtifactModel.cs
@@ -55,6 +55,30 @@ public bool NonShipping
             }
         }
 
+        public ArtifactVisibility Visibility
+        {
+            get
+            {
+                string val = Attributes.GetOrDefault(nameof(Visibility));
+                if (string.IsNullOrEmpty(val))
+                {
+                    return ArtifactVisibility.External;
+                }
+                else if (Enum.TryParse(val, out ArtifactVisibility visibility))
+                {
+                    return visibility;
+                }
+                else
+                {
+                    throw new ArgumentException($"Invalid value for {nameof(Visibility)}: {val}");
+                }
+            }
+            set
+            {
+                Attributes[nameof(Visibility)] = value.ToString();
+            }
+        }
+
         public string RepoOrigin
         {
             get => Attributes.GetOrDefault(nameof(RepoOrigin));

From 723c3bb2d4215f5b4d44714ba6d6ce98f27bd00f Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Tue, 17 Dec 2024 16:04:10 -0800
Subject: [PATCH 3/9] Fixup AnyHaveMetadataValue

---
 src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
index d272b1b45c9..44eaedde95e 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
@@ -225,7 +225,7 @@
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
-    <Error Condition="@(ItemsToPublishToBlobFeed->AnyWithMetadataValue('Visibility','Vertical')) and '$(DotNetBuildOrchestrator)' != 'true'"
+    <Error Condition="'@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true' and '$(DotNetBuildOrchestrator)' != 'true'"
            Text="Visibility 'Vertical' is only supported in vertical builds." />
 
     <ItemGroup>
@@ -235,11 +235,11 @@
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
-    <Error Condition="@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Vertical'))"
+    <Error Condition="'@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true'"
            Text="Visibility 'Vertical' is not supported for shipping artifacts." />
-    <Error Condition="@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Internal'))"
+    <Error Condition="'@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Internal'))' == 'true'"
            Text="Visibility 'Internal' is not supported for shipping artifacts." />
-    <Error Condition="@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical')) and '$(DotNetBuildOrchestrator)' != 'true'"
+    <Error Condition="'@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true' and '$(DotNetBuildOrchestrator)' != 'true'"
            Text="Visibility 'Vertical' is only supported in vertical builds." />
 
     <ItemGroup>

From 364bc5adf718aaaeb24d4e2e422e558b98c680bb Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Tue, 17 Dec 2024 16:05:15 -0800
Subject: [PATCH 4/9] Remove duplicate error check

---
 src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
index 44eaedde95e..7e4c134a471 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
@@ -225,9 +225,6 @@
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
-    <Error Condition="'@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true' and '$(DotNetBuildOrchestrator)' != 'true'"
-           Text="Visibility 'Vertical' is only supported in vertical builds." />
-
     <ItemGroup>
       <ItemsToPushToBlobFeed>
         <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.IsShipping)' == 'false'">%(ItemsToPushToBlobFeed.ManifestArtifactData);NonShipping=true</ManifestArtifactData>

From 5f63144afeb04910b44f387a10d8bcc11c427c06 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 18 Dec 2024 13:08:16 -0800
Subject: [PATCH 5/9] Fix empty string test cases

---
 .../BuildManifest/ManifestModelTests.cs                       | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs b/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
index 91d82e8acce..40598d31128 100644
--- a/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
+++ b/src/VersionTools/Microsoft.DotNet.VersionTools.Tests/BuildManifest/ManifestModelTests.cs
@@ -585,7 +585,7 @@ public void PackageArtifactModel_Visibility(string visibility)
                 }
             };
 
-            Assert.Equal((ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), packageArtifact.Visibility);
+            Assert.Equal(visibility is "" ? ArtifactVisibility.External : (ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), packageArtifact.Visibility);
         }
 
         [Fact]
@@ -691,7 +691,7 @@ public void BlobArtifactModel_Visibility(string visibility)
                 }
             };
 
-            Assert.Equal((ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), blobArtifact.Visibility);
+            Assert.Equal(visibility is "" ? ArtifactVisibility.External : (ArtifactVisibility)Enum.Parse(typeof(ArtifactVisibility), visibility), blobArtifact.Visibility);
         }
 
         private BuildModel CreatePackageOnlyBuildManifestModel()

From 6d91e8a6982a51f8ccc08f7ce8d583eed1dbd4d3 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 18 Dec 2024 13:43:31 -0800
Subject: [PATCH 6/9] More efficient filtering and better condition check

---
 src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj |  4 ++--
 .../src/PublishArtifactsInManifestBase.cs          | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
index 7e4c134a471..1006bd6acdf 100644
--- a/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
+++ b/src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj
@@ -228,7 +228,7 @@
     <ItemGroup>
       <ItemsToPushToBlobFeed>
         <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.IsShipping)' == 'false'">%(ItemsToPushToBlobFeed.ManifestArtifactData);NonShipping=true</ManifestArtifactData>
-        <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.IsShipping)' != 'false' and '$(ProducesDotNetReleaseShippingAssets)' == 'true'">%(ItemsToPushToBlobFeed.ManifestArtifactData);DotNetReleaseShipping=true</ManifestArtifactData>
+        <ManifestArtifactData Condition="'%(ItemsToPushToBlobFeed.IsShipping)' == 'true' and '$(ProducesDotNetReleaseShippingAssets)' == 'true'">%(ItemsToPushToBlobFeed.ManifestArtifactData);DotNetReleaseShipping=true</ManifestArtifactData>
       </ItemsToPushToBlobFeed>
     </ItemGroup>
 
@@ -236,7 +236,7 @@
            Text="Visibility 'Vertical' is not supported for shipping artifacts." />
     <Error Condition="'@(ItemsToPublishToBlobFeed->WithMetadataValue('IsShipping','true')->AnyHaveMetadataValue('Visibility','Internal'))' == 'true'"
            Text="Visibility 'Internal' is not supported for shipping artifacts." />
-    <Error Condition="'@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true' and '$(DotNetBuildOrchestrator)' != 'true'"
+    <Error Condition="'@(ItemsToPublishToBlobFeed->AnyHaveMetadataValue('Visibility','Vertical'))' == 'true' and '$(DotNetBuild)' != 'true'"
            Text="Visibility 'Vertical' is only supported in vertical builds." />
 
     <ItemGroup>
diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
index 2b1c2370970..b8908edc012 100644
--- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
+++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
@@ -831,13 +831,13 @@ protected async Task HandlePackagePublishingAsync(ReadOnlyDictionary<string, Ass
 
         private HashSet<PackageArtifactModel> FilterPackages(HashSet<PackageArtifactModel> packages, TargetFeedConfig feedConfig)
         {
-            IEnumerable<PackageArtifactModel> filteredPackages = new HashSet<PackageArtifactModel>(packages);
+            HashSet<PackageArtifactModel> filteredPackages = new HashSet<PackageArtifactModel>(packages);
             // We only publish externally visible packages to any feeds.
-            filteredPackages = filteredPackages.Where(p => p.Visibility == ArtifactVisibility.External);
+            filteredPackages.RemoveWhere(p => p.Visibility != ArtifactVisibility.External);
 
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => filteredPackages.ToHashSet(),
+                AssetSelection.All => filteredPackages,
                 AssetSelection.NonShippingOnly => filteredPackages.Where(p => p.NonShipping).ToHashSet(),
                 AssetSelection.ShippingOnly => filteredPackages.Where(p => !p.NonShipping).ToHashSet(),
 
@@ -1059,13 +1059,13 @@ protected async Task HandleBlobPublishingAsync(ReadOnlyDictionary<string, Asset>
         /// <returns></returns>
         private HashSet<BlobArtifactModel> FilterBlobs(HashSet<BlobArtifactModel> blobs, TargetFeedConfig feedConfig)
         {
-            IEnumerable<BlobArtifactModel> filteredBlobs = new HashSet<BlobArtifactModel>(blobs);
-            // We only publish externally visible blobs to any feeds.
-            filteredBlobs = filteredBlobs.Where(p => p.Visibility == ArtifactVisibility.External);
+            HashSet<BlobArtifactModel> filteredBlobs = new HashSet<BlobArtifactModel>(blobs);
+            // We only publish externally visible packages to any feeds.
+            filteredBlobs.RemoveWhere(p => p.Visibility != ArtifactVisibility.External);
 
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => filteredBlobs.ToHashSet(),
+                AssetSelection.All => filteredBlobs,
                 AssetSelection.NonShippingOnly => filteredBlobs.Where(p => p.NonShipping).ToHashSet(),
                 AssetSelection.ShippingOnly => filteredBlobs.Where(p => !p.NonShipping).ToHashSet(),
 

From 8a55c6720f888dfd83f45e8caf2c9aec562cc447 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 18 Dec 2024 14:07:33 -0800
Subject: [PATCH 7/9] Exclude assets that shouldn't be uploaded or tracked in
 an asset manifest from being inserted in the first place.

Remove changes to the publishing pipeline as it won't ever see a non External visibility any more.

Obsoletes https://github.com/dotnet/arcade-services/pull/4254
---
 .../src/PublishArtifactsInManifestBase.cs     | 20 ++++++-------------
 .../src/PushToBuildStorage.cs                 | 16 +++++++++++++++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
index b8908edc012..8a73efec7fb 100644
--- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
+++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs
@@ -831,15 +831,11 @@ protected async Task HandlePackagePublishingAsync(ReadOnlyDictionary<string, Ass
 
         private HashSet<PackageArtifactModel> FilterPackages(HashSet<PackageArtifactModel> packages, TargetFeedConfig feedConfig)
         {
-            HashSet<PackageArtifactModel> filteredPackages = new HashSet<PackageArtifactModel>(packages);
-            // We only publish externally visible packages to any feeds.
-            filteredPackages.RemoveWhere(p => p.Visibility != ArtifactVisibility.External);
-
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => filteredPackages,
-                AssetSelection.NonShippingOnly => filteredPackages.Where(p => p.NonShipping).ToHashSet(),
-                AssetSelection.ShippingOnly => filteredPackages.Where(p => !p.NonShipping).ToHashSet(),
+                AssetSelection.All => packages,
+                AssetSelection.NonShippingOnly => packages.Where(p => p.NonShipping).ToHashSet(),
+                AssetSelection.ShippingOnly => packages.Where(p => !p.NonShipping).ToHashSet(),
 
                 // Throw NIE here instead of logging an error because error would have already been logged in the
                 // parser for the user.
@@ -1059,15 +1055,11 @@ protected async Task HandleBlobPublishingAsync(ReadOnlyDictionary<string, Asset>
         /// <returns></returns>
         private HashSet<BlobArtifactModel> FilterBlobs(HashSet<BlobArtifactModel> blobs, TargetFeedConfig feedConfig)
         {
-            HashSet<BlobArtifactModel> filteredBlobs = new HashSet<BlobArtifactModel>(blobs);
-            // We only publish externally visible packages to any feeds.
-            filteredBlobs.RemoveWhere(p => p.Visibility != ArtifactVisibility.External);
-
             return feedConfig.AssetSelection switch
             {
-                AssetSelection.All => filteredBlobs,
-                AssetSelection.NonShippingOnly => filteredBlobs.Where(p => p.NonShipping).ToHashSet(),
-                AssetSelection.ShippingOnly => filteredBlobs.Where(p => !p.NonShipping).ToHashSet(),
+                AssetSelection.All => blobs,
+                AssetSelection.NonShippingOnly => blobs.Where(p => p.NonShipping).ToHashSet(),
+                AssetSelection.ShippingOnly => blobs.Where(p => !p.NonShipping).ToHashSet(),
 
                 // Throw NYI here instead of logging an error because error would have already been logged in the
                 // parser for the user.
diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
index af0863d07bb..ba75ed0a6c1 100644
--- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
+++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
@@ -218,6 +218,22 @@ public bool ExecuteTask(IFileSystem fileSystem,
                             .Where(blob => blob != null);
                     }
 
+                    // Filter out any "Internal" artifacts from the asset manifest.
+                    // We don't want to publish these outside of the build, so we exclude them from the manifest but still publish them.
+                    // We obviously want to include "External" artifacts as they are the ones we want to publish externally.
+                    packageArtifacts = packageArtifacts.Where(p => p.Visibility != Visibility.Internal);
+                    blobArtifacts = blobArtifacts.Where(p => p.Visibility != Visibility.Internal);
+
+                    if (!PushToLocalStorage)
+                    {
+                        // Inside the VMR, we publish to local storage to pass artifacts between repo sub-builds.
+                        // Outside the VMR, we publish to Azure DevOps artifacts storage.
+                        // "Vertical" artifacts should only ever be available within a build on a single machine, and should never be uploaded
+                        // to Azure DevOps artifacts storage.
+                        packageArtifacts = packageArtifacts.Where(p => p.Visibility != Visibility.Vertical);
+                        blobArtifacts = blobArtifacts.Where(p => p.Visibility != Visibility.Vertical);
+                    }
+
                     PublishingInfraVersion targetPublishingVersion = PublishingInfraVersion.Latest;
 
                     if (!string.IsNullOrEmpty(PublishingVersion))

From 8ece20c19ee15a2fcef3fd6d8a2666d6f1188fa5 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 18 Dec 2024 14:19:01 -0800
Subject: [PATCH 8/9] Fix typo

---
 .../src/PushToBuildStorage.cs                             | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
index ba75ed0a6c1..24e1cc16550 100644
--- a/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
+++ b/src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs
@@ -221,8 +221,8 @@ public bool ExecuteTask(IFileSystem fileSystem,
                     // Filter out any "Internal" artifacts from the asset manifest.
                     // We don't want to publish these outside of the build, so we exclude them from the manifest but still publish them.
                     // We obviously want to include "External" artifacts as they are the ones we want to publish externally.
-                    packageArtifacts = packageArtifacts.Where(p => p.Visibility != Visibility.Internal);
-                    blobArtifacts = blobArtifacts.Where(p => p.Visibility != Visibility.Internal);
+                    packageArtifacts = packageArtifacts.Where(p => p.Visibility != ArtifactVisibility.Internal);
+                    blobArtifacts = blobArtifacts.Where(p => p.Visibility != ArtifactVisibility.Internal);
 
                     if (!PushToLocalStorage)
                     {
@@ -230,8 +230,8 @@ public bool ExecuteTask(IFileSystem fileSystem,
                         // Outside the VMR, we publish to Azure DevOps artifacts storage.
                         // "Vertical" artifacts should only ever be available within a build on a single machine, and should never be uploaded
                         // to Azure DevOps artifacts storage.
-                        packageArtifacts = packageArtifacts.Where(p => p.Visibility != Visibility.Vertical);
-                        blobArtifacts = blobArtifacts.Where(p => p.Visibility != Visibility.Vertical);
+                        packageArtifacts = packageArtifacts.Where(p => p.Visibility != ArtifactVisibility.Vertical);
+                        blobArtifacts = blobArtifacts.Where(p => p.Visibility != ArtifactVisibility.Vertical);
                     }
 
                     PublishingInfraVersion targetPublishingVersion = PublishingInfraVersion.Latest;

From 243ee6a6a0b7e1cc39c1e144a24f194c78e38e94 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 18 Dec 2024 14:39:34 -0800
Subject: [PATCH 9/9] Add some docs

---
 Documentation/ArcadeSdk.md | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/ArcadeSdk.md b/Documentation/ArcadeSdk.md
index 4e86f9611b2..1c4577d71d4 100644
--- a/Documentation/ArcadeSdk.md
+++ b/Documentation/ArcadeSdk.md
@@ -290,6 +290,7 @@ These tools are only used for build operations performed outside of the reposito
 Customization of Authenticode signing process.
 
 Configurable item groups:
+
 - `ItemsToSign`
   List of files to sign in-place, during the build. May list individual files to sign (e.g. .dll, .exe, .ps1, etc.) as well as container files (.nupkg, .vsix, .zip, etc.). All files embedded in a container file are signed (recursively) unless specified otherwise.
 - `ItemsToSignPostBuild`
@@ -302,8 +303,12 @@ Configurable item groups:
   Specifies Authenticode certificate properties, such as whether a certificate allows dual signing.
 - `StrongNameSignInfo`
   Strong Name key to use to sign a specific managed assembly.
+- `Artifact`
+  List of files to sign and publish, either in-place or post-build depending on `PostBuildSign`. May list packages and blobs to publish.
+  More documentation available in the section on Publishing.props.
 
 Properties:
+
 - `AllowEmptySignList`
   True to allow ItemsToSign to be empty (the repository doesn't have any file to sign in-build).
 - `AllowEmptyPostBuildSignList`
@@ -321,6 +326,16 @@ To change the key used for strong-naming assemblies see `StrongNameKeyId` proper
 
 Customization of publishing process.
 
+Configurable item groups:
+
+- `Artifact`: List of files to publish. May list packages and blobs to publish.
+  Supported Metadata:
+  - `Visibility`
+    - Visibility of the artifact. Default is `External`. Visibility options are listed below:
+      - `External`: The artifact is visible outside of the build. It will be uploaded to AzDO artifacts and published to [the Build Asset Registry (BAR)](./Maestro/BuildAssetRegistry.md) as an artifact of the build.
+      - `Internal`: The artifact is visible only within the build. It will be uploaded to AzDO artifacts but not published to BAR.
+      - `Vertical`: The artifact is only visible within a given vertical in a Vertical Build. It will be copied into the local artifacts folder for the repository on disk during a vertical build. It will not be uploaded to AzDO artifacts, nor published to BAR. Any artifacts marked with `Vertical` visibility are also not available in any other Join Points in a Unified Build. See [here](./UnifiedBuild/Unified-Build-Join-Point.md) for more information. `Vertical` visibility is only available in a Vertical Build.
+
 ### /eng/AfterSolutionBuild.targets (optional)
 
 Targets executed in a step right after the solution is built.