Merge lp:~libertine-team/libertine-scope/cmake-fixes into lp:libertine-scope

Proposed by Kyle Nitzsche
Status: Merged
Approved by: Christopher Townsend
Approved revision: 40
Merged at revision: 38
Proposed branch: lp:~libertine-team/libertine-scope/cmake-fixes
Merge into: lp:libertine-scope
Diff against target: 75 lines (+9/-7)
4 files modified
CMakeLists.txt (+3/-1)
data/CMakeLists.txt (+4/-4)
libertine-scope/CMakeLists.txt (+1/-1)
po/libertine-scope.pot (+1/-1)
To merge this branch: bzr merge lp:~libertine-team/libertine-scope/cmake-fixes
Reviewer Review Type Date Requested Status
Christopher Townsend Approve
Review via email: mp+295365@code.launchpad.net

Description of the change

Hi.

This scope did not build (or runnably install) for me until I made the cmake changes in this MR.

In various CMakeLists.txts:
* install ini file names to be fully qualified
* install .so file as fully qualified name
* set install dir correctly

To post a comment you must log in.
Revision history for this message
Christopher Townsend (townsend) wrote :

Thanks for this MP. Please see inline comments.

review: Needs Fixing
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

On 05/31/2016 12:34 PM, Christopher Townsend wrote:
> Review: Needs Fixing
>
> Thanks for this MP. Please see inline comments.
>
> Diff comments:
>
>> === modified file 'CMakeLists.txt'
>> --- CMakeLists.txt 2016-05-04 13:56:45 +0000
>> +++ CMakeLists.txt 2016-05-20 21:00:48 +0000
>> @@ -42,11 +42,13 @@
>> ${LIBERTINE_INCLUDE_DIRS}
>> )
>>
>> +set(PACKAGE_NAME "libertine-scope.canonical")
>> +set(APP ${PROJECT})
>> +
>> # Important project paths
>> -set(SCOPE_INSTALL_DIR ${CMAKE_INSTALL_FULL_LIBDIR}/unity-scopes/libertine-scope/)
>> +set(SCOPE_INSTALL_DIR "/libertine-scope")
>
> Setting this will install the files to /libertine-scope when installing as a Debian package, which is what we don't want. Users will want to install this on Unity 8 desktop.

I was only aware of the need to install for click not debian, which have
different install paths. As we discussed in irc, perhaps you can add a
script to build the click (thus handling the install path issue) and we
can keep this MPs changes that install the click using fully qualified ID.

>
>> set(SCOPE_NAME "libertine-scope")
>> set(GETTEXT_PACKAGE "${SCOPE_NAME}")
>> -set(PACKAGE_NAME "libertine-scope.canonical")
>>
>> # If we need to refer to the scope's name or package in code, these definitions will help
>> add_definitions(-DPACKAGE_NAME="${PACKAGE_NAME}")
>> @@ -62,9 +64,9 @@
>> # Configure and install the click manifest and apparmor files
>> configure_file(manifest.json.in ${CMAKE_CURRENT_BINARY_DIR}/manifest.json)
>> install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json
>> - DESTINATION ${SCOPE_INSTALL_DIR})
>> + DESTINATION "/")
>
> Same idea as above, but this will install it to /. Definitely not what we want.
>
>> install(FILES "libertine-scope.apparmor"
>> - DESTINATION ${SCOPE_INSTALL_DIR})
>> + DESTINATION "/")
>
> See above.
>
>>
>> # Add our main directories
>> add_subdirectory(libertine-scope)
>
>

39. By Kyle Nitzsche

To accommodate the priority to build debian pkgs, this commit reverts the
three changes to the install dir for clicks (so for now building a click requires
manual intervention)

Revision history for this message
Christopher Townsend (townsend) wrote :

One very small nit inline and we are good.

40. By Kyle Nitzsche

remove extra empty line

Revision history for this message
Christopher Townsend (townsend) wrote :

Great, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-05-04 13:56:45 +0000
3+++ CMakeLists.txt 2016-05-31 19:53:32 +0000
4@@ -42,11 +42,13 @@
5 ${LIBERTINE_INCLUDE_DIRS}
6 )
7
8+set(PACKAGE_NAME "libertine-scope.canonical")
9+set(APP ${PROJECT})
10+
11 # Important project paths
12 set(SCOPE_INSTALL_DIR ${CMAKE_INSTALL_FULL_LIBDIR}/unity-scopes/libertine-scope/)
13 set(SCOPE_NAME "libertine-scope")
14 set(GETTEXT_PACKAGE "${SCOPE_NAME}")
15-set(PACKAGE_NAME "libertine-scope.canonical")
16
17 # If we need to refer to the scope's name or package in code, these definitions will help
18 add_definitions(-DPACKAGE_NAME="${PACKAGE_NAME}")
19
20=== modified file 'data/CMakeLists.txt'
21--- data/CMakeLists.txt 2016-01-07 19:35:25 +0000
22+++ data/CMakeLists.txt 2016-05-31 19:53:32 +0000
23@@ -3,13 +3,13 @@
24 # .so file so that the test tools can find them.
25 intltool_merge_translations(
26 "libertine-scope.ini.in"
27- "${CMAKE_CURRENT_BINARY_DIR}/${SCOPE_NAME}.ini"
28+ "${CMAKE_CURRENT_BINARY_DIR}/${PACKAGE_NAME}_${SCOPE_NAME}.ini"
29 ALL
30 UTF8
31 )
32 intltool_merge_translations(
33 "libertine-scope-settings.ini.in"
34- "${CMAKE_CURRENT_BINARY_DIR}/${SCOPE_NAME}-settings.ini"
35+ "${CMAKE_CURRENT_BINARY_DIR}/${PACKAGE_NAME}_${SCOPE_NAME}-settings.ini"
36 ALL
37 UTF8
38 )
39@@ -17,8 +17,8 @@
40 # Install the scope ini files
41 install(
42 FILES
43- "${CMAKE_CURRENT_BINARY_DIR}/${SCOPE_NAME}.ini"
44- "${CMAKE_CURRENT_BINARY_DIR}/${SCOPE_NAME}-settings.ini"
45+ "${CMAKE_CURRENT_BINARY_DIR}/${PACKAGE_NAME}_${SCOPE_NAME}.ini"
46+ "${CMAKE_CURRENT_BINARY_DIR}/${PACKAGE_NAME}_${SCOPE_NAME}-settings.ini"
47 DESTINATION
48 ${SCOPE_INSTALL_DIR}
49 )
50
51=== modified file 'libertine-scope/CMakeLists.txt'
52--- libertine-scope/CMakeLists.txt 2016-04-27 16:33:00 +0000
53+++ libertine-scope/CMakeLists.txt 2016-05-31 19:53:32 +0000
54@@ -27,7 +27,7 @@
55
56 set_target_properties(scope
57 PROPERTIES
58- OUTPUT_NAME "${SCOPE_NAME}"
59+ OUTPUT_NAME "${PACKAGE_NAME}_${SCOPE_NAME}"
60 )
61
62 install(TARGETS scope
63
64=== modified file 'po/libertine-scope.pot'
65--- po/libertine-scope.pot 2016-04-27 18:37:46 +0000
66+++ po/libertine-scope.pot 2016-05-31 19:53:32 +0000
67@@ -8,7 +8,7 @@
68 msgstr ""
69 "Project-Id-Version: PACKAGE VERSION\n"
70 "Report-Msgid-Bugs-To: \n"
71-"POT-Creation-Date: 2016-04-27 14:37-0400\n"
72+"POT-Creation-Date: 2016-05-20 20:52+0000\n"
73 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
74 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
75 "Language-Team: LANGUAGE <LL@li.org>\n"

Subscribers

People subscribed via source and target branches

to all changes: