ostree: Use --touch-if-changed

There are three different cases:

 * we expect commitid and it's there
 * we expect commitid and it's missing
 * we don't expect commitid

This patch helps differentiate between the second two. In former one we
should report an error and mark the phase as failed. The latter is
perfectly fine and no error should be reported

Fixes: https://pagure.io/pungi/issue/1046
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

pungi/ostree/tree.py

@@ -28,8 +28,17 @@ class Tree(OSTree):
     def _make_tree(self):
         """Compose OSTree tree"""
         log_file = make_log_file(self.logdir, 'create-ostree-repo')
-        cmd = ['rpm-ostree', 'compose', 'tree', '--repo=%s' % self.repo,
+        cmd = [
-               '--write-commitid-to=%s' % self.commitid_file]
+            "rpm-ostree",
+            "compose",
+            "tree",
+            "--repo=%s" % self.repo,
+            "--write-commitid-to=%s" % self.commitid_file,
+            # Touch the file if a new commit was created. This can help us tell
+            # if the commitid file is missing because no commit was created or
+            # because something went wrong.
+            "--touch-if-changed=%s.stamp" % self.commitid_file,
+        ]
         if self.version:
             # Add versioning metadata
             cmd.append('--add-metadata-string=version=%s' % self.version)

pungi/ostree/utils.py

@@ -56,16 +56,13 @@ def get_ref_from_treefile(treefile, arch=None, logger=None):
     return ref
 
 
-def get_commitid_from_commitid_file(commitid_file, logger=None):
+def get_commitid_from_commitid_file(commitid_file):
     """Return commit id which is read from the commitid file"""
-    logger = logger or logging.getLogger(__name__)
+    if not os.path.exists(commitid_file + ".stamp"):
-    commitid = None
+        # The stamp does not exist, so no new commit.
-    if os.path.isfile(commitid_file):
+        return None
-        with open(commitid_file, 'r') as f:
+    with open(commitid_file, 'r') as f:
-            commitid = f.read().replace('\n', '')
+        return f.read().replace('\n', '')
-    else:
-        logger.error('Unable to find commitid file')
-    return commitid
 
 
 def tweak_treeconf(treeconf, source_repos=None, keep_original_sources=False, update_dict=None):

pungi/phases/ostree.py

@@ -95,9 +95,13 @@ class OSTreeThread(WorkerThread):
         if compose.notifier:
             ref = get_ref_from_treefile(os.path.join(repodir, config['treefile']),
                                         arch, logger=self.pool._logger)
-            # 'pungi-make-ostree tree' writes commitid to commitid.log in logdir
+            # 'pungi-make-ostree tree' writes commitid to commitid.log in
-            commitid = get_commitid_from_commitid_file(os.path.join(self.logdir, 'commitid.log'),
+            # logdir, except if there was no new commit we will get None
-                                                       logger=self.pool._logger)
+            # instead. If the commit id could not be read, an exception will be
+            # raised.
+            commitid = get_commitid_from_commitid_file(
+                os.path.join(self.logdir, 'commitid.log')
+            )
             compose.notifier.send('ostree',
                                   variant=variant.uid,
                                   arch=arch,

tests/test_ostree_phase.py

@@ -145,6 +145,7 @@ class OSTreeThreadTest(helpers.PungiTestCase):
                 for filename in writefiles:
                     helpers.touch(os.path.join(logdir, filename),
                                   '\n'.join(writefiles[filename]))
+                    helpers.touch(os.path.join(logdir, filename + ".stamp"))
             return {'task_id': 1234, 'retcode': retcode, 'output': 'Foo bar\n'}
         return fake_runroot
 

tests/test_ostree_script.py

@@ -48,6 +48,7 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
             run.call_args_list,
             [mock.call(['rpm-ostree', 'compose', 'tree', '--repo=%s' % repo,
                         '--write-commitid-to=%s' % (self.topdir + '/logs/Atomic/commitid.log'),
+                        '--touch-if-changed=%s.stamp' % (self.topdir + '/logs/Atomic/commitid.log'),
                         self.topdir + '/fedora-atomic-docker-host.json'],
                        logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True)])
 
@@ -90,6 +91,7 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
             run.call_args_list,
             [mock.call(['rpm-ostree', 'compose', 'tree', '--repo=%s' % repo,
                         '--write-commitid-to=%s' % (self.topdir + '/logs/Atomic/commitid.log'),
+                        '--touch-if-changed=%s.stamp' % (self.topdir + '/logs/Atomic/commitid.log'),
                         self.topdir + '/fedora-atomic-docker-host.json'],
                        logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True)])
 
@@ -110,6 +112,7 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
             run.call_args_list,
             [mock.call(['rpm-ostree', 'compose', 'tree', '--repo=%s' % repo,
                         '--write-commitid-to=%s' % (self.topdir + '/logs/Atomic/commitid.log'),
+                        '--touch-if-changed=%s.stamp' % (self.topdir + '/logs/Atomic/commitid.log'),
                         self.topdir + '/fedora-atomic-docker-host.json'],
                        logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True),
              mock.call(['ostree', 'summary', '-u', '--repo=%s' % repo],
@@ -132,6 +135,7 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
             run.call_args_list,
             [mock.call(['rpm-ostree', 'compose', 'tree', '--repo=%s' % repo,
                         '--write-commitid-to=%s' % (self.topdir + '/logs/Atomic/commitid.log'),
+                        '--touch-if-changed=%s.stamp' % (self.topdir + '/logs/Atomic/commitid.log'),
                         '--add-metadata-string=version=24',
                         self.topdir + '/fedora-atomic-docker-host.json'],
                        logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True)])
@@ -219,6 +223,7 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
             run.call_args_list,
             [mock.call(['rpm-ostree', 'compose', 'tree', '--repo=%s' % repo,
                         '--write-commitid-to=%s' % (self.topdir + '/logs/Atomic/commitid.log'),
+                        '--touch-if-changed=%s.stamp' % (self.topdir + '/logs/Atomic/commitid.log'),
                         '--force-nocache',
                         self.topdir + '/fedora-atomic-docker-host.json'],
                        logfile=self.topdir + '/logs/Atomic/create-ostree-repo.log', show_cmd=True, stdout=True)])