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

Compare tar.gz files between VMR and current .NET builds #47667

Merged
merged 2 commits into from
Mar 18, 2025

Conversation

mmitche
Copy link
Member

@mmitche mmitche commented Mar 17, 2025

Adds tar.gz comparison, including assembly version comparison. Because our archives are basically always in the same order between base and diff, we avoid a ton of extra very expensive disk write time by finding the common files between base and diff, then reading the base and diff simultaneously, keeping a map of open streams. Every time we read a new common entry, we attempt to compare assembly versions and the remove the streams. This is drastically faster than extracting.

Also fixes a bug where version numbers weren't being removed from file paths within nupkgs.

Adds tar.gz comparison, including assembly version comparison.
Because our archives are basically always in the same order between base and diff, we avoid a ton of extra very expensive disk write time by finding the common files between base and diff, then reading the base and diff simultaneously, keeping a map of open streams. Every time we read a new common entry, we attempt to compare assembly versions and the remove the streams. This is drastically faster than extracting.
@Copilot Copilot bot review requested due to automatic review settings March 17, 2025 22:58
@mmitche mmitche requested review from a team as code owners March 17, 2025 22:58
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Mar 17, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to compare tar.gz archives (in addition to zip archives) between VMR and current .NET builds by comparing both file lists and assembly versions, while also refactoring some common routines.

  • Introduces new methods to list tar.gz archive files and compare their assembly versions asynchronously.
  • Updates copy stream methods to asynchronous versions and reduces the default parallelism from 16 to 8.
  • Refactors file version normalization to support comparisons across different archive formats.

}

/// <summary>
/// This method is called "USE ALL AVAILABLE MEMORY"
Copy link
Member

Choose a reason for hiding this comment

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

😄

private static string RemoveVersionsNormalized(string path)
{
string strippedPath = path.Replace("\\", "//");
string prevPath = path;
Copy link
Member

Choose a reason for hiding this comment

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

nit: this is overwritten on the next line anyway so initializing to path is unnecessary

@mmitche
Copy link
Member Author

mmitche commented Mar 18, 2025

/azp list

Copy link

CI/CD Pipelines for this repository:

@mmitche
Copy link
Member Author

mmitche commented Mar 18, 2025

/azp run sdk-unified-build

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

/// <summary>
/// Compare the file lists of packages, identifying missing and extra files.
/// </summary>
/// <param name="mapping"></param>
Copy link
Member

Choose a reason for hiding this comment

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

Nit, what value do empty doc tags provide? It bloats the code and erodes the value of the actual documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

None at all...will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mind if I fix this in the next iteration? There are some more tests to add to this.

Copy link
Member

Choose a reason for hiding this comment

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

That's fine with me.

@mmitche mmitche merged commit 4e5b509 into dotnet:main Mar 18, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants