Fix deployment of keepassxc-cli and keepassxc-proxy and the autotype plugin

[alcroito]

Fixes: #7475

Testing strategy

Tested manually

Type of change

• ✅ Bug fix (non-breaking change that fixes an issue)

[phoerious]

Thanks. The same procedure should probably be applied here as well:

keepassxc/src/autotype/mac/CMakeLists.txt

Lines 8 to 13 in 2423bed

	add_custom_command(TARGET keepassxc-autotype-cocoa
	POST_BUILD
	COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/libkeepassxc-autotype-cocoa.so ${PLUGIN_INSTALL_DIR}/libkeepassxc-autotype-cocoa.so
	COMMAND ${MACDEPLOYQT_EXE} ${PROGNAME}.app -executable=${PLUGIN_INSTALL_DIR}/libkeepassxc-autotype-cocoa.so -no-plugins 2> /dev/null
	WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
	COMMENT "Deploying autotype plugin")

Also, do you have any idea why it works for the main binary?

keepassxc/src/CMakeLists.txt

Lines 409 to 413 in 4f07103

	add_custom_command(TARGET ${PROGNAME}
	POST_BUILD
	COMMAND ${MACDEPLOYQT_EXE} ${PROGNAME}.app 2> /dev/null
	WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
	COMMENT "Deploying app bundle")

I am also confused about what has changed for this to be a problem at all. The POST_BUILD fixup definitely worked when I implemented it.

[alcroito]

Thanks. The same procedure should probably be applied here as well:

I guess i'll introduce a function to reuse the logic.

Also, do you have any idea why it works for the main binary?

I didn't investigate. If this is a recent regression, maybe i can bisect to see when it broke and figure out why.

[varjolintu]

Also, do you have any idea why it works for the main binary?

This is what I wondered as well. It always worked for the main binary, even with package or without.

[alcroito]

So i went ahead and investigated what's going on. The commit message has the full explanation.
But the gist of it is that the cli/proxy helpers in the .dmg had broken rpaths since the change that switched to using macdeployqt.

Also, do you have any idea why it works for the main binary?

The main binary does not do any POST_BUILD copies, so the install(TARGETS ${PROGNAME}) called implicitly by cpack successfully installed the app with proper rpaths into the staging area for .dmg creation.

The POST_BUILD cli / proxy copies are initially installed by install(TARGETS ${PROGNAME}) because installing a bundle installs all its contents, including the copies, but then they got overridden again by the install(TARGETS cli/proxy) calls, which would copy the non-macdeployqt'ed binaries.

[alcroito]

Note that while the current PR fixes the immediate issue, there is potential for improvement by delaying the macdeployqt call on the main app to install time.
That would improve incremental build times (no more POST_BUILD macdeployqt calls), and we can likely merge all 4 macdeployqt calls into a single one taking multiple -executable parameters, also speeding up install time.
I can try to implement that if it would be deemed acceptable.

[phoerious]

But the gist of it is that the cli/proxy helpers in the .dmg had broken rpaths since the change that switched to using macdeployqt.

I can say with certainty that that is not the case. First, because I tested it and the paths were fixed. And secondly because users would have been unable to use KeePassXC-Browser or the CLI on macOS.

[alcroito]

But the gist of it is that the cli/proxy helpers in the .dmg had broken rpaths since the change that switched to using macdeployqt.

I can say with certainty that that is not the case.

That's my assessment at least, you are free to think otherwise :)

I confirmed it by checking out the commit that introduced the usage of macdeployqt and doing a clean build and .dmg using it
af55d1b

I also tried the commit before it, which directly called install_name_tool -change instead of macdeployqt.
Note that in that case, the rpath was changed directly in the initial binary, not the POST_BUILD copy, so those changes are preserved during make install -> .dmg creation.

I did not try using older CMake versions, but I am skeptical that CMake/cpack changed their behavior / order of installation for .dmg creation.
You can also confirm my assessment about install overriding the macdeployqt'ed binaries by running cpack in --trace mode and grepping for the installation of the app bundle and the binaries (specifically the source of the copy).

If you still don't believe my assessment, you can try checking out that commit and do a .dmg build yourself. I would be happy to be proven wrong, in which case we'd have more data on whether this is somehow depended on the environment.

First, because I tested it and the paths were fixed. And secondly because users would have been unable to use KeePassXC-Browser or the CLI on macOS.

Yes, they would have been unable to use them. But how many users were using a 2.7.0 .dmg snapshot? Was there any snapshot up until recently? Wouldn't most testers build it from source, and not use the .dmg?
The cli/proxy macdeployqt change was not shipped in 2.6.6.

[droidmonkey]

I believe your assessment FWIW

[phoerious]

The issue was originally posted against 2.6.4 (#6042) and we definitely fixed it for follow-up stable versions (we kind of skipped 2.6.5 iirc, but 2.6.6 most certainly works). But you are right, this has apparently never been backported to the branch. The master branch still has the manual install_name_tool fixes.

[droidmonkey]

@phoerious ready to merge this?

[phoerious]

Go for it.

[phoerious]

@droidmonkey @alcroito I think we have to revert this and find another solution. The patch made it impossible to run the binary on the fly on macOS, because of signature mismatches. Running the binary through the debugger is already the only way to start KeePassXC from my IDE on ARM64, but with this patch even that doesn't work anymore.

[droidmonkey]

Ahhh so that's why appbundle was crashing on me...

[phoerious]

Yes, all binaries are signed locally and modifications create signature mismatches. I tested #7509 and it fixes the issue by deferring such changes until install time, which allows me to run binaries from my IDE again. I guess that settles it. In fact, it's even better than before, because I can finally run it without the debugger again. Previously, running KeePassXC from my IDE without the debugger would crash with a signature error.

[phoerious]

Seems like you can also run the binary from the local build directory directly again with the post-install method, but not from the DMG generated by make package. I suppose the ad-hoc signature is applied in between these steps now.

[alcroito]

Do you observe these issues only on an m1 mac? What's the exact error you get, so I can know what to look out for.

Also, which ad-hoc signing are you referring to? Afaik ad-hoc signing is done by the linker when linking the binaries / shared libraries.
And afaik, the CPack DragNDrop generator (.dmg) does no signing by default.

[phoerious]

The linker adds an ad-hoc signature, but when you patch a binary's rpath, the hash doesn't match anymore, so Apple Silicon Macs will kill the process immediately. Luckily, with your follow-up PR, it's only a problem with the binaries installed into a bundle, not with the binaries inside the build dir.