diff --git a/src/Filee.Engines/Infrastructure/CopyVersions.cs b/src/Filee.Engines/Infrastructure/CopyVersions.cs index b8c942988..5c0fe5440 100644 --- a/src/Filee.Engines/Infrastructure/CopyVersions.cs +++ b/src/Filee.Engines/Infrastructure/CopyVersions.cs @@ -65,7 +65,12 @@ public static bool IsOutdated(string id, string folder) => private static string Read(string id, string program) { if (id == "libreoffice" && OperatingSystem.IsWindows()) - return File.ReadAllText(program); + { + // LibreOffice 26 dropped MsiProductVersion from version.ini; soffice.exe next to it still says it. + var soffice = Path.Combine(Path.GetDirectoryName(program)!, "soffice.exe"); + var product = File.Exists(soffice) ? FileVersionInfo.GetVersionInfo(soffice).ProductVersion : null; + return File.ReadAllText(program) + (product is null ? "" : $"\nLibreOffice {product}"); + } var arguments = id == "ffmpeg" ? "-hide_banner -version" : "--version"; try { diff --git a/src/Filee.Engines/Infrastructure/EngineInstaller.cs b/src/Filee.Engines/Infrastructure/EngineInstaller.cs index 4e4a7d864..2f47e6949 100644 --- a/src/Filee.Engines/Infrastructure/EngineInstaller.cs +++ b/src/Filee.Engines/Infrastructure/EngineInstaller.cs @@ -456,6 +456,14 @@ private async Task UnpackMsiAsync(string id, string msi, string target, string l } if (process.ExitCode != 0) throw MsiFailure(id, process.ExitCode, log); + // The verbose log is only worth keeping when unpacking failed (the error names it); else it would stay in %TEMP%. + try + { + File.Delete(log); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + } } /// What the Engines card shows: the reasons a user can act on in their language (error.* keys). diff --git a/src/Filee.Engines/Infrastructure/ProcessRunner.cs b/src/Filee.Engines/Infrastructure/ProcessRunner.cs index 218e111cc..342dc7e34 100644 --- a/src/Filee.Engines/Infrastructure/ProcessRunner.cs +++ b/src/Filee.Engines/Infrastructure/ProcessRunner.cs @@ -93,7 +93,7 @@ public static async Task RunAsync( var finishedEarly = false; try { - var exited = process.WaitForExitAsync(timeoutCts.Token); + var exited = WaitForExitOnlyAsync(process, timeoutCts.Token); if (watch is null && outputReady is not null && await WatchOutputAsync(outputReady, exited, timeoutCts.Token)) { TryKill(process, job); @@ -123,10 +123,14 @@ public static async Task RunAsync( return new ProcessResult(0, stdout.ToString(), stderr.ToString()); } + // The program exited; the ones it started that still run (and may hold the output) end with the job. + job?.Terminate(); // Make sure the async output readers have drained. - process.WaitForExit(); + await DrainAsync(process, executable, stderr, cancellationToken); AppLocalRuntime.ThrowIfMissing(executable, process.ExitCode); - return new ProcessResult(process.ExitCode, stdout.ToString(), stderr.ToString()); + lock (stdout) + lock (stderr) + return new ProcessResult(process.ExitCode, stdout.ToString(), stderr.ToString()); } /// @@ -160,6 +164,43 @@ private static async Task WatchOutputAsync(Func ready, Task exited, internal static string Describe(TimeSpan limit) => limit >= TimeSpan.FromMinutes(1) ? $"{limit.TotalMinutes:0} min" : $"{limit.TotalSeconds:0} s"; + /// How long the rest of the output may take once the program has exited. + private static readonly TimeSpan DrainTimeout = TimeSpan.FromSeconds(30); + + /// + /// Waits for the program itself to exit. also waits for the end of its + /// output, which a program it started can hold open long after (soffice.bin after soffice.exe): see + /// . + /// + private static async Task WaitForExitOnlyAsync(Process process, CancellationToken cancellationToken) + { + var exited = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + process.EnableRaisingEvents = true; + process.Exited += (_, _) => exited.TrySetResult(); + if (process.HasExited) + exited.TrySetResult(); + await exited.Task.WaitAsync(cancellationToken); + } + + /// + /// Waits for the rest of the output after the program exited, for at most, rather than + /// block the conversion until its time limit: only the rest of the output is lost, with a note in standard error. + /// + private static async Task DrainAsync(Process process, string executable, StringBuilder stderr, CancellationToken cancellationToken) + { + using var drain = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + drain.CancelAfter(DrainTimeout); + try + { + await process.WaitForExitAsync(drain.Token); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + lock (stderr) + stderr.AppendLine($"(output not complete: a program {Path.GetFileName(executable)} started still holds it)"); + } + } + /// /// Below normal priority, so a long conversion on every core leaves room for the desktop and Filee's own input /// hook (#511); on an idle machine it is as fast. Programs it starts inherit it. A process that is already gone diff --git a/tests/Filee.Engines.Tests/OwnCopyTests.cs b/tests/Filee.Engines.Tests/OwnCopyTests.cs index cac063e11..1ae8eb6fc 100644 --- a/tests/Filee.Engines.Tests/OwnCopyTests.cs +++ b/tests/Filee.Engines.Tests/OwnCopyTests.cs @@ -122,9 +122,33 @@ public void LibreOffice_is_found_in_the_folder_its_installer_recorded() [InlineData("ghostscript", "10.04.0\r\n", "10.4")] [InlineData("libreoffice", "[Version]\nAllLanguages=en-US\nMsiProductVersion=25.2.4.3\n", "25.2")] [InlineData("libreoffice", "LibreOffice 26.2.6.2 98d5f8\n", "26.2")] + // LibreOffice 26.8's version.ini has no MsiProductVersion; Installed adds soffice.exe's product version to it. + [InlineData("libreoffice", "[Version]\nUpdateID=LibreOffice_26_en-US_de\nVendor=The Document Foundation\n", null)] + [InlineData("libreoffice", "[Version]\nUpdateID=LibreOffice_26_en-US_de\nVendor=The Document Foundation\nLibreOffice 26.8.1.1", "26.8")] public void Versions_are_read_from_each_program(string id, string output, string? expected) => Assert.Equal(expected is null ? null : Version.Parse(expected), CopyVersions.ParseVersion(id, output)); + [Fact] + public void A_libreoffice_without_msi_version_is_told_by_its_program() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "version.ini is read on Windows only"); + // Any program with a version resource stands in for soffice.exe. + var program = Environment.ProcessPath!; + var folder = Directory.CreateTempSubdirectory("filee-lo-version-").FullName; + try + { + File.WriteAllText(Path.Combine(folder, "version.ini"), "[Version]\nVendor=The Document Foundation\n"); + File.Copy(program, Path.Combine(folder, "soffice.exe")); + var product = System.Diagnostics.FileVersionInfo.GetVersionInfo(program); + + Assert.Equal(new Version(product.ProductMajorPart, product.ProductMinorPart), CopyVersions.Installed("libreoffice", folder)); + } + finally + { + Directory.Delete(folder, true); + } + } + [Fact] public void Expected_versions_come_from_engines_json() => Assert.NotNull(CopyVersions.Expected("ffmpeg")); diff --git a/tests/Filee.Engines.Tests/ProcessRunnerOutputTests.cs b/tests/Filee.Engines.Tests/ProcessRunnerOutputTests.cs index a4a0e6cb6..017ef2693 100644 --- a/tests/Filee.Engines.Tests/ProcessRunnerOutputTests.cs +++ b/tests/Filee.Engines.Tests/ProcessRunnerOutputTests.cs @@ -1,5 +1,6 @@ -// The output probe for a program that leaves a child holding Filee's output pipes (#1011): soffice.exe exits, but -// soffice.bin keeps the inherited pipes open, so waiting for the end of the output lasts until the timeout. +// A program that leaves a child holding Filee's output pipes (#1011): soffice.exe exits, but soffice.bin keeps the +// inherited pipes open, so waiting for the end of the output would last until the timeout. And the output probe, for +// a program that finished its file but doesn't exit. using System.Diagnostics; using Filee.Engines.Infrastructure; @@ -17,18 +18,16 @@ public sealed class ProcessRunnerOutputTests : IDisposable public void Dispose() => Directory.Delete(_root, recursive: true); [Fact] - public async Task A_finished_PDF_ends_the_run_and_the_child_that_holds_the_output() + public async Task A_finished_PDF_ends_a_run_that_goes_on() { Assert.SkipUnless(OperatingSystem.IsWindows(), "cmd.exe and a Job Object."); - // The child writes its process id and waits a minute with the script's output pipes, which it inherits like - // soffice.bin; the script writes a complete PDF and exits. + // The script writes a complete PDF and then waits a minute, like a soffice.exe that doesn't exit. var script = Script(""" @echo off - start /b %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe -NoProfile -NonInteractive -Command "[IO.File]::WriteAllText('%~dp0child.pid', $PID); Start-Sleep -Seconds 60" >"%~dp0out\out.pdf" echo %%PDF-1.4 >>"%~dp0out\out.pdf" echo %%%%EOF + ping -n 60 127.0.0.1 >nul """); - var pidFile = Path.Combine(_root, "child.pid"); var complete = LibreOfficeConverter.OutputComplete(Output); var held = 0; var started = Stopwatch.StartNew(); @@ -36,15 +35,42 @@ @echo off var result = await ProcessRunner.RunAsync(Cmd, ["/c", script], TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken, outputReady: () => { - var ready = File.Exists(pidFile) && complete(); + var ready = complete(); if (ready) held++; return ready; }); Assert.Equal(0, result.ExitCode); - Assert.True(held >= 4, $"the run ended by itself after {held} ready polls: the child did not hold the output"); + Assert.True(held >= 4, $"the run ended by itself after {held} ready polls"); + Assert.True(started.Elapsed < TimeSpan.FromSeconds(20), $"took {started.Elapsed}"); + } + + [Fact] + public async Task A_child_that_holds_the_output_ends_with_the_run() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "cmd.exe and a Job Object."); + // The child writes its process id and waits a minute with the script's output pipes, which it inherits like + // soffice.bin; the script writes a complete PDF and exits. + var script = Script(""" + @echo off + start /b %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe -NoProfile -NonInteractive -Command "[IO.File]::WriteAllText('%~dp0child.pid', $PID); Start-Sleep -Seconds 60" + >"%~dp0out\out.pdf" echo %%PDF-1.4 + >>"%~dp0out\out.pdf" echo %%%%EOF + """); + var pidFile = Path.Combine(_root, "child.pid"); + var complete = LibreOfficeConverter.OutputComplete(Output); + var started = Stopwatch.StartNew(); + + var result = await ProcessRunner.RunAsync(Cmd, ["/c", script], TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken, + outputReady: () => File.Exists(pidFile) && complete()); + + Assert.Equal(0, result.ExitCode); + // Under the 30 s the rest of the output may take: the job ended the child, so the output ended too. Assert.True(started.Elapsed < TimeSpan.FromSeconds(20), $"took {started.Elapsed}"); + Assert.DoesNotContain("output not complete", result.StandardError); + if (!File.Exists(pidFile)) + return; // The script exited before the child had started; the job ended it all the same. var pid = int.Parse(File.ReadAllText(pidFile).Trim()); try {