From 157261e210b8a6cc46fd85024e60fc360aa51bba Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Thu, 18 Jul 2019 11:25:24 -0400
Subject: [PATCH] compose: Add --parent option

This may seem like a backflip on #1829, but there's a common theme here:
in a promotion workflow, the parent (or lack of parent) of a commit is
an important parameter, so we need full flexibility in configuring it.

But again, like #1829, we still want e.g. change detection, versioning,
and various optimizations to happen on whatever the latest commit on
that ref is in the build repo.
---
 src/app/rpmostree-compose-builtin-tree.c  |  9 ++++++++-
 src/app/rpmostree-composeutil.c           | 17 +++++++++++++----
 tests/compose-tests/libbasic-test.sh      |  5 +++++
 tests/compose-tests/test-basic-unified.sh |  4 +++-
 tests/compose-tests/test-basic.sh         |  4 +++-
 5 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c
index fba193d96f..1aec35cd48 100644
--- a/src/app/rpmostree-compose-builtin-tree.c
+++ b/src/app/rpmostree-compose-builtin-tree.c
@@ -79,6 +79,7 @@ static char *opt_write_composejson_to;
 static gboolean opt_no_parent;
 static char *opt_write_lockfile;
 static char *opt_read_lockfile;
+static char *opt_parent;
 
 /* shared by both install & commit */
 static GOptionEntry common_option_entries[] = {
@@ -116,6 +117,7 @@ static GOptionEntry commit_option_entries[] = {
   { "write-commitid-to", 0, 0, G_OPTION_ARG_STRING, &opt_write_commitid_to, "File to write the composed commitid to instead of updating the ref", "FILE" },
   { "write-composejson-to", 0, 0, G_OPTION_ARG_STRING, &opt_write_composejson_to, "Write JSON to FILE containing information about the compose run", "FILE" },
   { "no-parent", 0, 0, G_OPTION_ARG_NONE, &opt_no_parent, "Always commit without a parent", NULL },
+  { "parent", 0, 0, G_OPTION_ARG_STRING, &opt_parent, "Commit with specific parent", "REV" },
   { NULL }
 };
 
@@ -1063,7 +1065,12 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self,
     }
 
   g_autofree char *parent_revision = NULL;
-  if (self->ref && !opt_no_parent)
+  if (opt_parent)
+    {
+      if (!ostree_repo_resolve_rev (self->repo, opt_parent, FALSE, &parent_revision, error))
+        return FALSE;
+    }
+  else if (self->ref && !opt_no_parent)
     {
       if (!ostree_repo_resolve_rev (self->repo, self->ref, TRUE, &parent_revision, error))
         return FALSE;
diff --git a/src/app/rpmostree-composeutil.c b/src/app/rpmostree-composeutil.c
index 702023051a..97053f9c0f 100644
--- a/src/app/rpmostree-composeutil.c
+++ b/src/app/rpmostree-composeutil.c
@@ -429,11 +429,20 @@ rpmostree_composeutil_write_composejson (OstreeRepo  *repo,
   g_autofree char *parent_revision = ostree_commit_get_parent (new_commit);
   if (path && parent_revision)
     {
-      g_autoptr(GVariant) diffv = NULL;
-      if (!rpm_ostree_db_diff_variant (repo, parent_revision, new_revision,
-                                       FALSE, &diffv, NULL, error))
+      /* don't error if the parent doesn't exist */
+      gboolean parent_exists;
+      if (!ostree_repo_has_object (repo, OSTREE_OBJECT_TYPE_COMMIT, parent_revision,
+                                   &parent_exists, NULL, error))
         return FALSE;
-      g_variant_builder_add (builder, "{sv}", "pkgdiff", diffv);
+
+      if (parent_exists)
+        {
+          g_autoptr(GVariant) diffv = NULL;
+          if (!rpm_ostree_db_diff_variant (repo, parent_revision, new_revision,
+                                           TRUE, &diffv, NULL, error))
+            return FALSE;
+          g_variant_builder_add (builder, "{sv}", "pkgdiff", diffv);
+        }
     }
 
   if (path)
diff --git a/tests/compose-tests/libbasic-test.sh b/tests/compose-tests/libbasic-test.sh
index df52af5e07..a7786c4085 100644
--- a/tests/compose-tests/libbasic-test.sh
+++ b/tests/compose-tests/libbasic-test.sh
@@ -108,4 +108,9 @@ echo "ok compose pkglist"
 ostree --repo=${repobuild} cat ${treeref} /usr/share/rpm-ostree/treefile.json > treefile.json
 assert_jq treefile.json '.basearch == "x86_64"'
 echo "ok basearch"
+
+ostree --repo=${repobuild} rev-parse ${treeref}^ > parent.txt
+assert_file_has_content parent.txt 01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b
+echo "ok --parent"
 }
+
diff --git a/tests/compose-tests/test-basic-unified.sh b/tests/compose-tests/test-basic-unified.sh
index 89a00c2b70..40fb1f8962 100755
--- a/tests/compose-tests/test-basic-unified.sh
+++ b/tests/compose-tests/test-basic-unified.sh
@@ -25,7 +25,9 @@ cat > metadata.json <<EOF
   "overrideme": "new val"
 }
 EOF
-runcompose --ex-unified-core --add-metadata-from-json metadata.json
+# Test --parent at the same time (hash is `echo | sha256sum`)
+runcompose --ex-unified-core --add-metadata-from-json metadata.json \
+  --parent 01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b
 
 # Run it again, but without RPMOSTREE_PRESERVE_TMPDIR. Should be a no-op. This
 # exercises fd handling in the tree context.
diff --git a/tests/compose-tests/test-basic.sh b/tests/compose-tests/test-basic.sh
index 3b57f71997..796faaf5f8 100755
--- a/tests/compose-tests/test-basic.sh
+++ b/tests/compose-tests/test-basic.sh
@@ -17,7 +17,9 @@ cat > metadata.json <<EOF
   "overrideme": "new val"
 }
 EOF
-runcompose --add-metadata-from-json metadata.json
+# Test --parent at the same time (hash is `echo | sha256sum`)
+runcompose --add-metadata-from-json metadata.json \
+  --parent 01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b
 
 . ${dn}/libbasic-test.sh
 basic_test