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.

Sign up for GitHub

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

C#: Improve log messages #16321

Merged
merged 2 commits into from
Apr 25, 2024
Merged

C#: Improve log messages #16321

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15c1fd9
C#: Improve log messages
tamasvajk Apr 25, 2024
dd9183c
Code quality improvements
tamasvajk Apr 25, 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
1 change: 1 addition & 0 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ void exitCallback(int ret, string msg, bool silent)

private HashSet<string> AddFrameworkDlls(HashSet<AssemblyLookupLocation> dllLocations)
{
logger.LogInfo("Adding .NET Framework DLLs");
var frameworkLocations = new HashSet<string>();

var frameworkReferences = Environment.GetEnvironmentVariable(EnvironmentVariableNames.DotnetFrameworkReferences);
Expand Down
4 changes: 2 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ public bool AddPackage(string folder, string package)
return dotnetCliInvoker.RunCommand(args);
}

public IList<string> GetListedRuntimes() => GetResultList("--list-runtimes", null, false);
public IList<string> GetListedRuntimes() => GetResultList("--list-runtimes", null, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: No need to supplu null and true as these are the defaults.


public IList<string> GetListedSdks() => GetResultList("--list-sdks", null, false);
public IList<string> GetListedSdks() => GetResultList("--list-sdks", null, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Same as above.


private IList<string> GetResultList(string args, string? workingDirectory = null, bool silent = true)
{
Expand Down
5 changes: 3 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public DotNetCliInvoker(ILogger logger, string exec)
{
this.logger = logger;
this.Exec = exec;
logger.LogInfo($"Using .NET CLI executable: '{Exec}'");
}

private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirectory)
Expand All @@ -43,15 +44,15 @@ private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirecto
private bool RunCommandAux(string args, string? workingDirectory, out IList<string> output, bool silent)
{
var dirLog = string.IsNullOrWhiteSpace(workingDirectory) ? "" : $" in {workingDirectory}";
logger.LogInfo($"Running {Exec} {args}{dirLog}");
logger.LogInfo($"Running '{Exec} {args}'{dirLog}");
var pi = MakeDotnetStartInfo(args, workingDirectory);
var threadId = Environment.CurrentManagedThreadId;
void onOut(string s) => logger.Log(silent ? Severity.Debug : Severity.Info, s, threadId);
void onError(string s) => logger.LogError(s, threadId);
var exitCode = pi.ReadOutput(out output, onOut, onError);
if (exitCode != 0)
{
logger.LogError($"Command {Exec} {args}{dirLog} failed with exit code {exitCode}");
logger.LogError($"Command '{Exec} {args}'{dirLog} failed with exit code {exitCode}");
return false;
}
return true;
Expand Down
29 changes: 19 additions & 10 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class FileProvider
private readonly Lazy<string[]> dlls;
private readonly Lazy<string[]> nugetConfigs;
private readonly Lazy<string[]> globalJsons;
private readonly Lazy<string[]> packagesConfigs;
private readonly Lazy<string[]> razorViews;
private readonly Lazy<string[]> resources;
private readonly Lazy<string?> rootNugetConfig;
Expand All @@ -32,31 +33,38 @@ public FileProvider(DirectoryInfo sourceDir, ILogger logger)

all = GetAllFiles();
allNonBinary = new Lazy<FileInfo[]>(() => all.Where(f => !binaryFileExtensions.Contains(f.Extension.ToLowerInvariant())).ToArray());
smallNonBinary = new Lazy<string[]>(() =>
{
var ret = SelectSmallFiles(allNonBinary.Value).SelectFileNames().ToArray();
logger.LogInfo($"Found {ret.Length} small non-binary files in {SourceDir}.");
return ret;
});
smallNonBinary = new Lazy<string[]>(() => ReturnAndLogFiles("small non-binary", SelectSmallFiles(allNonBinary.Value).SelectFileNames().ToArray()));
sources = new Lazy<string[]>(() => SelectTextFileNamesByExtension("source", ".cs"));
projects = new Lazy<string[]>(() => SelectTextFileNamesByExtension("project", ".csproj"));
solutions = new Lazy<string[]>(() => SelectTextFileNamesByExtension("solution", ".sln"));
dlls = new Lazy<string[]>(() => SelectBinaryFileNamesByExtension("DLL", ".dll"));
nugetConfigs = new Lazy<string[]>(() => allNonBinary.Value.SelectFileNamesByName("nuget.config").ToArray());
globalJsons = new Lazy<string[]>(() => allNonBinary.Value.SelectFileNamesByName("global.json").ToArray());
nugetConfigs = new Lazy<string[]>(() => SelectTextFileNamesByName("nuget.config"));
globalJsons = new Lazy<string[]>(() => SelectTextFileNamesByName("global.json"));
packagesConfigs = new Lazy<string[]>(() => SelectTextFileNamesByName("packages.config"));
razorViews = new Lazy<string[]>(() => SelectTextFileNamesByExtension("razor view", ".cshtml", ".razor"));
resources = new Lazy<string[]>(() => SelectTextFileNamesByExtension("resource", ".resx"));

rootNugetConfig = new Lazy<string?>(() => all.SelectRootFiles(SourceDir).SelectFileNamesByName("nuget.config").FirstOrDefault());
}

private string[] SelectTextFileNamesByExtension(string filetype, params string[] extensions)
private string[] ReturnAndLogFiles(string filetype, IEnumerable<string> files)
{
var ret = allNonBinary.Value.SelectFileNamesByExtension(extensions).ToArray();
var ret = files.ToArray();
logger.LogInfo($"Found {ret.Length} {filetype} files in {SourceDir}.");
return ret;
}

private string[] SelectTextFileNamesByExtension(string filetype, params string[] extensions)
=> ReturnAndLogFiles(filetype, allNonBinary.Value.SelectFileNamesByExtension(extensions));

private string[] SelectTextFileNamesByName(string name)
{
var ret = allNonBinary.Value.SelectFileNamesByName(name).ToArray();
var ending = ret.Length == 0 ? "." : $": {string.Join(", ", ret.OrderBy(s => s))}.";
logger.LogInfo($"Found {ret.Length} {name} files in {SourceDir}{ending}");
return ret;
}

private string[] SelectBinaryFileNamesByExtension(string filetype, params string[] extensions)
{
var ret = all.SelectFileNamesByExtension(extensions).ToArray();
Expand Down Expand Up @@ -117,6 +125,7 @@ private FileInfo[] GetAllFiles()
public ICollection<string> NugetConfigs => nugetConfigs.Value;
public string? RootNugetConfig => rootNugetConfig.Value;
public IEnumerable<string> GlobalJsons => globalJsons.Value;
public ICollection<string> PackagesConfigs => packagesConfigs.Value;
public ICollection<string> RazorViews => razorViews.Value;
public ICollection<string> Resources => resources.Value;
}
Expand Down
24 changes: 9 additions & 15 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ internal class NugetExeWrapper : IDisposable
/// <summary>
/// The list of package files.
/// </summary>
private readonly FileInfo[] packageFiles;
private readonly ICollection<string> packageFiles;

public int PackageCount => packageFiles.Length;
public int PackageCount => packageFiles.Count;

private readonly string? backupNugetConfig;
private readonly string? nugetConfigPath;
Expand All @@ -37,23 +37,21 @@ internal class NugetExeWrapper : IDisposable
/// <summary>
/// Create the package manager for a specified source tree.
/// </summary>
public NugetExeWrapper(string sourceDir, TemporaryDirectory packageDirectory, Util.Logging.ILogger logger)
public NugetExeWrapper(FileProvider fileProvider, TemporaryDirectory packageDirectory, Util.Logging.ILogger logger)
{
this.packageDirectory = packageDirectory;
this.logger = logger;

packageFiles = new DirectoryInfo(sourceDir)
.EnumerateFiles("packages.config", SearchOption.AllDirectories)
.ToArray();
packageFiles = fileProvider.PackagesConfigs;

if (packageFiles.Length > 0)
if (packageFiles.Count > 0)
{
logger.LogInfo($"Found {packageFiles.Length} packages.config files, trying to use nuget.exe for package restore");
nugetExe = ResolveNugetExe(sourceDir);
logger.LogInfo($"Found packages.config files, trying to use nuget.exe for package restore");
nugetExe = ResolveNugetExe(fileProvider.SourceDir.FullName);
if (HasNoPackageSource())
{
// We only modify or add a top level nuget.config file
nugetConfigPath = Path.Combine(sourceDir, "nuget.config");
nugetConfigPath = Path.Combine(fileProvider.SourceDir.FullName, "nuget.config");
try
{
if (File.Exists(nugetConfigPath))
Expand Down Expand Up @@ -86,10 +84,6 @@ public NugetExeWrapper(string sourceDir, TemporaryDirectory packageDirectory, Ut
}
}
}
else
{
logger.LogInfo("Found no packages.config file");
}
}

/// <summary>
Expand Down Expand Up @@ -195,7 +189,7 @@ private bool TryRestoreNugetPackage(string package)
/// </summary>
public int InstallPackages()
{
return packageFiles.Count(package => TryRestoreNugetPackage(package.FullName));
return packageFiles.Count(package => TryRestoreNugetPackage(package));
}

private bool HasNoPackageSource()
Expand Down
4 changes: 2 additions & 2 deletions csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public HashSet<AssemblyLookupLocation> Restore()
: [unresponsiveMissingPackageLocation];
}

using (var nuget = new NugetExeWrapper(fileProvider.SourceDir.FullName, legacyPackageDirectory, logger))
using (var nuget = new NugetExeWrapper(fileProvider, legacyPackageDirectory, logger))
{
var count = nuget.InstallPackages();

Expand Down Expand Up @@ -178,7 +178,7 @@ private List<string> GetReachableFallbackNugetFeeds()
logger.LogInfo($"No fallback Nuget feeds specified. Using default feed: {PublicNugetOrgFeed}");
}

logger.LogInfo($"Checking fallback Nuget feed reachability on feeds: {string.Join(", ", fallbackFeeds.OrderBy(f => f))}");
logger.LogInfo($"Checking fallback Nuget feed reachability on feeds: {string.Join(", ", fallbackFeeds.OrderBy(f => f))}");
var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: true);
var reachableFallbackFeeds = fallbackFeeds.Where(feed => IsFeedReachable(feed, initialTimeout, tryCount, allowExceptions: false)).ToList();
if (reachableFallbackFeeds.Count == 0)
Expand Down
9 changes: 6 additions & 3 deletions csharp/extractor/Semmle.Extraction.CSharp.Standalone/Extractor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,20 @@ public void Started(int item, int total, string source)

public void MissingType(string type)
{
logger.Log(Severity.Debug, "Missing type {0}", type);
logger.LogDebug($"Missing type {type}");
}

public void MissingNamespace(string @namespace)
{
logger.Log(Severity.Info, "Missing namespace {0}", @namespace);
logger.LogInfo($"Missing namespace {@namespace}");
}

public void MissingSummary(int missingTypes, int missingNamespaces)
{
logger.Log(Severity.Info, "Failed to resolve {0} types in {1} namespaces", missingTypes, missingNamespaces);
if (missingTypes > 0 || missingNamespaces > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

{
logger.LogInfo($"Failed to resolve {missingTypes} types in {missingNamespaces} namespaces");
}
}
}

Expand Down
Loading