Merge pull request #1409 from tgstation/1167-FinallyFixedThis [TGSDeploy]

Directory copying improvements (5.2.1)

build/Version.props

@@ -3,7 +3,7 @@
   <!-- Integration tests will ensure they match across the board -->
   <Import Project="ControlPanelVersion.props" />
   <PropertyGroup>
-    <TgsCoreVersion>5.2.0</TgsCoreVersion>
+    <TgsCoreVersion>5.2.1</TgsCoreVersion>
     <TgsConfigVersion>4.4.0</TgsConfigVersion>
     <TgsApiVersion>9.7.0</TgsApiVersion>
     <TgsApiLibraryVersion>10.1.0</TgsApiLibraryVersion>

src/Tgstation.Server.Host/Components/InstanceFactory.cs

@@ -251,6 +251,7 @@ namespace Tgstation.Server.Host.Components
 				repositoryCommands,
 				repoIoManager,
 				eventConsumer,
+				postWriteHandler,
 				gitRemoteFeaturesFactory,
 				loggerFactory.CreateLogger<Repository.Repository>(),
 				loggerFactory.CreateLogger<RepositoryManager>());

src/Tgstation.Server.Host/Components/Repository/Repository.cs

@@ -91,6 +91,11 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// </summary>
 		readonly ICredentialsProvider credentialsProvider;
 
+		/// <summary>
+		/// The <see cref="IPostWriteHandler"/> for the <see cref="Repository"/>.
+		/// </summary>
+		readonly IPostWriteHandler postWriteHandler;
+
 		/// <summary>
 		/// The <see cref="IGitRemoteFeatures"/> for the <see cref="Repository"/>.
 		/// </summary>
@@ -119,6 +124,7 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// <param name="ioMananger">The value of <see cref="ioMananger"/>.</param>
 		/// <param name="eventConsumer">The value of <see cref="eventConsumer"/>.</param>
 		/// <param name="credentialsProvider">The value of <see cref="credentialsProvider"/>.</param>
+		/// <param name="postWriteHandler">The value of <see cref="postWriteHandler"/>.</param>
 		/// <param name="gitRemoteFeaturesFactory">The <see cref="IGitRemoteFeaturesFactory"/> to provide the value of <see cref="gitRemoteFeatures"/>.</param>
 		/// <param name="logger">The value of <see cref="logger"/>.</param>
 		/// <param name="onDispose">The value if <see cref="onDispose"/>.</param>
@@ -128,6 +134,7 @@ namespace Tgstation.Server.Host.Components.Repository
 			IIOManager ioMananger,
 			IEventConsumer eventConsumer,
 			ICredentialsProvider credentialsProvider,
+			IPostWriteHandler postWriteHandler,
 			IGitRemoteFeaturesFactory gitRemoteFeaturesFactory,
 			ILogger<Repository> logger,
 			Action onDispose)
@@ -137,6 +144,7 @@ namespace Tgstation.Server.Host.Components.Repository
 			this.ioMananger = ioMananger ?? throw new ArgumentNullException(nameof(ioMananger));
 			this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer));
 			this.credentialsProvider = credentialsProvider ?? throw new ArgumentNullException(nameof(credentialsProvider));
+			this.postWriteHandler = postWriteHandler ?? throw new ArgumentNullException(nameof(postWriteHandler));
 			if (gitRemoteFeaturesFactory == null)
 				throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory));
 
@@ -500,7 +508,18 @@ namespace Tgstation.Server.Host.Components.Repository
 			if (path == null)
 				throw new ArgumentNullException(nameof(path));
 			logger.LogTrace("Copying to {0}...", path);
-			await ioMananger.CopyDirectory(ioMananger.ResolvePath(), path, new List<string> { ".git" }, cancellationToken);
+			await ioMananger.CopyDirectory(
+				ioMananger.ResolvePath(),
+				path,
+				new List<string> { ".git" },
+				(src, dest) =>
+				{
+					if (postWriteHandler.NeedsPostWrite(src))
+						postWriteHandler.HandleWrite(dest);
+
+					return Task.CompletedTask;
+				},
+				cancellationToken);
 		}
 
 		/// <inheritdoc />

src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs

@@ -42,6 +42,11 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// </summary>
 		readonly IEventConsumer eventConsumer;
 
+		/// <summary>
+		/// The <see cref="IPostWriteHandler"/> for the <see cref="RepositoryManager"/>.
+		/// </summary>
+		readonly IPostWriteHandler postWriteHandler;
+
 		/// <summary>
 		/// The <see cref="IGitRemoteFeaturesFactory"/> for the <see cref="RepositoryManager"/>.
 		/// </summary>
@@ -69,6 +74,7 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// <param name="commands">The value of <see cref="commands"/>.</param>
 		/// <param name="ioManager">The value of <see cref="ioManager"/>.</param>
 		/// <param name="eventConsumer">The value of <see cref="eventConsumer"/>.</param>
+		/// <param name="postWriteHandler">The value of <see cref="postWriteHandler"/>.</param>
 		/// <param name="gitRemoteFeaturesFactory">The value of <see cref="gitRemoteFeaturesFactory"/>.</param>
 		/// <param name="repositoryLogger">The value of <see cref="repositoryLogger"/>.</param>
 		/// <param name="logger">The value of <see cref="logger"/>.</param>
@@ -77,6 +83,7 @@ namespace Tgstation.Server.Host.Components.Repository
 			ILibGit2Commands commands,
 			IIOManager ioManager,
 			IEventConsumer eventConsumer,
+			IPostWriteHandler postWriteHandler,
 			IGitRemoteFeaturesFactory gitRemoteFeaturesFactory,
 			ILogger<Repository> repositoryLogger,
 			ILogger<RepositoryManager> logger)
@@ -85,6 +92,7 @@ namespace Tgstation.Server.Host.Components.Repository
 			this.commands = commands ?? throw new ArgumentNullException(nameof(commands));
 			this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager));
 			this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer));
+			this.postWriteHandler = postWriteHandler ?? throw new ArgumentNullException(nameof(postWriteHandler));
 			this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory));
 			this.repositoryLogger = repositoryLogger ?? throw new ArgumentNullException(nameof(repositoryLogger));
 			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
@@ -206,6 +214,7 @@ namespace Tgstation.Server.Host.Components.Repository
 						ioManager,
 						eventConsumer,
 						repositoryFactory,
+						postWriteHandler,
 						gitRemoteFeaturesFactory,
 						repositoryLogger,
 						() =>

src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs

@@ -180,7 +180,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles
 				var dmeExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, dmeFile), cancellationToken);
 				var headFileExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, CodeModificationsHeadFile), cancellationToken);
 				var tailFileExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, CodeModificationsTailFile), cancellationToken);
-				var copyTask = ioManager.CopyDirectory(CodeModificationsSubdirectory, destination, null, cancellationToken);
+				var copyTask = ioManager.CopyDirectory(CodeModificationsSubdirectory, destination, null, null, cancellationToken);
 
 				await Task.WhenAll(dmeExistsTask, headFileExistsTask, tailFileExistsTask, copyTask);
 

src/Tgstation.Server.Host/IO/DefaultIOManager.cs

@@ -88,7 +88,12 @@ namespace Tgstation.Server.Host.IO
 		}
 
 		/// <inheritdoc />
-		public async Task CopyDirectory(string src, string dest, IEnumerable<string> ignore, CancellationToken cancellationToken)
+		public async Task CopyDirectory(
+			string src,
+			string dest,
+			IEnumerable<string> ignore,
+			Func<string, string, Task> postCopyCallback,
+			CancellationToken cancellationToken)
 		{
 			if (src == null)
 				throw new ArgumentNullException(nameof(src));
@@ -97,8 +102,7 @@ namespace Tgstation.Server.Host.IO
 
 			src = ResolvePath(src);
 			dest = ResolvePath(dest);
-			foreach (var directoryCopy in CopyDirectoryImpl(src, dest, ignore, cancellationToken))
-				await directoryCopy;
+			await Task.WhenAll(CopyDirectoryImpl(src, dest, ignore, postCopyCallback, cancellationToken));
 		}
 
 		/// <inheritdoc />
@@ -368,37 +372,49 @@ namespace Tgstation.Server.Host.IO
 		/// <param name="src">The source directory path.</param>
 		/// <param name="dest">The destination directory path.</param>
 		/// <param name="ignore">Files and folders to ignore at the root level.</param>
+		/// <param name="postCopyCallback">The optional callback called for each source/dest file pair post copy.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="IEnumerable{T}"/> of <see cref="Task"/>s representing the running operation.</returns>
-		IEnumerable<Task> CopyDirectoryImpl(string src, string dest, IEnumerable<string> ignore, CancellationToken cancellationToken)
+		IEnumerable<Task> CopyDirectoryImpl(
+			string src,
+			string dest,
+			IEnumerable<string> ignore,
+			Func<string, string, Task> postCopyCallback,
+			CancellationToken cancellationToken)
 		{
 			var dir = new DirectoryInfo(src);
-			var atLeastOneSubDir = false;
 			foreach (var subDirectory in dir.EnumerateDirectories())
 			{
 				if (ignore != null && ignore.Contains(subDirectory.Name))
 					continue;
-				foreach (var copyTask in CopyDirectoryImpl(subDirectory.FullName, Path.Combine(dest, subDirectory.Name), null, cancellationToken))
-				{
-					atLeastOneSubDir = true;
+				foreach (var copyTask in CopyDirectoryImpl(subDirectory.FullName, Path.Combine(dest, subDirectory.Name), null, postCopyCallback, cancellationToken))
 					yield return copyTask;
-				}
 			}
 
 			async Task CopyThisDirectory()
 			{
-				if (!atLeastOneSubDir)
-					await CreateDirectory(dest, cancellationToken); // save on createdir calls
+				await CreateDirectory(dest, cancellationToken);
 
-				var tasks = new List<Task>();
+				var fileCopyTasks = new List<Task>();
 				foreach (var fileInfo in dir.EnumerateFiles())
 				{
 					if (ignore != null && ignore.Contains(fileInfo.Name))
 						return;
-					tasks.Add(CopyFile(fileInfo.FullName, Path.Combine(dest, fileInfo.Name), cancellationToken));
+
+					var sourceFile = fileInfo.FullName;
+					var destFile = Path.Combine(dest, fileInfo.Name);
+
+					async Task CopyThisFile()
+					{
+						await CopyFile(sourceFile, destFile, cancellationToken);
+						if (postCopyCallback != null)
+							await postCopyCallback(sourceFile, destFile);
+					}
+
+					fileCopyTasks.Add(CopyThisFile());
 				}
 
-				await Task.WhenAll(tasks);
+				await Task.WhenAll(fileCopyTasks);
 			}
 
 			yield return CopyThisDirectory();

src/Tgstation.Server.Host/IO/IIOManager.cs

@@ -51,9 +51,15 @@ namespace Tgstation.Server.Host.IO
 		/// <param name="src">The source directory path.</param>
 		/// <param name="dest">The destination directory path.</param>
 		/// <param name="ignore">Files and folders to ignore at the root level.</param>
+		/// <param name="postCopyCallback">The optional callback called for each source/dest file pair post copy.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
-		Task CopyDirectory(string src, string dest, IEnumerable<string> ignore, CancellationToken cancellationToken);
+		Task CopyDirectory(
+			string src,
+			string dest,
+			IEnumerable<string> ignore,
+			Func<string, string, Task> postCopyCallback,
+			CancellationToken cancellationToken);
 
 		/// <summary>
 		/// Check that the file at <paramref name="path"/> exists.

src/Tgstation.Server.Host/IO/IPostWriteHandler.cs

@@ -5,6 +5,13 @@
 	/// </summary>
 	interface IPostWriteHandler
 	{
+		/// <summary>
+		/// Check if a given <paramref name="sourceFilePath"/> will need <see cref="HandleWrite(string)"/> called on a copy of it.
+		/// </summary>
+		/// <param name="sourceFilePath">The path of the source file to check.</param>
+		/// <returns><see langword="true"/> if <see cref="HandleWrite(string)"/> should be called on copies of <paramref name="sourceFilePath"/>, <see langword="false"/> otherwise.</returns>
+		public bool NeedsPostWrite(string sourceFilePath);
+
 		/// <summary>
 		/// For handling system specific necessities after a write.
 		/// </summary>

src/Tgstation.Server.Host/IO/PosixPostWriteHandler.cs

@@ -25,6 +25,18 @@ namespace Tgstation.Server.Host.IO
 			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
 		}
 
+		/// <inheritdoc />
+		public bool NeedsPostWrite(string sourceFilePath)
+		{
+			if (sourceFilePath == null)
+				throw new ArgumentNullException(nameof(sourceFilePath));
+
+			if (Syscall.stat(sourceFilePath, out var stat) != 0)
+				throw new UnixIOException(Stdlib.GetLastError());
+
+			return stat.st_mode.HasFlag(FilePermissions.S_IXUSR);
+		}
+
 		/// <inheritdoc />
 		public void HandleWrite(string filePath)
 		{

src/Tgstation.Server.Host/IO/WindowsPostWriteHandler.cs

@@ -7,6 +7,15 @@ namespace Tgstation.Server.Host.IO
 	/// </summary>
 	sealed class WindowsPostWriteHandler : IPostWriteHandler
 	{
+		/// <inheritdoc />
+		public bool NeedsPostWrite(string sourceFilePath)
+		{
+			if (sourceFilePath == null)
+				throw new ArgumentNullException(nameof(sourceFilePath));
+
+			return false;
+		}
+
 		/// <inheritdoc />
 		public void HandleWrite(string filePath)
 		{

tests/Tgstation.Server.Host.Tests/IO/TestPostWriteHandler.cs

@@ -24,6 +24,7 @@ namespace Tgstation.Server.Host.IO.Tests
 				postWriteHandler = new PosixPostWriteHandler(Mock.Of<ILogger<PosixPostWriteHandler>>());
 
 			Assert.ThrowsException<ArgumentNullException>(() => postWriteHandler.HandleWrite(null));
+			Assert.ThrowsException<ArgumentNullException>(() => postWriteHandler.NeedsPostWrite(null));
 		}
 
 		[TestMethod]
@@ -40,12 +41,14 @@ namespace Tgstation.Server.Host.IO.Tests
 			var tmpFile = Path.GetTempFileName();
 			try
 			{
+				Assert.IsFalse(postWriteHandler.NeedsPostWrite(tmpFile));
 				postWriteHandler.HandleWrite(tmpFile);
 
 				if (isWindows)
 					return; //you do nothing
 
 				//ensure it is now executable
+				Assert.IsTrue(postWriteHandler.NeedsPostWrite(tmpFile));
 				File.WriteAllBytes(tmpFile, Encoding.UTF8.GetBytes("#!/bin/sh\n"));
 
 				using (var process = Process.Start(tmpFile))

tests/Tgstation.Server.Tests/IntegrationTest.cs

@@ -1073,6 +1073,7 @@ namespace Tgstation.Server.Tests
 				Mock.Of<Host.IO.IIOManager>(),
 				Mock.Of<IEventConsumer>(),
 				Mock.Of<ICredentialsProvider>(),
+				Mock.Of<Host.IO.IPostWriteHandler>(),
 				Mock.Of<IGitRemoteFeaturesFactory>(),
 				Mock.Of<ILogger<Repository>>(),
 				() => { });