GNOME Bugzilla – Bug 764091
Don't fail pruning partial commits
Last modified: 2016-03-23 19:45:48 UTC
If you've done a commit only pull and then try to prune, ostree will promptly blow trying to traverse a commit that obviously doesn't have all its objects.
Created attachment 324604 [details] [review] traverse: Require variant when traversing dirtree The dirtree object is required for traversing, so don't use the load_variant_if_exists() function. This will return a G_IO_ERROR_NOT_FOUND to the caller rather than trying to ref a NULL variant in ostree_repo_commit_traverse_iter_init_dirtree() if the object is missing.
Created attachment 324605 [details] [review] prune: Don't fail on partial commits If a commit only pull has been done, then the commit object exists in the object store in addition to the commitpartial file. Traversing this partial commit will likely fail, but that's expected. If traverse returns a G_IO_ERROR_NOT_FOUND in this case, continue with pruning.
Review of attachment 324604 [details] [review]: LGTM
I don't have permissions to push. Can you? Thanks.
Review of attachment 324605 [details] [review]: One minor style thing. Also, would you consider trying to write a test case? ::: src/libostree/ostree-repo-prune.c @@ +315,3 @@ + { + /* Don't fail traversing a partial commit */ + if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL && Can you parenthesize the bitflag operators, so (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0 ? The trap here is specifically with `==` having lower precedence than `&`, so I always parenthesize bitflags out of sanity. cd ~/src/linux git grep 'if (.* & .*&&' shows the Linux kernel mostly uses this style too.
Created attachment 324617 [details] [review] prune: Don't fail on partial commits If a commit only pull has been done, then the commit object exists in the object store in addition to the commitpartial file. Traversing this partial commit will likely fail, but that's expected. If traverse returns a G_IO_ERROR_NOT_FOUND in this case, continue with pruning.
This test case seems to work: From 7232b2940b763258a01c3ee086e2fe90fa56b9eb Mon Sep 17 00:00:00 2001 From: Colin Walters <walters@verbum.org> Date: Wed, 23 Mar 2016 15:32:06 -0400 Subject: [PATCH] tests: Add a commitpartial + prune test Followup for previous commit. --- tests/test-prune.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test-prune.sh b/tests/test-prune.sh index a1322d9..7184ea9 100755 --- a/tests/test-prune.sh +++ b/tests/test-prune.sh @@ -23,7 +23,7 @@ set -euo pipefail setup_fake_remote_repo1 "archive-z2" -echo '1..1' +echo '1..2' cd ${test_tmpdir} mkdir repo @@ -126,3 +126,11 @@ ${CMD_PREFIX} ostree --repo=repo static-delta list | wc -l > deltascount assert_file_has_content deltascount "^1$" echo "ok prune" + +rm repo -rf +ostree --repo=repo init --mode=bare-user +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo +${CMD_PREFIX} ostree --repo=repo pull --depth=-1 --commit-metadata-only origin test +ostree --repo=repo prune + +echo "ok prune with partial repo" -- 1.8.3.1
Merged both + test case commit.
Thanks for the test case! Sorry, I meant to do that myself.