Fix packages being deleted too early during upload

2 files changed, 34 insertions, 9 deletions

diff --git a/modules/buildsystem/templates/cleaner.rb b/modules/buildsystem/templates/cleaner.rb
index d9457757..da13a8e8 100755
--- a/modules/buildsystem/templates/cleaner.rb
+++ b/modules/buildsystem/templates/cleaner.rb
@@ -121,8 +121,12 @@ end
 # - Mark used src.rpm (if one is never marked, the src.rpm is obsolete)
 
 def packages(path)
-  `urpmf --synthesis "#{path}/media_info/synthesis.hdlist.cz" --qf '%sourcerpm:%filename' ":"`.each_line{|l|
-    yield *(l.rstrip.split(':'))
+  `urpmf --synthesis "#{path}/media_info/synthesis.hdlist.cz" --qf '%sourcerpm:%filename:%buildtime' ":"`.each_line{|l|
+    l2 = l.split(':')
+    sourcerpm = l2[0]
+    filename = l2[1]
+    buildtime = l2[2].to_i
+    yield(sourcerpm, filename, buildtime)
   }
 end
 
@@ -133,7 +137,7 @@ end
 def check_binaries(srcs, srcages, path, used_srcs)
   used_here_srcs = {}
   all_versions = {}
-  packages(path) {|src, filename|
+  packages(path) {|src, filename, buildtime|
     used_srcs[src] = true if used_srcs != nil
     if filename =~ /noarch.rpm$/ then
       # We need to mark the src.rpm present on this arch only for full noarch packages
@@ -149,7 +153,7 @@ def check_binaries(srcs, srcages, path, used_srcs)
     end
   }
   old_binaries = []
-  packages(path) {|src, filename|
+  packages(path) {|src, filename, buildtime|
     if ! srcs[src] then
       srcname = name_from_filename(src)
       if srcages[srcname] then
@@ -157,6 +161,8 @@ def check_binaries(srcs, srcages, path, used_srcs)
         # Only delete old binaries after 7d or if there is a new version
         name = name_from_filename(filename)
         next unless (srcages[srcname][1] < Time.now.to_i - 24*60*60*7 || all_versions[name].include?(src))
+        # Do not delete newer binaries, upload may be in progress
+        next unless buildtime < srcages[srcname][1]
         # Do not delete if the new version of the package hasn't been built for this arch yet
         next unless used_here_srcs[srcages[srcname][0]]
       end
diff --git a/modules/buildsystem/templates/cleaner_test.rb b/modules/buildsystem/templates/cleaner_test.rb
index d1f343e2..7e03b80d 100644
--- a/modules/buildsystem/templates/cleaner_test.rb
+++ b/modules/buildsystem/templates/cleaner_test.rb
@@ -10,12 +10,18 @@ class TestCleaner < Test::Unit::TestCase
   end
 
   def packages(path)
-    @pkgs.map{|p| yield *p.split(":")}
+    @pkgs.map{|p|
+      l2 = p.split(':')
+      sourcerpm = l2[0]
+      filename = l2[1]
+      buildtime = l2[2].to_i
+      yield(sourcerpm, filename, buildtime)
+    }
   end
 
   def test_old
     # Package was built on this arch and src.rpm for new version is 15d old
-    setpackages(['foo-43-1.src.rpm:libfoo2-43-1.armv7hl.rpm', 'foo-42-1.src.rpm:libfoo1-42-1.armv7hl.rpm'])
+    setpackages(['foo-43-1.src.rpm:libfoo2-43-1.armv7hl.rpm:43', 'foo-42-1.src.rpm:libfoo1-42-1.armv7hl.rpm:42'])
     srcages = {}
     srcages['foo'] = [ 'foo-43-1.src.rpm', Time.now.to_i - 15*24*3600 ]
     srcs = {}
@@ -25,7 +31,7 @@ class TestCleaner < Test::Unit::TestCase
 
   def test_recent
     # Package was built on this arch but src.rpm for new version is only 1d old
-    setpackages(['foo-43-1.src.rpm:foo-43-1.armv7hl.rpm', 'foo-42-1.src.rpm:foo-42-1.armv7hl.rpm'])
+    setpackages(['foo-43-1.src.rpm:foo-43-1.armv7hl.rpm:43', 'foo-42-1.src.rpm:foo-42-1.armv7hl.rpm:42'])
     srcages = {}
     srcages['foo'] = [ 'foo-43.src.rpm', Time.now.to_i - 24*3600 ]
     srcs = {}
@@ -35,7 +41,7 @@ class TestCleaner < Test::Unit::TestCase
 
   def test_arm_late
     # Package was not yet built on this arch
-    setpackages(['foo-42-1.src.rpm:foo-42-1.armv7hl.rpm'])
+    setpackages(['foo-42-1.src.rpm:foo-42-1.armv7hl.rpm:42'])
     srcages = {}
     srcages['foo'] = [ 'foo-43.src.rpm', Time.now.to_i - 24*3600 ]
     srcs = {}
@@ -46,11 +52,24 @@ class TestCleaner < Test::Unit::TestCase
   def test_multiple_versions
     # Old package remains (usually happens to noarch due to youri bug)
     $noarch = { 'foo' => true }
-    setpackages(['foo-42-1.src.rpm:foo-42-1.noarch.rpm', 'foo-42-2.src.rpm:foo-42-2.noarch.rpm'])
+    setpackages(['foo-42-1.src.rpm:foo-42-1.noarch.rpm:42', 'foo-42-2.src.rpm:foo-42-2.noarch.rpm:43'])
     srcages = {}
     srcages['foo'] = [ 'foo-42-2.src.rpm', Time.now.to_i - 24*3600 ]
     srcs = {}
     srcs['foo-42-2.src.rpm'] = true
     assert_equal(['foo-42-1.noarch.rpm'], check_binaries(srcs, srcages, '', nil))
   end
+
+  def test_newer_binary
+    # Package built after src remains (could happen due to race condition with upload)
+    $noarch = { 'foo' => true }
+    srctime = Time.now.to_i - 10*24*3600
+    bintime = Time.now.to_i - 3600
+    setpackages(["foo-42-2.src.rpm:foo-42-2.noarch.rpm:#{bintime}"])
+    srcages = {}
+    srcages['foo'] = [ 'foo-42-1.src.rpm', srctime ]
+    srcs = {}
+    srcs['foo-42-1.src.rpm'] = true
+    assert_equal([], check_binaries(srcs, srcages, '', nil))
+  end
 end