From 4f9a19bcf4d59cd03ff39c296121af3fb2dd0401 Mon Sep 17 00:00:00 2001 From: gmipf Date: Sun, 12 Jul 2026 18:58:56 +0200 Subject: [PATCH 1/3] Root the default output path outside Windows DefaultOutputPath defaults to the relative "ISO". A relative path resolves against the working directory, not against the application directory. That is exactly right for the portable Windows layout, where the folder ends up next to the executable. An installed application has no folder next to the executable. The default then sends dumps wherever MPF happened to be started from, and the browse handler opens there as well, since it runs the value through Path.GetFullPath(). Launched from a desktop entry, the output path field reads ISO/track_20260712-1843/track_20260712-1843.bin and the dump lands in the user's home directory, or in / for a system service. Give the default the same platform-dependent shape the tool paths already have (DefaultAaruPath and friends): Windows keeps "ISO" unchanged, everything else gets $HOME/ISO. MPF creates the directory itself, so it need not exist. Both places that carry the default are updated; the loader fallback alone would leave the property initializer relative. Verified on Fedora 44 against the packaged 3.8.3 build. --- MPF.Frontend.Test/OptionsTests.cs | 50 +++++++++++++++++++++++++++++ MPF.Frontend/Options.cs | 34 ++++++++++++++++++-- MPF.Frontend/Tools/OptionsLoader.cs | 2 +- 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 MPF.Frontend.Test/OptionsTests.cs diff --git a/MPF.Frontend.Test/OptionsTests.cs b/MPF.Frontend.Test/OptionsTests.cs new file mode 100644 index 000000000..488b3e018 --- /dev/null +++ b/MPF.Frontend.Test/OptionsTests.cs @@ -0,0 +1,50 @@ +using System; +using System.IO; +using Xunit; + +namespace MPF.Frontend.Test +{ + public class OptionsTests + { + /// + /// The default output path must not be relative anywhere but Windows + /// + /// + /// A relative path resolves against the working directory, not against the application + /// directory. That is what the portable Windows layout wants, where the folder sits next to + /// the executable. An installed application has no such folder, so a relative default sends + /// dumps wherever MPF happened to be started from. + /// + [Fact] + public void DefaultOutputPath_OutsideWindows_IsRooted() + { + var options = new Options(); + + string? actual = options.Dumping.DefaultOutputPath; + + if (Environment.OSVersion.Platform == PlatformID.Unix + || Environment.OSVersion.Platform == PlatformID.MacOSX) + Assert.True(Path.IsPathRooted(actual), $"Expected a rooted path, found '{actual}'"); + else + Assert.Equal("ISO", actual); + } + + /// + /// Clearing the default output path restores the platform default, not a relative one + /// + [Fact] + public void DefaultOutputPath_SetToNull_RestoresPlatformDefault() + { + var options = new Options(); + + options.Dumping.DefaultOutputPath = null; + + string? actual = options.Dumping.DefaultOutputPath; + if (Environment.OSVersion.Platform == PlatformID.Unix + || Environment.OSVersion.Platform == PlatformID.MacOSX) + Assert.True(Path.IsPathRooted(actual), $"Expected a rooted path, found '{actual}'"); + else + Assert.Equal("ISO", actual); + } + } +} diff --git a/MPF.Frontend/Options.cs b/MPF.Frontend/Options.cs index b3d69147f..2fd742128 100644 --- a/MPF.Frontend/Options.cs +++ b/MPF.Frontend/Options.cs @@ -281,6 +281,36 @@ internal static string DefaultRedumperPath } } + /// + /// Default output path for dumps + /// + /// + /// Windows keeps the portable layout, where the relative path lands in a folder next to the + /// executable. An installed application has no such folder: the path resolves against the + /// working directory instead, so dumps land wherever MPF happened to be started from. + /// + [JsonIgnore] + internal static string DefaultOutputPathValue + { + get + { +#if NET20 || NET35 + return "ISO"; +#else + bool portable = Environment.OSVersion.Platform != PlatformID.Unix + && Environment.OSVersion.Platform != PlatformID.MacOSX; + if (portable) + return "ISO"; + + string homeDir = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrEmpty(homeDir)) + return "ISO"; + + return Path.Combine(homeDir, "ISO"); +#endif + } + } + #endregion #region Internal Program @@ -350,9 +380,9 @@ public string? DefaultOutputPath get; set { - field = value ?? "ISO"; + field = value ?? DefaultOutputPathValue; } - } = "ISO"; + } = DefaultOutputPathValue; /// /// Default system if none can be detected diff --git a/MPF.Frontend/Tools/OptionsLoader.cs b/MPF.Frontend/Tools/OptionsLoader.cs index 7a9ddb7d3..496be895e 100644 --- a/MPF.Frontend/Tools/OptionsLoader.cs +++ b/MPF.Frontend/Tools/OptionsLoader.cs @@ -386,7 +386,7 @@ private static Options ConvertFromDictionary(Dictionary? source options.Dumping.DreamdumpPath = GetStringSetting(source, "DreamdumpPath", DumpSettings.DefaultDreamdumpPath) ?? DumpSettings.DefaultDreamdumpPath; options.Dumping.RedumperPath = GetStringSetting(source, "RedumperPath", DumpSettings.DefaultRedumperPath) ?? DumpSettings.DefaultRedumperPath; - options.Dumping.DefaultOutputPath = GetStringSetting(source, "DefaultOutputPath", "ISO"); + options.Dumping.DefaultOutputPath = GetStringSetting(source, "DefaultOutputPath", DumpSettings.DefaultOutputPathValue); valueString = GetStringSetting(source, "DefaultSystem", PhysicalSystem.IBMPCcompatible.ToString()); options.Dumping.DefaultSystem = (valueString ?? string.Empty).ToPhysicalSystem(); options.Dumping.DumpSpeeds.CD = GetInt32Setting(source, "PreferredDumpSpeedCD", 24); From 1e38f3fda297ce96f0fec723f442a40c3ab812b4 Mon Sep 17 00:00:00 2001 From: gmipf Date: Mon, 13 Jul 2026 12:30:34 +0200 Subject: [PATCH 2/3] Root the default output path only when the current folder is unwritable Replaces the platform-dependent default with the shape requested in review: the relative "ISO" stays the default on every platform, and the path is only rooted in the home directory when a file cannot be created in the current folder. This mirrors GetConfigurationPath, which probes the portable location first and moves elsewhere when writing fails. Co-Authored-By: Claude Opus 4.8 --- CHANGELIST.md | 1 + MPF.Frontend.Test/OptionsTests.cs | 73 ++++++++++++++++++++--------- MPF.Frontend/Options.cs | 76 +++++++++++++++++++++++++------ 3 files changed, 113 insertions(+), 37 deletions(-) diff --git a/CHANGELIST.md b/CHANGELIST.md index eace94f8c..f69832e08 100644 --- a/CHANGELIST.md +++ b/CHANGELIST.md @@ -44,6 +44,7 @@ - Fix resizing of main window in WPF - Use placeholder for media type for Generic processor - Use input file as only output file in generic processor +- Root the default output path when the current folder is unwritable (gmipf) ### 3.8.3 (2026-07-06) diff --git a/MPF.Frontend.Test/OptionsTests.cs b/MPF.Frontend.Test/OptionsTests.cs index 488b3e018..4ae5d769e 100644 --- a/MPF.Frontend.Test/OptionsTests.cs +++ b/MPF.Frontend.Test/OptionsTests.cs @@ -1,5 +1,5 @@ -using System; using System.IO; +using SabreTools.IO; using Xunit; namespace MPF.Frontend.Test @@ -7,44 +7,73 @@ namespace MPF.Frontend.Test public class OptionsTests { /// - /// The default output path must not be relative anywhere but Windows + /// A writable current directory keeps the relative default + /// + [Fact] + public void GetDefaultOutputPath_WritableDirectory_ReturnsRelativePath() + { + string currentDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(currentDirectory); + + try + { + string actual = DumpSettings.GetDefaultOutputPath(currentDirectory); + Assert.Equal("ISO", actual); + } + finally + { + Directory.Delete(currentDirectory); + } + } + + /// + /// A current directory that can not hold a file falls back to the home directory /// /// - /// A relative path resolves against the working directory, not against the application - /// directory. That is what the portable Windows layout wants, where the folder sits next to - /// the executable. An installed application has no such folder, so a relative default sends - /// dumps wherever MPF happened to be started from. + /// A file stands in for the directory because that is the only way of making the creation + /// fail on every platform. Denied permissions end up in the same branch. /// [Fact] - public void DefaultOutputPath_OutsideWindows_IsRooted() + public void GetDefaultOutputPath_UnwritableDirectory_ReturnsHomePath() { - var options = new Options(); + string currentDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + File.Create(currentDirectory).Dispose(); - string? actual = options.Dumping.DefaultOutputPath; + try + { + string actual = DumpSettings.GetDefaultOutputPath(currentDirectory); + Assert.Equal(Path.Combine(PathTool.GetHomeDirectory(), "ISO"), actual); + } + finally + { + File.Delete(currentDirectory); + } + } - if (Environment.OSVersion.Platform == PlatformID.Unix - || Environment.OSVersion.Platform == PlatformID.MacOSX) - Assert.True(Path.IsPathRooted(actual), $"Expected a rooted path, found '{actual}'"); - else - Assert.Equal("ISO", actual); + /// + /// A missing current directory falls back to the home directory + /// + [Fact] + public void GetDefaultOutputPath_MissingDirectory_ReturnsHomePath() + { + string currentDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + string actual = DumpSettings.GetDefaultOutputPath(currentDirectory); + + Assert.Equal(Path.Combine(PathTool.GetHomeDirectory(), "ISO"), actual); } /// - /// Clearing the default output path restores the platform default, not a relative one + /// Clearing the default output path restores the default instead of leaving it empty /// [Fact] - public void DefaultOutputPath_SetToNull_RestoresPlatformDefault() + public void DefaultOutputPath_SetToNull_RestoresDefault() { var options = new Options(); options.Dumping.DefaultOutputPath = null; - string? actual = options.Dumping.DefaultOutputPath; - if (Environment.OSVersion.Platform == PlatformID.Unix - || Environment.OSVersion.Platform == PlatformID.MacOSX) - Assert.True(Path.IsPathRooted(actual), $"Expected a rooted path, found '{actual}'"); - else - Assert.Equal("ISO", actual); + Assert.Equal(DumpSettings.DefaultOutputPathValue, options.Dumping.DefaultOutputPath); } } } diff --git a/MPF.Frontend/Options.cs b/MPF.Frontend/Options.cs index 7bb1cfea7..3946d1088 100644 --- a/MPF.Frontend/Options.cs +++ b/MPF.Frontend/Options.cs @@ -1,6 +1,7 @@ using System; using System.IO; using Newtonsoft.Json; +using SabreTools.IO; using SabreTools.RedumpLib.Data; using AaruDumpSettings = MPF.ExecutionContexts.Aaru.DumpSettings; using DiscImageCreatorDumpSettings = MPF.ExecutionContexts.DiscImageCreator.DumpSettings; @@ -281,34 +282,79 @@ internal static string DefaultRedumperPath } } + /// + /// Default output directory name + /// + private const string DefaultOutputDirectoryName = "ISO"; + /// /// Default output path for dumps /// /// - /// Windows keeps the portable layout, where the relative path lands in a folder next to the - /// executable. An installed application has no such folder: the path resolves against the - /// working directory instead, so dumps land wherever MPF happened to be started from. + /// The relative path is kept wherever the current directory can be written to, which is the + /// portable layout. It only resolves to the home directory when the current directory can + /// not be written to, such as an installed copy launched from a read-only location. /// [JsonIgnore] internal static string DefaultOutputPathValue { get { -#if NET20 || NET35 - return "ISO"; -#else - bool portable = Environment.OSVersion.Platform != PlatformID.Unix - && Environment.OSVersion.Platform != PlatformID.MacOSX; - if (portable) - return "ISO"; + if (field is not null) + return field; + + field = GetDefaultOutputPath(Environment.CurrentDirectory); + return field; + } + } - string homeDir = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - if (string.IsNullOrEmpty(homeDir)) - return "ISO"; + /// + /// Attempt to determine the default output path + /// + /// Directory that a relative path would resolve against + internal static string GetDefaultOutputPath(string currentDirectory) + { + // Portable output + if (CanCreateFile(currentDirectory)) + return DefaultOutputDirectoryName; - return Path.Combine(homeDir, "ISO"); -#endif + // User output + string homeDir = PathTool.GetHomeDirectory(); + if (!string.IsNullOrEmpty(homeDir)) + return Path.Combine(homeDir, DefaultOutputDirectoryName); + + // This should not happen + return DefaultOutputDirectoryName; + } + + /// + /// Indicates if a new file can be created in a directory + /// + /// Creating a file is the only portable way of asking; the file is removed again + private static bool CanCreateFile(string directory) + { + if (string.IsNullOrEmpty(directory)) + return false; + + string probePath = Path.Combine(directory, Path.GetRandomFileName()); + + // Only the creation decides, so that a failed cleanup does not read as unwritable + try + { + File.Create(probePath).Dispose(); } + catch + { + return false; + } + + try + { + File.Delete(probePath); + } + catch { } + + return true; } #endregion From 6ec71841274fde9bb532130ef2e7dbbe5bdbb1cc Mon Sep 17 00:00:00 2001 From: gmipf Date: Fri, 17 Jul 2026 20:42:39 +0200 Subject: [PATCH 3/3] Apply review feedback: inline the output-path writability check and hoist the constant - GetDefaultOutputPath now probes inline like OptionsLoader.GetConfigurationPath; CanCreateFile removed - DefaultOutputDirectoryName moved to the top of DumpSettings Co-Authored-By: Claude Opus 4.8 --- MPF.Frontend/Options.cs | 57 ++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/MPF.Frontend/Options.cs b/MPF.Frontend/Options.cs index 3946d1088..d1707f1f6 100644 --- a/MPF.Frontend/Options.cs +++ b/MPF.Frontend/Options.cs @@ -180,6 +180,11 @@ public Options(Options? source) /// public class DumpSettings { + /// + /// Default output directory name + /// + private const string DefaultOutputDirectoryName = "ISO"; + #region Default Paths /// @@ -282,11 +287,6 @@ internal static string DefaultRedumperPath } } - /// - /// Default output directory name - /// - private const string DefaultOutputDirectoryName = "ISO"; - /// /// Default output path for dumps /// @@ -314,9 +314,20 @@ internal static string DefaultOutputPathValue /// Directory that a relative path would resolve against internal static string GetDefaultOutputPath(string currentDirectory) { - // Portable output - if (CanCreateFile(currentDirectory)) - return DefaultOutputDirectoryName; + // Portable output -- keep the relative path wherever the current directory can hold a + // file. Creating one is the only portable way of asking; the probe is removed again, and + // only the creation decides so that a failed cleanup does not read as unwritable. + if (!string.IsNullOrEmpty(currentDirectory)) + { + try + { + string probePath = Path.Combine(currentDirectory, Path.GetRandomFileName()); + File.Create(probePath).Dispose(); + try { File.Delete(probePath); } catch { } + return DefaultOutputDirectoryName; + } + catch { } + } // User output string homeDir = PathTool.GetHomeDirectory(); @@ -327,36 +338,6 @@ internal static string GetDefaultOutputPath(string currentDirectory) return DefaultOutputDirectoryName; } - /// - /// Indicates if a new file can be created in a directory - /// - /// Creating a file is the only portable way of asking; the file is removed again - private static bool CanCreateFile(string directory) - { - if (string.IsNullOrEmpty(directory)) - return false; - - string probePath = Path.Combine(directory, Path.GetRandomFileName()); - - // Only the creation decides, so that a failed cleanup does not read as unwritable - try - { - File.Create(probePath).Dispose(); - } - catch - { - return false; - } - - try - { - File.Delete(probePath); - } - catch { } - - return true; - } - #endregion #region Internal Program