Tarsnap / tarsnap-gui

Cross-platform GUI for the Tarsnap backup service.
https://www.tarsnap.com
BSD 2-Clause "Simplified" License
251 stars 25 forks source link

macOS build failure as Qt no longer adds deprecated CFBundleGetInfoString #557

Closed cho-m closed 10 months ago

cho-m commented 2 years ago

Issue happens at: https://github.com/Tarsnap/tarsnap-gui/blob/b155a354b91f0ecfd7acdd981e7d1f0c3b925ad5/Tarsnap.pro#L299-L300

The specific build error with Qt 5.15 is:

/usr/libexec/PlistBuddy -c "Set :CFBundleGetInfoString 1.0.2" /private/tmp/tarsnap-gui-20220722-154-w5fyn/tarsnap-gui-1.0.2/Tarsnap.app/Contents/Info.plist ;
Set: Entry, ":CFBundleGetInfoString", Does Not Exist
make: *** [Tarsnap.app/Contents/MacOS/Tarsnap] Error 1

This may relate to https://bugreports.qt.io/browse/QTBUG-74872 and corresponding fix where deprecated CFBundleGetInfoString was removed from Info.plist.

Since Set only works on existing values, the command fails as the key is missing.


On side note, CFBundleGetInfoString has been deprecated for a while. Version information should probably use CFBundleVersion and CFBundleShortVersionString.

The manual way would be to modify command to run something like Add :CFBundleVersionString string $${VERSION}.

I think an alternative way is to maintain your own Info.plist file and use QMAKE_INFO_PLIST for placeholder substitution.


To fix Homebrew formula (https://github.com/Homebrew/homebrew-core/pull/106421), I added manual way for now, but it may be cleaner to use Info.plist placeholders.

diff --git a/Tarsnap.pro b/Tarsnap.pro
index 9954fc5c..560621b1 100644
--- a/Tarsnap.pro
+++ b/Tarsnap.pro
@@ -131,5 +131,8 @@ osx {

     # Add VERSION to the app bundle.  (Why doesn't qmake do this?)
     INFO_PLIST_PATH = $$shell_quote($${OUT_PWD}/$${TARGET}.app/Contents/Info.plist)
-    QMAKE_POST_LINK += /usr/libexec/PlistBuddy -c \"Set :CFBundleGetInfoString $${VERSION}\" $${INFO_PLIST_PATH} ;
+    QMAKE_POST_LINK += /usr/libexec/PlistBuddy \
+                            -c \"Add :CFBundleVersionString string $${VERSION}\" \
+                            -c \"Add :CFBundleShortVersionString string $${VERSION}\" \
+                            $${INFO_PLIST_PATH} ;
 }
gperciva commented 10 months ago

Hi @cho-m,

Thanks so much for the patch, and especially for the fix to the Homebrew recipe!

That fix works great for a stand-alone compile, but if one tries to compile it again after modifying something (say, make && touch src/main.cpp && make) then it fails. /usr/libexec/PlistBuddy is very temperamental: the Add command fails if the key already exists:

/usr/libexec/PlistBuddy -c "Add :CFBundleVersionString string 1.1.0-unreleased" -c "Add :CFBundleShortVersionString string 1.1.0-unreleased" /Users/gperciva/src/tarsnap-gui/Tarsnap.app/Contents/Info.plist ;
Add: ":CFBundleVersionString" Entry Already Exists
Add: ":CFBundleShortVersionString" Entry Already Exists
make: *** [Tarsnap.app/Contents/MacOS/Tarsnap] Error 1

I fumbled around for a bit, and eventually discovered that PlistBuddy's Merge command can handle it. At least, as of the version in macOS 13.6, if the keys already exist then it prints a warning rather than failing. I've implemented this solution in #561.

(I'm not at all claiming that this is the ideal solution! But unfortunately it's the only one I've come up with, other than maintaining the Info.plist file myself, which seems dangerous given my lack of knowledge of macOS!)

gperciva commented 10 months ago

I merged #561, so I'll close this issue.

I'll submit a PR to homebrew once we release tarsnap-gui 1.0.3 to remove the fix to the homebrew formula, since that diff wouldn't apply to the new Tarsnap.pro.

Thanks again for fix!