Merge branch '41972-make-sure-no-warnings' into 'master'

Fail static-analysis if there's output to stderr

Closes #41972

See merge request gitlab-org/gitlab-ce!16648

.gitlab-ci.yml

@@ -321,6 +321,7 @@ setup-test-env:
     expire_in: 7d
     paths:
       - tmp/tests
+      - config/secrets.yml
 rspec-pg 0 27: *rspec-metadata-pg
 rspec-pg 1 27: *rspec-metadata-pg

Gemfile

@@ -325,7 +325,7 @@ group :development, :test do
   gem 'spinach-rerun-reporter', '~> 0.0.2'
   gem 'rspec_profiling', '~> 0.0.5'
   gem 'rspec-set', '~> 0.1.3'
-  gem 'rspec-parameterized'
+  gem 'rspec-parameterized', require: false
   # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826)
   gem 'minitest', '~> 5.7.0'

Gemfile.lock

@@ -304,7 +304,7 @@ GEM
       mime-types (>= 1.16)
       posix-spawn (~> 0.3)
     gitlab-markup (1.6.3)
-    gitlab-styles (2.3.1)
+    gitlab-styles (2.3.2)
       rubocop (~> 0.51)
       rubocop-gitlab-security (~> 0.1.0)
       rubocop-rspec (~> 1.19)

bin/upgrade.rb

-require_relative "../lib/gitlab/upgrader"
-
-Gitlab::Upgrader.new.execute

config/application.rb

@@ -6,6 +6,7 @@ Bundler.require(:default, Rails.env)
 module Gitlab
   class Application < Rails::Application
+    require_dependency Rails.root.join('lib/gitlab/redis/wrapper')
     require_dependency Rails.root.join('lib/gitlab/redis/cache')
     require_dependency Rails.root.join('lib/gitlab/redis/queues')
     require_dependency Rails.root.join('lib/gitlab/redis/shared_state')

lib/gitlab/popen.rb

@@ -5,7 +5,17 @@ module Gitlab
   module Popen
     extend self
-    def popen(cmd, path = nil, vars = {})
+    Result = Struct.new(:cmd, :stdout, :stderr, :status, :duration)
+
+    # Returns [stdout + stderr, status]
+    def popen(cmd, path = nil, vars = {}, &block)
+      result = popen_with_detail(cmd, path, vars, &block)
+
+      [result.stdout << result.stderr, result.status&.exitstatus]
+    end
+
+    # Returns Result
+    def popen_with_detail(cmd, path = nil, vars = {})
       unless cmd.is_a?(Array)
         raise "System commands must be given as an array of strings"
       end
@@ -18,18 +28,21 @@ module Gitlab
         FileUtils.mkdir_p(path)
       end
-      cmd_output = ""
-      cmd_status = 0
+      cmd_stdout = ''
+      cmd_stderr = ''
+      cmd_status = nil
+      start = Time.now
+
       Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
         yield(stdin) if block_given?
         stdin.close
-        cmd_output << stdout.read
-        cmd_output << stderr.read
-        cmd_status = wait_thr.value.exitstatus
+        cmd_stdout = stdout.read
+        cmd_stderr = stderr.read
+        cmd_status = wait_thr.value
       end
-      [cmd_output, cmd_status]
+      Result.new(cmd, cmd_stdout, cmd_stderr, cmd_status, Time.now - start)
     end
   end
 end

lib/gitlab/popen/runner.rb

+module Gitlab
+  module Popen
+    class Runner
+      attr_reader :results
+
+      def initialize
+        @results = []
+      end
+
+      def run(commands, &block)
+        commands.each do |cmd|
+          # yield doesn't support blocks, so we need to use a block variable
+          block.call(cmd) do # rubocop:disable Performance/RedundantBlockCall
+            cmd_result = Gitlab::Popen.popen_with_detail(cmd)
+
+            results << cmd_result
+
+            cmd_result
+          end
+        end
+      end
+
+      def all_success_and_clean?
+        all_success? && all_stderr_empty?
+      end
+
+      def all_success?
+        results.all? { |result| result.status.success? }
+      end
+
+      def all_stderr_empty?
+        results.all? { |result| result.stderr.empty? }
+      end
+
+      def failed_results
+        results.reject { |result| result.status.success? }
+      end
+
+      def warned_results
+        results.select do |result|
+          result.status.success? && !result.stderr.empty?
+        end
+      end
+    end
+  end
+end

lib/gitlab/redis/cache.rb

 # please require all dependencies below:
-require_relative 'wrapper' unless defined?(::Gitlab::Redis::Wrapper)
+require_relative 'wrapper' unless defined?(::Rails) && ::Rails.root.present?
 module Gitlab
   module Redis

lib/tasks/gitlab/task_helpers.rb → lib/gitlab/task_helpers.rb

 require 'rainbow/ext/string'
 require 'gitlab/utils/strong_memoize'
+# rubocop:disable Rails/Output
 module Gitlab
   TaskFailedError = Class.new(StandardError)
   TaskAbortedByUserError = Class.new(StandardError)
@@ -96,11 +97,9 @@ module Gitlab
     end
     def gid_for(group_name)
-      begin
-        Etc.getgrnam(group_name).gid
-      rescue ArgumentError # no group
-        "group #{group_name} doesn't exist"
-      end
+      Etc.getgrnam(group_name).gid
+    rescue ArgumentError # no group
+      "group #{group_name} doesn't exist"
     end
     def gitlab_user

lib/gitlab/upgrader.rb

-require_relative "popen"
-require_relative "version_info"
-
 module Gitlab
   class Upgrader
     def execute

lib/system_check/helpers.rb

-require 'tasks/gitlab/task_helpers'
-
 module SystemCheck
   module Helpers
     include ::Gitlab::TaskHelpers

lib/tasks/flay.rake

 desc 'Code duplication analyze via flay'
 task :flay do
-  output = `bundle exec flay --mass 35 app/ lib/gitlab/`
+  output = `bundle exec flay --mass 35 app/ lib/gitlab/ 2> #{File::NULL}`
   if output.include? "Similar code found"
     puts output

lib/tasks/gitlab/backup.rake

@@ -4,7 +4,7 @@ namespace :gitlab do
   namespace :backup do
     # Create backup of GitLab system
     desc "GitLab | Create a backup of the GitLab system"
-    task create: :environment do
+    task create: :gitlab_environment do
       warn_user_is_not_gitlab
       configure_cron_mode
@@ -25,7 +25,7 @@ namespace :gitlab do
     # Restore backup of GitLab system
     desc 'GitLab | Restore a previously created backup'
-    task restore: :environment do
+    task restore: :gitlab_environment do
       warn_user_is_not_gitlab
       configure_cron_mode
@@ -73,7 +73,7 @@ namespace :gitlab do
     end
     namespace :repo do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping repositories ...".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("repositories")
@@ -84,7 +84,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring repositories ...".color(:blue)
         Backup::Repository.new.restore
         $progress.puts "done".color(:green)
@@ -92,7 +92,7 @@ namespace :gitlab do
     end
     namespace :db do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping database ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("db")
@@ -103,7 +103,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring database ... ".color(:blue)
         Backup::Database.new.restore
         $progress.puts "done".color(:green)
@@ -111,7 +111,7 @@ namespace :gitlab do
     end
     namespace :builds do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping builds ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("builds")
@@ -122,7 +122,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring builds ... ".color(:blue)
         Backup::Builds.new.restore
         $progress.puts "done".color(:green)
@@ -130,7 +130,7 @@ namespace :gitlab do
     end
     namespace :uploads do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping uploads ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("uploads")
@@ -141,7 +141,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring uploads ... ".color(:blue)
         Backup::Uploads.new.restore
         $progress.puts "done".color(:green)
@@ -149,7 +149,7 @@ namespace :gitlab do
     end
     namespace :artifacts do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping artifacts ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("artifacts")
@@ -160,7 +160,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring artifacts ... ".color(:blue)
         Backup::Artifacts.new.restore
         $progress.puts "done".color(:green)
@@ -168,7 +168,7 @@ namespace :gitlab do
     end
     namespace :pages do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping pages ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("pages")
@@ -179,7 +179,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring pages ... ".color(:blue)
         Backup::Pages.new.restore
         $progress.puts "done".color(:green)
@@ -187,7 +187,7 @@ namespace :gitlab do
     end
     namespace :lfs do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping lfs objects ... ".color(:blue)
         if ENV["SKIP"] && ENV["SKIP"].include?("lfs")
@@ -198,7 +198,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring lfs objects ... ".color(:blue)
         Backup::Lfs.new.restore
         $progress.puts "done".color(:green)
@@ -206,7 +206,7 @@ namespace :gitlab do
     end
     namespace :registry do
-      task create: :environment do
+      task create: :gitlab_environment do
         $progress.puts "Dumping container registry images ... ".color(:blue)
         if Gitlab.config.registry.enabled
@@ -221,7 +221,7 @@ namespace :gitlab do
         end
       end
-      task restore: :environment do
+      task restore: :gitlab_environment do
         $progress.puts "Restoring container registry images ... ".color(:blue)
         if Gitlab.config.registry.enabled

lib/tasks/gitlab/check.rake

-# Temporary hack, until we migrate all checks to SystemCheck format
-require 'system_check'
-require 'system_check/helpers'
-
 namespace :gitlab do
   desc 'GitLab | Check the configuration of GitLab and its environment'
   task check: %w{gitlab:gitlab_shell:check
@@ -12,7 +8,7 @@ namespace :gitlab do
   namespace :app do
     desc 'GitLab | Check the configuration of the GitLab Rails app'
-    task check: :environment  do
+    task check: :gitlab_environment do
       warn_user_is_not_gitlab
       checks = [
@@ -43,7 +39,7 @@ namespace :gitlab do
   namespace :gitlab_shell do
     desc "GitLab | Check the configuration of GitLab Shell"
-    task check: :environment  do
+    task check: :gitlab_environment do
       warn_user_is_not_gitlab
       start_checking "GitLab Shell"
@@ -251,7 +247,7 @@ namespace :gitlab do
   namespace :sidekiq do
     desc "GitLab | Check the configuration of Sidekiq"
-    task check: :environment  do
+    task check: :gitlab_environment do
       warn_user_is_not_gitlab
       start_checking "Sidekiq"
@@ -310,7 +306,7 @@ namespace :gitlab do
   namespace :incoming_email do
     desc "GitLab | Check the configuration of Reply by email"
-    task check: :environment  do
+    task check: :gitlab_environment do
       warn_user_is_not_gitlab
       if Gitlab.config.incoming_email.enabled
@@ -333,7 +329,7 @@ namespace :gitlab do
   end
   namespace :ldap do
-    task :check, [:limit] => :environment do |_, args|
+    task :check, [:limit] => :gitlab_environment do |_, args|
       # Only show up to 100 results because LDAP directories can be very big.
       # This setting only affects the `rake gitlab:check` script.
       args.with_defaults(limit: 100)
@@ -389,7 +385,7 @@ namespace :gitlab do
   namespace :repo do
     desc "GitLab | Check the integrity of the repositories managed by GitLab"
-    task check: :environment do
+    task check: :gitlab_environment do
       puts "This task is deprecated. Please use gitlab:git:fsck instead".color(:red)
       Rake::Task["gitlab:git:fsck"].execute
     end
@@ -397,7 +393,7 @@ namespace :gitlab do
   namespace :orphans do
     desc 'Gitlab | Check for orphaned namespaces and repositories'
-    task check: :environment do
+    task check: :gitlab_environment do
       warn_user_is_not_gitlab
       checks = [
         SystemCheck::Orphans::NamespaceCheck,
@@ -408,7 +404,7 @@ namespace :gitlab do
     end
     desc 'GitLab | Check for orphaned namespaces in the repositories path'
-    task check_namespaces: :environment do
+    task check_namespaces: :gitlab_environment do
       warn_user_is_not_gitlab
       checks = [SystemCheck::Orphans::NamespaceCheck]
@@ -416,7 +412,7 @@ namespace :gitlab do
     end
     desc 'GitLab | Check for orphaned repositories in the repositories path'
-    task check_repositories: :environment do
+    task check_repositories: :gitlab_environment do
       warn_user_is_not_gitlab
       checks = [SystemCheck::Orphans::RepositoryCheck]
@@ -426,7 +422,7 @@ namespace :gitlab do
   namespace :user do
     desc "GitLab | Check the integrity of a specific user's repositories"
-    task :check_repos, [:username] => :environment do |t, args|
+    task :check_repos, [:username] => :gitlab_environment do |t, args|
       username = args[:username] || prompt("Check repository integrity for username? ".color(:blue))
       user = User.find_by(username: username)
       if user

lib/tasks/gitlab/cleanup.rake

@@ -5,7 +5,7 @@ namespace :gitlab do
     HASHED_REPOSITORY_NAME = '@hashed'.freeze
     desc "GitLab | Cleanup | Clean namespaces"
-    task dirs: :environment  do
+    task dirs: :gitlab_environment do
       warn_user_is_not_gitlab
       remove_flag = ENV['REMOVE']
@@ -49,7 +49,7 @@ namespace :gitlab do
     end
     desc "GitLab | Cleanup | Clean repositories"
-    task repos: :environment  do
+    task repos: :gitlab_environment do
       warn_user_is_not_gitlab
       move_suffix = "+orphaned+#{Time.now.to_i}"
@@ -78,7 +78,7 @@ namespace :gitlab do
     end
     desc "GitLab | Cleanup | Block users that have been removed in LDAP"
-    task block_removed_ldap_users: :environment  do
+    task block_removed_ldap_users: :gitlab_environment do
       warn_user_is_not_gitlab
       block_flag = ENV['BLOCK']
@@ -109,7 +109,7 @@ namespace :gitlab do
     # released. So likely this should only be run once on gitlab.com
     # Faulty refs are moved so they are kept around, else some features break.
     desc 'GitLab | Cleanup | Remove faulty deployment refs'
-    task move_faulty_deployment_refs: :environment do
+    task move_faulty_deployment_refs: :gitlab_environment do
       projects = Project.where(id: Deployment.select(:project_id).distinct)
       projects.find_each do |project|

lib/tasks/gitlab/git.rake

 namespace :gitlab do
   namespace :git do
     desc "GitLab | Git | Repack"
-    task repack: :environment do
+    task repack: :gitlab_environment do
       failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} repack -a --quiet), "Repacking repo")
       if failures.empty?
         puts "Done".color(:green)
@@ -11,7 +11,7 @@ namespace :gitlab do
     end
     desc "GitLab | Git | Run garbage collection on all repos"
-    task gc: :environment do
+    task gc: :gitlab_environment do
       failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} gc --auto --quiet), "Garbage Collecting")
       if failures.empty?
         puts "Done".color(:green)
@@ -21,7 +21,7 @@ namespace :gitlab do
     end
     desc "GitLab | Git | Prune all repos"
-    task prune: :environment do
+    task prune: :gitlab_environment do
       failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} prune), "Git Prune")
       if failures.empty?
         puts "Done".color(:green)
@@ -31,7 +31,7 @@ namespace :gitlab do
     end
     desc 'GitLab | Git | Check all repos integrity'
-    task fsck: :environment do
+    task fsck: :gitlab_environment do
       failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") do |repo|
         check_config_lock(repo)
         check_ref_locks(repo)

lib/tasks/gitlab/gitaly.rake

 namespace :gitlab do
   namespace :gitaly do
     desc "GitLab | Install or upgrade gitaly"
-    task :install, [:dir, :repo] => :environment do |t, args|
+    task :install, [:dir, :repo] => :gitlab_environment do |t, args|
       require 'toml'
       warn_user_is_not_gitlab

lib/tasks/gitlab/helpers.rake

-require 'tasks/gitlab/task_helpers'
-
 # Prevent StateMachine warnings from outputting during a cron task
 StateMachines::Machine.ignore_method_conflicts = true if ENV['CRON']
-namespace :gitlab do
+task gitlab_environment: :environment do
   extend SystemCheck::Helpers
 end

lib/tasks/gitlab/info.rake

 namespace :gitlab do
   namespace :env do
     desc "GitLab | Show information about GitLab and its environment"
-    task info: :environment  do
+    task info: :gitlab_environment do
       # check if there is an RVM environment
       rvm_version = run_and_match(%w(rvm --version), /[\d\.]+/).try(:to_s)
       # check Ruby version

lib/tasks/gitlab/setup.rake

 namespace :gitlab do
   desc "GitLab | Setup production application"
-  task setup: :environment do
+  task setup: :gitlab_environment do
     setup_db
   end