9.2.0 OTR generation crashes on macOS

[homeisfar]

I just tried a clean installation of Ship. The OTR generation step after the first launch crashes when choosing "yes" on version 9.2.0.

I downloaded 9.1.2 and was able to generate OTR files, choose the ROM, exit, then launch the 9.2.0 version and actually get into the game.

OS version macOS Tahoe 26.3.1 (a)

[koplas]

Do you have a crash log, or at least stacktrace?

[homeisfar]

I don't have time to make generate a crash log, but I have an observation: After deleting ~/Library/Application Support/com.shipofharkinian.soh and running soh through the terminal (./soh.app/Contents/MacOS/soh) I did not experience a crash and soh created files in my pwd.

When. launched the .app from Finder AND when I instead pre-created the com.shipofharkinian.soh directory myself it also ran normally. That would probably be enough info for me to contribute a patch myself, but I won't have a chance to look until after the workday.

[homeisfar]

So I realized that probably the relevant changes should go into libultraship. First, here is the backtrace

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x0000000199f955b0 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x0000000199fcf888 libsystem_pthread.dylib`pthread_kill + 296
    frame #2: 0x0000000199ed4850 libsystem_c.dylib`abort + 124
    frame #3: 0x0000000199f83858 libc++abi.dylib`__abort_message + 132
    frame #4: 0x0000000199f724bc libc++abi.dylib`demangling_terminate_handler() + 280
    frame #5: 0x0000000199b9c424 libobjc.A.dylib`_objc_terminate() + 172
    frame #6: 0x0000000199f82c2c libc++abi.dylib`std::__terminate(void (*)()) + 16
    frame #7: 0x0000000199f86394 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 88
    frame #8: 0x0000000199f8633c libc++abi.dylib`__cxa_throw + 92
    frame #9: 0x0000000199f3e6f8 libc++.1.dylib`void std::__1::__fs::filesystem::__throw_filesystem_error[abi:ne200100]<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&, std::__1::__fs::filesystem::path const&, std::__1::error_code const&>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&, std::__1::__fs::filesystem::path const&, std::__1::error_code const&) + 92
    frame #10: 0x0000000199f3efd4 libc++.1.dylib`std::__1::__fs::filesystem::detail::ErrorHandler<void>::report(std::__1::error_code const&) const + 164
    frame #11: 0x0000000199f3eefc libc++.1.dylib`std::__1::__fs::filesystem::directory_iterator::directory_iterator(std::__1::__fs::filesystem::path const&, std::__1::error_code*, std::__1::__fs::filesystem::directory_options) + 212
    frame #12: 0x0000000100a1e5dc soh`std::__1::__fs::filesystem::directory_iterator::directory_iterator[abi:ne200100](this=0x000000016f935ca8, __p=0x000000016f935c90) at directory_iterator.h:52:72
    frame #13: 0x0000000102682328 soh`Extractor::GetRoms(this=0x000000016f936bb8, roms=size=0) at Extract.cpp:271:29
    frame #14: 0x00000001028fb304 soh`OTRGlobals::RunExtract(this=0x000000010a8bc970, argc=1, argv=0x000000016f937a40) at OTRGlobals.cpp:631:33
    frame #15: 0x0000000102901cd8 soh`InitOTR(argc=1, argv=0x000000016f937a40) at OTRGlobals.cpp:1460:27
    frame #16: 0x0000000102fbc940 soh`main(argc=1, argv=0x000000016f937a40) at main.c:63:5
    frame #17: 0x0000000199c05d54 dyld`start + 7184

Creating the proper application directory in ~/Library/Application Support/ for some reason counts as a valid directory for NSString *bundlePath = [[NSBundle mainBundle] resourcePath]; (I'm not a macOS expert to explain why this works, because resourcePath is in the .app itself, not Application Support). I figured it would be best to to just create that Support directory before the point it is first used. Presumably doing that would help 2ship as well, but I didn't test 2ship.

Here is my patch, but again all these changes were made in libultraship.

diff --git a/include/ship/utils/AppleFolderManager.h b/include/ship/utils/AppleFolderManager.h
index 904b6ec..9629f7f 100644
--- a/include/ship/utils/AppleFolderManager.h
+++ b/include/ship/utils/AppleFolderManager.h
@@ -56,6 +56,7 @@ class FolderManager {
     FolderManager();
     ~FolderManager();
 
+    void createAppSupportDirectory(const char* appName);
     const char* getMainBundlePath();
     const char* pathForDirectory(SearchPathDirectory directory, SearchPathDomainMask domainMask);
     const char* pathForDirectoryAppropriateForItemAtPath(SearchPathDirectory directory, SearchPathDomainMask domainMask,
diff --git a/src/ship/Context.cpp b/src/ship/Context.cpp
index 88ab182..4c9fa63 100644
--- a/src/ship/Context.cpp
+++ b/src/ship/Context.cpp
@@ -1,5 +1,6 @@
 #include "ship/Context.h"
 #include "ship/controller/controldevice/controller/mapping/keyboard/KeyboardScancodes.h"
+#include <cstring>
 #include <iostream>
 #include <spdlog/sinks/rotating_file_sink.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
@@ -468,7 +469,10 @@ std::string Context::GetAppDirectoryPath(std::string appName) {
 #endif
 
 #if defined(__APPLE__)
+    FolderManager foldermanager;
     if (char* fpath = std::getenv("SHIP_HOME")) {
+        char* applicationId = strrchr(fpath, '/') + 1;
+        foldermanager.createAppSupportDirectory(applicationId);
         if (fpath[0] == '~') {
             const char* home = getenv("HOME") ? getenv("HOME") : getpwuid(getuid())->pw_dir;
             return std::string(home) + std::string(fpath).substr(1);
diff --git a/src/ship/utils/AppleFolderManager.mm b/src/ship/utils/AppleFolderManager.mm
index ac82da3..ccd755c 100644
--- a/src/ship/utils/AppleFolderManager.mm
+++ b/src/ship/utils/AppleFolderManager.mm
@@ -18,6 +18,20 @@
     [(NSAutoreleasePool *)m_autoreleasePool release];
 }
 
+void FolderManager::createAppSupportDirectory(const char * appName) {
+    NSString *appNameString = [NSString stringWithUTF8String:appName];
+    NSFileManager *fileManager = [NSFileManager defaultManager];
+    NSString *appSupportDirectory = [NSSearchPathForDirectoriesInDomains(NSSearchPathDirectory::NSApplicationSupportDirectory, Ship::NSUserDomainMask, YES) firstObject];
+    NSString *appSpecificDir = [appSupportDirectory stringByAppendingPathComponent:appNameString];
+
+    NSError *error = nil;
+    if (![fileManager fileExistsAtPath:appSpecificDir]) {
+        if (![fileManager createDirectoryAtPath:appSpecificDir withIntermediateDirectories:NO attributes:Nil error:&error]) {
+            NSLog(@"Error creating directory: %@", error);
+        }
+    }
+}
+
 const char * FolderManager::getMainBundlePath() {
     NSString *bundlePath = [[NSBundle mainBundle] resourcePath];
     return [bundlePath UTF8String];

If that looks satisfactory, I can open a PR in libultraship with these changes. As a note, this will not modify that behavior I mentioned earlier where opening Ship through the terminal will instead put files in the pwd from which ship gets invoked.

Final note, in the patch the bit of weirdness with NSSearchPathDirectory::NSApplicationSupportDirectory, Ship::NSUserDomainMask was to resolve ambiguity for the compiler with this kind of hybrid ObjC++ and C++.

[Malkierian]

I have to imagine it's something that changed with the permissions in our app packager between the two, since obviously it was working just fine without the manual folder creation. Isn't it always supposed to be automatic on MacOS's part?

[homeisfar]

Now that you say that, I think the more obvious starting point would have been for me to bisect for the change that caused the crash. I can do that later today when I have free time.

[homeisfar]

It's not a permissions problem.

Shipwright/soh/soh/OTRGlobals.cpp

Line 774 in 704ace8

CheckAndCreateModFolder();

In this commit the call to CheckAndCreateModFolder() was moved to OTRGlobals::RunExtract(), notably after the call to extract.GetRoms(args);

Since CheckAndCreateModFolder() would create the mod directory and any missing intermediate directory, it incidentally created ~/Library/Application Support/com.shipofharkinian.soh as if it called mkdir -p. In other words, before 704ace8 the directory com.shipofharkinian.soh/mods was created early enough to mask the problem of not deliberately creating com.shipofharkinian.soh.

So there are at least two fix options. One is my above proposed changes in libultraship (except instead of using strrchr I should just change it to use appShortName... didn't notice that at first), and another is to just move CheckAndCreateModFolder() earlier in the execution in soh. I'd be happy to open a PR, either way.

[Malkierian]

I'd like to do the latter right now, since I don't know how long it would take to get LUS stuff in (and we're already waiting on a couple of them for Ackbar Bravo).

[Malkierian]

Make sure to target develop-ackbar if you're going to make a PR.

[serprex]

re LUS proposal, make sure to check if strrchr returns nullptr. In that case probably applicationId = fpath is fine