Libertine

Merge lp:~larryprice/libertine/listmodel-crash into lp:libertine

  1. listmodel-crash
  2. Merge into devel
Proposed by Larry Price
Status: Merged
Approved by: Christopher Townsend
Approved revision: 274
Merged at revision: 287
Proposed branch: lp:~larryprice/libertine/listmodel-crash
Merge into: lp:libertine
Diff against target: 173 lines (+28/-23)
6 files modified
libertine/ContainerAppsList.cpp (+2/-7)
libertine/ContainerAppsList.h (+2/-1)
libertine/ContainerArchivesList.cpp (+2/-7)
libertine/ContainerArchivesList.h (+2/-1)
libertine/qml/ContainersView.qml (+16/-6)
python/libertine/ChrootContainer.py (+4/-1)
To merge this branch: bzr merge lp:~larryprice/libertine/listmodel-crash
Reviewer Review Type Date Requested Status
Libertine CI Bot continuous-integration Approve
Christopher Townsend (community) Approve
Review via email: mp+302854@code.launchpad.net

Commit message

Return user to homepage when container has been destroyed from under them.

Description of the change

Return user to homepage when container has been destroyed from under them.

Also check our listmodels for null before trying to call methods on them, especially from noexcept methods (:

To post a comment you must log in.
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

PASSED: Continuous integration, rev:273
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/98/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/283
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/233
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/233
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/233
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/233
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/233
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/233
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/215/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/285
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/269
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/269
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/269
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/262/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/262/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/262/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/262/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/262/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/262
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/262/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/98/rebuild

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Works well and no more crash.

Just one inline comment.

Thanks!

review: Needs Information
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

PASSED: Continuous integration, rev:274
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/99/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/284
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/234
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/234
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/234
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/234
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/234
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/234
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/216/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/286
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/270
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/270
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/270
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/263/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/263/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/263/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/263/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/263/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/263
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/263/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/99/rebuild

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Ok, looks good. Thanks!

review: Approve
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://jenkins.canonical.com/libertine/job/lp-libertine-autoland/39/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/285
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/235
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/235
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/235
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/235
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/235
    FAILURE: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/235/console
    None: https://jenkins.canonical.com/libertine/job/lp-generic-land-mp/64/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/287
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/271
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/271
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/271
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/264/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/264/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/264/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/264/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/264/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/264
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/264/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Libertine CI Bot (libertine-ci-bot) :
review: Approve (continuous-integration)
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
https://jenkins.canonical.com/libertine/job/lp-libertine-autoland/40/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/288
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/238
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/238
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/238
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/238
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/238
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/238
    None: https://jenkins.canonical.com/libertine/job/lp-generic-land-mp/65/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/290
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/274
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/274
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/274
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/267/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/267/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/267/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/267/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/267/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/267
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/267/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Libertine CI Bot (libertine-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libertine/ContainerAppsList.cpp'
2--- libertine/ContainerAppsList.cpp 2016-03-24 14:15:26 +0000
3+++ libertine/ContainerAppsList.cpp 2016-08-15 16:00:34 +0000
4@@ -27,11 +27,6 @@
5 { }
6
7
8-ContainerAppsList::
9-~ContainerAppsList()
10-{ }
11-
12-
13 void ContainerAppsList::
14 setContainerApps(QString const& container_id)
15 {
16@@ -51,12 +46,12 @@
17
18 bool ContainerAppsList::
19 empty() const noexcept
20-{ return apps_->empty(); }
21+{ return apps_ == nullptr || apps_->empty(); }
22
23
24 ContainerAppsList::size_type ContainerAppsList::
25 size() const noexcept
26-{ return apps_->count(); }
27+{ return apps_ != nullptr ? apps_->count() : 0;}
28
29
30 int ContainerAppsList::
31
32=== modified file 'libertine/ContainerAppsList.h'
33--- libertine/ContainerAppsList.h 2016-03-24 14:15:26 +0000
34+++ libertine/ContainerAppsList.h 2016-08-15 16:00:34 +0000
35@@ -53,7 +53,8 @@
36 ContainerAppsList(ContainerConfigList* container_config_list,
37 QObject* parent = nullptr);
38
39- ~ContainerAppsList();
40+ virtual
41+ ~ContainerAppsList() = default;
42
43 Q_INVOKABLE void
44 setContainerApps(QString const& container_id);
45
46=== modified file 'libertine/ContainerArchivesList.cpp'
47--- libertine/ContainerArchivesList.cpp 2016-03-10 21:33:25 +0000
48+++ libertine/ContainerArchivesList.cpp 2016-08-15 16:00:34 +0000
49@@ -27,11 +27,6 @@
50 { }
51
52
53-ContainerArchivesList::
54-~ContainerArchivesList()
55-{ }
56-
57-
58 void ContainerArchivesList::
59 setContainerArchives(QString const& container_id)
60 {
61@@ -44,12 +39,12 @@
62
63 bool ContainerArchivesList::
64 empty() const noexcept
65-{ return archives_->empty(); }
66+{ return archives_ == nullptr || archives_->empty(); }
67
68
69 ContainerArchivesList::size_type ContainerArchivesList::
70 size() const noexcept
71-{ return archives_->count(); }
72+{ return archives_ != nullptr ? archives_->count() : 0; }
73
74
75 int ContainerArchivesList::
76
77=== modified file 'libertine/ContainerArchivesList.h'
78--- libertine/ContainerArchivesList.h 2016-03-10 21:33:25 +0000
79+++ libertine/ContainerArchivesList.h 2016-08-15 16:00:34 +0000
80@@ -53,7 +53,8 @@
81 ContainerArchivesList(ContainerConfigList* container_config_list,
82 QObject* parent = nullptr);
83
84- ~ContainerArchivesList();
85+ virtual
86+ ~ContainerArchivesList() = default;
87
88 Q_INVOKABLE void
89 setContainerArchives(QString const& container_id);
90
91=== modified file 'libertine/qml/ContainersView.qml'
92--- libertine/qml/ContainersView.qml 2016-07-15 18:39:58 +0000
93+++ libertine/qml/ContainersView.qml 2016-08-15 16:00:34 +0000
94@@ -55,8 +55,12 @@
95 }
96 model: containerConfigList
97
98- function edit(containerId) {
99- mainView.currentContainer = containerId
100+ function edit(id, status) {
101+ if (status === "removing") {
102+ mainView.error(i18n.tr("Container Unavailable"), i18n.tr("Container is being destroyed and is no longer editable."))
103+ return
104+ }
105+ mainView.currentContainer = id
106 containerAppsList.setContainerApps(mainView.currentContainer)
107 pageStack.push(Qt.resolvedUrl("HomeView.qml"), {"currentContainer": mainView.currentContainer})
108 }
109@@ -82,7 +86,7 @@
110 running: containerActivity.visible
111 }
112
113- onClicked: { containersList.edit(containerId) }
114+ onClicked: { containersList.edit(containerId, installStatus) }
115
116 leadingActions: ListItemActions {
117 actions: [
118@@ -92,8 +96,8 @@
119 description: i18n.tr("Delete Container")
120 onTriggered: {
121 var worker = Qt.createComponent("ContainerManager.qml").createObject(mainView)
122+ worker.error.connect(mainView.error)
123 worker.destroyContainer(containerId)
124- mainView.currentContainer = containerId
125 }
126 }
127 ]
128@@ -117,7 +121,7 @@
129 visible: (installStatus === i18n.tr("ready") ||
130 installStatus === i18n.tr("updating")) ? true : false
131 onTriggered: {
132- containersList.edit(containerId)
133+ containersList.edit(containerId, installStatus)
134 }
135 }
136 ]
137@@ -128,13 +132,19 @@
138 Component.onCompleted: {
139 containerConfigList.configChanged.connect(updateContainerList)
140 }
141-
142+
143 Component.onDestruction: {
144 containerConfigList.configChanged.disconnect(updateContainerList)
145 }
146
147 function updateContainerList() {
148 containerConfigList.reloadContainerList()
149+
150+ if (mainView.currentContainer && !containerConfigList.getContainerStatus(mainView.currentContainer) && pageStack.currentPage !== containersView) {
151+ pageStack.pop()
152+ mainView.currentContainer = ""
153+ mainView.error(i18n.tr("Container Unavailable"), i18n.tr("This container has been destroyed and is no longer valid. You have been returned to the containers overview."))
154+ }
155 }
156
157 function showPasswordDialog(enableMultiarch, containerName) {
158
159=== modified file 'python/libertine/ChrootContainer.py'
160--- python/libertine/ChrootContainer.py 2016-08-09 13:58:26 +0000
161+++ python/libertine/ChrootContainer.py 2016-08-15 16:00:34 +0000
162@@ -61,7 +61,10 @@
163
164 def destroy_libertine_container(self):
165 container_root = os.path.join(utils.get_libertine_containers_dir_path(), self.container_id)
166- shutil.rmtree(container_root)
167+ try:
168+ shutil.rmtree(container_root)
169+ except Exception as e:
170+ print("%s" % e)
171
172 def create_libertine_container(self, password=None, multiarch=False, verbosity=1):
173 # Create the actual chroot

Subscribers

People subscribed via source and target branches