Keeper

Merge lp://qastaging/~charlesk/keeper/untar into lp://qastaging/keeper/devel

untar
Merge into devel

Proposed by Charles Kerr on 2016-12-14

Status:	Merged
Approved by:	Xavi Garcia on 2016-12-19
Approved revision:	160
Merged at revision:	123
Proposed branch:	lp://qastaging/~charlesk/keeper/untar
Merge into:	lp://qastaging/keeper/devel
Diff against target:	1362 lines (+984/-108) 18 files modified debian/control (+5/-1) debian/keeper.install (+2/-1) src/helper/folder-backup.sh.in (+1/-1) src/tar/CMakeLists.txt (+72/-27) src/tar/untar-main.cpp (+169/-0) src/tar/untar.cpp (+144/-0) src/tar/untar.h (+38/-0) tests/CMakeLists.txt (+2/-1) tests/com_canonical_keeper.py (+80/-45) tests/unit/tar/CMakeLists.txt (+104/-29) tests/unit/tar/keeper-untar-test.cpp (+232/-0) tests/unit/tar/ku-invoke-nobus.sh.in (+1/-0) tests/unit/tar/ku-invoke.sh.in (+1/-0) tests/unit/tar/untar-test.cpp (+120/-0) tests/utils/file-utils.cpp (+2/-1) tests/utils/file-utils.h (+1/-1) tests/utils/keeper-dbusmock-fixture.h (+9/-0) tests/utils/main.cpp (+1/-1)
To merge this branch:	bzr merge lp://qastaging/~charlesk/keeper/untar
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Xavi Garcia (community)		2016-12-14	Approve on 2016-12-19
unity-api-1-bot	continuous-integration		Approve on 2016-12-16
Review via email: mp+313189@code.qastaging.launchpad.net

Commit message

Add untar

Description of the change

Add untar.

* The Untar class itself, which manages a couple of QProcesses for xzcat and tar x. Add tar & xz to debian dependencies.

* Add untar-test unit test for the untar class

* Add keeper-untar executable that gets a restore socket from keeper, then untars from it

* Add keeper-untar-test that tests keeper-untar against the dbusmock service. Implement restore in the dbusmock service.

* Rename keeper-tar-create as keeper-tar for symmetry with keeper-untar

* Not pushed/completed: integration tests

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-12-14:

FAILED: Continuous integration, rev:148
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/148/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1266/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1273/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/148/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-12-14:

FAILED: Continuous integration, rev:148
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/149/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1267/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1274/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/149/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Xavi Garcia (xavi-garcia-mena) wrote on 2016-12-14:

Thanks, Charles.
It looks good and I've tested it with the service and the command line client and it works fine.

I tried to trigger a rebuild myself but jenkins is still complaining.

I've added a comment inline...

A found a minor issue, non blocking, but that in cases that the restore file is very big we could be trying to restore even we had an error in the step method. (the error in not checked when calling step)

And even more minor: the coding style for the brackets is different in some cases.
There are some cases like:
if (blah) {
blah
}

Shouldn't it be:?
if (blah)
{
blah
}

review: Needs Fixing

lp://qastaging/~charlesk/keeper/untar updated on 2016-12-15

149. By Charles Kerr on 2016-12-14: in debian/control, remove errant newline
150. By Charles Kerr on 2016-12-14: in keeper-untar-test, extract out common test code into helper methods
151. By Charles Kerr on 2016-12-14: in keeper-untar, exit with failure if Untar::step() fails
152. By Charles Kerr on 2016-12-15: in Untar class, add finish() method to check that xz/tar finish and check their return values
153. By Charles Kerr on 2016-12-15: in keeper-untar, exit with failure if Untar.step() or Untar.finish() fail
154. By Charles Kerr on 2016-12-15: fix end-of-namespace semicolon warning
155. By Charles Kerr on 2016-12-15: in untar-test, use Untar.finish()
156. By Charles Kerr on 2016-12-15: in keeper-untar-test, add test cases for restoring corrupt archives
157. By Charles Kerr on 2016-12-15: in service dbusmock, wait for helpers to exit

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-12-15:

FAILED: Continuous integration, rev:157
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/153/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1271/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1278
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1059/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1059/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1059/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1059/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1059/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1059/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/153/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~charlesk/keeper/untar updated on 2016-12-16

158. By Charles Kerr on 2016-12-16: fix bad whitespace that caused whitespace test to fail
159. By Charles Kerr on 2016-12-16: fix flake8 warnings in service dbusmock
160. By Charles Kerr on 2016-12-16: fix minor int conversion warning in tests/utils/main.cpp