App bundle arg, improved run-path search algorithm, handle frameworks #49
Conversation
| @@ -77,7 +87,7 @@ fixes dependencies where bundled libraries depend on each other. If this option | |||
| > If the output directory does not exist, create it. | |||
|
|
|||
| A command may look like | |||
| `% dylibbundler -od -b -x ./HelloWorld.app/Contents/MacOS/helloworld -d ./HelloWorld.app/Contents/libs/` | |||
| `% dylibbundler -cd -b -f -a ./HelloWorld.app -x ./HelloWorld.app/Contents/PlugIns/printsupport` | |||
There was a problem hiding this comment.
Not sure about the use of "PlugIns/printsupport" here, this might be needlessly specific and confusing. What was wrong with the previous example?
There was a problem hiding this comment.
to show that the -x flag can be used to add additional executable files in an app bundle (besides the primary executable and its LC_LOAD_DYLIB dependencies. The -a flag provides an alternative way to fix-up app bundles. While most simple apps may only need the -a flag with the location of the app bundle, more complicated app bundles may require additional binary files (plugins) fixed up by adding them with -x flags. I've added back your (excellent) example. Thank you for creating this very useful tool.
| @@ -107,72 +63,85 @@ Dependency::Dependency(std::string path) | |||
| } | |||
|
|
|||
| // check if given path is a symlink | |||
| if (original_file != rtrim(path)) | |||
There was a problem hiding this comment.
General note, I wish this had been submitted as a series of smaller pull requests. This pull request is so large it basically rewrites the entire app into a different product, that is exceedinly hard to review for me. I'm asking this without any sarcasm, if I merge this PR there's basically very little of my old code left and this would become your app basically, so would you actually be willing to take ownership of it and maintain it? I wrote this as a small utility years ago and no longer have time/interest to maintain it so TBH if you're interested in doing huge changes it might actually be simpler to actually make you maintainer of this app
There was a problem hiding this comment.
Sure, I'm willing to maintain if you'd like. I understand it's a relatively large PR, so how about creating a new branch for it while I work on breaking this pull request into smaller PRs?
There was a problem hiding this comment.
This way it can be more thoroughly tested and gain additional feedback/review.
There was a problem hiding this comment.
I've invited you as a collaborator, as I'm more than happy to have help on this. But yes, I like your idea of creating a branch and breaking it into smaller PRs. After all, it is good practice to keep commits fairly focused around one thing - makes things much easier to follow than huge commits that contain unrelated changes
There was a problem hiding this comment.
Thank you for the invitation. I'm more than happy to help.
There was a problem hiding this comment.
Ah, another suggestion : the Qt stuff seems quite mixed/intertwined with other code. It might be preferable to move Qt stuff to its own file. Mixing Qt-specific code within the generic bundling risks putting too much unrelated stuff in the same file
There was a problem hiding this comment.
@SCG82 We're using your fork on OBS for GitHub Actions CI right now and it works without issues, but when trying to bundle the app with self-compiled Qt Frameworks I run into errors. You have issues disabled on your fork, what would be the best way to reach out/report?
There was a problem hiding this comment.
I enabled issues on my fork. Thanks.
-a MyApp.app(or--app MyApp.app) automatically adds the main executable (in MyApp.app/Contents/MacOS/) to the list of files to fix up.MyApp.app/Contents/Frameworksand default install name prefix to@executable_path/../Frameworksas per Apple's app bundle structure guidelines. If the '-a' flag is not used, the default destination & install name prefix are set to./libs@executable_path/../libsrespectively (same as current defaults).-f, --frameworksbundles (copies and fixes) dependencies that are frameworks.MyApp.app/Contents/PlugIns/and output a qt.conf file inMyApp.app/Contents/Resources. Runningmacdeployqtis no longer needed.otool -lto gather dependent libraries (LC_LOAD_DYLIB) instead ofotool -Lbecause the latter output contains the ID and weakly linked dylibs.@rpath/instead of the default install name prefix.-q, --quietfor less verbose output