Merge pull request #1441 from tgstation/DDExe

dd.exe Support

build/Version.props

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

src/Tgstation.Server.Host/Components/Byond/ByondExecutableLock.cs

@@ -20,6 +20,9 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// <inheritdoc />
 		public string DreamMakerPath { get; }
 
+		/// <inheritdoc />
+		public bool SupportsCli { get; }
+
 		/// <summary>
 		/// The <see cref="IIOManager"/> for the <see cref="ByondExecutableLock"/>.
 		/// </summary>
@@ -44,13 +47,15 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// <param name="dreamDaemonPath">The value of <see cref="DreamDaemonPath"/>.</param>
 		/// <param name="dreamMakerPath">The value of <see cref="DreamMakerPath"/>.</param>
 		/// <param name="trustedFilePath">The value of <see cref="trustedFilePath"/>.</param>
+		/// <param name="supportsCli">The value of <see cref="SupportsCli"/>.</param>
 		public ByondExecutableLock(
 			IIOManager ioManager,
 			SemaphoreSlim trustedFileSemaphore,
 			Version version,
 			string dreamDaemonPath,
 			string dreamMakerPath,
-			string trustedFilePath)
+			string trustedFilePath,
+			bool supportsCli)
 		{
 			this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager));
 			this.trustedFileSemaphore = trustedFileSemaphore ?? throw new ArgumentNullException(nameof(trustedFileSemaphore));
@@ -58,6 +63,8 @@ namespace Tgstation.Server.Host.Components.Byond
 			DreamDaemonPath = dreamDaemonPath ?? throw new ArgumentNullException(nameof(dreamDaemonPath));
 			DreamMakerPath = dreamMakerPath ?? throw new ArgumentNullException(nameof(dreamMakerPath));
 			this.trustedFilePath = trustedFilePath ?? throw new ArgumentNullException(nameof(trustedFilePath));
+
+			SupportsCli = supportsCli;
 		}
 
 		// at one point in design, byond versions were to delete themselves if they weren't the active version

src/Tgstation.Server.Host/Components/Byond/ByondInstallerBase.cs

@@ -18,9 +18,6 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// </summary>
 		const string CacheDirectoryName = "cache";
 
-		/// <inheritdoc />
-		public abstract string DreamDaemonName { get; }
-
 		/// <inheritdoc />
 		public abstract string DreamMakerName { get; }
 
@@ -53,6 +50,9 @@ namespace Tgstation.Server.Host.Components.Byond
 			Logger = logger ?? throw new ArgumentNullException(nameof(logger));
 		}
 
+		/// <inheritdoc />
+		public abstract string GetDreamDaemonName(Version version, out bool supportsCli);
+
 		/// <inheritdoc />
 		public async Task CleanCache(CancellationToken cancellationToken)
 		{
@@ -77,7 +77,10 @@ namespace Tgstation.Server.Host.Components.Byond
 		}
 
 		/// <inheritdoc />
-		public abstract Task InstallByond(string path, Version version, CancellationToken cancellationToken);
+		public abstract Task InstallByond(Version version, string path, CancellationToken cancellationToken);
+
+		/// <inheritdoc />
+		public abstract Task UpgradeInstallation(Version version, string path, CancellationToken cancellationToken);
 
 		/// <inheritdoc />
 		public Task<MemoryStream> DownloadVersion(Version version, CancellationToken cancellationToken)

src/Tgstation.Server.Host/Components/Byond/ByondManager.cs

@@ -166,7 +166,7 @@ namespace Tgstation.Server.Host.Components.Byond
 				ioManager.ResolvePath(
 					ioManager.ConcatPath(
 						binPathForVersion,
-						byondInstaller.DreamDaemonName)),
+						byondInstaller.GetDreamDaemonName(versionToUse, out var supportsCli))),
 				ioManager.ResolvePath(
 					ioManager.ConcatPath(
 						binPathForVersion,
@@ -175,7 +175,8 @@ namespace Tgstation.Server.Host.Components.Byond
 					ioManager.ConcatPath(
 						byondInstaller.PathToUserByondFolder,
 						CfgDirectoryName,
-						TrustedDmbFileName)));
+						TrustedDmbFileName)),
+				supportsCli);
 		}
 
 		/// <inheritdoc />
@@ -211,6 +212,8 @@ namespace Tgstation.Server.Host.Components.Byond
 			await ioManager.CreateDirectory(byondDirectory, cancellationToken);
 			var directories = await ioManager.GetDirectories(byondDirectory, cancellationToken);
 
+			var installedVersionPaths = new Dictionary<string, Version>();
+
 			async Task ReadVersion(string path)
 			{
 				var versionFile = ioManager.ConcatPath(path, VersionFileName);
@@ -231,6 +234,7 @@ namespace Tgstation.Server.Host.Components.Byond
 						{
 							logger.LogDebug("Adding detected BYOND version {0}...", key);
 							installedVersions.Add(key, Task.CompletedTask);
+							installedVersionPaths.Add(ioManager.ResolvePath(key), version);
 							return;
 						}
 				}
@@ -238,7 +242,10 @@ namespace Tgstation.Server.Host.Components.Byond
 				await ioManager.DeleteDirectory(path, cancellationToken);
 			}
 
-			await Task.WhenAll(directories.Select(x => ReadVersion(x)));
+			await Task.WhenAll(directories.Select(ReadVersion));
+
+			logger.LogTrace("Upgrading BYOND installations...");
+			await Task.WhenAll(installedVersionPaths.Select(kvp => byondInstaller.UpgradeInstallation(kvp.Value, kvp.Key, cancellationToken)));
 
 			var activeVersionBytes = await activeVersionBytesTask;
 			if (activeVersionBytes != null)
@@ -339,7 +346,7 @@ namespace Tgstation.Server.Host.Components.Byond
 						await ioManager.ZipToDirectory(extractPath, versionZipStream, cancellationToken);
 					}
 
-					await byondInstaller.InstallByond(extractPath, version, cancellationToken);
+					await byondInstaller.InstallByond(version, extractPath, cancellationToken);
 
 					// make sure to do this last because this is what tells us we have a valid version in the future
 					await ioManager.WriteAllBytes(

src/Tgstation.Server.Host/Components/Byond/IByondExecutableLock.cs

@@ -24,6 +24,11 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// </summary>
 		string DreamMakerPath { get; }
 
+		/// <summary>
+		/// If <see cref="DreamDaemonPath"/> supports being run as a command-line application.
+		/// </summary>
+		bool SupportsCli { get; }
+
 		/// <summary>
 		/// Call if, during a detach, this version should not be deleted.
 		/// </summary>

src/Tgstation.Server.Host/Components/Byond/IByondInstaller.cs

@@ -10,11 +10,6 @@ namespace Tgstation.Server.Host.Components.Byond
 	/// </summary>
 	interface IByondInstaller
 	{
-		/// <summary>
-		/// Get the file name of the DreamDaemon executable.
-		/// </summary>
-		string DreamDaemonName { get; }
-
 		/// <summary>
 		/// Get the file name of the DreamMaker executable.
 		/// </summary>
@@ -25,6 +20,14 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// </summary>
 		string PathToUserByondFolder { get; }
 
+		/// <summary>
+		/// Get the file name of the DreamDaemon executable.
+		/// </summary>
+		/// <param name="version">The <see cref="Version"/> of BYOND to select the executable name for.</param>
+		/// <param name="supportsCli">Whether or not the returned path supports being run as a command-line application.</param>
+		/// <returns>The file name of the DreamDaemon executable.</returns>
+		string GetDreamDaemonName(Version version, out bool supportsCli);
+
 		/// <summary>
 		/// Download a given BYOND <paramref name="version"/>.
 		/// </summary>
@@ -36,11 +39,20 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// <summary>
 		/// Does actions necessary to get an extracted BYOND installation working.
 		/// </summary>
-		/// <param name="path">The path to the BYOND installation.</param>
 		/// <param name="version">The <see cref="Version"/> of BYOND being installed.</param>
+		/// <param name="path">The path to the BYOND installation.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
-		Task InstallByond(string path, Version version, CancellationToken cancellationToken);
+		Task InstallByond(Version version, string path, CancellationToken cancellationToken);
+
+		/// <summary>
+		/// Does actions necessary to get upgrade a BYOND version installed by a previous version of TGS.
+		/// </summary>
+		/// <param name="version">The <see cref="Version"/> of BYOND being installed.</param>
+		/// <param name="path">The path to the BYOND installation.</param>
+		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
+		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
+		Task UpgradeInstallation(Version version, string path, CancellationToken cancellationToken);
 
 		/// <summary>
 		/// Attempts to cleans the BYOND cache folder for the system.

src/Tgstation.Server.Host/Components/Byond/PosixByondInstaller.cs

@@ -30,9 +30,6 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// </summary>
 		const string ShellScriptExtension = ".sh";
 
-		/// <inheritdoc />
-		public override string DreamDaemonName => DreamDaemonExecutableName + ShellScriptExtension;
-
 		/// <inheritdoc />
 		public override string DreamMakerName => DreamMakerExecutableName + ShellScriptExtension;
 
@@ -66,13 +63,23 @@ namespace Tgstation.Server.Host.Components.Byond
 		}
 
 		/// <inheritdoc />
-		public override Task InstallByond(string path, Version version, CancellationToken cancellationToken)
+		public override string GetDreamDaemonName(Version version, out bool supportsCli)
 		{
-			if (path == null)
-				throw new ArgumentNullException(nameof(path));
 			if (version == null)
 				throw new ArgumentNullException(nameof(version));
 
+			supportsCli = true;
+			return DreamDaemonExecutableName + ShellScriptExtension;
+		}
+
+		/// <inheritdoc />
+		public override Task InstallByond(Version version, string path, CancellationToken cancellationToken)
+		{
+			if (version == null)
+				throw new ArgumentNullException(nameof(version));
+			if (path == null)
+				throw new ArgumentNullException(nameof(path));
+
 			// write the scripts for running the ting
 			// need to add $ORIGIN to LD_LIBRARY_PATH
 			const string StandardScript = "#!/bin/sh\nexport LD_LIBRARY_PATH=\"\\$ORIGIN:$LD_LIBRARY_PATH\"\nBASEDIR=$(dirname \"$0\")\nexec \"$BASEDIR/{0}\" \"$@\"\n";
@@ -89,12 +96,29 @@ namespace Tgstation.Server.Host.Components.Byond
 
 			var basePath = IOManager.ConcatPath(path, ByondManager.BinPath);
 
-			var task = Task.WhenAll(WriteAndMakeExecutable(IOManager.ConcatPath(basePath, DreamDaemonName), dreamDaemonScript), WriteAndMakeExecutable(IOManager.ConcatPath(basePath, DreamMakerName), dreamMakerScript));
+			var task = Task.WhenAll(
+				WriteAndMakeExecutable(
+					IOManager.ConcatPath(basePath, GetDreamDaemonName(version, out var _)),
+					dreamDaemonScript),
+				WriteAndMakeExecutable(
+					IOManager.ConcatPath(basePath, DreamMakerName),
+					dreamMakerScript));
 
 			postWriteHandler.HandleWrite(IOManager.ConcatPath(basePath, DreamDaemonExecutableName));
 			postWriteHandler.HandleWrite(IOManager.ConcatPath(basePath, DreamMakerExecutableName));
 
 			return task;
 		}
+
+		/// <inheritdoc />
+		public override Task UpgradeInstallation(Version version, string path, CancellationToken cancellationToken)
+		{
+			if (version == null)
+				throw new ArgumentNullException(nameof(version));
+			if (path == null)
+				throw new ArgumentNullException(nameof(path));
+
+			return Task.CompletedTask;
+		}
 	}
 }

src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs

@@ -41,8 +41,10 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// </summary>
 		const string ByondDXDir = "byond/directx";
 
-		/// <inheritdoc />
-		public override string DreamDaemonName => "dreamdaemon.exe";
+		/// <summary>
+		/// The file TGS uses to determine if dd.exe has been firewalled.
+		/// </summary>
+		const string TgsFirewalledDDFile = "TGSFirewalledDD";
 
 		/// <inheritdoc />
 		public override string DreamMakerName => "dm.exe";
@@ -96,7 +98,17 @@ namespace Tgstation.Server.Host.Components.Byond
 		public void Dispose() => semaphore.Dispose();
 
 		/// <inheritdoc />
-		public override Task InstallByond(string path, Version version, CancellationToken cancellationToken)
+		public override string GetDreamDaemonName(Version version, out bool supportsCli)
+		{
+			if (version == null)
+				throw new ArgumentNullException(nameof(version));
+
+			supportsCli = version.Major >= 515 && version.Minor >= 1598;
+			return supportsCli ? "dd.exe" : "dreamdaemon.exe";
+		}
+
+		/// <inheritdoc />
+		public override Task InstallByond(Version version, string path, CancellationToken cancellationToken)
 		{
 			var tasks = new List<Task>
 			{
@@ -105,11 +117,33 @@ namespace Tgstation.Server.Host.Components.Byond
 			};
 
 			if (!generalConfiguration.SkipAddingByondFirewallException)
-				tasks.Add(AddDreamDaemonToFirewall(path, cancellationToken));
+				tasks.Add(AddDreamDaemonToFirewall(version, path, cancellationToken));
 
 			return Task.WhenAll(tasks);
 		}
 
+		/// <inheritdoc />
+		public override async Task UpgradeInstallation(Version version, string path, CancellationToken cancellationToken)
+		{
+			if (version == null)
+				throw new ArgumentNullException(nameof(version));
+			if (path == null)
+				throw new ArgumentNullException(nameof(path));
+
+			if (generalConfiguration.SkipAddingByondFirewallException)
+				return;
+
+			GetDreamDaemonName(version, out var usesDDExe);
+			if (!usesDDExe)
+				return;
+
+			if (await IOManager.FileExists(IOManager.ConcatPath(path, TgsFirewalledDDFile), cancellationToken))
+				return;
+
+			Logger.LogInformation("BYOND Version {version} needs dd.exe added to firewall", version);
+			await AddDreamDaemonToFirewall(version, path, cancellationToken);
+		}
+
 		/// <summary>
 		/// Creates the BYOND cfg file that prevents the trusted mode dialog from appearing when launching DreamDaemon.
 		/// </summary>
@@ -177,17 +211,21 @@ namespace Tgstation.Server.Host.Components.Byond
 		/// <summary>
 		/// Attempt to add the DreamDaemon executable as an exception to the Windows firewall.
 		/// </summary>
+		/// <param name="version">The BYOND <see cref="Version"/>.</param>
 		/// <param name="path">The path to the BYOND installation.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
-		async Task AddDreamDaemonToFirewall(string path, CancellationToken cancellationToken)
+		async Task AddDreamDaemonToFirewall(Version version, string path, CancellationToken cancellationToken)
 		{
+			var dreamDaemonName = GetDreamDaemonName(version, out var supportsCli);
+
 			var dreamDaemonPath = IOManager.ResolvePath(
 				IOManager.ConcatPath(
 					path,
 					ByondManager.BinPath,
-					DreamDaemonName));
+					dreamDaemonName));
 
+			Logger.LogInformation("Adding Windows Firewall exception for {path}...", dreamDaemonPath);
 			try
 			{
 				using var netshProcess = processExecutor.LaunchProcess(
@@ -210,6 +248,12 @@ namespace Tgstation.Server.Host.Components.Byond
 
 				if (exitCode != 0)
 					throw new JobException(ErrorCode.ByondDreamDaemonFirewallFail, new JobException($"Invalid exit code: {exitCode}"));
+
+				if (supportsCli)
+					await IOManager.WriteAllBytes(
+						IOManager.ConcatPath(path, TgsFirewalledDDFile),
+						Array.Empty<byte>(),
+						cancellationToken);
 			}
 			catch (Exception ex)
 			{

src/Tgstation.Server.Host/Components/Chat/ChatManager.cs

@@ -679,7 +679,7 @@ namespace Tgstation.Server.Host.Components.Chat
 				message.User.Channel.IsAdminChannel = mappingChannelRepresentation.IsAdminChannel;
 			}
 
-			var splits = new List<string>(message.Content.Trim().Split(' '));
+			var splits = new List<string>(message.Content.Trim().Split(' ', StringSplitOptions.RemoveEmptyEntries));
 			var address = splits[0];
 			if (address.Length > 1 && (address.Last() == ':' || address.Last() == ','))
 				address = address[0..^1];

src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs

@@ -282,23 +282,22 @@ namespace Tgstation.Server.Host.Components.Session
 					Guid? logFileGuid = null;
 					var arguments = String.Format(
 						CultureInfo.InvariantCulture,
-						"{0} -port {1} -ports 1-65535 {2}-close -verbose -logself -{3} -{4}{5}{6} -params \"{7}\"",
+						"{0} -port {1} -ports 1-65535 {2}-close -verbose -{3} -{4}{5}{6} -params \"{7}\"",
 						dmbProvider.DmbName,
 						launchParameters.Port.Value,
 						launchParameters.AllowWebClient.Value ? "-webclient " : String.Empty,
 						SecurityWord(launchParameters.SecurityLevel.Value),
 						VisibilityWord(launchParameters.Visibility.Value),
-						platformIdentifier.IsWindows
-							? $" -log {logFileGuid = Guid.NewGuid()}"
-							: String.Empty, // Just use stdout on linux
+						!byondLock.SupportsCli
+							? $" -logself -log {logFileGuid = Guid.NewGuid()}"
+							: !platformIdentifier.IsWindows // Just use stdout on if CLI is supported
+								? " -logself"
+								: String.Empty, // Windows doesn't output anything to dd.exe if -logself is set?
 						launchParameters.StartProfiler.Value
 							? " -profile"
 							: String.Empty,
 						parameters);
 
-					// See https://github.com/tgstation/tgstation-server/issues/719
-					var noShellExecute = !platformIdentifier.IsWindows;
-
 					if (!apiValidate && dmbProvider.CompileJob.DMApiVersion == null)
 						logger.LogDebug("Session will have no DMAPI support!");
 
@@ -307,14 +306,15 @@ namespace Tgstation.Server.Host.Components.Session
 						byondLock.DreamDaemonPath,
 						dmbProvider.Directory,
 						arguments,
-						noShellExecute,
-						noShellExecute,
-						noShellExecute: noShellExecute);
+						byondLock.SupportsCli,
+						byondLock.SupportsCli,
+						byondLock.SupportsCli);
 
+					var cliSupported = byondLock.SupportsCli;
 					async Task<string> GetDDOutput()
 					{
 						// DCT x2: None available
-						if (!platformIdentifier.IsWindows)
+						if (cliSupported)
 							return await process.GetCombinedOutput(default);
 
 						var logFilePath = ioManager.ConcatPath(dmbProvider.Directory, logFileGuid.ToString());

src/Tgstation.Server.Host/System/Process.cs

@@ -112,7 +112,7 @@ namespace Tgstation.Server.Host.System
 					}
 					catch (Exception ex)
 					{
-						logger.LogDebug(ex, "Error on WaitForInputIdle()!");
+						logger.LogTrace(ex, "WaitForInputIdle() failed, this is normal.");
 					}
 				},
 				default, // DCT: None available

tests/Tgstation.Server.Host.Tests/Components/Byond/TestPosixByondInstaller.cs

@@ -65,9 +65,9 @@ namespace Tgstation.Server.Host.Components.Byond.Tests
 
 			const string FakePath = "fake";
 			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => installer.InstallByond(null, null, default));
-			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => installer.InstallByond(FakePath, null, default));
+			await Assert.ThrowsExceptionAsync<ArgumentNullException>(() => installer.InstallByond(new Version(123,252345), null, default));
 
-			await installer.InstallByond(FakePath, new Version(511, 1385), default);
+			await installer.InstallByond(new Version(511, 1385), FakePath, default);
 
 			mockPostWriteHandler.Verify(x => x.HandleWrite(It.IsAny<string>()), Times.Exactly(4));
 		}

tests/Tgstation.Server.Tests/Instance/ByondTest.cs

@@ -1,5 +1,4 @@
-using Castle.Core.Logging;
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
@@ -103,6 +102,8 @@ namespace Tgstation.Server.Tests.Instance
 					new DefaultIOManager(new AssemblyInformationProvider()),
 					Mock.Of<ILogger<PosixByondInstaller>>());
 
+			using var windowsByondInstaller = byondInstaller as WindowsByondInstaller;
+
 			// get the bytes for stable
 			using var stableBytesMs = await byondInstaller.DownloadVersion(TestVersion, cancellationToken);