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..940dc964ad1 100644 --- a/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs +++ b/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs @@ -32,31 +32,48 @@ 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++) + if (!File.Exists(expectedMigrationFilename)) { - try + // Only run migrations that have not already been run + int highestMigrationRun = GetHighestMigrationRun(migrationsDirectory); + for (int i = highestMigrationRun + 1; i < Migrations.Count; i++) { - 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); + try + { + 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); + } + catch { } } - catch { } } - mutex.ReleaseMutex(); } } } + + 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) 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..cff096f7ef5 --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs @@ -0,0 +1,47 @@ +// 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 System.Threading.Tasks; +using NuGet.Common.Migrations; +using Xunit; + +namespace NuGet.Common.Test +{ + [CollectionDefinition("MigrationRunner", DisableParallelization = true)] + public class MigrationRunnerTests + { + [Fact] + public void WhenExecutedInParallelOnlyOneFileIsCreatedForEveryMigration_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)); + Assert.Equal(1, Directory.GetFiles(directory).Length); + } + } +} 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); + } + } +}