Merge pull request #2 from blue-systems/master

Merge master upstream

ci-tooling/lib/lint/log/build_log_segmenter.rb

@@ -1,11 +1,14 @@
 # Split a segment out of a build log by defining a start maker and an end marker
 module BuildLogSegmenter
+  class SegmentMissingError < StandardError; end
+
   module_function
 
   def segmentify(data, start_marker, end_marker)
     start_index = data.index(start_marker)
     end_index = data.index(end_marker)
-    return [] unless start_index && end_index
+    raise SegmentMissingError, "missing #{start_marker}" unless start_index
+    raise SegmentMissingError, "missing #{end_marker}" unless end_index
     data = data.slice(start_index..end_index).split("\n")
     data.shift # Ditch start line
     data.pop # Ditch end line

ci-tooling/lib/lint/log/cmake.rb

@@ -1,3 +1,23 @@
+# frozen_string_literal: true
+#
+# Copyright (C) 2016 Harald Sitter <sitter@kde.org>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) version 3, or any
+# later version accepted by the membership of KDE e.V. (or its
+# successor approved by the membership of KDE e.V.), which shall
+# act as a proxy defined in Section 6 of version 3 of the license.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
 require_relative '../linter'
 require_relative 'build_log_segmenter'
 
@@ -26,28 +46,32 @@ class CMake < Linter
       def lint(data)
         r = Result.new
         data = segmentify(data, 'dh_auto_configure', 'dh_auto_build')
-        r.valid = !data.empty?
+        r.valid = true
         parse(data, r)
-        r.informations.uniq!
-        r.warnings.uniq!
-        r.errors.uniq!
+        r.uniq
+      rescue BuildLogSegmenter::SegmentMissingError => e
+        puts "#{self.class}: in log #{e.message}"
         r
       end
 
       private
 
+      def warnings(line, data)
+        METHS.each do |id, meth|
+          next unless line.include?(id)
+          ret = send(meth, line, data)
+          @ignores.each do |ignore|
+            ret.reject! { |d| ignore.match?(d) }
+          end
+          return ret
+        end
+        []
+      end
+
       def parse(data, result)
         until data.empty?
           line = data.shift
-          METHS.each do |id, meth|
-            next unless line.include?(id)
-            ret = send(meth, line, data)
-            @ignores.each do |ignore|
-              ret.reject! { |d| ignore.match?(d) }
-            end
-            result.warnings += ret
-            break
-          end
+          result.warnings += warnings(line, data)
         end
       end
 
@@ -88,7 +112,7 @@ def parse_warning(line, _data)
         return [] if line.include?('CMake Warning at ' \
           '/usr/share/kde4/apps/cmake/modules/MacroOptionalFindPackage.cmake')
         # Lines coming from find_package (from old parsing).
-        return [] if line.match(/CMake Warning at [^ :]+:\d+ \(find_package\)/)
+        return [] if line =~ /CMake Warning at [^ :]+:\d+ \(find_package\)/
         # Lines coming from warnings inside the actual CMakeLists.txt as those
         # can be arbitrary.
         # ref: "CMake Warning at src/worker/CMakeLists.txt:33 (message):"

ci-tooling/lib/lint/log/lintian.rb

@@ -31,11 +31,14 @@ class Lintian < Linter
       def lint(data)
         r = Result.new
         data = segmentify(data, "=== Start lintian\n", "=== End lintian\n")
-        r.valid = !data.empty?
+        r.valid = true
         data.each do |line|
           lint_line(line, r)
         end
         r
+      rescue BuildLogSegmenter::SegmentMissingError => e
+        puts "#{self.class}: in log #{e.message}"
+        r
       end
 
       private

ci-tooling/lib/lint/log/list_missing.rb

@@ -3,6 +3,7 @@
 
 module Lint
   class Log
+    # Parses list-missing block from a build log.
     class ListMissing < Linter
       include BuildLogSegmenter
 
@@ -11,10 +12,15 @@ def lint(data)
         data = segmentify(data,
                           "=== Start list-missing\n",
                           "=== End list-missing\n")
-        r.valid = !data.empty?
-        data.each do |line|
-          r.errors << line
-        end
+        # TODO: This doesn't really make sense? What does valid mean anyway?
+        #  should probably be if the linting was able to be done, which is not
+        #  asserted by this at all. segmentify would need to raise on
+        # missing blocks
+        r.valid = true
+        data.each { |line| r.errors << line }
+        r
+      rescue BuildLogSegmenter::SegmentMissingError => e
+        puts "#{self.class}: in log #{e.message}"
         r
       end
     end

ci-tooling/lib/lint/result.rb

@@ -20,6 +20,13 @@ def merge!(other)
       @informations += other.informations
     end
 
+    def uniq
+      @errors.uniq!
+      @warnings.uniq!
+      @informations.uniq!
+      self
+    end
+
     def all
       @errors + @warnings + @informations
     end

ci-tooling/lib/projects.rb

@@ -40,6 +40,9 @@ class Error < Exception; end
   class TransactionError < Error; end
   class BzrTransactionError < TransactionError; end
   class GitTransactionError < TransactionError; end
+  # Derives from RuntimeError because someone decided to resuce Transaction
+  # and Runtime in factories only...
+  class GitNoBranchError < RuntimeError; end
 
   extend Deprecate
 
@@ -228,14 +231,25 @@ def get_bzr(uri, dest)
       raise BzrTransactionError, "Could not checkout #{uri}"
     end
 
+    # Separation method. Within the context of a checkout an execution error
+    # is not a transaction error, but simply means the branch does not exist.
+    # This type of error is different from a TransactionError in that it is
+    # not retriable (or rather it makes no sense to retry as the branch is not
+    # going to magically appear).
+    def git_checkout(repo, branch)
+      repo.checkout("origin/#{branch}")
+    rescue Git::GitExecuteError
+      raise GitNoBranchError, "origin/#{branch}"
+    end
+
     def update_git(branch, dir = Dir.pwd)
       repo = Git.open(dir)
       repo.clean(force: true, d: true)
       repo.reset(nil, hard: true)
       repo.gc
       repo.config('remote.origin.prune', true)
       repo.fetch('origin')
-      repo.checkout("origin/#{branch}")
+      git_checkout(repo, branch)
     rescue Git::GitExecuteError => e
       raise GitTransactionError, e
     end
@@ -280,7 +294,7 @@ def init_packaging_scm(url_base, branch)
   end
 
   def get
-    Retry.retry_it(errors: [TransactionError], times: 5) do
+    Retry.retry_it(errors: [TransactionError], times: 2, sleep: 5) do
       if @component == 'launchpad'
         self.class.get_bzr(@packaging_scm.url, @name)
       else
@@ -290,7 +304,7 @@ def get
   end
 
   def update(branch)
-    Retry.retry_it(errors: [TransactionError], times: 5) do
+    Retry.retry_it(errors: [TransactionError], times: 2, sleep: 5) do
       if @component == 'launchpad'
         self.class.update_bzr(branch)
       else

ci-tooling/nci/lib/lint/result_test.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+#
+# Copyright (C) 2016 Harald Sitter <sitter@kde.org>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) version 3, or any
+# later version accepted by the membership of KDE e.V. (or its
+# successor approved by the membership of KDE e.V.), which shall
+# act as a proxy defined in Section 6 of version 3 of the license.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+# FIXME: we manually load the reporter here because we install it from git
+#   and would need bundler to load it properly, alas, bundler can't help
+#   either because in containers we throw away gemfile and friends on
+#   account of only using ci-tooling/
+#   Ideally we'd simply have the gem updated properly so we don't need
+#   git anymore.
+begin
+  require 'ci/reporter/rake/test_unit_loader'
+rescue LoadError
+  REPORTER = 'ci_reporter_test_unit-5c6c30d120a3'.freeze
+  require format("#{Gem.default_dir}/bundler/gems/#{REPORTER}/lib/%s",
+                 'ci/reporter/rake/test_unit_loader')
+end
+require 'test/unit'
+
+module Lint
+  # Convenience class to test lint results
+  class ResultTest < Test::Unit::TestCase
+    def join(array)
+      # Add terminating nils to get an extra newlines
+      (array + [nil, nil]).join("\n")
+    end
+
+    def result_notify(array)
+      notify(join(array)) unless array.empty?
+    end
+
+    def result_flunk(array)
+      flunk(join(array)) unless array.empty?
+    end
+
+    def assert_result(result)
+      result_notify(result.warnings)
+      result_notify(result.informations)
+      # Flunking fails the test entirely, so this needs to be at the very end!
+      result_flunk(result.errors)
+      # FIXME: valid means nothing concrete so we skip it for now
+      # assert(result.valid, "Lint result not valid ::\n #{result.inspect}")
+    end
+  end
+end

ci-tooling/nci/lint_bin.rb

@@ -19,80 +19,12 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-REPORTER = 'ci_reporter_test_unit-5c6c30d120a3'.freeze
-require format("#{Gem.default_dir}/bundler/gems/#{REPORTER}/lib/%s",
-               'ci/reporter/rake/test_unit_loader')
-require 'logger'
-require 'logger/colors'
-require 'open-uri'
-require 'test/unit'
-
-require_relative '../lib/lint/control'
-require_relative '../lib/lint/log'
-require_relative '../lib/lint/merge_marker'
-require_relative '../lib/lint/result'
-require_relative '../lib/lint/series'
-require_relative '../lib/lint/symbols'
-
 ENV['CI_REPORTS'] = "#{Dir.pwd}/reports".freeze
-
+# FIXME: should use env not the marker file
 # BUILD_URL = ENV.fetch('BUILD_URL')
-BUILD_URL = File.read('build_url')
-LOG_URL = "#{BUILD_URL}/consoleText".freeze
-
-module Lint
-  class TestCase < Test::Unit::TestCase
-    def assert_result(result)
-      assert(result.valid, "Lint result not valid ::\n #{result}")
-      notify(result.warnings.join("\n")) unless result.warnings.empty?
-      notify(result.informations.join("\n")) unless result.informations.empty?
-      # Flunking fails the test entirely, so this needs to be at the very end!
-      flunk(result.errors.join("\n")) unless result.errors.empty?
-    end
-  end
-
-  class TestLog < TestCase
-    def initialize(*args)
-      super
-      @log_orig = open(LOG_URL).read
-    end
-
-    def setup
-      @log = @log_orig.dup
-    end
-
-    def test_cmake
-      assert_result Log::CMake.new.lint(@log)
-    end
-
-    def test_lintian
-      assert_result Log::Lintian.new.lint(@log)
-    end
-
-    def test_list_missing
-      assert_result Log::ListMissing.new.lint(@log)
-    end
-  end
-
-  class TestPackaging < TestCase
-    def setup
-      @dir = 'build'.freeze
-    end
-
-    def test_control
-      assert_result Control.new(@dir).lint
-    end
-
-    def test_series
-      assert_result Series.new(@dir).lint
-    end
-
-    def test_symbols
-      assert_result Symbols.new(@dir).lint
-    end
+BUILD_URL = File.read('build_url').strip
+ENV['LOG_URL'] = "#{BUILD_URL}/consoleText".freeze
 
-    def test_merge_markers
-      assert_result MergeMarker.new(@dir).lint
-    end
-  end
+Dir.glob(File.expand_path('lint_bin/test_*.rb', __dir__)).each do |file|
+  require file
 end