Merge lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot

Status: Merged
Approved by: David Planella
Approved revision: 209
Merged at revision: 208
Proposed branch: lp:~popey/ubuntu-terminal-app/add-snapcraft-config
Merge into: lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot
Diff against target: 61 lines (+38/-1)
2 files modified
snapcraft.yaml (+37/-0)
src/plugin/qmltermwidget/CMakeLists.txt (+1/-1)
To merge this branch: bzr merge lp:~popey/ubuntu-terminal-app/add-snapcraft-config
Reviewer Review Type Date Requested Status
David Planella Approve
Jenkins Bot continuous-integration Approve
Review via email: mp+305206@code.launchpad.net

Commit message

Adds snapcraft.yaml to do builds of ubuntu-terminal-app.
Also fixes broken xenial build - thanks to Dan Chapman

Description of the change

Adds snapcraft.yaml to do builds of ubuntu-terminal-app.
Also fixes broken xenial build - thanks to Dan Chapman

To post a comment you must log in.
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:208
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~popey/ubuntu-terminal-app/add-snapcraft-config/+merge/305206/+edit-commit-message

https://core-apps-jenkins.ubuntu.com/job/terminal-app-ci/57/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/972/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/terminal-app-ci/57/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
David Planella (dpm) wrote :

Nice work!

I've added some comments. Right now, if I understand it correctly, the terminal will only run on Unity 7, whereas we'd like to get it running on Unity 8.

I'm just adding it as a reminder, which shouldn't block this MP to get approved after reviewing the comments, but it's something to have a look at on the next iteration.

To get it running on Unity 8 will either require a wrapper [1] or the desktop-launcher to include these lines from the wrapper (plus a couple of extra dependencies, which IIRC did not add much to the size of the snap).

[1] https://github.com/ubuntu/snappy-playpen/blob/master/ubuntu-clock-app/clock.wrapper#L19

review: Needs Information
Revision history for this message
Dan Chapman ξƒΏ (dpniel) :
Revision history for this message
David Planella (dpm) wrote :

I've added another comment, but I also just noticed that this branch attempts to fix the same thing, but adding the CMake rule somewhere else:

https://code.launchpad.net/~larryprice/ubuntu-terminal-app/fix-cmake-xenial/+merge/297394

Revision history for this message
Dan Chapman ξƒΏ (dpniel) wrote :

Added a comment inline.

Revision history for this message
David Planella (dpm) wrote :

Replied inline again, thanks!

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

Thanks for the comments - fixing!

209. By Alan Pope 🍺🐧🐱 πŸ¦„

Set click mode, tidy out comments, remove unnecessary delete from cmake, as per comments.

Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
David Planella (dpm) wrote :

Looks good to me, thanks!

I think we can merge it as it is. CLICK_MODE=on is not necessary and installs extra files [1], but it seems the snap works, so we might as well leave it as it is for a first pass.

[1] http://bazaar.launchpad.net/~ubuntu-terminal-dev/ubuntu-terminal-app/reboot/view/head:/CMakeLists.txt#L24

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'snapcraft.yaml'
2--- snapcraft.yaml 1970-01-01 00:00:00 +0000
3+++ snapcraft.yaml 2016-09-15 10:24:26 +0000
4@@ -0,0 +1,37 @@
5+name: ubuntu-terminal-app
6+version: 0.7.207
7+summary: Ubuntu Terminal app
8+description: The terminal app for all Ubuntu devices.
9+
10+confinement: devmode
11+
12+apps:
13+ ubuntu-terminal-app:
14+ command: desktop-launch $SNAP/usr/lib/*/qt5/bin/qmlscene $SNAP/qml/ubuntu-terminal-app.qml
15+ plugs: [unity7, opengl, network, network-bind, home]
16+
17+parts:
18+ terminal:
19+ plugin: cmake
20+ configflags: [-DCMAKE_INSTALL_PREFIX=/usr, -DCLICK_MODE=on]
21+ source: .
22+ build-packages:
23+ - build-essential
24+ - cmake
25+ - gettext
26+ - intltool
27+ - python3
28+ - qtbase5-dev
29+ - qtdeclarative5-dev
30+ - libqtermwidget5-0-dev
31+ - libpam0g-dev
32+ stage-packages:
33+ - ubuntu-sdk-libs
34+ - qtubuntu-desktop
35+ - libqtermwidget5-0
36+ - libqt5systeminfo5
37+ - qml-module-qtsysteminfo
38+ snap:
39+ - -usr/share/doc
40+ - -usr/include
41+ after: [desktop-qt5]
42
43=== modified file 'src/plugin/qmltermwidget/CMakeLists.txt'
44--- src/plugin/qmltermwidget/CMakeLists.txt 2014-11-15 14:39:05 +0000
45+++ src/plugin/qmltermwidget/CMakeLists.txt 2016-09-15 10:24:26 +0000
46@@ -70,6 +70,7 @@
47
48 # Copy the plugin, the qmldir file and other assets to the build dir for running in QtCreator
49 if(NOT "${CMAKE_CURRENT_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_BINARY_DIR}")
50+ file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR})
51 add_custom_target(qmltermwidget-qmldir ALL
52 COMMAND cp ${CMAKE_CURRENT_SOURCE_DIR}/src/qmldir ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR}
53 DEPENDS ${QMLFILES}
54@@ -80,7 +81,6 @@
55 DEPENDS ${QMLFILES}
56 )
57 add_custom_command(TARGET qmltermwidget POST_BUILD
58- COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR}
59 COMMENT "Creating plugin directory layout in the build dir"
60 COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:qmltermwidget> ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR}
61 COMMENT "Copying to output directory"

Subscribers

People subscribed via source and target branches