From d063d91086702ba8efcd0f9a1a3aa817409b2b67 Mon Sep 17 00:00:00 2001 From: vpoguru Date: Wed, 23 Sep 2026 14:05:04 +0200 Subject: [PATCH 1/2] Fix ODF loading through symlinked OrganDefinitions on macOS Native macOS file selection resolves paths through a symlinked OrganDefinitions directory before returning the selected ODF. Restore the logical ODF path through known Hauptwerk library roots before deriving the organ root. This preserves the relationship between OrganDefinitions and OrganInstallationPackages and also ensures that the logical path is stored as the last opened organ. Add regression coverage for a resolved ODF path underneath a symlinked OrganDefinitions directory. --- src/mp_audio/MasterpieceProcessor.cpp | 36 +++++++++++++----- src/mp_core/OdfLoader.cpp | 54 +++++++++++++++++++++++++++ src/mp_core/OdfLoader.h | 8 ++++ tests/test_core.cpp | 7 ++++ 4 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/mp_audio/MasterpieceProcessor.cpp b/src/mp_audio/MasterpieceProcessor.cpp index 45edd27..11bb643 100644 --- a/src/mp_audio/MasterpieceProcessor.cpp +++ b/src/mp_audio/MasterpieceProcessor.cpp @@ -2507,7 +2507,6 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( // symlinks -- OrganDefinitions or OrganInstallationPackages relocated onto // another drive -- where the plain parent walk can land somewhere that no // longer has OrganInstallationPackages beside it. - const juce::File root(mp::deriveOrganRoot(odfFile.getFullPathName().toStdString())); // What this organ was last set to. Has to happen before a byte of audio is // read: the resident format, streaming and the preload head all decide how @@ -2528,7 +2527,25 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( p->setValueNotifyingHost(p->convertTo0to1(1.0f)); loadGlobalDefaults(); - loadSettingsFor(odfFile); + seedSampleLibraries(); + + std::vector libraryRoots; + libraryRoots.reserve(libraries_.size()); + + for (const auto& library : libraries_) + libraryRoots.push_back( + library.getFullPathName().toStdString()); + + const juce::File effectiveOdf( + mp::restoreLogicalOdfPath( + odfFile.getFullPathName().toStdString(), + libraryRoots)); + + const juce::File root( + mp::deriveOrganRoot( + effectiveOdf.getFullPathName().toStdString())); + + loadSettingsFor(effectiveOdf); OdfLoader loader; OdfLoader::Options opts; @@ -2544,8 +2561,8 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( } OrganModel loaded; - if (!loader.load(odfFile.getFullPathName().toStdString(), opts, loaded, - result.diagnostics)) { + if (!loader.load(effectiveOdf.getFullPathName().toStdString(), + opts, loaded, result.diagnostics)) { result.error = result.diagnostics.errors.empty() ? "the organ definition could not be parsed" : result.diagnostics.errors.front(); @@ -2576,7 +2593,7 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( model_ = std::move(loaded); organRootDir_ = opts.organRootDir; - loadedOdf_ = odfFile; + loadedOdf_ = effectiveOdf; // Console click -> stop. Without this a drawstop would move on screen and // the organ would stay silent, which is the worst of both. @@ -2888,9 +2905,10 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( samples_.setCacheDir(cacheDirectory().getFullPathName().toStdString()); samples_.setCacheIdentity( organKey(), - odfFile.getFullPathName().toStdString() + "|" + - std::to_string(odfFile.getSize()) + "|" + - std::to_string(odfFile.getLastModificationTime().toMilliseconds())); + effectiveOdf.getFullPathName().toStdString() + "|" + + std::to_string(effectiveOdf.getSize()) + "|" + + std::to_string( + effectiveOdf.getLastModificationTime().toMilliseconds())); result.samples = samples_.loadAll(model_, opts.organRootDir, head, LoopSelection::Longest, &loadProgress_, @@ -2971,7 +2989,7 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( // Only now, having got this far: an organ that failed to load is not one // worth reopening on the next start. - setLastOrgan(odfFile); + setLastOrgan(effectiveOdf); // And the library it came from, so a definition moved away from its audio // later can still be matched to it. if (!graphicsOnly) rememberSampleLibrary(juce::File(organRootDir_)); diff --git a/src/mp_core/OdfLoader.cpp b/src/mp_core/OdfLoader.cpp index 3e24e15..d163f3e 100644 --- a/src/mp_core/OdfLoader.cpp +++ b/src/mp_core/OdfLoader.cpp @@ -2021,6 +2021,60 @@ bool hasInstallationPackages(const std::filesystem::path& root) { } // namespace +std::string restoreLogicalOdfPath( + const std::string& odfPath, + const std::vector& libraryRoots) { + namespace fs = std::filesystem; + + std::error_code ec; + const fs::path selected = + fs::weakly_canonical(fs::path(odfPath), ec); + + if (ec) return odfPath; + + for (const auto& root : libraryRoots) { + const fs::path logicalDefinitions = + fs::path(root) / "OrganDefinitions"; + + ec.clear(); + if (!fs::is_directory(logicalDefinitions, ec)) + continue; + + ec.clear(); + const fs::path physicalDefinitions = + fs::weakly_canonical(logicalDefinitions, ec); + + if (ec) + continue; + + const fs::path relative = + selected.lexically_relative(physicalDefinitions); + + if (relative.empty() || relative.is_absolute()) + continue; + + bool escapes = false; + for (const auto& part : relative) { + if (part == "..") { + escapes = true; + break; + } + } + + if (escapes) + continue; + + const fs::path candidate = + logicalDefinitions / relative; + + ec.clear(); + if (fs::is_regular_file(candidate, ec)) + return candidate.string(); + } + + return odfPath; +} + std::string findLibraryHolding(const std::vector& roots, const OrganModel& model) { std::vector wanted; diff --git a/src/mp_core/OdfLoader.h b/src/mp_core/OdfLoader.h index 4eaaf1d..b99db81 100644 --- a/src/mp_core/OdfLoader.h +++ b/src/mp_core/OdfLoader.h @@ -91,6 +91,14 @@ class OdfLoader { OdfDiagnostics& outDiag); }; +// A native file chooser may resolve a symlinked OrganDefinitions directory +// before returning the selected file. If the physical file belongs underneath +// the OrganDefinitions directory of one of the known library roots, rebuild +// the equivalent path through that logical directory. +std::string restoreLogicalOdfPath( + const std::string& odfPath, + const std::vector& libraryRoots); + // Given the path to a *.Organ_Hauptwerk_xml (or *.CustomOrgan_Hauptwerk_xml) // file, return the directory OrganInstallationPackages and OrganDefinitions // hang off. Ordinarily that is just the ODF's grandparent — / diff --git a/tests/test_core.cpp b/tests/test_core.cpp index 8172026..e8d4c6d 100644 --- a/tests/test_core.cpp +++ b/tests/test_core.cpp @@ -6164,6 +6164,13 @@ class SymlinkedOrganTest final : public mp::test::Test { // Through the resolved path it cannot, and must not invent one. const fs::path resolved = defsElsewhere / "test.Organ_Hauptwerk_xml"; + const std::string restored = + mp::restoreLogicalOdfPath(resolved.string(), {setRoot.string()}); + + MP_CHECK(fs::path(restored).lexically_normal() == + throughLink.lexically_normal(), + "layout D: a resolved ODF path must be restored through the " + "known OrganDefinitions symlink"); const std::string derived = mp::deriveOrganRoot(resolved.string()); MP_CHECK(!fs::is_directory(fs::path(derived) / "OrganInstallationPackages", ec), "layout D: a resolved path genuinely has no packages to find"); From 48333fe0a8ed0b3c7581831a9348619ac798bbe6 Mon Sep 17 00:00:00 2001 From: Bonni Date: Wed, 23 Sep 2026 19:43:31 -0300 Subject: [PATCH 2/2] Keep the organ-root comment beside the code it describes The root is now derived after the logical path is restored, so the comment explaining deriveOrganRoot moves down to that line, and the restoration gets a comment of its own. Wrapping follows the rest of the file. No change in behaviour. --- src/mp_audio/MasterpieceProcessor.cpp | 41 +++++++++++++-------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/mp_audio/MasterpieceProcessor.cpp b/src/mp_audio/MasterpieceProcessor.cpp index 11bb643..f62dedc 100644 --- a/src/mp_audio/MasterpieceProcessor.cpp +++ b/src/mp_audio/MasterpieceProcessor.cpp @@ -2500,14 +2500,6 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( return result; } - // A Hauptwerk set puts its definitions in /OrganDefinitions and its - // audio in /OrganInstallationPackages, so the root is the definition's - // grandparent. deriveOrganRoot (mp_core, shared with the loader so the two - // never disagree) also copes with a set that has been reorganised with - // symlinks -- OrganDefinitions or OrganInstallationPackages relocated onto - // another drive -- where the plain parent walk can land somewhere that no - // longer has OrganInstallationPackages beside it. - // What this organ was last set to. Has to happen before a byte of audio is // read: the resident format, streaming and the preload head all decide how // the samples are read and cannot be changed afterwards. @@ -2529,21 +2521,27 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( loadGlobalDefaults(); seedSampleLibraries(); + // The path the definition is known by. A native file chooser can hand back + // a symlinked OrganDefinitions already resolved, which loses the folder its + // packages sit beside; when the file lies under a known library's linked + // OrganDefinitions, the path is rebuilt through that link. Everything below + // -- the root, the settings, the cache and the organ to reopen -- uses it. std::vector libraryRoots; libraryRoots.reserve(libraries_.size()); - for (const auto& library : libraries_) - libraryRoots.push_back( - library.getFullPathName().toStdString()); - - const juce::File effectiveOdf( - mp::restoreLogicalOdfPath( - odfFile.getFullPathName().toStdString(), - libraryRoots)); + libraryRoots.push_back(library.getFullPathName().toStdString()); + const juce::File effectiveOdf(mp::restoreLogicalOdfPath( + odfFile.getFullPathName().toStdString(), libraryRoots)); + // A Hauptwerk set puts its definitions in /OrganDefinitions and its + // audio in /OrganInstallationPackages, so the root is the definition's + // grandparent. deriveOrganRoot (mp_core, shared with the loader so the two + // never disagree) also copes with a set that has been reorganised with + // symlinks -- OrganDefinitions or OrganInstallationPackages relocated onto + // another drive -- where the plain parent walk can land somewhere that no + // longer has OrganInstallationPackages beside it. const juce::File root( - mp::deriveOrganRoot( - effectiveOdf.getFullPathName().toStdString())); + mp::deriveOrganRoot(effectiveOdf.getFullPathName().toStdString())); loadSettingsFor(effectiveOdf); @@ -2561,8 +2559,8 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( } OrganModel loaded; - if (!loader.load(effectiveOdf.getFullPathName().toStdString(), - opts, loaded, result.diagnostics)) { + if (!loader.load(effectiveOdf.getFullPathName().toStdString(), opts, loaded, + result.diagnostics)) { result.error = result.diagnostics.errors.empty() ? "the organ definition could not be parsed" : result.diagnostics.errors.front(); @@ -2907,8 +2905,7 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( organKey(), effectiveOdf.getFullPathName().toStdString() + "|" + std::to_string(effectiveOdf.getSize()) + "|" + - std::to_string( - effectiveOdf.getLastModificationTime().toMilliseconds())); + std::to_string(effectiveOdf.getLastModificationTime().toMilliseconds())); result.samples = samples_.loadAll(model_, opts.organRootDir, head, LoopSelection::Longest, &loadProgress_,