ubuntu-touch-coreapps-reviewers team mailing list archive

[David Planella <david.planella@xxxxxxxxxx>]

Review: Needs Information

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

Diff comments:

> === added file 'snapcraft.yaml'
> --- snapcraft.yaml	1970-01-01 00:00:00 +0000
> +++ snapcraft.yaml	2016-09-08 11:44:05 +0000
> @@ -0,0 +1,40 @@
> +name: ubuntu-terminal-app
> +version: 0.7.207
> +summary: Ubuntu Terminal app
> +description: The terminal app for all Ubuntu devices.
> +
> +confinement: strict
> +

Don't we want to make it devmode for now?

> +apps:
> +  ubuntu-terminal-app:
> +    # If CLICK_MODE=off
> +    #command: desktop-launch $SNAP/usr/lib/*/qt5/bin/qmlscene $SNAP/usr/share/terminal/qml/ubuntu-terminal-app.qml
> +    # If CLICK_MODE=on
> +    command: desktop-launch $SNAP/usr/lib/*/qt5/bin/qmlscene $SNAP/qml/ubuntu-terminal-app.qml

I'd suggest choosing one of the two and removing the commented code. For clock app, as an example, chose to build with CLICK_MODE=off. If I'm not mistaken, this will allow executing the terminal without the password prompt on the desktop.

> +    plugs: [unity7, opengl, network, network-bind, home]
> +
> +parts:
> +  terminal:
> +    plugin: cmake
> +    configflags: [-DCMAKE_INSTALL_PREFIX=/usr, -DCLICK_MODE=on]

See comment above about using CLICK_MODE=off

> +    source: .
> +    build-packages:
> +      - build-essential
> +      - cmake
> +      - gettext
> +      - intltool
> +      - python3
> +      - qtbase5-dev
> +      - qtdeclarative5-dev
> +      - libqtermwidget5-0-dev
> +      - libpam0g-dev
> +    stage-packages:
> +      - ubuntu-sdk-libs
> +      - qtubuntu-desktop
> +      - libqtermwidget5-0
> +      - libqt5systeminfo5
> +      - qml-module-qtsysteminfo
> +    snap:
> +      - -usr/share/doc
> +      - -usr/include
> +    after: [desktop-qt5]
> 
> === modified file 'src/plugin/qmltermwidget/CMakeLists.txt'
> --- src/plugin/qmltermwidget/CMakeLists.txt	2014-11-15 14:39:05 +0000
> +++ src/plugin/qmltermwidget/CMakeLists.txt	2016-09-08 11:44:05 +0000
> @@ -70,6 +70,7 @@
>  
>  # Copy the plugin, the qmldir file and other assets to the build dir for running in QtCreator
>  if(NOT "${CMAKE_CURRENT_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_BINARY_DIR}")
> +    file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR})
>      add_custom_target(qmltermwidget-qmldir ALL

Just out of interest, do we know exactly what changed in CMake that stopped the previous CMakeList.txt from building?

>          COMMAND cp ${CMAKE_CURRENT_SOURCE_DIR}/src/qmldir ${CMAKE_CURRENT_BINARY_DIR}/../${PLUGIN_DIR}
>          DEPENDS ${QMLFILES}


-- 
https://code.launchpad.net/~popey/ubuntu-terminal-app/add-snapcraft-config/+merge/305206
Your team Ubuntu Terminal Developers is subscribed to branch lp:ubuntu-terminal-app.