Page MenuHomePhabricator

[CMAKE] Prepare the application bundle for localization
ClosedPublic

Authored by Fabien on Tue, Nov 5, 14:46.

Details

Summary

This diff adds the files required to setup localization for the
application bundle (lproj). There is no associated translation at the
moment, they will be added in a later diff.

Test Plan
ninja BitcoinABC-Qt

The generated .app should run on OSX.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Tue, Nov 5, 14:46
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Nov 5, 14:46
jasonbcox requested changes to this revision.Thu, Nov 7, 18:13
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/qt/CMakeLists.txt
320 ↗(On Diff #13921)

Is there a reason to join these together rather than specifying each on it's own RESOURCE line below?

336 ↗(On Diff #13921)

The CMake documentation isn't clear on how this is handled. If it's in Resources/... why is the property not similar to RESOURCE "${BITCOINQT_BUNDLE_RESOURCES}"? Is the InfoPlist.strings a bundle and not a resource despite the location?

This revision now requires changes to proceed.Thu, Nov 7, 18:13
Fabien requested review of this revision.Thu, Nov 7, 19:37
Fabien added inline comments.
src/qt/CMakeLists.txt
320 ↗(On Diff #13921)

Yes, RESOURCE takes a single argument, which can be a list. The join here is the way CMake does list serialization.

336 ↗(On Diff #13921)

Good question :), this is a resource of the bundle just like the others. The difference is that is should be located in a subdirectory of the bundle's Resources folder (the variable is not only the file name, but the relative path). This is not possible with the standard RESOURCE property, because it will try to create the file without creating the intermediate folder first, which the MACOSX_PACKAGE_LOCATION does.

jasonbcox accepted this revision.Thu, Nov 7, 19:52
jasonbcox added inline comments.
src/qt/CMakeLists.txt
336 ↗(On Diff #13921)

Got it. thanks.

This revision is now accepted and ready to land.Thu, Nov 7, 19:52
jasonbcox added inline comments.Thu, Nov 7, 19:53
src/qt/CMakeLists.txt
336 ↗(On Diff #13921)

Maybe consider writing that in as a comment.

Fabien updated this revision to Diff 14010.Fri, Nov 8, 20:25

Add a comment to explain the need for MACOSX_PACKAGE_LOCATION.

jasonbcox accepted this revision.Fri, Nov 8, 20:27
This revision was automatically updated to reflect the committed changes.