From 69f500fe0ddb26ed95acc3c06c4465b590e05550 Mon Sep 17 00:00:00 2001 From: JoC0de <53140583+JoC0de@users.noreply.github.com> Date: Sat, 4 Jan 2025 00:46:07 +0100 Subject: [PATCH] Add an option to not strip out PDB symbols from nutget feeds (#689) --- .editorconfig | 3 ++ .../Assets/Tests/Editor/NuGetTests.cs | 37 +++++++++++++++++++ .../Editor/Configuration/NugetConfigFile.cs | 36 +++++++++++++++--- .../Editor/Helper/PortableSymbolFileHelper.cs | 25 +++++++++++++ .../Helper/PortableSymbolFileHelper.cs.meta | 11 ++++++ .../Editor/InstalledPackagesManager.cs | 2 +- .../Editor/NugetPackageInstaller.cs | 4 +- .../Editor/PackageContentManager.cs | 18 +++++++-- .../PackageSource/NugetPackageSourceV2.cs | 14 ++----- .../PackageSource/NugetPackageSourceV3.cs | 22 +++++------ .../Editor/Ui/NugetPreferences.cs | 19 +++++++++- 11 files changed, 157 insertions(+), 34 deletions(-) create mode 100644 src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs create mode 100644 src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs.meta diff --git a/.editorconfig b/.editorconfig index 3fde7d04..d7456d7b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -149,6 +149,9 @@ dotnet_diagnostic.CA2234.severity=none dotnet_diagnostic.CA2007.severity=none dotnet_diagnostic.CA1056.severity=none # URI-like properties should not be strings dotnet_diagnostic.CA1054.severity=none # URI parameters should not be strings +dotnet_diagnostic.CA2249.severity=none # Cant use String.Contains as it doesn't support StringComparison type on .net framework +dotnet_diagnostic.CA1307.severity=none # Cant use String.Replace with StringComparison type on .net framework +dotnet_diagnostic.CA1865.severity=none # Cant use String.StartsWith with char on .net framework # ReSharper properties resharper_wrap_after_declaration_lpar=true diff --git a/src/NuGetForUnity.Tests/Assets/Tests/Editor/NuGetTests.cs b/src/NuGetForUnity.Tests/Assets/Tests/Editor/NuGetTests.cs index 58af74ee..cfc35155 100644 --- a/src/NuGetForUnity.Tests/Assets/Tests/Editor/NuGetTests.cs +++ b/src/NuGetForUnity.Tests/Assets/Tests/Editor/NuGetTests.cs @@ -15,6 +15,7 @@ using NUnit.Framework; using UnityEditor; using UnityEngine; +using UnityEngine.TestTools; public class NuGetTests { @@ -1129,6 +1130,42 @@ public void NativeSettingsTest(string runtime, BuildTarget buildTarget) $"Native mapping for {runtime} is missing build target {buildTarget}"); } + [Test] + public void KeepPdbFileTest() + { + ConfigureNugetConfig(InstallMode.ApiV3Only); + var nugetConfigFile = ConfigurationManager.NugetConfigFile; + nugetConfigFile.KeepingPdbFiles = true; + LogAssert.ignoreFailingMessages = true; + + try + { + var package = new NugetPackageIdentifier("Fody", "6.8.2") { IsManuallyInstalled = true }; + Assume.That( + InstalledPackagesManager.IsInstalled(package, false), + Is.False, + "The package IS installed: {0} {1}", + package.Id, + package.Version); + + NugetPackageInstaller.InstallIdentifier(package); + Assert.IsTrue( + InstalledPackagesManager.IsInstalled(package, false), + "The package was NOT installed: {0} {1}", + package.Id, + package.Version); + + var packageDirectory = Path.Combine(ConfigurationManager.NugetConfigFile.RepositoryPath, $"{package.Id}.{package.Version}"); + + Assert.That(Path.Combine(packageDirectory, "netclassictask", "Mono.Cecil.pdb"), Does.Exist); + } + finally + { + LogAssert.ignoreFailingMessages = false; + nugetConfigFile.KeepingPdbFiles = false; + } + } + private static void ConfigureNugetConfig(InstallMode installMode) { var nugetConfigFile = ConfigurationManager.NugetConfigFile; diff --git a/src/NuGetForUnity/Editor/Configuration/NugetConfigFile.cs b/src/NuGetForUnity/Editor/Configuration/NugetConfigFile.cs index 9b3cfc58..9ed55224 100644 --- a/src/NuGetForUnity/Editor/Configuration/NugetConfigFile.cs +++ b/src/NuGetForUnity/Editor/Configuration/NugetConfigFile.cs @@ -1,5 +1,7 @@ #pragma warning disable SA1512,SA1124 // Single-line comments should not be followed by blank line +#region No ReShaper + using System; using System.Collections.Generic; using System.Globalization; @@ -12,8 +14,6 @@ using NugetForUnity.PackageSource; using UnityEngine; -#region No ReShaper - // ReSharper disable All // needed because 'JetBrains.Annotations.NotNull' and 'System.Diagnostics.CodeAnalysis.NotNull' collide if this file is compiled with a never version of Unity / C# using SuppressMessageAttribute = System.Diagnostics.CodeAnalysis.SuppressMessageAttribute; @@ -54,6 +54,8 @@ public class NugetConfigFile private const string PreferNetStandardOverNetFrameworkConfigKey = "PreferNetStandardOverNetFramework"; + private const string KeepingPdbFilesConfigKey = "KeepingPdbFiles"; + private const string ProtocolVersionAttributeName = "protocolVersion"; private const string PasswordAttributeName = "password"; @@ -189,10 +191,15 @@ public string RelativePackagesConfigDirectoryPath public int RequestTimeoutSeconds { get; set; } = DefaultRequestTimeout; /// - /// Gets or sets the value indicating whether .NET Standard is preferred over .NET Framework as the TargetFramework. + /// Gets or sets a value indicating whether .NET Standard is preferred over .NET Framework as the TargetFramework. /// public bool PreferNetStandardOverNetFramework { get; set; } + /// + /// Gets or sets a value indicating whether PDB files included in NuGet packages should not be deleted if they can be read by Unity. + /// + internal bool KeepingPdbFiles { get; set; } + /// /// Gets the value that tells the system how to determine where the packages are to be installed and configurations are to be stored. /// @@ -209,6 +216,10 @@ public string RelativePackagesConfigDirectoryPath /// The full file-path to the NuGet.config file to load. /// The newly loaded . [NotNull] + [SuppressMessage( + "Usage", + "CA2263:Prefer generic overload when type is known", + Justification = "When targeting .net-framework there is no generic overload.")] public static NugetConfigFile Load([NotNull] string filePath) { var configFile = new NugetConfigFile @@ -238,7 +249,10 @@ public static NugetConfigFile Load([NotNull] string filePath) var updateSearchBatchSizeString = add.Attribute(UpdateSearchBatchSizeAttributeName)?.Value; if (!string.IsNullOrEmpty(updateSearchBatchSizeString)) { - sourceV3.UpdateSearchBatchSize = Mathf.Clamp(int.Parse(updateSearchBatchSizeString), 1, int.MaxValue); + sourceV3.UpdateSearchBatchSize = Mathf.Clamp( + int.Parse(updateSearchBatchSizeString, CultureInfo.InvariantCulture), + 1, + int.MaxValue); } var supportsPackageIdSearchFilterString = add.Attribute(SupportsPackageIdSearchFilterAttributeName)?.Value; @@ -318,7 +332,7 @@ public static NugetConfigFile Load([NotNull] string filePath) if (string.Equals(key, "packageInstallLocation", StringComparison.OrdinalIgnoreCase)) { - configFile.InstallLocation = (PackageInstallLocation)Enum.Parse(typeof(PackageInstallLocation), value); + configFile.InstallLocation = (PackageInstallLocation)Enum.Parse(typeof(PackageInstallLocation), value, true); } else if (string.Equals(key, "repositoryPath", StringComparison.OrdinalIgnoreCase)) { @@ -356,6 +370,10 @@ public static NugetConfigFile Load([NotNull] string filePath) { configFile.PreferNetStandardOverNetFramework = bool.Parse(value); } + else if (string.Equals(key, KeepingPdbFilesConfigKey, StringComparison.OrdinalIgnoreCase)) + { + configFile.KeepingPdbFiles = bool.Parse(value); + } } return configFile; @@ -547,6 +565,14 @@ public void Save([NotNull] string filePath) config.Add(addElement); } + if (KeepingPdbFiles) + { + addElement = new XElement("add"); + addElement.Add(new XAttribute("key", KeepingPdbFilesConfigKey)); + addElement.Add(new XAttribute("value", KeepingPdbFiles.ToString().ToLowerInvariant())); + config.Add(addElement); + } + var configuration = new XElement("configuration"); configuration.Add(packageSources); configuration.Add(disabledPackageSources); diff --git a/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs b/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs new file mode 100644 index 00000000..1804cb36 --- /dev/null +++ b/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs @@ -0,0 +1,25 @@ +using System.IO; + +namespace NugetForUnity.Helper +{ + /// + /// Provides methods for working with symbol files. + /// + internal static class PortableSymbolFileHelper + { + /// + /// Determines whether the specified PDB file is a portable symbol file. + /// + /// The path to the PDB file. + /// + /// true if the specified PDB file is a portable symbol file; otherwise, false. + /// + public static bool IsPortableSymbolFile(string filePath) + { + using (var stream = File.OpenRead(filePath)) + { + return stream.ReadByte() == 'B' && stream.ReadByte() == 'S' && stream.ReadByte() == 'J' && stream.ReadByte() == 'B'; + } + } + } +} diff --git a/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs.meta b/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs.meta new file mode 100644 index 00000000..c390b043 --- /dev/null +++ b/src/NuGetForUnity/Editor/Helper/PortableSymbolFileHelper.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6a82531c47c548d47b2823a530fd58be +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/src/NuGetForUnity/Editor/InstalledPackagesManager.cs b/src/NuGetForUnity/Editor/InstalledPackagesManager.cs index 866c46e1..565efe03 100644 --- a/src/NuGetForUnity/Editor/InstalledPackagesManager.cs +++ b/src/NuGetForUnity/Editor/InstalledPackagesManager.cs @@ -362,7 +362,7 @@ internal static List GetInstalledRootPackages() return roots; } - private static void AddPackageToInstalledInternal([NotNull] INugetPackage package, ref int manuallyInstalledPackagesNumber) + private static void AddPackageToInstalledInternal([NotNull] NugetPackageLocal package, ref int manuallyInstalledPackagesNumber) { var packages = InstalledPackagesDictionary; if (!packages.ContainsKey(package.Id)) diff --git a/src/NuGetForUnity/Editor/NugetPackageInstaller.cs b/src/NuGetForUnity/Editor/NugetPackageInstaller.cs index 0345e337..25b07576 100644 --- a/src/NuGetForUnity/Editor/NugetPackageInstaller.cs +++ b/src/NuGetForUnity/Editor/NugetPackageInstaller.cs @@ -449,7 +449,7 @@ private static void WriteInitialExcludeAllPluginImporterConfig([NotNull] string } private static void TryExtractBestFrameworkSources( - [NotNull] [ItemNotNull] IReadOnlyDictionary> frameworks, + [NotNull] [ItemNotNull] Dictionary> frameworks, [NotNull] string sourceDirName, [NotNull] INugetPackage package, [NotNull] PackageConfig packageConfig) @@ -477,7 +477,7 @@ private static void TryExtractBestFrameworkSources( } private static void FillFrameworkZipEntries( - IDictionary> frameworkZipEntries, + Dictionary> frameworkZipEntries, string framework, ZipArchiveEntry entry) { diff --git a/src/NuGetForUnity/Editor/PackageContentManager.cs b/src/NuGetForUnity/Editor/PackageContentManager.cs index 97b527c2..c59e99f4 100644 --- a/src/NuGetForUnity/Editor/PackageContentManager.cs +++ b/src/NuGetForUnity/Editor/PackageContentManager.cs @@ -114,7 +114,7 @@ internal static void CleanInstallationDirectory([NotNull] INugetPackageIdentifie /// True if the file can be skipped, is not needed. internal static bool ShouldSkipUnpackingOnPath([NotNull] string path, PackageInstallOperationResultEntry operationResultEntry) { - if (path.EndsWith("/")) + if (path.EndsWith("/", StringComparison.Ordinal)) { // We do not want to extract empty directory entries. If there are empty directories within the .nupkg, we // expect them to have a file named '_._' in them that indicates that it should be extracted, usually as a @@ -208,8 +208,9 @@ internal static bool ShouldSkipUnpackingOnPath([NotNull] string path, PackageIns return true; } - // Skip all PDB files since Unity uses Mono and requires MDB files, which causes it to output "missing MDB" errors - if (path.EndsWith(".pdb", StringComparison.OrdinalIgnoreCase)) + // If not configured skip all PDB files. Unity uses Mono and requires MDB files or portable PDB files, else it produces "missing MDB" errors. + if (!ConfigurationManager.NugetConfigFile.KeepingPdbFiles && + (path.EndsWith(".pdb", StringComparison.OrdinalIgnoreCase) || path.EndsWith(".pdb.meta", StringComparison.OrdinalIgnoreCase))) { return true; } @@ -325,6 +326,17 @@ internal static string ExtractPackageEntry([NotNull] ZipArchiveEntry entry, [Not entry.ExtractToFile(filePath, true); + if (filePath.EndsWith(".pdb", StringComparison.OrdinalIgnoreCase) && !PortableSymbolFileHelper.IsPortableSymbolFile(filePath)) + { + // Unity uses Mono and requires MDB files or portable PDB files, else it produces "missing MDB" errors. + new FileInfo(filePath).Delete(); + new FileInfo($"{filePath}.meta").Delete(); + NugetLogger.LogVerbose( + "The PDB file '{0}' doesn't have the new PDB file format that can be read by Unity so we delete it.", + filePath); + return null; + } + if (ConfigurationManager.NugetConfigFile.ReadOnlyPackageFiles) { var extractedFile = new FileInfo(filePath); diff --git a/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV2.cs b/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV2.cs index 730225b5..36aee09d 100644 --- a/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV2.cs +++ b/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV2.cs @@ -393,10 +393,7 @@ public void DownloadNupkgToFile(INugetPackageIdentifier package, string outputFi } #if TEST_GET_UPDATES_FALLBACK - private static void ComparePackageLists( - List updates, - List updatesReplacement, - string errorMessageToDisplayIfListsDoNotMatch) + private static void ComparePackageLists(List updates, List updatesReplacement, string errorMessageToDisplayIfListsDoNotMatch) { var matchingComparison = new StringBuilder(); var missingComparison = new StringBuilder(); @@ -426,12 +423,7 @@ private static void ComparePackageLists( if (missingComparison.Length > 0 || extraComparison.Length > 0) { - Debug.LogWarningFormat( - "{0}\n{1}\n{2}\n{3}", - errorMessageToDisplayIfListsDoNotMatch, - matchingComparison, - missingComparison, - extraComparison); + Debug.LogWarningFormat("{0}\n{1}\n{2}\n{3}", errorMessageToDisplayIfListsDoNotMatch, matchingComparison, missingComparison, extraComparison); } } #endif @@ -528,7 +520,7 @@ private List GetUpdatesFallback( return updates; } - private void CopyIsManuallyInstalled(List newPackages, ICollection packagesToUpdate) + private static void CopyIsManuallyInstalled(List newPackages, ICollection packagesToUpdate) { foreach (var newPackage in newPackages) { diff --git a/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV3.cs b/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV3.cs index dbe1473c..aad9a439 100644 --- a/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV3.cs +++ b/src/NuGetForUnity/Editor/PackageSource/NugetPackageSourceV3.cs @@ -305,6 +305,17 @@ public Task> GetPackageDetailsAsync( return ApiClient.GetPackageDetailsAsync(this, package, cancellationToken); } + private static void CopyIsManuallyInstalled(List newPackages, ICollection packagesToUpdate) + { + foreach (var newPackage in newPackages) + { + newPackage.IsManuallyInstalled = + packagesToUpdate.FirstOrDefault(packageToUpdate => packageToUpdate.Id.Equals(newPackage.Id, StringComparison.OrdinalIgnoreCase)) + ?.IsManuallyInstalled ?? + false; + } + } + [NotNull] private NugetApiClientV3 InitializeApiClient() { @@ -318,16 +329,5 @@ private NugetApiClientV3 InitializeApiClient() ApiClientCache.Add(SavedPath, apiClient); return apiClient; } - - private void CopyIsManuallyInstalled(List newPackages, ICollection packagesToUpdate) - { - foreach (var newPackage in newPackages) - { - newPackage.IsManuallyInstalled = - packagesToUpdate.FirstOrDefault(packageToUpdate => packageToUpdate.Id.Equals(newPackage.Id, StringComparison.OrdinalIgnoreCase)) - ?.IsManuallyInstalled ?? - false; - } - } } } diff --git a/src/NuGetForUnity/Editor/Ui/NugetPreferences.cs b/src/NuGetForUnity/Editor/Ui/NugetPreferences.cs index 1110c3e7..3caa24a5 100644 --- a/src/NuGetForUnity/Editor/Ui/NugetPreferences.cs +++ b/src/NuGetForUnity/Editor/Ui/NugetPreferences.cs @@ -44,6 +44,8 @@ public class NugetPreferences : SettingsProvider private const float LabelPading = 5; + private static readonly string[] SearchKeywords = { "NuGet", "Packages" }; + private readonly GUIContent deleteX = new GUIContent("\u2716"); private readonly GUIContent downArrow = new GUIContent("\u25bc"); @@ -79,8 +81,12 @@ public class NugetPreferences : SettingsProvider /// Initializes a new instance of the class. /// Path of packages.config file is checked here as well in case it was manually changed. /// + [SuppressMessage( + "Usage", + "CA2249:Consider using 'string.Contains' instead of 'string.IndexOf'", + Justification = ".Net Framework doesn't have a string contains that is case insensitive.")] private NugetPreferences() - : base(MenuItemLocation, SettingsScope.Project, new[] { "NuGet", "Packages" }) + : base(MenuItemLocation, SettingsScope.Project, SearchKeywords) { shouldShowPackagesConfigPathWarning = ConfigurationManager.NugetConfigFile.InstallLocation == PackageInstallLocation.CustomWithinAssets && !UnityPathHelper.IsPathInAssets(ConfigurationManager.NugetConfigFile.PackagesConfigDirectoryPath); @@ -129,6 +135,17 @@ public override void OnGUI([CanBeNull] string searchContext) ConfigurationManager.NugetConfigFile.InstallFromCache = installFromCache; } + var keepingPdbFiles = EditorGUILayout.Toggle( + new GUIContent( + "Keep PDB Files", + "Do not delete PDB files included in a NuGet package if they can be read by Unity (have the new portable PDB format)."), + ConfigurationManager.NugetConfigFile.KeepingPdbFiles); + if (keepingPdbFiles != ConfigurationManager.NugetConfigFile.KeepingPdbFiles) + { + preferencesChangedThisFrame = true; + ConfigurationManager.NugetConfigFile.KeepingPdbFiles = keepingPdbFiles; + } + var readOnlyPackageFiles = EditorGUILayout.Toggle("Read-Only Package Files", ConfigurationManager.NugetConfigFile.ReadOnlyPackageFiles); if (readOnlyPackageFiles != ConfigurationManager.NugetConfigFile.ReadOnlyPackageFiles) {