Skip to content

Commit afe8f07

Browse files
michaelnebelmichaelnebel
committed
C#: Re-factor more feed sets into the feed manager (and further use immutable hash sets).
1 parent 07dbdc9 commit afe8f07

3 files changed

Lines changed: 70 additions & 26 deletions

File tree

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,79 @@ internal sealed partial class FeedManager : IDisposable
2424
private readonly FileProvider fileProvider;
2525
private readonly DependabotProxy? dependabotProxy;
2626
private readonly DependencyDirectory emptyPackageDirectory;
27+
private readonly ImmutableHashSet<string> privateRegistryFeeds;
2728

28-
public ImmutableHashSet<string> PrivateRegistryFeeds { get; }
2929
public bool HasPrivateRegistryFeeds { get; }
3030
public bool CheckNugetFeedResponsiveness { get; } = EnvironmentVariables.GetBooleanOptOut(EnvironmentVariableNames.CheckNugetFeedResponsiveness);
3131

3232
private readonly Lazy<ImmutableHashSet<string>> lazyExplicitFeeds;
33+
34+
/// <summary>
35+
/// Gets the list of NuGet feeds that are explicitly configured
36+
/// - NuGet configuration files.
37+
/// - Private package registries that are configured for C#.
38+
/// </summary>
3339
public ImmutableHashSet<string> ExplicitFeeds => lazyExplicitFeeds.Value;
3440

3541
private readonly Lazy<ImmutableHashSet<string>> lazyAllFeeds;
42+
43+
/// <summary>
44+
/// Gets the list of all NuGet feeds that are configured in the environment. That is
45+
/// - Explicit feeds
46+
/// - Inherited feeds from the machine and environment (if not explicitly disabled by a
47+
/// root directory NuGet configuration).
48+
/// </summary>
3649
public ImmutableHashSet<string> AllFeeds => lazyAllFeeds.Value;
3750

51+
/// <summary>
52+
/// Gets the list of inherited NuGet feeds that are configured in the environment.
53+
/// </summary>
54+
public ImmutableHashSet<string> InheritedFeeds => AllFeeds.Except(ExplicitFeeds).ToImmutableHashSet();
55+
56+
private readonly Lazy<(bool, ImmutableHashSet<string>)> lazyReachableExplicitFeeds;
57+
58+
/// <summary>
59+
/// Gets whether there was a timeout when checking the reachability of the explicitly configured NuGet feeds.
60+
/// </summary>
61+
public bool ExplicitFeedTimeout => lazyReachableExplicitFeeds.Value.Item1;
62+
63+
/// <summary>
64+
/// Gets the list of reachable NuGet feeds that are explicitly configured.
65+
/// </summary>
66+
public ImmutableHashSet<string> ReachableExplicitFeeds => lazyReachableExplicitFeeds.Value.Item2;
67+
68+
private readonly Lazy<ImmutableHashSet<string>> lazyReachableFeeds;
69+
/// <summary>
70+
/// Gets the list of reachable NuGet feeds that are configured in the environment.
71+
/// </summary>
72+
public ImmutableHashSet<string> ReachableFeeds => lazyReachableFeeds.Value;
73+
3874
public FeedManager(ILogger logger, IDotNet dotnet, DependabotProxy? dependabotProxy, FileProvider fileProvider)
3975
{
4076
this.logger = logger;
4177
this.dotnet = dotnet;
4278
this.dependabotProxy = dependabotProxy;
4379
this.fileProvider = fileProvider;
44-
PrivateRegistryFeeds = dependabotProxy?.RegistryURLs.ToImmutableHashSet() ?? [];
45-
HasPrivateRegistryFeeds = PrivateRegistryFeeds.Count > 0;
80+
privateRegistryFeeds = dependabotProxy?.RegistryURLs.ToImmutableHashSet() ?? [];
81+
HasPrivateRegistryFeeds = privateRegistryFeeds.Count > 0;
4682
emptyPackageDirectory = new DependencyDirectory("empty", "empty package", logger);
4783

4884
lazyExplicitFeeds = new Lazy<ImmutableHashSet<string>>(GetExplicitFeeds);
4985
lazyAllFeeds = new Lazy<ImmutableHashSet<string>>(GetAllFeeds);
86+
lazyReachableExplicitFeeds = new Lazy<(bool, ImmutableHashSet<string>)>(() =>
87+
{
88+
var timeout = CheckSpecifiedFeeds(ExplicitFeeds, out var reachableFeeds);
89+
return (timeout, reachableFeeds);
90+
});
91+
lazyReachableFeeds = new Lazy<ImmutableHashSet<string>>(() =>
92+
{
93+
// Inherited feeds should only be used, if they are indeed reachable (as they may be environment specific).
94+
CheckSpecifiedFeeds(InheritedFeeds, out var reachableInheritedFeeds);
95+
return ReachableExplicitFeeds.Union(reachableInheritedFeeds).ToImmutableHashSet();
96+
});
5097
}
5198

99+
52100
private string? GetDirectoryName(string path)
53101
{
54102
try
@@ -124,15 +172,15 @@ public string FeedsToRestoreArgument(IEnumerable<string> feeds, string sourceArg
124172
/// <param name="path">Path to project/solution</param>
125173
/// <param name="reachableFeeds">The set of reachable NuGet feeds.</param>
126174
/// <returns>The list of NuGet feeds to use for this restore.</returns>
127-
public IEnumerable<string> FeedsToUse(string path, HashSet<string> reachableFeeds)
175+
public IEnumerable<string> FeedsToUse(string path, ImmutableHashSet<string> reachableFeeds)
128176
{
129177
// Find the path specific feeds.
130178
var folder = GetDirectoryName(path);
131179
var feedsToConsider = folder is not null ? GetFeedsFromFolder(folder).ToHashSet() : new HashSet<string>();
132180

133181
if (HasPrivateRegistryFeeds)
134182
{
135-
feedsToConsider.UnionWith(PrivateRegistryFeeds);
183+
feedsToConsider.UnionWith(privateRegistryFeeds);
136184
}
137185

138186
var feedsToUse = CheckNugetFeedResponsiveness
@@ -150,7 +198,7 @@ public IEnumerable<string> FeedsToUse(string path, HashSet<string> reachableFeed
150198
/// <param name="path">Path to project/solution</param>
151199
/// <param name="reachableFeeds">The set of reachable NuGet feeds.</param>
152200
/// <returns>A string representing the NuGet sources argument for the restore command.</returns>
153-
public string? MakeDotnetRestoreSourcesArgument(string path, HashSet<string> reachableFeeds)
201+
public string? MakeDotnetRestoreSourcesArgument(string path, ImmutableHashSet<string> reachableFeeds)
154202
{
155203
// Do not construct a set of explicit NuGet sources to use for restore.
156204
if (!CheckNugetFeedResponsiveness && !HasPrivateRegistryFeeds)
@@ -280,7 +328,7 @@ private HashSet<string> GetExcludedFeeds()
280328
/// True if there is a timeout when trying to reach the feeds (excluding any feeds that are configured
281329
/// to be excluded from the check) or false otherwise.
282330
/// </returns>
283-
public bool CheckSpecifiedFeeds(ImmutableHashSet<string> feeds, out HashSet<string> reachableFeeds)
331+
private bool CheckSpecifiedFeeds(ImmutableHashSet<string> feeds, out ImmutableHashSet<string> reachableFeeds)
284332
{
285333
// Exclude any feeds from the feed check that are configured by the corresponding environment variable.
286334
// These feeds are always assumed to be reachable.
@@ -296,10 +344,10 @@ public bool CheckSpecifiedFeeds(ImmutableHashSet<string> feeds, out HashSet<stri
296344
return true;
297345
}).ToHashSet();
298346

299-
reachableFeeds = GetReachableNuGetFeeds(feedsToCheck, isFallback: false, out var isTimeout).ToHashSet();
347+
var reachable = GetReachableNuGetFeeds(feedsToCheck, isFallback: false, out var isTimeout);
300348

301349
// Always consider feeds excluded for the reachability check as reachable.
302-
reachableFeeds.UnionWith(feeds.Where(feed => excludedFeeds.Contains(feed)));
350+
reachableFeeds = reachable.Union(feeds.Where(feed => excludedFeeds.Contains(feed))).ToImmutableHashSet();
303351

304352
return isTimeout;
305353
}
@@ -396,8 +444,8 @@ private ImmutableHashSet<string> GetExplicitFeeds()
396444
// in addition to the ones that are configured in `nuget.config` files.
397445
if (HasPrivateRegistryFeeds)
398446
{
399-
logger.LogInfo($"Found {PrivateRegistryFeeds.Count} private registry feeds configured for C#: {string.Join(", ", PrivateRegistryFeeds.OrderBy(f => f))}");
400-
explicitFeeds.UnionWith(PrivateRegistryFeeds);
447+
logger.LogInfo($"Found {privateRegistryFeeds.Count} private registry feeds configured for C#: {string.Join(", ", privateRegistryFeeds.OrderBy(f => f))}");
448+
explicitFeeds.UnionWith(privateRegistryFeeds);
401449
}
402450

403451
return explicitFeeds.ToImmutableHashSet();

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public HashSet<AssemblyLookupLocation> Restore()
110110
logger.LogInfo($"Checking NuGet feed responsiveness: {feedManager.CheckNugetFeedResponsiveness}");
111111
compilationInfoContainer.CompilationInfos.Add(("NuGet feed responsiveness checked", feedManager.CheckNugetFeedResponsiveness ? "1" : "0"));
112112

113-
HashSet<string> reachableFeeds = [];
113+
ImmutableHashSet<string> reachableFeeds = [];
114114

115115
EmitNugetConfigDiagnostics();
116116

@@ -122,20 +122,17 @@ public HashSet<AssemblyLookupLocation> Restore()
122122

123123
if (feedManager.CheckNugetFeedResponsiveness)
124124
{
125-
var inheritedFeeds = allFeeds.Except(explicitFeeds).ToImmutableHashSet();
125+
var inheritedFeeds = feedManager.InheritedFeeds;
126126

127127
if (inheritedFeeds.Count > 0)
128128
{
129129
compilationInfoContainer.CompilationInfos.Add(("Inherited NuGet feed count", inheritedFeeds.Count.ToString()));
130130
}
131131

132-
var timeout = feedManager.CheckSpecifiedFeeds(explicitFeeds, out var reachableExplicitFeeds);
133-
reachableFeeds.UnionWith(reachableExplicitFeeds);
134-
135-
var allExplicitReachable = explicitFeeds.Count == reachableExplicitFeeds.Count;
132+
var allExplicitReachable = explicitFeeds.Count == feedManager.ReachableExplicitFeeds.Count;
136133
EmitUnreachableFeedsDiagnostics(allExplicitReachable);
137134

138-
if (timeout)
135+
if (feedManager.ExplicitFeedTimeout)
139136
{
140137
// If we experience a timeout, we use this fallback.
141138
// todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too.
@@ -145,9 +142,7 @@ public HashSet<AssemblyLookupLocation> Restore()
145142
: [unresponsiveMissingPackageLocation];
146143
}
147144

148-
// Inherited feeds should only be used, if they are indeed reachable (as they may be environment specific).
149-
feedManager.CheckSpecifiedFeeds(inheritedFeeds, out var reachableInheritedFeeds);
150-
reachableFeeds.UnionWith(reachableInheritedFeeds);
145+
reachableFeeds = feedManager.ReachableFeeds;
151146
}
152147

153148
try
@@ -224,7 +219,7 @@ public HashSet<AssemblyLookupLocation> Restore()
224219
/// Populates dependencies with the relevant dependencies from the assets files generated by the restore.
225220
/// Returns a list of projects that are up to date with respect to restore.
226221
/// </summary>
227-
private IEnumerable<string> RestoreSolutions(HashSet<string> reachableFeeds, out DependencyContainer dependencies)
222+
private IEnumerable<string> RestoreSolutions(ImmutableHashSet<string> reachableFeeds, out DependencyContainer dependencies)
228223
{
229224
var successCount = 0;
230225
var nugetSourceFailures = 0;
@@ -269,7 +264,7 @@ private IEnumerable<string> RestoreSolutions(HashSet<string> reachableFeeds, out
269264
/// </summary>
270265
/// <param name="projects">A list of paths to project files.</param>
271266
/// <param name="reachableFeeds">The set of reachable NuGet feeds.</param>
272-
private void RestoreProjects(IEnumerable<string> projects, HashSet<string> reachableFeeds, out ConcurrentBag<DependencyContainer> dependencies)
267+
private void RestoreProjects(IEnumerable<string> projects, ImmutableHashSet<string> reachableFeeds, out ConcurrentBag<DependencyContainer> dependencies)
273268
{
274269
var successCount = 0;
275270
var nugetSourceFailures = 0;

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/PackagesConfigRestorer.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Collections.Immutable;
34
using System.Diagnostics;
45
using System.IO;
56
using System.Linq;
@@ -33,7 +34,7 @@ internal interface IPackagesConfigRestore
3334
/// </summary>
3435
internal class PackagesConfigRestoreFactory
3536
{
36-
public static IPackagesConfigRestore Create(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, HashSet<string> reachableFeeds)
37+
public static IPackagesConfigRestore Create(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, ImmutableHashSet<string> reachableFeeds)
3738
{
3839
if (SystemBuildActions.Instance.IsWindows() || SystemBuildActions.Instance.IsMonoInstalled())
3940
{
@@ -64,7 +65,7 @@ private class NugetExeWrapper : IPackagesConfigRestore
6465
/// </summary>
6566
private readonly DependencyDirectory packageDirectory;
6667
private readonly FeedManager feedManager;
67-
private readonly HashSet<string> reachableFeeds;
68+
private readonly ImmutableHashSet<string> reachableFeeds;
6869

6970
private bool IsWindows => SystemBuildActions.Instance.IsWindows();
7071

@@ -77,7 +78,7 @@ private class NugetExeWrapper : IPackagesConfigRestore
7778
/// <summary>
7879
/// Create the package manager for a specified source tree.
7980
/// </summary>
80-
public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, HashSet<string> reachableFeeds)
81+
public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, FeedManager feedManager, ImmutableHashSet<string> reachableFeeds)
8182
{
8283
this.fileProvider = fileProvider;
8384
this.packageDirectory = packageDirectory;

0 commit comments

Comments
 (0)