diff --git a/src/NuGet.Core/NuGet.Common/Migrations/Migration1.cs b/src/NuGet.Core/NuGet.Common/Migrations/Migration1.cs index f7281c461f5..e0d86cc4c8e 100644 --- a/src/NuGet.Core/NuGet.Common/Migrations/Migration1.cs +++ b/src/NuGet.Core/NuGet.Common/Migrations/Migration1.cs @@ -79,7 +79,7 @@ void AddAllParentDirectoriesUpToHome(string path) { pathsToCheck.Add(path); - if (!path.StartsWith(homePath, StringComparison.Ordinal)) + if (!path.StartsWith(homePath + Path.DirectorySeparatorChar, StringComparison.Ordinal)) { return; } @@ -156,13 +156,13 @@ private static void FixPermissions(string path, PosixPermissions umask) { PosixPermissions correctedPermissions = permissions.Value.WithUmask(umask); string correctedPermissionsString = correctedPermissions.ToString(); - Exec("chmod", correctedPermissionsString + " " + path); + Exec("chmod", correctedPermissionsString + " " + path.FormatWithDoubleQuotes()); } } internal static PosixPermissions? GetPermissions(string path) { - string output = Exec("ls", "-ld " + path); + string output = Exec("ls", "-ld " + path.FormatWithDoubleQuotes()); if (output == null) { return null; diff --git a/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs b/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs index 8ce07392c6f..f8f913bc8c6 100644 --- a/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs +++ b/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs @@ -2,9 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; using System.IO; using System.Threading; @@ -12,17 +9,10 @@ namespace NuGet.Common.Migrations { public static class MigrationRunner { - private static readonly IReadOnlyList Migrations = new List() - { - Migration1.Run - }; private const string MaxMigrationFilename = "1"; public static void Run() { - // since migrations run once per machine, optimize for the scenario where the migration has already run - Debug.Assert(MaxMigrationFilename == Migrations.Count.ToString(CultureInfo.InvariantCulture)); - string migrationsDirectory = GetMigrationsDirectory(); var expectedMigrationFilename = Path.Combine(migrationsDirectory, MaxMigrationFilename); @@ -32,31 +22,45 @@ public static void Run() // so use a global mutex and then check if someone else already did the work. using (var mutex = new Mutex(false, "NuGet-Migrations")) { - bool signal = mutex.WaitOne(TimeSpan.FromMinutes(1), false); - if (signal && !File.Exists(expectedMigrationFilename)) + if (WaitForMutex(mutex)) { - // Only run migrations that have not already been run - int highestMigrationRun = GetHighestMigrationRun(migrationsDirectory); - for (int i = highestMigrationRun + 1; i < Migrations.Count; i++) + try { - try + // Only run migrations that have not already been run + if (!File.Exists(expectedMigrationFilename)) { - Migrations[i](); - // Create file for every migration run, so that if an older version of NuGet is run, it doesn't try to run - // migrations again. - string migrationFile = Path.Combine(migrationsDirectory, (i + 1).ToString(CultureInfo.InvariantCulture)); - File.WriteAllText(migrationFile, string.Empty); + Migration1.Run(); + // Create file for the migration run, so that if an older version of NuGet is run, it doesn't try to run migrations again. + File.WriteAllText(expectedMigrationFilename, string.Empty); } - catch { } } - - mutex.ReleaseMutex(); + catch { } + finally + { + mutex.ReleaseMutex(); + } } } + } + } + + private static bool WaitForMutex(Mutex mutex) + { + bool captured; + + try + { + captured = mutex.WaitOne(TimeSpan.FromMinutes(1), false); } + catch (AbandonedMutexException) + { + captured = true; + } + + return captured; } - private static string GetMigrationsDirectory() + internal static string GetMigrationsDirectory() { string migrationsDirectory; if (RuntimeEnvironmentHelper.IsWindows) @@ -73,19 +77,5 @@ private static string GetMigrationsDirectory() Directory.CreateDirectory(migrationsDirectory); return migrationsDirectory; } - - private static int GetHighestMigrationRun(string directory) - { - for (int i = Migrations.Count - 1; i >= 0; --i) - { - var filename = Path.Combine(directory, (i + 1).ToString(CultureInfo.InvariantCulture)); - if (File.Exists(filename)) - { - return i; - } - } - - return -1; - } } } diff --git a/src/NuGet.Core/NuGet.Common/StringExtensions.cs b/src/NuGet.Core/NuGet.Common/StringExtensions.cs new file mode 100644 index 00000000000..6c740ff2b29 --- /dev/null +++ b/src/NuGet.Core/NuGet.Common/StringExtensions.cs @@ -0,0 +1,13 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace NuGet.Common +{ + internal static class StringExtensions + { + internal static string FormatWithDoubleQuotes(this string s) + { + return s == null ? s : $@"""{s}"""; + } + } +} diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/Migration1Tests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/Migration1Tests.cs index dcd8704ce14..6c2608b88a5 100644 --- a/test/NuGet.Core.Tests/NuGet.Common.Test/Migration1Tests.cs +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/Migration1Tests.cs @@ -57,11 +57,11 @@ public void EnsureExpectedPermissions_Directories_Success(string currentPermissi { using (var testDirectory = TestDirectory.Create()) { - var v3cachePath = Path.Combine(testDirectory, "v3-cache"); + var v3cachePath = Path.Combine(testDirectory, "my documents", "v3-cache"); var v3cacheSubDirectoryInfo = Directory.CreateDirectory(Path.Combine(v3cachePath, "subDirectory")); Migration1.Exec("chmod", currentPermissions + " " + testDirectory.Path); - Migration1.Exec("chmod", currentPermissions + " " + v3cachePath); - Migration1.Exec("chmod", currentPermissions + " " + v3cacheSubDirectoryInfo.FullName); + Migration1.Exec("chmod", currentPermissions + " " + v3cachePath.FormatWithDoubleQuotes()); + Migration1.Exec("chmod", currentPermissions + " " + v3cacheSubDirectoryInfo.FullName.FormatWithDoubleQuotes()); HashSet pathsToCheck = new HashSet() { testDirectory.Path, v3cachePath, v3cacheSubDirectoryInfo.FullName }; Migration1.EnsureExpectedPermissions(pathsToCheck, PosixPermissions.Parse(umask)); diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs new file mode 100644 index 00000000000..c2560e1fb54 --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using NuGet.Common.Migrations; +using Xunit; + +namespace NuGet.Common.Test +{ + [CollectionDefinition("MigrationRunner", DisableParallelization = true)] + public class MigrationRunnerTests + { + [Fact] + public void Run_WhenExecutedOnSingleThreadThenOneMigrationFileIsCreated_Success() + { + // Arrange + string directory = MigrationRunner.GetMigrationsDirectory(); + if (Directory.Exists(directory)) + Directory.Delete(path: directory, recursive: true); + + // Act + MigrationRunner.Run(); + + // Assert + Assert.True(Directory.Exists(directory)); + var files = Directory.GetFiles(directory); + Assert.Equal(1, files.Length); + Assert.Equal(Path.Combine(directory, "1"), files[0]); + } + + [Fact] + public void Run_WhenExecutedInParallelThenOnlyOneMigrationFileIsCreated_Success() + { + var threads = new List(); + int numThreads = 5; + int timeoutInSeconds = 90; + + // Arrange + string directory = MigrationRunner.GetMigrationsDirectory(); + if (Directory.Exists(directory)) + Directory.Delete(path: directory, recursive: true); + + // Act + for (int i = 0; i < numThreads; i++) + { + var thread = new Thread(MigrationRunner.Run); + thread.Start(); + threads.Add(thread); + } + + foreach (var thread in threads) + { + thread.Join(timeout: TimeSpan.FromSeconds(timeoutInSeconds)); + } + + // Assert + Assert.True(Directory.Exists(directory)); + var files = Directory.GetFiles(directory); + Assert.Equal(1, files.Length); + Assert.Equal(Path.Combine(directory, "1"), files[0]); + } + } +} diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/StringExtensionsTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/StringExtensionsTests.cs new file mode 100644 index 00000000000..82b5cf05d5c --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/StringExtensionsTests.cs @@ -0,0 +1,30 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace NuGet.Common.Test +{ + public class StringExtensionsTests + { + [Fact] + public void FormatWithDoubleQuotes_WhenNullIsPassedReturnsNull_Success() + { + string actual = null; + string formatted = actual.FormatWithDoubleQuotes(); + Assert.Equal(actual, formatted); + } + + [Theory] + [InlineData("")] + [InlineData("/home/user/NuGet AppData/share")] + [InlineData("/home/user/NuGet/share")] + [InlineData(@"c:\users\NuGet App\Share")] + [InlineData(@"c:\users\NuGet\Share")] + public void FormatWithDoubleQuotes_WhenNonNullStringIsPassedReturnsFormattedString_Success(string actual) + { + string formatted = actual.FormatWithDoubleQuotes(); + Assert.Equal($@"""{actual}""", formatted); + } + } +}