Skip to content

Conversation

@mbg
Copy link
Member

@mbg mbg commented Jan 29, 2026

This (sort of) fixes a (sort of) bug in the computation of allFeeds. We assume that allFeeds includes explicitFeeds and that is generally true, except for one case. On systems with case-sensitive file systems, we may discover a configuration file such as Nuget.Config, which is not recognised by dotnet nuget list source (which e.g. only recognises NuGet.config and nuget.config). In that case, our call to GetNugetFeeds will retrieve the feeds from that file (because it is accepted when provided explicitly as --configfile argument), but the call to GetNugetFeedsFromFolder will not.

Arguably, another option here would be to make our configuration file discovery more strict and only recognise the same case-sensitive filenames as dotnet nuget list source. However, that may catch out customers by surprise who normally work on case-insensitive systems, but who use Default Setup.

Note that this doesn't address scenarios where GetNugetFeedsFromFolder misses feeds from parent directories because they have names that dotnet nuget list source doesn't recognise.

@mbg mbg self-assigned this Jan 29, 2026
Copilot AI review requested due to automatic review settings January 29, 2026 21:48
@mbg mbg requested a review from a team as a code owner January 29, 2026 21:48
@github-actions github-actions bot added the C# label Jan 29, 2026
Copy link
Contributor

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 pull request fixes a bug in the computation of allFeeds in the NuGet package restoration logic. The PR ensures that allFeeds always includes explicitFeeds, which is normally the case except when configuration files with non-standard casing (like Nuget.Config instead of nuget.config) are encountered on case-sensitive file systems.

Changes:

  • Added logic to explicitly union explicitFeeds into allFeeds to handle the edge case where case-sensitive filesystems cause dotnet nuget list source to miss configuration files that are otherwise valid
  • Added comprehensive comments explaining the rationale and the specific edge case being addressed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

That is some excellent 🕵️ work!!!

@michaelnebel
Copy link
Contributor

We should also run DCA.

@mbg mbg force-pushed the mbg/csharp/fix-registry-feeds branch from 8857553 to 1aba0b2 Compare January 30, 2026 13:20
@mbg mbg requested review from a team as code owners January 30, 2026 13:20
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

Fantastic!
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants