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.

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

Join assets from verticals in the final join point #43627

Merged
merged 23 commits into from
Oct 17, 2024
Merged
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0270b04
merge vertical manifests
MilenaHristova Sep 23, 2024
260f356
exclude verticals from join
MilenaHristova Sep 25, 2024
c5c14ac
add comment
MilenaHristova Sep 25, 2024
0aa6762
list duplicate assets
MilenaHristova Sep 27, 2024
d1784b7
parallelism
MilenaHristova Sep 27, 2024
745f552
download build artifact
MilenaHristova Sep 27, 2024
6f70d33
fix path
MilenaHristova Sep 27, 2024
82c18d0
add comment
MilenaHristova Sep 27, 2024
ed711dc
add extra parallelism
MilenaHristova Oct 7, 2024
c34e878
Update src/SourceBuild/content/eng/tools/tasks/Microsoft.DotNet.Unifi…
MilenaHristova Oct 7, 2024
717bfcb
Merge branch 'main' into mhristova/join-verticals
MilenaHristova Oct 7, 2024
a08dc29
add comment
MilenaHristova Oct 9, 2024
0a9cb47
Merge branch 'mhristova/join-verticals' of https://github.com/MilenaH…
MilenaHristova Oct 9, 2024
afd565d
Merge branch 'main' into mhristova/join-verticals
mmitche Oct 10, 2024
1093ff3
Merge branch 'main' into mhristova/join-verticals
mmitche Oct 10, 2024
ea8dd95
Update src/SourceBuild/content/eng/join-verticals.proj
MilenaHristova Oct 15, 2024
90400d7
Update src/SourceBuild/content/eng/pipelines/ci.yml
MilenaHristova Oct 15, 2024
2e256ba
Update src/SourceBuild/content/eng/pipelines/pr.yml
MilenaHristova Oct 15, 2024
0d501d1
Remove the unused target
ViktorHofer Oct 16, 2024
5d36eba
Update src/SourceBuild/content/eng/pipelines/ci.yml
ViktorHofer Oct 16, 2024
d33abcc
Update src/SourceBuild/content/eng/pipelines/pr.yml
ViktorHofer Oct 16, 2024
37d25d5
Merge branch 'main' into mhristova/join-verticals
ViktorHofer Oct 17, 2024
875c991
Update vmr-build.yml
ViktorHofer Oct 17, 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
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class JoinVerticals : Microsoft.Build.Utilities.Task
public required ITaskItem[] VerticalManifest { get; init; }

/// <summary>
/// Name of the main vertical that we'll take all artifacts from
/// Name of the main vertical that we'll take all artifacts from if they exist in this vertical, and at least one other vertical.
/// </summary>
[Required]
public required string MainVertical { get; init; }
Expand Down Expand Up @@ -102,8 +102,11 @@ private async Task ExecuteAsync()
List<string> addedPackageIds = AddMissingElements(packageElements, mainVerticalManifest, _packageElementName);
List<string> addedBlobIds = AddMissingElements(blobElements, mainVerticalManifest, _blobElementName);

CopyMainVerticalAssets(Path.Combine(MainVerticalArtifactsFolder, _packagesFolderName), Path.Combine(OutputFolder, _packagesFolderName));
CopyMainVerticalAssets(Path.Combine(MainVerticalArtifactsFolder, _assetsFolderName), Path.Combine(OutputFolder, _assetsFolderName));
string packagesOutputDirectory = Path.Combine(OutputFolder, _packagesFolderName);
string blobsOutputDirectory = Path.Combine(OutputFolder, _assetsFolderName);

CopyMainVerticalAssets(Path.Combine(MainVerticalArtifactsFolder, _packagesFolderName), packagesOutputDirectory);
CopyMainVerticalAssets(Path.Combine(MainVerticalArtifactsFolder, _assetsFolderName), blobsOutputDirectory);

using var clientThrottle = new SemaphoreSlim(16, 16);
List<Task> downloadTasks = new();
Expand All @@ -121,14 +124,25 @@ private async Task ExecuteAsync()
addedPackageIds = AddMissingElements(packageElements, verticalManifest, _packageElementName);
addedBlobIds = AddMissingElements(blobElements, verticalManifest, _blobElementName);

if (addedPackageIds.Count > 0 || addedBlobIds.Count > 0)
if (addedPackageIds.Count > 0)
{
downloadTasks.Add(
DownloadArtifactFiles(
BuildId,
$"{verticalName}{_artifactNameSuffix}",
addedPackageIds,
packagesOutputDirectory,
clientThrottle));
}

if (addedBlobIds.Count > 0)
{
downloadTasks.Add(
DownloadArtifactFiles(
BuildId,
$"{verticalName}{_artifactNameSuffix}",
addedBlobIds,
blobsOutputDirectory,
clientThrottle));
}
}
Expand Down Expand Up @@ -159,86 +173,87 @@ private async Task ExecuteAsync()
}

/// <summary>
/// Downloads specified packages and symbols from a specific build artifact and stores them in an output folder,
/// either flat or with the same relative path as in the artifact.
/// Downloads specified packages and symbols from a specific build artifact and stores them in an output folder
/// </summary>
private async Task DownloadArtifactFiles(
string buildId,
string artifactName,
List<string> packageFileNames,
List<string> assetFileNames,
List<string> fileNamesToDownload,
string outputDirectory,
SemaphoreSlim clientThrottle)
{
string packagesOutputPath = Path.Combine(OutputFolder, _packagesFolderName);
string assetsOutputPath = Path.Combine(OutputFolder, _assetsFolderName);

using AzureDevOpsClient azureDevOpsClient = new(AzureDevOpsToken, AzureDevOpsBaseUri, AzureDevOpsProject, Log);

ArtifactFiles filesInformation = await azureDevOpsClient.GetArtifactFilesInformation(buildId, artifactName, _retryCount);

await Task.WhenAll(fileNamesToDownload.Select(async fileName =>
await DownloadFileFromArtifact(
filesInformation,
artifactName,
azureDevOpsClient,
buildId,
fileName,
outputDirectory,
clientThrottle)));
}

private async Task DownloadFileFromArtifact(
ArtifactFiles artifactFilesMetadata,
string azureDevOpsArtifact,
AzureDevOpsClient azureDevOpsClient,
string buildId,
string manifestFile,
string destinationDirectory,
SemaphoreSlim clientThrottle)
{
try
{
await clientThrottle.WaitAsync();

ArtifactItem fileItem;

ArtifactFiles filesInformation = await azureDevOpsClient.GetArtifactFilesInformation(buildId, artifactName, _retryCount);
var matchingFilePaths = artifactFilesMetadata.Items.Where(f => Path.GetFileName(f.Path) == Path.GetFileName(manifestFile));

foreach (var package in packageFileNames)
if (!matchingFilePaths.Any())
{
await DownloadFileFromArtifact(filesInformation, artifactName, azureDevOpsClient, buildId, package, packagesOutputPath);
throw new ArgumentException($"File {manifestFile} not found in source files.");
}

foreach (var asset in assetFileNames)
if (matchingFilePaths.Count() > 1)
{
await DownloadFileFromArtifact(filesInformation, artifactName, azureDevOpsClient, buildId, asset, assetsOutputPath);
}
}
finally
{
clientThrottle.Release();
}
}

private async Task DownloadFileFromArtifact(ArtifactFiles artifactFilesMetadata, string azureDevOpsArtifact, AzureDevOpsClient azureDevOpsClient, string buildId, string manifestFile, string destinationDirectory)
{
ArtifactItem fileItem;

var matchingFilePaths = artifactFilesMetadata.Items.Where(f => Path.GetFileName(f.Path) == Path.GetFileName(manifestFile));

if (!matchingFilePaths.Any())
{
throw new ArgumentException($"File {manifestFile} not found in source files.");
}

if (matchingFilePaths.Count() > 1)
{
// Picking the first one until https://github.com/dotnet/source-build/issues/4596 is resolved
if (manifestFile.Contains("productVersion.txt"))
{
fileItem = matchingFilePaths.First();
// Picking the first one until https://github.com/dotnet/source-build/issues/4596 is resolved
if (manifestFile.Contains("productVersion.txt"))
{
fileItem = matchingFilePaths.First();
}
else
{
fileItem = matchingFilePaths
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

If it's not redundant, can you add a comment as to its purpose?

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 added a comment, this is for the sdk zip which exists in 2 places in the artifact - for example Windows_x64_Artifacts/assets/Release/dotnet-sdk-*-win-x64.zip and Windows_x64_Artifacts/assets/Release/Sdk/*/dotnet-sdk-*-win-x64.zip but it's listed only once in the vertical manifest
I assume it's getting copied somewhere after the manifest creation but couldn't find where
Comparing the full path solves the issue

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, and my guess is that it's not enough to initially compare with the full path in all cases?

We need to add some tests for this merging code to ensure that the desired selection is being made: dotnet/source-build#4661

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 found some places where Contains doesn't work - both Microsoft.DotNet.Wpf.GitHub.9.0.0-rc.1.24413.1.nupkg and runtime.win-x86.Microsoft.DotNet.Wpf.GitHub.9.0.0-rc.1.24413.1.nupkg contain the same package name
So we'd need to check both that the filename fully matches and compare the path, since we need to check the path only in few cases it made sense to do it this way

.SingleOrDefault(f => f.Path.Contains(manifestFile) || f.Path.Contains(manifestFile.Replace("/", @"\")))
?? throw new ArgumentException($"File {manifestFile} not found in source files.");
}
}
else
{
fileItem = matchingFilePaths.SingleOrDefault(f => f.Path.Contains(manifestFile) || f.Path.Contains(manifestFile.Replace("/", @"\")))
?? throw new ArgumentException($"File {manifestFile} not found in source files.");
fileItem = matchingFilePaths.Single();
}
}
else
{
fileItem = matchingFilePaths.Single();
}

string itemId = fileItem.Blob.Id;
string artifactSubPath = fileItem.Path;
string itemId = fileItem.Blob.Id;
string artifactSubPath = fileItem.Path;

string destinationFilePath = Path.Combine(destinationDirectory, Path.GetFileName(manifestFile));
string destinationFilePath = Path.Combine(destinationDirectory, Path.GetFileName(manifestFile));

try
{
await azureDevOpsClient.DownloadSingleFileFromArtifact(buildId, azureDevOpsArtifact, itemId, artifactSubPath, destinationFilePath, _retryCount);
}
catch (Exception ex)
{
Log.LogError($"Failed to download file {manifestFile} from artifact {azureDevOpsArtifact}: {ex.Message}");
throw;
}
finally
{
clientThrottle.Release();
}
}

/// <summary>
Expand Down Expand Up @@ -314,4 +329,4 @@ private static string GetRequiredRootAttribute(XDocument document, string attrib
}

private record AddedElement(string VerticalName, XElement Element);
}
}