Multiple macOS fixes and include keepassxc-cli in DMG#2165
Multiple macOS fixes and include keepassxc-cli in DMG#2165droidmonkey merged 7 commits intokeepassxreboot:developfrom
Conversation
varjolintu
left a comment
There was a problem hiding this comment.
This will not work. You are just copying keepassxc-cli inside the application image but you are not fixing the linking. This needs a similar kind of fix keepassxc-proxy is using in the makefile.
When inspecting this copied keepassxc-cli via otool -L it shows the following linking to Qt's libraries (just one line is enough for example):
/usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore
While the correct one should be linking to the libraries inside the app image, as follows:
@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore
|
Thank you so much for fixing this stupid issue. As @varjolintu mentioned, you need to fix the linking, though, otherwise the proxy will just crash on other systems where there is no Qt installed under /usr/local/opt/qt |
Getting warmer. What other libraries need linking fixed too? |
|
@jtl999 Everything should be linked from the app image, including libgcrypt, libargon, libsodium etc.. only the basic libs (libc++ and libSystem) can be linked from outside. |
|
This looks better now |
|
When trying the I will fix that very soon. |
src/cli/CMakeLists.txt
Outdated
| add_custom_command(TARGET keepassxc-cli | ||
| POST_BUILD | ||
| COMMAND ${CMAKE_INSTALL_NAME_TOOL} | ||
| -change /usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore |
There was a problem hiding this comment.
You cannot use static paths for Qt libraries. User might have installed them manually or through MacPorts etc.. You can use Qt5_PREFIX instead:
-change ${Qt5_PREFIX}/lib/QtCore.framework/Versions/5/QtCore
"@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
There was a problem hiding this comment.
Where exactly is ${Qt5_PREFIX} defined? When I tried replacing /usr/local/opt/qt with ${Qt5_PREFIX} for each of the linking changes needed, built and checked the linking of the binary again, the linking was not changed from the static paths.
Or is ${Qt5_PREFIX} manually defined by the user?
There was a problem hiding this comment.
Also, which variable (if any) should I use to refer to the libsodium library?
There was a problem hiding this comment.
keepassxc/CMakeLists.txt:
310: get_filename_component(Qt5_PREFIX ${Qt5_DIR}/../../.. REALPATH)
Of course the path remains the same in your computer. For libsodium you could try using ${sodium_LIBRARY_DEBUG} and ${sodium_LIBRARY_RELEASE}.
There was a problem hiding this comment.
Thanks
I'm wondering the best way to go about this because currently the way I have the paths set in src/cli/CMakeLists.txt is like this.
-change ${sodium_LIBRARY_RELEASE}
"@executable_path/../Frameworks/libsodium.23.dylib"
-change /usr/local/opt/libsodium/lib/libsodium.23.dylib
"@executable_path/../Frameworks/libsodium.23.dylib"
According to build/CMakeLists.txt sodium_LIBRARY_RELEASE:FILEPATH=/usr/local/Cellar/libsodium/1.0.16/lib/libsodium.dylib doesn't match up with which was linked by default for libsodium in keepassxc-cli on my system.
Which is /usr/local/opt/libsodium/lib/libsodium.23.dylib (compatibility version 25.0.0, current version 25.0.0)
There was a problem hiding this comment.
I think this is the best way yes. Also with ${Qt5_PREFIX}. Looking the the proxy's CMakeList.txt, both prefix and fixed location is used to ensure it would work on different environments. libyubikey and libykpers should be also added to the list. At least for me otool shows them in the list also (using WITH_XC_ALL=ON).
For me this works OK:
Code block of app bundle
if(APPLE AND WITH_APP_BUNDLE)
set(CLI_BINARY_DIR "${CMAKE_BINARY_DIR}/src/cli/keepassxc-cli")
set(CLI_APP_DIR "KeePassXC.app/Contents/MacOS/keepassxc-cli")
list(GET YUBIKEY_LIBRARIES 0 YUBIKEY_CORE_LIBRARY)
list(GET YUBIKEY_LIBRARIES 1 YUBIKEY_PERS_LIBRARY)
add_custom_command(TARGET keepassxc-cli
POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy ${CLI_BINARY_DIR} ${CLI_APP_DIR}
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
COMMENT "Copying keepassxc-cli inside the application")
add_custom_command(TARGET keepassxc-cli
POST_BUILD
COMMAND ${CMAKE_INSTALL_NAME_TOOL}
-change ${Qt5_PREFIX}lib/QtCore.framework/Versions/5/QtCore
"@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
-change /usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore
"@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
-change ${Qt5_PREFIX}/lib/QtGui.framework/Versions/5/QtGui
"@executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui"
-change /usr/local/opt/qt/lib/QtGui.framework/Versions/5/QtGui
"@executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui"
-change ${Qt5_PREFIX}/lib/QtMacExtras.framework/Versions/5/QtMacExtras
"@executable_path/../Frameworks/QtMacExtras.framework/Versions/5/QtMacExtras"
-change /usr/local/opt/qt/lib/QtMacExtras.framework/Versions/5/QtMacExtras
"@executable_path/../Frameworks/QtMacExtras.framework/Versions/5/QtMacExtras"
-change ${Qt5_PREFIX}/lib/QtConcurrent.framework/Versions/5/QtConcurrent
"@executable_path/../Frameworks/QtConcurrent.framework/Versions/5/QtConcurrent"
-change /usr/local/opt/qt/lib/QtConcurrent.framework/Versions/5/QtConcurrent
"@executable_path/../Frameworks/QtConcurrent.framework/Versions/5/QtConcurrent"
-change ${Qt5_PREFIX}/lib/QtCore.framework/Versions/5/QtCore
"@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
-change /usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore
"@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
-change ${Qt5_PREFIX}/lib/QtNetwork.framework/Versions/5/QtNetwork
"@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork"
-change /usr/local/opt/qt/lib/QtNetwork.framework/Versions/5/QtNetwork
"@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork"
-change ${Qt5_PREFIX}/lib/QtWidgets.framework/Versions/5/QtWidgets
"@executable_path/../Frameworks/QtWidgets.framework/Versions/5/QtWidgets"
-change /usr/local/opt/qt/lib/QtWidgets.framework/Versions/5/QtWidgets
"@executable_path/../Frameworks/QtWidgets.framework/Versions/5/QtWidgets"
-change ${GCRYPT_LIBRARIES}
"@executable_path/../Frameworks/libgcrypt.20.dylib"
-change /usr/local/opt/libgcrypt/lib/libgcrypt.20.dylib
"@executable_path/../Frameworks/libgcrypt.20.dylib"
-change ${ARGON2_LIBRARIES}
"@executable_path/../Frameworks/libargon2.1.dylib"
-change /usr/local/opt/argon2/lib/libargon2.1.dylib
"@executable_path/../Frameworks/libargon2.1.dylib"
-change ${GPGERROR_LIBRARIES}
"@executable_path/../Frameworks/libgpg-error.0.dylib"
-change /usr/local/opt/libgpg-error/lib/libgpg-error.0.dylib
"@executable_path/../Frameworks/libgpg-error.0.dylib"
-change ${sodium_LIBRARY_RELEASE}
"@executable_path/../Frameworks/libsodium.23.dylib"
-change /usr/local/opt/libsodium/lib/libsodium.23.dylib
"@executable_path/../Frameworks/libsodium.23.dylib"
-change ${YUBIKEY_CORE_LIBRARY}
"@executable_path/../Frameworks/libyubikey.0.dylib"
-change /usr/local/opt/libyubikey/lib/libyubikey.0.dylib
"@executable_path/../Frameworks/libyubikey.0.dylib"
-change ${YUBIKEY_PERS_LIBRARY}
"@executable_path/../Frameworks/libykpers-1.1.dylib"
-change /usr/local/opt/ykpers/lib/libykpers-1.1.dylib
"@executable_path/../Frameworks/libykpers-1.1.dylib"
${CLI_APP_DIR}
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
COMMENT "Changing linking of keepassxc-cli")
endif()
This gives the following result:
otool output
@executable_path/../Frameworks/libgcrypt.20.dylib (compatibility version 23.0.0, current version 23.3.0)
@executable_path/../Frameworks/libargon2.1.dylib (compatibility version 0.0.0, current version 0.0.0)
@executable_path/../Frameworks/libgpg-error.0.dylib (compatibility version 25.0.0, current version 25.3.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
@executable_path/../Frameworks/libsodium.23.dylib (compatibility version 25.0.0, current version 25.0.0)
@executable_path/../Frameworks/QtWidgets.framework/Versions/5/QtWidgets (compatibility version 5.11.0, current version 5.11.1)
@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork (compatibility version 5.11.0, current version 5.11.1)
@executable_path/../Frameworks/QtConcurrent.framework/Versions/5/QtConcurrent (compatibility version 5.11.0, current version 5.11.1)
@executable_path/../Frameworks/libyubikey.0.dylib (compatibility version 2.0.0, current version 2.7.0)
@executable_path/../Frameworks/libykpers-1.1.dylib (compatibility version 21.0.0, current version 21.0.0)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1452.23.0)
@executable_path/../Frameworks/QtMacExtras.framework/Versions/5/QtMacExtras (compatibility version 5.11.0, current version 5.11.1)
@executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui (compatibility version 5.11.0, current version 5.11.1)
@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore (compatibility version 5.11.0, current version 5.11.1)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.50.4)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1452.23.0)
There was a problem hiding this comment.
On my system I get this error after updating that section of src/cli/CMakeLists.txt to match yours.
Copying keepassxc-cli inside the application
Changing linking of keepassxc-cli
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: more than one input file specified (@executable_path/../Frameworks/libsodium.23.dylib and /usr/local/opt/libsodium/lib/libsodium.23.dylib)
Usage: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool [-change old new] ... [-rpath old new] ... [-add_rpath new] ... [-delete_rpath old] ... [-id name] input
make[2]: *** [src/cli/keepassxc-cli] Error 1
make[2]: *** Deleting file `src/cli/keepassxc-cli'
make[1]: *** [src/cli/CMakeFiles/keepassxc-cli.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
Is there an easy way to list the full command line (including arguments) of install_name_tool? called by cmake?
If I remove this line:
-change ${sodium_LIBRARY_RELEASE}
"@executable_path/../Frameworks/libsodium.23.dylib"
My otool -L output is equivalent to yours.
There was a problem hiding this comment.
That libsodium variable was giving me some headaches too. I'm not sure if that line above is even necessary. I haven't found another way to include the libs.
|
Any updates to this PR? |
|
Sorry. I've been busy with university and other projects. From memory and the above comments the only major issue that's blocking me from completing this PR is the path of the libsodium library isn't being properly detected for the variable to correct the linking for the |
|
I guess the only solution for libsodium is to use a static path and hope nothing breaks.. |
c289c9e to
e87e9b0
Compare
|
Whew. Haven't taken a look at this in a while (busy with other things). Glad to see the PR moving forward in my absence. |
| set(PLUGIN_INSTALL_DIR ".") | ||
| set(DATA_INSTALL_DIR "share") | ||
| elseif(APPLE AND WITH_APP_BUNDLE) | ||
| set(CLI_INSTALL_DIR "/usr/local/bin") |
There was a problem hiding this comment.
Yes that is a great solution and exposes the command line tools as an option. This can be made available in 2.5
|
Everything builds and links fine. keepassxc-cli tests fail for me (12 of them), and running the |
|
Also, linking QtSvg, libqrencode and YubiKey libraries are not correct when inspecting with keepassxc-cli: EDIT: Pushed a fix for these. The error message above disappeared also. Tests still fail. |
|
Just need to fix the gui tests and this will be ready for merge |
fb87e73 to
c0fa0df
Compare
- New Database Wizard [#1952] - Advanced Search [#1797] - Automatic update checker [#2648] - KeeShare database synchronization [#2109, #1992, #2738, #2742, #2746, #2739] - Improve favicon fetching; transition to Duck-Duck-Go [#2795, #2011, #2439] - Remove KeePassHttp support [#1752] - CLI: output info to stderr for easier scripting [#2558] - CLI: Add --quiet option [#2507] - CLI: Add create command [#2540] - CLI: Add recursive listing of entries [#2345] - CLI: Fix stdin/stdout encoding on Windows [#2425] - SSH Agent: Support OpenSSH for Windows [#1994] - macOS: TouchID Quick Unlock [#1851] - macOS: Multiple improvements; include CLI in DMG [#2165, #2331, #2583] - Linux: Prevent Klipper from storing secrets in clipboard [#1969] - Linux: Use polling based file watching for NFS [#2171] - Linux: Enable use of browser plugin in Snap build [#2802] - TOTP QR Code Generator [#1167] - High-DPI Scaling for 4k screens [#2404] - Make keyboard shortcuts more consistent [#2431] - Warn user if deleting referenced entries [#1744] - Allow toolbar to be hidden and repositioned [#1819, #2357] - Increase max allowed database timeout to 12 hours [#2173] - Password generator uses existing password length by default [#2318] - Improve alert message box button labels [#2376] - Show message when a database merge makes no changes [#2551] - Browser Integration Enhancements [#1497, #2253, #1904, #2232, #1850, #2218, #2391, #2396, #2542, #2622, #2637, #2790] - Overall Code Improvements [#2316, #2284, #2351, #2402, #2410, #2419, #2422, #2443, #2491, #2506, #2610, #2667, #2709, #2731]

Description
This PR solves the issues that when building KeePassXC on Mac, the resulting KeePassXC.dmg bundle doesn't include the
keepassxc-cliMotivation and context
This PR solves open issue #1697. One caveat to note if the user wishes to use
keepassxc-clithey either need to use the full path to the executable, or symlink the binary to a location in their PATH. Sublime Text recommends this method.How has this been tested?
I built KeePassXC with this patch on an OS X 10.11 machine, inside the resulting DMG bundle is the
keepassxc-clibinary located atKeePassXC.app/Contents/MacOS/keepassxc-cliTypes of changes
Checklist:
For some reason
testguifails on my machine. Also it scared me when it started doing stuff on it's own :P-DWITH_ASAN=ON. [REQUIRED](To be fair the documentation for
keepassxc-cliis almost nonexistent. And I feel writing aboutkeepassxc-cliitself is outside the scope of this PR. Something should be done though, noting the above caveat.)