fix(xcode.application): bundle external dylibs in macOS apps#7453
fix(xcode.application): bundle external dylibs in macOS apps#7453Akaps316 wants to merge 2 commits intoxmake-io:devfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Xcode application rule to improve the handling of dependent dynamic libraries and frameworks, ensuring they are correctly copied into the application bundle. The review feedback highlights that the accompanying test case hardcodes architecture and platform-specific paths, which limits portability. Additionally, the refactoring of the copy logic in the build rule was suggested to improve code clarity and maintainability by separating collection logic from file system operations.
Collect non-target shared library dependencies for macOS app bundles in addition to shared target and framework dependencies. Resolve declared shared links against merged linkdirs, reuse existing target/package library collection, and then walk the app binary's dylib dependencies to copy non-system dylibs into Contents/Frameworks. Add a regression test that builds an external dylib and verifies xcode.application bundles it into a macOS app.
96b07e4 to
5714bab
Compare
|
This method is somewhat of a hack and doesn't guarantee that all dylibs will be reliably bundled into the bundle. The fact that macOS also includes system dylibs under /Library/ and the Xcode built-in dylib (/Applications/Xcode.app/) system libraries is completely unreliable. Additionally, the extra At least for now, I cannot accept the current implementation. |
| local xmake = path.absolute(path.join(os.projectdir(), "build", "xmake")) | ||
| local xmake_program_dir = path.absolute(path.join(os.projectdir(), "xmake")) | ||
| os.setenv("XMAKE_PROGRAM_FILE", xmake) | ||
| os.setenv("XMAKE_PROGRAM_DIR", xmake_program_dir) |
| os.setenv("XMAKE_PROGRAM_DIR", xmake_program_dir) | ||
|
|
||
| os.execv(xmake, {"f", "-p", "macosx", "-a", arch, "-c"}) | ||
| os.execv(xmake, {"-vD"}) |
| and not libfile:startswith("/System/Library/") | ||
| end | ||
|
|
||
| local function _get_target_linkdirs(target) |
| return table.unique(linkdirs) | ||
| end | ||
|
|
||
| local function _get_target_linklibfiles(target) |
| local libfiles = {} | ||
| target_utils.get_target_libfiles(target, libfiles, target:targetfile(), {}) | ||
| table.join2(libfiles, _get_target_linklibfiles(target)) | ||
| local dependfiles = get_depend_libraries(target:targetfile(), { |
There was a problem hiding this comment.
Please refer to the implementation details in the installation guide.
The get_target_libfiles function has already called get_depend_libraries internally. Then, _get_target_linklibfiles performs a very time-consuming find_library function, which then calls get_depend_libraries again.
| local frameworks_to_copy = {} | ||
| local framework_targetfiles = {} | ||
| for _, dep in ipairs(target:orderdeps()) do | ||
| if dep:kind() == "shared" then |
There was a problem hiding this comment.
The handling of dependencies on non-framework dynamic libraries is also missing here.
|
Understood. The current approach is too heuristic and duplicates dependency scanning, I also see that this refactor is missing the clearer handling for direct I’m fine dropping this version rather than pushing the implementation |
Summary
Bundle non-target macOS dylibs into
xcode.applicationapp bundles inaddition to shared target and framework dependencies.
This fixes app bundles that link external or package-provided
.dylibfiles and otherwise end up with a binary that depends on
@rpath/...entries that are never copied into
Contents/Frameworks.Fixes #7452.
Changes
get_target_libfiles()linksagainst mergedlinkdirsto find non-target dylibsContents/FrameworksWhy
xcode.applicationalready performs partial app-bundle assembly by rewritingrpath and copying shared target/framework dependencies.
However, it did not bundle non-target dylibs linked through raw
add_linkdirs()/add_links()usage, leaving the generated.appincomplete relative to the app binary's load commands.
This patch makes that behavior consistent for macOS app bundles without
changing the separate duplicate-
LC_RPATHissue, which is intentionallyleft out of this change.
Testing
build/xmake l tests/run.lua macapp_external_dylib1 test(s) succeedtests/projects/objc/macapp_with_sharedtests/projects/objc/macapp_with_framework