bin/fsck: Make ref binding verification optional

Today the rpm-ostree test suite uses `refs --create` to save
commits.  I think this is a legitimate use case, and other
people may be doing something similar.

On the other hand, I think we should probably be changing the rpm-ostree test
suite to create "unbound" commits. But let's be maximially compatible here since
we hit a real-world case where something needed to change.

Closes: #1379
Approved by: pwithnall

bash/ostree

@@ -963,6 +963,7 @@ _ostree_fsck() {
         --add-tombstones
         --delete
         --quiet -q
+        --verify-bindings
         --verify-back-refs
     "
 

man/ostree-fsck.xml

@@ -86,14 +86,24 @@ Boston, MA 02111-1307, USA.
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--verify-bindings</option></term>
+                <listitem><para>
+                  Verify that the commits pointed to by each ref have that
+                  ref in the binding set.  You should usually add this
+                  option; it only defaults to off for backwards compatibility.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--verify-back-refs</option></term>
                 <listitem><para>
-                   Verify that all the refs listed in a commit’s ref-bindings
+                  Verify that all the refs listed in a commit’s ref-bindings
-                   point to that commit. This cannot be used in repositories
+                  point to that commit. This cannot be used in repositories
-                   where the target of refs is changed over time as new commits
+                  where the target of refs is changed over time as new commits
-                   are added, but can be used in repositories which are
+                  are added, but can be used in repositories which are
-                   regenerated from scratch for each commit.
+                  regenerated from scratch for each commit.
+                  Implies <literal>--verify-bindings</literal> as well.
                 </para></listitem>
             </varlistentry>
         </variablelist>

src/ostree/ot-builtin-fsck.c

@@ -30,6 +30,7 @@
 static gboolean opt_quiet;
 static gboolean opt_delete;
 static gboolean opt_add_tombstones;
+static gboolean opt_verify_bindings;
 static gboolean opt_verify_back_refs;
 
 /* ATTENTION:
@@ -41,7 +42,8 @@ static GOptionEntry options[] = {
   { "add-tombstones", 0, 0, G_OPTION_ARG_NONE, &opt_add_tombstones, "Add tombstones for missing commits", NULL },
   { "quiet", 'q', 0, G_OPTION_ARG_NONE, &opt_quiet, "Only print error messages", NULL },
   { "delete", 0, 0, G_OPTION_ARG_NONE, &opt_delete, "Remove corrupted objects", NULL },
-  { "verify-back-refs", 0, 0, G_OPTION_ARG_NONE, &opt_verify_back_refs, "Verify back-references", NULL },
+  { "verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_verify_bindings, "Verify ref bindings", NULL },
+  { "verify-back-refs", 0, 0, G_OPTION_ARG_NONE, &opt_verify_back_refs, "Verify back-references (implies --verify-bindings)", NULL },
   { NULL }
 };
 
@@ -162,8 +164,11 @@ fsck_commit_for_ref (OstreeRepo    *repo,
     }
 
   /* Check its bindings. */
-  if (!ostree_cmd__private__ ()->ostree_repo_verify_bindings (collection_id, ref_name, commit, error))
+  if (opt_verify_bindings)
-    return glnx_prefix_error (error, "Commit %s", checksum);
+    {
+      if (!ostree_cmd__private__ ()->ostree_repo_verify_bindings (collection_id, ref_name, commit, error))
+        return glnx_prefix_error (error, "Commit %s", checksum);
+    }
 
   return TRUE;
 }
@@ -238,6 +243,9 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation,
   if (opt_add_tombstones)
     tombstones = g_ptr_array_new_with_free_func (g_free);
 
+  if (opt_verify_back_refs)
+    opt_verify_bindings = TRUE;
+
   guint n_partial = 0;
   g_hash_table_iter_init (&hash_iter, objects);
   while (g_hash_table_iter_next (&hash_iter, &key, &value))

tests/test-fsck-collections.sh

@@ -92,8 +92,10 @@ echo "ok 2 fsck-collections in old repository"
 set_up_repo_with_collection_id
 ${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref1-checksum)
 
+# For compatibility we don't check for this by default
+${CMD_PREFIX} ostree fsck --repo=repo
 # fsck should now fail
-if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then
+if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then
     assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!"
 fi
 assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)"
@@ -106,7 +108,7 @@ set_up_repo_with_collection_id
 ${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:new-ref $(cat ref1-checksum)
 
 # fsck should now fail
-if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then
+if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then
     assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!"
 fi
 assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)"
@@ -120,7 +122,7 @@ set_up_repo_with_collection_id
 ${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:ref1 $(cat ref1-checksum)
 
 # fsck should now fail
-if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then
+if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then
     assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!"
 fi
 assert_file_has_content fsck-error "Commit has collection ID ‘org.example.Collection’ in collection binding metadata, while the remote it came from has collection ID ‘org.example.Collection2’"
@@ -156,7 +158,7 @@ echo "ok 7 fsck ignores unreferenced ref bindings"
 set_up_repo_without_collection_id
 
 # fsck at this point should succeed
-${CMD_PREFIX} ostree fsck --repo=repo > fsck
+${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck
 assert_file_has_content fsck "^Validating refs in collections...$"
 
 # Drop the commit the ref points to, and drop the original ref so that fsck doesn’t prematurely fail on that.
@@ -166,7 +168,7 @@ assert_file_has_content commitcount "^1$"
 rm repo/refs/heads/ref3
 
 # fsck should now fail
-if ${CMD_PREFIX} ostree fsck --repo=repo > fsck; then
+if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck; then
     assert_not_reached "fsck unexpectedly succeeded after deleting commit!"
 fi
 assert_file_has_content fsck "^Validating refs...$"
@@ -179,7 +181,7 @@ set_up_repo_without_collection_id
 ${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref3-checksum)
 
 # fsck should now fail
-if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then
+if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then
     assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!"
 fi
 assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref3’, ‘ref4’)"