Merge branch 'dz-update-deps' into 'master'

Update rubocop

See merge request gitlab-org/gitlab-shell!190

Gemfile

 source "http://rubygems.org"
 
 group :development, :test do
-  gem 'simplecov', require: false
-  gem 'rspec', '~> 2.14.0'
-  gem 'webmock'
   gem 'guard'
   gem 'guard-rspec'
+  gem 'rspec', '~> 2.14.0'
+  gem 'rubocop', '0.49.1', require: false
+  gem 'simplecov', require: false
   gem 'vcr'
-  gem 'rubocop', '0.28.0', require: false
+  gem 'webmock'
 end

Gemfile.lock

@@ -2,11 +2,9 @@ GEM
   remote: http://rubygems.org/
   specs:
     addressable (2.3.2)
-    ast (2.0.0)
-    astrolabe (1.3.0)
-      parser (>= 2.2.0.pre.3, < 3.0)
+    ast (2.4.0)
     coderay (1.0.8)
-    crack (0.3.1)
+    crack (0.3.2)
     diff-lcs (1.2.5)
     docile (1.1.5)
     guard (1.5.4)
@@ -21,14 +19,17 @@ GEM
     lumberjack (1.0.2)
     method_source (0.8.1)
     multi_json (1.10.1)
-    parser (2.2.0.2)
-      ast (>= 1.1, < 3.0)
-    powerpack (0.0.9)
+    parallel (1.12.1)
+    parser (2.5.0.2)
+      ast (~> 2.4.0)
+    powerpack (0.1.1)
     pry (0.9.10)
       coderay (~> 1.0.5)
       method_source (~> 0.8)
       slop (~> 3.3.1)
-    rainbow (2.0.0)
+    rainbow (2.2.2)
+      rake
+    rake (12.3.0)
     rspec (2.14.1)
       rspec-core (~> 2.14.0)
       rspec-expectations (~> 2.14.0)
@@ -37,13 +38,14 @@ GEM
     rspec-expectations (2.14.5)
       diff-lcs (>= 1.1.3, < 2.0)
     rspec-mocks (2.14.6)
-    rubocop (0.28.0)
-      astrolabe (~> 1.3)
-      parser (>= 2.2.0.pre.7, < 3.0)
-      powerpack (~> 0.0.6)
+    rubocop (0.49.1)
+      parallel (~> 1.10)
+      parser (>= 2.3.3.1, < 3.0)
+      powerpack (~> 0.1)
       rainbow (>= 1.99.1, < 3.0)
-      ruby-progressbar (~> 1.4)
-    ruby-progressbar (1.7.1)
+      ruby-progressbar (~> 1.7)
+      unicode-display_width (~> 1.0, >= 1.0.1)
+    ruby-progressbar (1.9.0)
     simplecov (0.9.1)
       docile (~> 1.1.0)
       multi_json (~> 1.0)
@@ -51,6 +53,7 @@ GEM
     simplecov-html (0.8.0)
     slop (3.3.3)
     thor (0.19.1)
+    unicode-display_width (1.3.0)
     vcr (2.4.0)
     webmock (1.9.0)
       addressable (>= 2.2.7)
@@ -63,7 +66,7 @@ DEPENDENCIES
   guard
   guard-rspec
   rspec (~> 2.14.0)
-  rubocop (= 0.28.0)
+  rubocop (= 0.49.1)
   simplecov
   vcr
   webmock

hooks/post-receive

@@ -4,7 +4,7 @@
 # will be processed properly.
 
 refs = $stdin.read
-key_id  = ENV.delete('GL_ID')
+key_id = ENV.delete('GL_ID')
 gl_repository = ENV.delete('GL_REPOSITORY')
 repo_path = Dir.pwd
 

lib/gitlab_access_status.rb

@@ -14,12 +14,12 @@ class GitAccessStatus
 
   def self.create_from_json(json)
     values = JSON.parse(json)
-    self.new(values["status"],
-             values["message"],
-             gl_repository: values["gl_repository"],
-             gl_username: values["gl_username"],
-             repository_path: values["repository_path"],
-             gitaly: values["gitaly"])
+    new(values["status"],
+        values["message"],
+        gl_repository: values["gl_repository"],
+        gl_username: values["gl_username"],
+        repository_path: values["repository_path"],
+        gitaly: values["gitaly"])
   end
 
   def allowed?

lib/gitlab_custom_hook.rb

@@ -47,7 +47,7 @@ class GitlabCustomHook
     # Submit changes to the hook via its stdin.
     begin
       IO.copy_stream(StringIO.new(changes), stdin_writer)
-    rescue Errno::EPIPE
+    rescue Errno::EPIPE # rubocop:disable Lint/HandleExceptions
       # It is not an error if the hook does not consume all of its input.
     end
 

lib/gitlab_keys.rb

@@ -4,19 +4,26 @@ require_relative 'gitlab_config'
 require_relative 'gitlab_logger'
 require_relative 'gitlab_metrics'
 
-class GitlabKeys
-  class KeyError < StandardError ; end
+class GitlabKeys # rubocop:disable Metrics/ClassLength
+  class KeyError < StandardError; end
 
   attr_accessor :auth_file, :key
 
   def self.command(key_id)
-    raise KeyError.new("Invalid key_id: #{key_id.inspect}") unless /\A[a-z0-9-]+\z/ =~ key_id
+    unless /\A[a-z0-9-]+\z/ =~ key_id
+      raise KeyError, "Invalid key_id: #{key_id.inspect}"
+    end
+
     "#{ROOT_PATH}/bin/gitlab-shell #{key_id}"
   end
 
   def self.key_line(key_id, public_key)
     public_key.chomp!
-    raise KeyError.new("Invalid public_key: #{public_key.inspect}") if public_key.include?("\n")
+
+    if public_key.include?("\n")
+      raise KeyError, "Invalid public_key: #{public_key.inspect}"
+    end
+
     "command=\"#{command(key_id)}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}"
   end
 
@@ -31,19 +38,19 @@ class GitlabKeys
   def exec
     GitlabMetrics.measure("command-#{@command}") do
       case @command
-      when 'add-key';
+      when 'add-key'
         add_key
-      when 'batch-add-keys';
+      when 'batch-add-keys'
         batch_add_keys
-      when 'rm-key';
+      when 'rm-key'
         rm_key
-      when 'list-keys';
+      when 'list-keys'
         list_keys
-      when 'list-key-ids';
+      when 'list-key-ids'
         list_key_ids
-      when 'clear';
+      when 'clear'
         clear
-      when 'check-permissions';
+      when 'check-permissions'
         check_permissions
       else
         $logger.warn "Attempt to execute invalid gitlab-keys command #{@command.inspect}."
@@ -111,7 +118,7 @@ class GitlabKeys
     lock do
       $logger.info "Removing key #{@key_id}"
       open_auth_file('r+') do |f|
-        while line = f.gets do
+        while line = f.gets # rubocop:disable Style/AssignmentInCondition
           next unless line.start_with?("command=\"#{self.class.command(@key_id)}\"")
           f.seek(-line.length, IO::SEEK_CUR)
           # Overwrite the line with #'s. Because the 'line' variable contains
@@ -145,7 +152,7 @@ class GitlabKeys
     File.open(lock_file, "w+") do |f|
       begin
         f.flock File::LOCK_EX
-        Timeout::timeout(timeout) { yield }
+        Timeout.timeout(timeout) { yield }
       ensure
         f.flock File::LOCK_UN
       end
@@ -157,8 +164,8 @@ class GitlabKeys
   end
 
   def open_auth_file(mode)
-    open(auth_file, mode, 0600) do |file|
-      file.chmod(0600)
+    open(auth_file, mode, 0o600) do |file|
+      file.chmod(0o600)
       yield file
     end
   end

lib/gitlab_lfs_authentication.rb

@@ -11,12 +11,10 @@ class GitlabLfsAuthentication
   end
 
   def self.build_from_json(json)
-    begin
-      values = JSON.parse(json)
-      self.new(values['username'], values['lfs_token'], values['repository_http_path'])
-    rescue
-      nil
-    end
+    values = JSON.parse(json)
+    new(values['username'], values['lfs_token'], values['repository_http_path'])
+  rescue
+    nil
   end
 
   def authentication_payload

lib/gitlab_logger.rb

@@ -2,7 +2,7 @@ require 'logger'
 
 require_relative 'gitlab_config'
 
-def convert_log_level log_level
+def convert_log_level(log_level)
   Logger.const_get(log_level.upcase)
 rescue NameError
   $stderr.puts "WARNING: Unrecognized log level #{log_level.inspect}."

lib/gitlab_net.rb

@@ -8,7 +8,7 @@ require_relative 'gitlab_access'
 require_relative 'gitlab_lfs_authentication'
 require_relative 'httpunix'
 
-class GitlabNet
+class GitlabNet # rubocop:disable Metrics/ClassLength
   class ApiUnreachableError < StandardError; end
   class NotFound < StandardError; end
 
@@ -16,7 +16,7 @@ class GitlabNet
   READ_TIMEOUT = 300
 
   def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {})
-    changes = changes.join("\n") unless changes.kind_of?(String)
+    changes = changes.join("\n") unless changes.is_a?(String)
 
     params = {
       action: cmd,
@@ -28,9 +28,9 @@ class GitlabNet
     }
 
     if actor =~ /\Akey\-\d+\Z/
-      params.merge!(key_id: actor.gsub("key-", ""))
+      params[:key_id] = actor.gsub("key-", "")
     elsif actor =~ /\Auser\-\d+\Z/
-      params.merge!(user_id: actor.gsub("user-", ""))
+      params[:user_id] = actor.gsub("user-", "")
     end
 
     url = "#{host}/allowed"
@@ -73,7 +73,7 @@ class GitlabNet
   end
 
   def merge_request_urls(gl_repository, repo_path, changes)
-    changes = changes.join("\n") unless changes.kind_of?(String)
+    changes = changes.join("\n") unless changes.is_a?(String)
     changes = changes.encode('UTF-8', 'ASCII', invalid: :replace, replace: '')
     url = "#{host}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
     url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
@@ -141,7 +141,7 @@ class GitlabNet
   protected
 
   def sanitize_path(repo)
-    repo.gsub("'", "")
+    repo.delete("'")
   end
 
   def config
@@ -152,12 +152,12 @@ class GitlabNet
     "#{config.gitlab_url}/api/v4/internal"
   end
 
-  def http_client_for(uri, options={})
-    if uri.is_a?(URI::HTTPUNIX)
-      http = Net::HTTPUNIX.new(uri.hostname)
-    else
-      http = Net::HTTP.new(uri.host, uri.port)
-    end
+  def http_client_for(uri, options = {})
+    http = if uri.is_a?(URI::HTTPUNIX)
+             Net::HTTPUNIX.new(uri.hostname)
+           else
+             Net::HTTP.new(uri.host, uri.port)
+           end
 
     http.read_timeout = options[:read_timeout] || read_timeout
 
@@ -189,7 +189,7 @@ class GitlabNet
     request
   end
 
-  def request(method, url, params = {}, options={})
+  def request(method, url, params = {}, options = {})
     $logger.debug "Performing #{method.to_s.upcase} #{url}"
 
     uri = URI.parse(url)
@@ -205,7 +205,7 @@ class GitlabNet
       raise ApiUnreachableError
     ensure
       $logger.info do
-        sprintf('%s %s %0.5f', method.to_s.upcase, url, Time.new - start_time)
+        sprintf('%s %s %0.5f', method.to_s.upcase, url, Time.new - start_time) # rubocop:disable Style/FormatString
       end
     end
 
@@ -218,7 +218,7 @@ class GitlabNet
     response
   end
 
-  def get(url, options={})
+  def get(url, options = {})
     request(:get, url, {}, options)
   end
 
@@ -231,13 +231,11 @@ class GitlabNet
       store = OpenSSL::X509::Store.new
       store.set_default_paths
 
-      if ca_file = config.http_settings['ca_file']
-        store.add_file(ca_file)
-      end
+      ca_file = config.http_settings['ca_file']
+      store.add_file(ca_file) if ca_file
 
-      if ca_path = config.http_settings['ca_path']
-        store.add_path(ca_path)
-      end
+      ca_path = config.http_settings['ca_path']
+      store.add_path(ca_path) if ca_path
 
       store
     end

lib/gitlab_post_receive.rb

@@ -13,7 +13,8 @@ class GitlabPostReceive
   def initialize(gl_repository, repo_path, actor, changes)
     @config = GitlabConfig.new
     @gl_repository = gl_repository
-    @repo_path, @actor = repo_path.strip, actor
+    @repo_path = repo_path.strip
+    @actor = actor
     @changes = changes
     @jid = SecureRandom.hex(12)
   end
@@ -47,11 +48,12 @@ class GitlabPostReceive
   end
 
   def print_merge_request_link(merge_request)
-    if merge_request["new_merge_request"]
-      message = "To create a merge request for #{merge_request["branch_name"]}, visit:"
-    else
-      message = "View merge request for #{merge_request["branch_name"]}:"
-    end
+    message =
+      if merge_request["new_merge_request"]
+        "To create a merge request for #{merge_request['branch_name']}, visit:"
+      else
+        "View merge request for #{merge_request['branch_name']}:"
+      end
 
     puts message
     puts((" " * 2) + merge_request["url"])
@@ -64,7 +66,7 @@ class GitlabPostReceive
 
     # Git prefixes remote messages with "remote: ", so this width is subtracted
     # from the width available to us.
-    total_width -= "remote: ".length
+    total_width -= "remote: ".length # rubocop:disable Performance/FixedSize
 
     # Our centered text shouldn't start or end right at the edge of the window,
     # so we add some horizontal padding: 2 chars on either side.

lib/gitlab_shell.rb

@@ -4,7 +4,7 @@ require 'pathname'
 require_relative 'gitlab_net'
 require_relative 'gitlab_metrics'
 
-class GitlabShell
+class GitlabShell # rubocop:disable Metrics/ClassLength
   class AccessDeniedError < StandardError; end
   class DisallowedCommandError < StandardError; end
   class InvalidRepositoryPathError < StandardError; end
@@ -12,9 +12,9 @@ class GitlabShell
   GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-lfs-authenticate).freeze
   GITALY_MIGRATED_COMMANDS = {
     'git-upload-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-pack'),
-    'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack'),
-  }
-  API_COMMANDS = %w(2fa_recovery_codes)
+    'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack')
+  }.freeze
+  API_COMMANDS = %w(2fa_recovery_codes).freeze
   GL_PROTOCOL = 'ssh'.freeze
 
   attr_accessor :key_id, :gl_repository, :repo_name, :command, :git_access, :username
@@ -44,7 +44,7 @@ class GitlabShell
     process_cmd(args)
 
     true
-  rescue GitlabNet::ApiUnreachableError => ex
+  rescue GitlabNet::ApiUnreachableError
     $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
     false
   rescue AccessDeniedError => ex
@@ -53,13 +53,13 @@ class GitlabShell
 
     $stderr.puts "GitLab: #{ex.message}"
     false
-  rescue DisallowedCommandError => ex
+  rescue DisallowedCommandError
     message = "gitlab-shell: Attempt to execute disallowed command <#{origin_cmd}> by #{log_username}."
     $logger.warn message
 
     $stderr.puts "GitLab: Disallowed command"
     false
-  rescue InvalidRepositoryPathError => ex
+  rescue InvalidRepositoryPathError
     $stderr.puts "GitLab: Invalid repository path"
     false
   end
@@ -113,7 +113,7 @@ class GitlabShell
   end
 
   def process_cmd(args)
-    return self.send("api_#{@command}") if API_COMMANDS.include?(@command)
+    return send("api_#{@command}") if API_COMMANDS.include?(@command)
 
     if @command == 'git-lfs-authenticate'
       GitlabMetrics.measure('lfs-authenticate') do
@@ -126,7 +126,7 @@ class GitlabShell
     executable = @command
     args = [repo_path]
 
-    if GITALY_MIGRATED_COMMANDS.has_key?(executable) && @gitaly
+    if GITALY_MIGRATED_COMMANDS.key?(executable) && @gitaly
       executable = GITALY_MIGRATED_COMMANDS[executable]
 
       gitaly_address = @gitaly['address']
@@ -172,15 +172,15 @@ class GitlabShell
     end
 
     if git_trace_available?
-      env.merge!({
+      env.merge!(
         'GIT_TRACE' => @config.git_trace_log_file,
         'GIT_TRACE_PACKET' => @config.git_trace_log_file,
-        'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file,
-      })
+        'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file
+      )
     end
 
     # We use 'chdir: ROOT_PATH' to let the next executable know where config.yml is.
-    Kernel::exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
+    Kernel.exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
   end
 
   def api

lib/httpunix.rb

@@ -11,7 +11,7 @@ module URI
   class HTTPUNIX < HTTP
     def hostname
       # decode %XX from path to file
-      v = self.host
+      v = host
       URI.decode(v)
     end
 
@@ -30,9 +30,9 @@ end
 # - Net::HTTP::connect
 module Net
   class HTTPUNIX < HTTP
-    def initialize(socketpath, port=nil)
+    def initialize(socketpath, port = nil)
       super(socketpath, port)
-      @port    = nil    # HTTP will set it to default - override back -> set DEFAULT_PORT
+      @port = nil # HTTP will set it to default - override back -> set DEFAULT_PORT
     end
 
     # override to prevent ":<port>" being appended to HTTP_HOST

support/go-format

@@ -5,7 +5,7 @@ def main(check)
   cmd = %w[gofmt -s -l]
   cmd << '-w' unless check
   cmd += go_files
-  output = IO.popen(cmd, 'r') { |io| io.read }
+  output = IO.popen(cmd, 'r', &:read)
   $stdout.write(output)
   abort 'gofmt failed' unless $?.success?
   if check && output.lines.any? { |l| l != "\n" }

support/go_build.rb

@@ -6,14 +6,14 @@ require 'fileutils'
 require_relative '../lib/gitlab_init'
 
 module GoBuild
-  GO_DIR = 'go'
+  GO_DIR = 'go'.freeze
   BUILD_DIR = File.join(ROOT_PATH, 'go_build')
   GO_PACKAGE = File.join('gitlab.com/gitlab-org/gitlab-shell', GO_DIR)
 
   GO_ENV = {
     'GOPATH' => BUILD_DIR,
-    'GO15VENDOREXPERIMENT' => '1',
-  }
+    'GO15VENDOREXPERIMENT' => '1'
+  }.freeze
 
   def create_fresh_build_dir
     FileUtils.rm_rf(BUILD_DIR)
@@ -25,8 +25,8 @@ module GoBuild
   def run!(env, cmd)
     raise "env must be a hash" unless env.is_a?(Hash)
     raise "cmd must be an array" unless cmd.is_a?(Array)
-  
-    if !system(env, *cmd)
+
+    unless system(env, *cmd)
       abort "command failed: #{env.inspect} #{cmd.join(' ')}"
     end
   end