-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple macOS fixes and include keepassxc-cli in DMG #2165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, which variable (if any) should I use to refer to the libsodium library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
if(MINGW) | ||
set(CLI_INSTALL_DIR ".") | ||
set(PROXY_INSTALL_DIR ".") | ||
set(BIN_INSTALL_DIR ".") | ||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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-cli
Motivation and context
This PR solves open issue #1697. One caveat to note if the user wishes to use
keepassxc-cli
they 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-cli
binary located atKeePassXC.app/Contents/MacOS/keepassxc-cli
Types of changes
Checklist:
For some reason
testgui
fails 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-cli
is almost nonexistent. And I feel writing aboutkeepassxc-cli
itself is outside the scope of this PR. Something should be done though, noting the above caveat.)