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

Add local package patterns to source-mappings for online feeds #44076

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

NikolaMilosavljevic
Copy link
Member

Fixes: dotnet/source-build#4651

This was a bug in original implementation (ddfd74a), that was not noticed in source-build, due to limited number of repos using package source mappings. Code had a correct comment describing the behavior, and now the implementation matches that intent.

In online builds, if a repo has package source-mappings, it could cause issues in unified build due to the way we update NuGet.config to add package source mappings for all feeds. All packages built locally should also be added as package-source mapping patterns to online feeds, so any package with a different version can be resolved from those online feeds.

This code is complex. It was added to original, old, codebase with ddfd74a. Due to limited time available for that feature, new code was implemented in a way that followed the original design. This is not easy to maintain and should be refactored - I've created the issue to track that work: dotnet/source-build#4666

@NikolaMilosavljevic NikolaMilosavljevic requested review from a team as code owners October 10, 2024 18:52
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Oct 10, 2024
@mmitche
Copy link
Member

mmitche commented Oct 10, 2024

We can combine this with the windowsdesktop update for further verification if you want.

@NikolaMilosavljevic
Copy link
Member Author

We can combine this with the windowsdesktop update for further verification if you want.

That PR will hit the same issues as this one, with nuget source auditing. We can certainly cherry-pick this change into that PR, which would make this one obsolete and we'd close it. Source auditing issue will be fixed with #44078

Copy link
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

Please prioritize adding unit tests for this logic after unblocking the dependency flows.

@NikolaMilosavljevic
Copy link
Member Author

Investigating a unified build failure.

@NikolaMilosavljevic
Copy link
Member Author

Pushed a fix for custom sources. Original change in this PR regressed the content of package source mappings for custom feeds. We were missing all of the original package patterns that were present in original NuGet.config file. Since custom sources are online and we aren't enumerating those, we can only add all enumerated local packages and all patterns present in original nuget.config.

The issue with unified build was related to this. Custom feed, named net-sdk-supporting-feed points at the same feed as dotnet9. While mappings for dotnet9 feed were correct, i.e. included microsoft.*, mappings for the custom feed were incorrect, and only included enumerated local packages.

Nuget code supposedly discovers feed mappings in order. Custom feed is last in the config file, so it seems that nuget replaces all patterns previously discovered from dotnet9 mappings section - as both mappings actually point at the same feed.

@NikolaMilosavljevic NikolaMilosavljevic merged commit c9b7a34 into dotnet:main Oct 11, 2024
36 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.

Package source mapping causes problems with VMR restores
4 participants