From d3c5b07962127be3b29e4602eef51abcb4c2c7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 3 Sep 2018 17:20:57 -0300 Subject: Remove Rugged and shell code from Gitlab::Git --- app/models/project.rb | 1 - app/models/repository.rb | 4 - changelogs/unreleased/clean-gitlab-git.yml | 5 + db/fixtures/development/17_cycle_analytics.rb | 14 - doc/development/diffs.md | 16 +- lib/gitlab/git/committer_with_hooks.rb | 47 -- lib/gitlab/git/diff.rb | 76 +-- lib/gitlab/git/gitlab_projects.rb | 253 ------- lib/gitlab/git/hook.rb | 108 --- lib/gitlab/git/hooks_service.rb | 35 - lib/gitlab/git/index.rb | 150 ----- lib/gitlab/git/operation_service.rb | 173 ----- lib/gitlab/git/popen.rb | 112 ---- lib/gitlab/git/repository.rb | 209 +----- lib/gitlab/git/tree.rb | 45 -- lib/gitlab/git/version.rb | 2 - lib/gitlab/git/wiki.rb | 14 - lib/gitlab/shell.rb | 9 - .../projects/import_export/import_file_spec.rb | 7 +- spec/lib/gitlab/git/committer_with_hooks_spec.rb | 156 ----- spec/lib/gitlab/git/diff_spec.rb | 82 +-- spec/lib/gitlab/git/gitlab_projects_spec.rb | 321 --------- spec/lib/gitlab/git/hook_spec.rb | 111 --- spec/lib/gitlab/git/hooks_service_spec.rb | 50 -- spec/lib/gitlab/git/index_spec.rb | 239 ------- spec/lib/gitlab/git/popen_spec.rb | 179 ----- spec/lib/gitlab/git/repository_spec.rb | 745 ++++++--------------- .../lib/gitlab/import_export/repo_restorer_spec.rb | 6 +- spec/lib/gitlab/shell_spec.rb | 5 - spec/models/repository_spec.rb | 243 ------- spec/support/helpers/git_helpers.rb | 11 + spec/support/helpers/test_env.rb | 4 - 32 files changed, 284 insertions(+), 3148 deletions(-) create mode 100644 changelogs/unreleased/clean-gitlab-git.yml delete mode 100644 lib/gitlab/git/committer_with_hooks.rb delete mode 100644 lib/gitlab/git/gitlab_projects.rb delete mode 100644 lib/gitlab/git/hook.rb delete mode 100644 lib/gitlab/git/hooks_service.rb delete mode 100644 lib/gitlab/git/popen.rb delete mode 100644 spec/lib/gitlab/git/committer_with_hooks_spec.rb delete mode 100644 spec/lib/gitlab/git/gitlab_projects_spec.rb delete mode 100644 spec/lib/gitlab/git/hook_spec.rb delete mode 100644 spec/lib/gitlab/git/hooks_service_spec.rb delete mode 100644 spec/lib/gitlab/git/index_spec.rb delete mode 100644 spec/lib/gitlab/git/popen_spec.rb create mode 100644 spec/support/helpers/git_helpers.rb diff --git a/app/models/project.rb b/app/models/project.rb index 8928bffd36c..f057c63afdf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -569,7 +569,6 @@ class Project < ActiveRecord::Base end def cleanup - @repository&.cleanup @repository = nil end diff --git a/app/models/repository.rb b/app/models/repository.rb index e98021af818..99b2e5a6686 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -82,10 +82,6 @@ class Repository alias_method :raw, :raw_repository - def cleanup - @raw_repository&.cleanup - end - # Don't use this! It's going away. Use Gitaly to read or write from repos. def path_to_repo @path_to_repo ||= diff --git a/changelogs/unreleased/clean-gitlab-git.yml b/changelogs/unreleased/clean-gitlab-git.yml new file mode 100644 index 00000000000..d7086b8eea0 --- /dev/null +++ b/changelogs/unreleased/clean-gitlab-git.yml @@ -0,0 +1,5 @@ +--- +title: Remove Rugged and shell code from Gitlab::Git +merge_request: 21488 +author: +type: other diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 7b9a4bad449..285436f4324 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -6,20 +6,6 @@ class Gitlab::Seeder::CycleAnalytics @project = project @user = User.admins.first @issue_count = perf ? 1000 : 5 - stub_git_pre_receive! - end - - # The GitLab API needn't be running for the fixtures to be - # created. Since we're performing a number of git actions - # here (like creating a branch or committing a file), we need - # to disable the `pre_receive` hook in order to remove this - # dependency on the GitLab API. - def stub_git_pre_receive! - Gitlab::Git::HooksService.class_eval do - def run_hook(name) - [true, ''] - end - end end def seed_metrics! diff --git a/doc/development/diffs.md b/doc/development/diffs.md index 2738b1b5635..5e8e8cc7541 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -15,9 +15,9 @@ We're constantly moving Rugged calls to Gitaly and the progress can be followed When refreshing a Merge Request (pushing to a source branch, force-pushing to target branch, or if the target branch now contains any commits from the MR) we fetch the comparison information using `Gitlab::Git::Compare`, which fetches `base` and `head` data using Gitaly and diff between them through -`Gitlab::Git::Diff.between` (which uses _Gitaly_ if it's enabled, otherwise _Rugged_). +`Gitlab::Git::Diff.between`. The diffs fetching process _limits_ single file diff sizes and the overall size of the whole diff through a series of constant values. Raw diff files are -then persisted on `merge_request_diff_files` table. +then persisted on `merge_request_diff_files` table. Even though diffs higher than 10kb are collapsed (`Gitlab::Git::Diff::COLLAPSE_LIMIT`), we still keep them on Postgres. However, diff files over _safety limits_ (see the [Diff limits section](#diff-limits)) are _not_ persisted. @@ -63,34 +63,34 @@ File diffs will be collapsed (but be expandable) if 100 files have already been ```ruby -Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 +Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 ``` File diffs will be collapsed (but be expandable) if 5000 lines have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes +Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes ``` File diffs will be collapsed (but be expandable) if 500 kilobytes have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000 +Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000 ``` No more files will be rendered at all if 1000 files have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000 +Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000 ``` No more files will be rendered at all if 50,000 lines have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes +Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes ``` No more files will be rendered at all if 5 megabytes have already been rendered. @@ -131,7 +131,7 @@ File diff will be suppressed (technically different from collapsed, but behaves ## Viewers Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to map metadata about each type of Diff File. It has information -whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. +whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. `DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type in order to check if it can be rendered. diff --git a/lib/gitlab/git/committer_with_hooks.rb b/lib/gitlab/git/committer_with_hooks.rb deleted file mode 100644 index 4198be7c9c9..00000000000 --- a/lib/gitlab/git/committer_with_hooks.rb +++ /dev/null @@ -1,47 +0,0 @@ -module Gitlab - module Git - class CommitterWithHooks < Gollum::Committer - attr_reader :gl_wiki - - def initialize(gl_wiki, options = {}) - @gl_wiki = gl_wiki - super(gl_wiki.gollum_wiki, options) - end - - def commit - # TODO: Remove after 10.8 - return super unless allowed_to_run_hooks? - - result = Gitlab::Git::OperationService.new(git_user, gl_wiki.repository).with_branch( - @wiki.ref, - start_branch_name: @wiki.ref - ) do |start_commit| - super(false) - end - - result[:newrev] - rescue Gitlab::Git::PreReceiveError => e - message = "Custom Hook failed: #{e.message}" - raise Gitlab::Git::Wiki::OperationError, message - end - - private - - # TODO: Remove after 10.8 - def allowed_to_run_hooks? - @options[:user_id] != 0 && @options[:username].present? - end - - def git_user - @git_user ||= Gitlab::Git::User.new(@options[:username], - @options[:name], - @options[:email], - gitlab_id) - end - - def gitlab_id - Gitlab::GlId.gl_id_from_id_value(@options[:user_id]) - end - end - end -end diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 61ce10ca131..f6b51dc3982 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -1,6 +1,3 @@ -# Gitaly note: JV: needs RPC for Gitlab::Git::Diff.between. - -# Gitlab::Git::Diff is a wrapper around native Rugged::Diff object module Gitlab module Git class Diff @@ -52,20 +49,31 @@ module Gitlab repo.diff(common_commit, head, actual_options, *paths) end - # Return a copy of the +options+ hash containing only keys that can be - # passed to Rugged. Allowed options are: + # Return a copy of the +options+ hash containing only recognized keys. + # Allowed options are: # # :ignore_whitespace_change :: # If true, changes in amount of whitespace will be ignored. # - # :disable_pathspec_match :: - # If true, the given +*paths+ will be applied as exact matches, - # instead of as fnmatch patterns. + # :max_files :: + # Limit how many files will patches be allowed for before collapsing + # + # :max_lines :: + # Limit how many patch lines (across all files) will be allowed for + # before collapsing # + # :limits :: + # A hash with additional limits to check before collapsing patches. + # Allowed keys are: `max_bytes`, `safe_max_files`, `safe_max_lines` + # and `safe_max_bytes` + # + # :expanded :: + # If true, patch raw data will not be included in the diff after + # `max_files`, `max_lines` or any of the limits in `limits` are + # exceeded def filter_diff_options(options, default_options = {}) - allowed_options = [:ignore_whitespace_change, - :disable_pathspec_match, :paths, - :max_files, :max_lines, :limits, :expanded] + allowed_options = [:ignore_whitespace_change, :max_files, :max_lines, + :limits, :expanded] if default_options actual_defaults = default_options.dup @@ -93,7 +101,7 @@ module Gitlab # # "Binary files a/file/path and b/file/path differ\n" # This is used when we detect that a diff is binary - # using CharlockHolmes when Rugged treats it as text. + # using CharlockHolmes. def binary_message(old_path, new_path) "Binary files #{old_path} and #{new_path} differ\n" end @@ -106,8 +114,6 @@ module Gitlab when Hash init_from_hash(raw_diff) prune_diff_if_eligible - when Rugged::Patch, Rugged::Diff::Delta - init_from_rugged(raw_diff) when Gitlab::GitalyClient::Diff init_from_gitaly(raw_diff) prune_diff_if_eligible @@ -184,31 +190,6 @@ module Gitlab private - def init_from_rugged(rugged) - if rugged.is_a?(Rugged::Patch) - init_from_rugged_patch(rugged) - d = rugged.delta - else - d = rugged - end - - @new_path = encode!(d.new_file[:path]) - @old_path = encode!(d.old_file[:path]) - @a_mode = d.old_file[:mode].to_s(8) - @b_mode = d.new_file[:mode].to_s(8) - @new_file = d.added? - @renamed_file = d.renamed? - @deleted_file = d.deleted? - end - - def init_from_rugged_patch(patch) - # Don't bother initializing diffs that are too large. If a diff is - # binary we're not going to display anything so we skip the size check. - return if !patch.delta.binary? && prune_large_patch(patch) - - @diff = encode!(strip_diff_headers(patch.to_s)) - end - def init_from_hash(hash) raw_diff = hash.symbolize_keys @@ -262,23 +243,6 @@ module Gitlab false end - - # Strip out the information at the beginning of the patch's text to match - # Grit's output - def strip_diff_headers(diff_text) - # Delete everything up to the first line that starts with '---' or - # 'Binary' - diff_text.sub!(/\A.*?^(---|Binary)/m, '\1') - - if diff_text.start_with?('---', 'Binary') - diff_text - else - # If the diff_text did not contain a line starting with '---' or - # 'Binary', return the empty string. No idea why; we are just - # preserving behavior from before the refactor. - '' - end - end end end end diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb deleted file mode 100644 index 5ff15a787f0..00000000000 --- a/lib/gitlab/git/gitlab_projects.rb +++ /dev/null @@ -1,253 +0,0 @@ -module Gitlab - module Git - class GitlabProjects - include Gitlab::Git::Popen - include Gitlab::Utils::StrongMemoize - - # Name of shard where repositories are stored. - # Example: nfs-file06 - attr_reader :shard_name - - # Relative path is a directory name for repository with .git at the end. - # Example: gitlab-org/gitlab-test.git - attr_reader :repository_relative_path - - # This is the path at which the gitlab-shell hooks directory can be found. - # It's essential for integration between git and GitLab proper. All new - # repositories should have their hooks directory symlinked here. - attr_reader :global_hooks_path - - attr_reader :logger - - def initialize(shard_name, repository_relative_path, global_hooks_path:, logger:) - @shard_name = shard_name - @repository_relative_path = repository_relative_path - - @logger = logger - @global_hooks_path = global_hooks_path - @output = StringIO.new - end - - def output - io = @output.dup - io.rewind - io.read - end - - # Absolute path to the repository. - # Example: /home/git/repositorities/gitlab-org/gitlab-test.git - # Probably will be removed when we fully migrate to Gitaly, part of - # https://gitlab.com/gitlab-org/gitaly/issues/1124. - def repository_absolute_path - strong_memoize(:repository_absolute_path) do - File.join(shard_path, repository_relative_path) - end - end - - def shard_path - strong_memoize(:shard_path) do - Gitlab.config.repositories.storages.fetch(shard_name).legacy_disk_path - end - end - - # Import project via git clone --bare - # URL must be publicly cloneable - def import_project(source, timeout) - git_import_repository(source, timeout) - end - - def fork_repository(new_shard_name, new_repository_relative_path) - git_fork_repository(new_shard_name, new_repository_relative_path) - end - - def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil, prune: true) - logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." - cmd = fetch_remote_command(name, tags, prune, force) - - setup_ssh_auth(ssh_key, known_hosts) do |env| - run_with_timeout(cmd, timeout, repository_absolute_path, env).tap do |success| - unless success - logger.error "Fetching remote #{name} for repository #{repository_absolute_path} failed." - end - end - end - end - - def push_branches(remote_name, timeout, force, branch_names) - logger.info "Pushing branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %W(#{Gitlab.config.git.bin_path} push) - cmd << '--force' if force - cmd += %W(-- #{remote_name}).concat(branch_names) - - success = run_with_timeout(cmd, timeout, repository_absolute_path) - - unless success - logger.error("Pushing branches to remote #{remote_name} failed.") - end - - success - end - - def delete_remote_branches(remote_name, branch_names) - branches = branch_names.map { |branch_name| ":#{branch_name}" } - - logger.info "Pushing deleted branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}).concat(branches) - - success = run(cmd, repository_absolute_path) - - unless success - logger.error("Pushing deleted branches to remote #{remote_name} failed.") - end - - success - end - - protected - - def run(*args) - output, exitstatus = popen(*args) - @output << output - - exitstatus&.zero? - end - - def run_with_timeout(*args) - output, exitstatus = popen_with_timeout(*args) - @output << output - - exitstatus&.zero? - rescue Timeout::Error - @output.puts('Timed out') - - false - end - - def mask_password_in_url(url) - result = URI(url) - result.password = "*****" unless result.password.nil? - result.user = "*****" unless result.user.nil? # it's needed for oauth access_token - result - rescue - url - end - - def remove_origin_in_repo - cmd = %W(#{Gitlab.config.git.bin_path} remote rm origin) - run(cmd, repository_absolute_path) - end - - # Builds a small shell script that can be used to execute SSH with a set of - # custom options. - # - # Options are expanded as `'-oKey="Value"'`, so SSH will correctly interpret - # paths with spaces in them. We trust the user not to embed single or double - # quotes in the key or value. - def custom_ssh_script(options = {}) - args = options.map { |k, v| %Q{'-o#{k}="#{v}"'} }.join(' ') - - [ - "#!/bin/sh", - "exec ssh #{args} \"$@\"" - ].join("\n") - end - - # Known hosts data and private keys can be passed to gitlab-shell in the - # environment. If present, this method puts them into temporary files, writes - # a script that can substitute as `ssh`, setting the options to respect those - # files, and yields: { "GIT_SSH" => "/tmp/myScript" } - def setup_ssh_auth(key, known_hosts) - options = {} - - if key - key_file = Tempfile.new('gitlab-shell-key-file') - key_file.chmod(0o400) - key_file.write(key) - key_file.close - - options['IdentityFile'] = key_file.path - options['IdentitiesOnly'] = 'yes' - end - - if known_hosts - known_hosts_file = Tempfile.new('gitlab-shell-known-hosts') - known_hosts_file.chmod(0o400) - known_hosts_file.write(known_hosts) - known_hosts_file.close - - options['StrictHostKeyChecking'] = 'yes' - options['UserKnownHostsFile'] = known_hosts_file.path - end - - return yield({}) if options.empty? - - script = Tempfile.new('gitlab-shell-ssh-wrapper') - script.chmod(0o755) - script.write(custom_ssh_script(options)) - script.close - - yield('GIT_SSH' => script.path) - ensure - key_file&.close! - known_hosts_file&.close! - script&.close! - end - - private - - def fetch_remote_command(name, tags, prune, force) - %W(#{Gitlab.config.git.bin_path} fetch #{name} --quiet).tap do |cmd| - cmd << '--prune' if prune - cmd << '--force' if force - cmd << (tags ? '--tags' : '--no-tags') - end - end - - def git_import_repository(source, timeout) - # Skip import if repo already exists - return false if File.exist?(repository_absolute_path) - - masked_source = mask_password_in_url(source) - - logger.info "Importing project from <#{masked_source}> to <#{repository_absolute_path}>." - cmd = %W(#{Gitlab.config.git.bin_path} clone --bare -- #{source} #{repository_absolute_path}) - - success = run_with_timeout(cmd, timeout, nil) - - unless success - logger.error("Importing project from <#{masked_source}> to <#{repository_absolute_path}> failed.") - FileUtils.rm_rf(repository_absolute_path) - return false - end - - Gitlab::Git::Repository.create_hooks(repository_absolute_path, global_hooks_path) - - # The project was imported successfully. - # Remove the origin URL since it may contain password. - remove_origin_in_repo - - true - end - - def git_fork_repository(new_shard_name, new_repository_relative_path) - from_path = repository_absolute_path - new_shard_path = Gitlab.config.repositories.storages.fetch(new_shard_name).legacy_disk_path - to_path = File.join(new_shard_path, new_repository_relative_path) - - # The repository cannot already exist - if File.exist?(to_path) - logger.error "fork-repository failed: destination repository <#{to_path}> already exists." - return false - end - - # Ensure the namepsace / hashed storage directory exists - FileUtils.mkdir_p(File.dirname(to_path), mode: 0770) - - logger.info "Forking repository from <#{from_path}> to <#{to_path}>." - cmd = %W(#{Gitlab.config.git.bin_path} clone --bare --no-local -- #{from_path} #{to_path}) - - run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) - end - end - end -end diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb deleted file mode 100644 index 94ff5b4980a..00000000000 --- a/lib/gitlab/git/hook.rb +++ /dev/null @@ -1,108 +0,0 @@ -# Gitaly note: JV: looks like this is only used by Gitlab::Git::HooksService in -# app/services. We shouldn't bother migrating this until we know how -# Gitlab::Git::HooksService will be migrated. - -module Gitlab - module Git - class Hook - GL_PROTOCOL = 'web'.freeze - attr_reader :name, :path, :repository - - def initialize(name, repository) - @name = name - @repository = repository - @path = File.join(repo_path, 'hooks', name) - end - - def repo_path - repository.path - end - - def exists? - File.exist?(path) - end - - def trigger(gl_id, gl_username, oldrev, newrev, ref) - return [true, nil] unless exists? - - Bundler.with_clean_env do - case name - when "pre-receive", "post-receive" - call_receive_hook(gl_id, gl_username, oldrev, newrev, ref) - when "update" - call_update_hook(gl_id, gl_username, oldrev, newrev, ref) - end - end - end - - private - - def call_receive_hook(gl_id, gl_username, oldrev, newrev, ref) - changes = [oldrev, newrev, ref].join(" ") - - exit_status = false - exit_message = nil - - vars = { - 'GL_ID' => gl_id, - 'GL_USERNAME' => gl_username, - 'PWD' => repo_path, - 'GL_PROTOCOL' => GL_PROTOCOL, - 'GL_REPOSITORY' => repository.gl_repository - } - - options = { - chdir: repo_path - } - - Open3.popen3(vars, path, options) do |stdin, stdout, stderr, wait_thr| - exit_status = true - stdin.sync = true - - # in git, pre- and post- receive hooks may just exit without - # reading stdin. We catch the exception to avoid a broken pipe - # warning - begin - # inject all the changes as stdin to the hook - changes.lines do |line| - stdin.puts line - end - rescue Errno::EPIPE - end - - stdin.close - - unless wait_thr.value == 0 - exit_status = false - exit_message = retrieve_error_message(stderr, stdout) - end - end - - [exit_status, exit_message] - end - - def call_update_hook(gl_id, gl_username, oldrev, newrev, ref) - env = { - 'GL_ID' => gl_id, - 'GL_USERNAME' => gl_username, - 'PWD' => repo_path - } - - options = { - chdir: repo_path - } - - args = [ref, oldrev, newrev] - - stdout, stderr, status = Open3.capture3(env, path, *args, options) - [status.success?, stderr.presence || stdout] - end - - def retrieve_error_message(stderr, stdout) - err_message = stderr.read - err_message = err_message.blank? ? stdout.read : err_message - err_message - end - end - end -end diff --git a/lib/gitlab/git/hooks_service.rb b/lib/gitlab/git/hooks_service.rb deleted file mode 100644 index e67cacdb95a..00000000000 --- a/lib/gitlab/git/hooks_service.rb +++ /dev/null @@ -1,35 +0,0 @@ -module Gitlab - module Git - class HooksService - attr_accessor :oldrev, :newrev, :ref - - def execute(pusher, repository, oldrev, newrev, ref) - @repository = repository - @gl_id = pusher.gl_id - @gl_username = pusher.username - @oldrev = oldrev - @newrev = newrev - @ref = ref - - %w(pre-receive update).each do |hook_name| - status, message = run_hook(hook_name) - - unless status - raise PreReceiveError, message - end - end - - yield(self).tap do - run_hook('post-receive') - end - end - - private - - def run_hook(name) - hook = Gitlab::Git::Hook.new(name, @repository) - hook.trigger(@gl_id, @gl_username, oldrev, newrev, ref) - end - end - end -end diff --git a/lib/gitlab/git/index.rb b/lib/gitlab/git/index.rb index d94082a3e30..c2e4274e3ee 100644 --- a/lib/gitlab/git/index.rb +++ b/lib/gitlab/git/index.rb @@ -1,157 +1,7 @@ -# Gitaly note: JV: When the time comes I think we will want to copy this -# class into Gitaly. None of its methods look like they should be RPC's. -# The RPC's will be at a higher level. - module Gitlab module Git class Index IndexError = Class.new(StandardError) - - DEFAULT_MODE = 0o100644 - - ACTIONS = %w(create create_dir update move delete).freeze - ACTION_OPTIONS = %i(file_path previous_path content encoding).freeze - - attr_reader :repository, :raw_index - - def initialize(repository) - @repository = repository - @raw_index = repository.rugged.index - end - - delegate :read_tree, :get, to: :raw_index - - def apply(action, options) - validate_action!(action) - public_send(action, options.slice(*ACTION_OPTIONS)) # rubocop:disable GitlabSecurity/PublicSend - end - - def write_tree - raw_index.write_tree(repository.rugged) - end - - def dir_exists?(path) - raw_index.find { |entry| entry[:path].start_with?("#{path}/") } - end - - def create(options) - options = normalize_options(options) - - if get(options[:file_path]) - raise IndexError, "A file with this name already exists" - end - - add_blob(options) - end - - def create_dir(options) - options = normalize_options(options) - - if get(options[:file_path]) - raise IndexError, "A file with this name already exists" - end - - if dir_exists?(options[:file_path]) - raise IndexError, "A directory with this name already exists" - end - - options = options.dup - options[:file_path] += '/.gitkeep' - options[:content] = '' - - add_blob(options) - end - - def update(options) - options = normalize_options(options) - - file_entry = get(options[:file_path]) - unless file_entry - raise IndexError, "A file with this name doesn't exist" - end - - add_blob(options, mode: file_entry[:mode]) - end - - def move(options) - options = normalize_options(options) - - file_entry = get(options[:previous_path]) - unless file_entry - raise IndexError, "A file with this name doesn't exist" - end - - if get(options[:file_path]) - raise IndexError, "A file with this name already exists" - end - - raw_index.remove(options[:previous_path]) - - add_blob(options, mode: file_entry[:mode]) - end - - def delete(options) - options = normalize_options(options) - - unless get(options[:file_path]) - raise IndexError, "A file with this name doesn't exist" - end - - raw_index.remove(options[:file_path]) - end - - private - - def normalize_options(options) - options = options.dup - options[:file_path] = normalize_path(options[:file_path]) if options[:file_path] - options[:previous_path] = normalize_path(options[:previous_path]) if options[:previous_path] - options - end - - def normalize_path(path) - unless path - raise IndexError, "You must provide a file path" - end - - pathname = Gitlab::Git::PathHelper.normalize_path(path.dup) - - pathname.each_filename do |segment| - if segment == '..' - raise IndexError, 'Path cannot include directory traversal' - end - end - - pathname.to_s - end - - def add_blob(options, mode: nil) - content = options[:content] - unless content - raise IndexError, "You must provide content" - end - - content = Base64.decode64(content) if options[:encoding] == 'base64' - - detect = CharlockHolmes::EncodingDetector.new.detect(content) - unless detect && detect[:type] == :binary - # When writing to the repo directly as we are doing here, - # the `core.autocrlf` config isn't taken into account. - content.gsub!("\r\n", "\n") if repository.autocrlf - end - - oid = repository.rugged.write(content, :blob) - - raw_index.add(path: options[:file_path], oid: oid, mode: mode || DEFAULT_MODE) - rescue Rugged::IndexError => e - raise IndexError, e.message - end - - def validate_action!(action) - unless ACTIONS.include?(action.to_s) - raise ArgumentError, "Unknown action '#{action}'" - end - end end end end diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index 57d748343be..0584629ac84 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -1,8 +1,6 @@ module Gitlab module Git class OperationService - include Gitlab::Git::Popen - BranchUpdate = Struct.new(:newrev, :repo_created, :branch_created) do alias_method :repo_created?, :repo_created alias_method :branch_created?, :branch_created @@ -17,177 +15,6 @@ module Gitlab ) end end - - attr_reader :user, :repository - - def initialize(user, new_repository) - if user - user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id) - @user = user - end - - # Refactoring aid - Gitlab::Git.check_namespace!(new_repository) - - @repository = new_repository - end - - def add_branch(branch_name, newrev) - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name - oldrev = Gitlab::Git::BLANK_SHA - - update_ref_in_hooks(ref, newrev, oldrev) - end - - def rm_branch(branch) - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch.name - oldrev = branch.target - newrev = Gitlab::Git::BLANK_SHA - - update_ref_in_hooks(ref, newrev, oldrev) - end - - def add_tag(tag_name, newrev, options = {}) - ref = Gitlab::Git::TAG_REF_PREFIX + tag_name - oldrev = Gitlab::Git::BLANK_SHA - - with_hooks(ref, newrev, oldrev) do |service| - # We want to pass the OID of the tag object to the hooks. For an - # annotated tag we don't know that OID until after the tag object - # (raw_tag) is created in the repository. That is why we have to - # update the value after creating the tag object. Only the - # "post-receive" hook will receive the correct value in this case. - raw_tag = repository.rugged.tags.create(tag_name, newrev, options) - service.newrev = raw_tag.target_id - end - end - - def rm_tag(tag) - ref = Gitlab::Git::TAG_REF_PREFIX + tag.name - oldrev = tag.target - newrev = Gitlab::Git::BLANK_SHA - - update_ref_in_hooks(ref, newrev, oldrev) do - repository.rugged.tags.delete(tag_name) - end - end - - # Whenever `start_branch_name` is passed, if `branch_name` doesn't exist, - # it would be created from `start_branch_name`. - # If `start_repository` is passed, and the branch doesn't exist, - # it would try to find the commits from it instead of current repository. - def with_branch( - branch_name, - start_branch_name: nil, - start_repository: repository, - &block) - - Gitlab::Git.check_namespace!(start_repository) - start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository) - - start_branch_name = nil if start_repository.empty? - - if start_branch_name && !start_repository.branch_exists?(start_branch_name) - raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.relative_path}" - end - - update_branch_with_hooks(branch_name) do - repository.with_repo_branch_commit( - start_repository, - start_branch_name || branch_name, - &block) - end - end - - def update_branch(branch_name, newrev, oldrev) - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name - update_ref_in_hooks(ref, newrev, oldrev) - end - - private - - # Returns [newrev, should_run_after_create, should_run_after_create_branch] - def update_branch_with_hooks(branch_name) - update_autocrlf_option - - was_empty = repository.empty? - - # Make commit - newrev = yield - - unless newrev - raise Gitlab::Git::CommitError.new('Failed to create commit') - end - - branch = repository.find_branch(branch_name) - oldrev = find_oldrev_from_branch(newrev, branch) - - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name - update_ref_in_hooks(ref, newrev, oldrev) - - BranchUpdate.new(newrev, was_empty, was_empty || Gitlab::Git.blank_ref?(oldrev)) - end - - def find_oldrev_from_branch(newrev, branch) - return Gitlab::Git::BLANK_SHA unless branch - - oldrev = branch.target - - merge_base = repository.merge_base(newrev, branch.target) - raise Gitlab::Git::Repository::InvalidRef unless merge_base - - if oldrev == merge_base - oldrev - else - raise Gitlab::Git::CommitError.new('Branch diverged') - end - end - - def update_ref_in_hooks(ref, newrev, oldrev) - with_hooks(ref, newrev, oldrev) do - update_ref(ref, newrev, oldrev) - end - end - - def with_hooks(ref, newrev, oldrev) - Gitlab::Git::HooksService.new.execute( - user, - repository, - oldrev, - newrev, - ref) do |service| - - yield(service) - end - end - - # Gitaly note: JV: wait with migrating #update_ref until we know how to migrate its call sites. - def update_ref(ref, newrev, oldrev) - # We use 'git update-ref' because libgit2/rugged currently does not - # offer 'compare and swap' ref updates. Without compare-and-swap we can - # (and have!) accidentally reset the ref to an earlier state, clobbering - # commits. See also https://github.com/libgit2/libgit2/issues/1534. - command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z] - - output, status = popen( - command, - repository.path) do |stdin| - stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00") - end - - unless status.zero? - Gitlab::GitLogger.error("'git update-ref' in #{repository.path}: #{output}") - raise Gitlab::Git::CommitError.new( - "Could not update branch #{Gitlab::Git.branch_name(ref)}." \ - " Please refresh and try again.") - end - end - - def update_autocrlf_option - if repository.autocrlf != :input - repository.autocrlf = :input - end - end end end end diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb deleted file mode 100644 index 7426688fc55..00000000000 --- a/lib/gitlab/git/popen.rb +++ /dev/null @@ -1,112 +0,0 @@ -# Gitaly note: JV: no RPC's here. - -require 'open3' - -module Gitlab - module Git - module Popen - FAST_GIT_PROCESS_TIMEOUT = 15.seconds - - def popen(cmd, path, vars = {}, lazy_block: nil) - unless cmd.is_a?(Array) - raise "System commands must be given as an array of strings" - end - - path ||= Dir.pwd - vars['PWD'] = path - options = { chdir: path } - - cmd_output = "" - cmd_status = 0 - Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| - stdout.set_encoding(Encoding::ASCII_8BIT) - - # stderr and stdout pipes can block if stderr/stdout aren't drained: https://bugs.ruby-lang.org/issues/9082 - # Mimic what Ruby does with capture3: https://github.com/ruby/ruby/blob/1ec544695fa02d714180ef9c34e755027b6a2103/lib/open3.rb#L257-L273 - err_reader = Thread.new { stderr.read } - - yield(stdin) if block_given? - stdin.close - - if lazy_block - cmd_output = lazy_block.call(stdout.lazy) - cmd_status = 0 - break - else - cmd_output << stdout.read - end - - cmd_output << err_reader.value - cmd_status = wait_thr.value.exitstatus - end - - [cmd_output, cmd_status] - end - - def popen_with_timeout(cmd, timeout, path, vars = {}) - unless cmd.is_a?(Array) - raise "System commands must be given as an array of strings" - end - - path ||= Dir.pwd - vars['PWD'] = path - - unless File.directory?(path) - FileUtils.mkdir_p(path) - end - - rout, wout = IO.pipe - rerr, werr = IO.pipe - - pid = Process.spawn(vars, *cmd, out: wout, err: werr, chdir: path, pgroup: true) - # stderr and stdout pipes can block if stderr/stdout aren't drained: https://bugs.ruby-lang.org/issues/9082 - # Mimic what Ruby does with capture3: https://github.com/ruby/ruby/blob/1ec544695fa02d714180ef9c34e755027b6a2103/lib/open3.rb#L257-L273 - out_reader = Thread.new { rout.read } - err_reader = Thread.new { rerr.read } - - begin - # close write ends so we could read them - wout.close - werr.close - - status = process_wait_with_timeout(pid, timeout) - - cmd_output = out_reader.value - cmd_output << err_reader.value # Copying the behaviour of `popen` which merges stderr into output - - [cmd_output, status.exitstatus] - rescue Timeout::Error => e - kill_process_group_for_pid(pid) - - raise e - ensure - wout.close unless wout.closed? - werr.close unless werr.closed? - - rout.close - rerr.close - end - end - - def process_wait_with_timeout(pid, timeout) - deadline = timeout.seconds.from_now - wait_time = 0.01 - - while deadline > Time.now - sleep(wait_time) - _, status = Process.wait2(pid, Process::WNOHANG) - - return status unless status.nil? - end - - raise Timeout::Error, "Timeout waiting for process ##{pid}" - end - - def kill_process_group_for_pid(pid) - Process.kill("KILL", -pid) - Process.wait(pid) - rescue Errno::ESRCH - end - end - end -end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 74a1bfb273a..1b8d320ff3b 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -6,7 +6,6 @@ module Gitlab module Git class Repository include Gitlab::Git::RepositoryMirroring - include Gitlab::Git::Popen include Gitlab::EncodingHelper include Gitlab::Utils::StrongMemoize @@ -73,7 +72,7 @@ module Gitlab # Relative path of repo attr_reader :relative_path - attr_reader :gitlab_projects, :storage, :gl_repository, :relative_path + attr_reader :storage, :gl_repository, :relative_path # This initializer method is only used on the client side (gitlab-ce). # Gitaly-ruby uses a different initializer. @@ -82,13 +81,6 @@ module Gitlab @relative_path = relative_path @gl_repository = gl_repository - @gitlab_projects = Gitlab::Git::GitlabProjects.new( - storage, - relative_path, - global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, - logger: Rails.logger - ) - @name = @relative_path.split("/").last end @@ -121,10 +113,6 @@ module Gitlab raise NoRepository.new('no repository for such path') end - def cleanup - @rugged&.close - end - def circuit_breaker @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(storage) end @@ -148,10 +136,6 @@ module Gitlab end end - def reload_rugged - @rugged = nil - end - # Directly find a branch with a simple name (e.g. master) # def find_branch(name) @@ -250,15 +234,6 @@ module Gitlab end end - # Returns an Array of all ref names, except when it's matching pattern - # - # regexp - The pattern for ref names we don't want - def all_ref_names_except(prefixes) - rugged.references.reject do |ref| - prefixes.any? { |p| ref.name.start_with?(p) } - end.map(&:name) - end - def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:) ref ||= root_ref commit = Gitlab::Git::Commit.find(self, ref) @@ -331,7 +306,7 @@ module Gitlab (size.to_f / 1024).round(2) end - # Use the Rugged Walker API to build an array of commits. + # Build an array of commits. # # Usage. # repo.log( @@ -591,19 +566,6 @@ module Gitlab end end - def check_revert_content(target_commit, source_sha) - args = [target_commit.sha, source_sha] - args << { mainline: 1 } if target_commit.merge_commit? - - revert_index = rugged.revert_commit(*args) - return false if revert_index.conflicts? - - tree_id = revert_index.write_tree(rugged) - return false unless diff_exists?(source_sha, tree_id) - - tree_id - end - def cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) args = { user: user, @@ -619,10 +581,6 @@ module Gitlab end end - def diff_exists?(sha1, sha2) - rugged.diff(sha1, sha2).size > 0 - end - def user_to_committer(user) Gitlab::Git.committer_hash(email: user.email, name: user.name) end @@ -746,48 +704,6 @@ module Gitlab end end - def with_repo_branch_commit(start_repository, start_branch_name) - Gitlab::Git.check_namespace!(start_repository) - start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository) - - return yield nil if start_repository.empty? - - if start_repository.same_repository?(self) - yield commit(start_branch_name) - else - start_commit_id = start_repository.commit_id(start_branch_name) - - return yield nil unless start_commit_id - - if branch_commit = commit(start_commit_id) - yield branch_commit - else - with_repo_tmp_commit( - start_repository, start_branch_name, start_commit_id) do |tmp_commit| - yield tmp_commit - end - end - end - end - - def with_repo_tmp_commit(start_repository, start_branch_name, sha) - source_ref = start_branch_name - - unless Gitlab::Git.branch_ref?(source_ref) - source_ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{source_ref}" - end - - tmp_ref = fetch_ref( - start_repository, - source_ref: source_ref, - target_ref: "refs/tmp/#{SecureRandom.hex}" - ) - - yield commit(sha) - ensure - delete_refs(tmp_ref) if tmp_ref - end - def fetch_source_branch!(source_repository, source_branch, local_ref) wrapped_gitaly_errors do gitaly_repository_client.fetch_source_branch(source_repository, source_branch, local_ref) @@ -817,21 +733,6 @@ module Gitlab end end - # This method, fetch_ref, is used from within - # Gitlab::Git::OperationService. OperationService will eventually only - # exist in gitaly-ruby. When we delete OperationService from gitlab-ce - # we can also remove fetch_ref. - def fetch_ref(source_repository, source_ref:, target_ref:) - Gitlab::Git.check_namespace!(source_repository) - source_repository = RemoteRepository.new(source_repository) unless source_repository.is_a?(RemoteRepository) - - args = %W(fetch --no-tags -f #{GITALY_INTERNAL_URL} #{source_ref}:#{target_ref}) - message, status = run_git(args, env: source_repository.fetch_env) - raise Gitlab::Git::CommandError, message if status != 0 - - target_ref - end - # Refactoring aid; allows us to copy code from app/models/repository.rb def commit(ref = 'HEAD') Gitlab::Git::Commit.find(self, ref) @@ -899,24 +800,6 @@ module Gitlab end end - def push_remote_branches(remote_name, branch_names, forced: true) - success = @gitlab_projects.push_branches(remote_name, GITLAB_PROJECTS_TIMEOUT, forced, branch_names) - - success || gitlab_projects_error - end - - def delete_remote_branches(remote_name, branch_names) - success = @gitlab_projects.delete_remote_branches(remote_name, branch_names) - - success || gitlab_projects_error - end - - def delete_remote_branches(remote_name, branch_names) - success = @gitlab_projects.delete_remote_branches(remote_name, branch_names) - - success || gitlab_projects_error - end - def bundle_to_disk(save_path) wrapped_gitaly_errors do gitaly_repository_client.create_bundle(save_path) @@ -1064,37 +947,12 @@ module Gitlab end end - def shell_blame(sha, path) - output, _status = run_git(%W(blame -p #{sha} -- #{path})) - output - end - def last_commit_for_path(sha, path) wrapped_gitaly_errors do gitaly_commit_client.last_commit_for_path(sha, path) end end - def rev_list(including: [], excluding: [], options: [], objects: false, &block) - args = ['rev-list'] - - args.push(*rev_list_param(including)) - - exclude_param = *rev_list_param(excluding) - if exclude_param.any? - args.push('--not') - args.push(*exclude_param) - end - - args.push('--objects') if objects - - if options.any? - args.push(*options) - end - - run_git!(args, lazy_block: block) - end - def checksum # The exists? RPC is much cheaper, so we perform this request first raise NoRepository, "Repository does not exists" unless exists? @@ -1112,44 +970,6 @@ module Gitlab end end - def run_git(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block) - cmd = [Gitlab.config.git.bin_path, *args] - cmd.unshift("nice") if nice - - object_directories = alternate_object_directories - if object_directories.any? - env['GIT_ALTERNATE_OBJECT_DIRECTORIES'] = object_directories.join(File::PATH_SEPARATOR) - end - - circuit_breaker.perform do - popen(cmd, chdir, env, lazy_block: lazy_block, &block) - end - end - - def run_git!(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block) - output, status = run_git(args, chdir: chdir, env: env, nice: nice, lazy_block: lazy_block, &block) - - raise GitError, output unless status.zero? - - output - end - - def run_git_with_timeout(args, timeout, env: {}) - circuit_breaker.perform do - popen_with_timeout([Gitlab.config.git.bin_path, *args], timeout, path, env) - end - end - - def git_env_for_user(user) - { - 'GIT_COMMITTER_NAME' => user.name, - 'GIT_COMMITTER_EMAIL' => user.email, - 'GL_ID' => Gitlab::GlId.gl_id(user), - 'GL_PROTOCOL' => Gitlab::Git::Hook::GL_PROTOCOL, - 'GL_REPOSITORY' => gl_repository - } - end - def gitaly_merged_branch_names(branch_names, root_sha) qualified_branch_names = branch_names.map { |b| "refs/heads/#{b}" } @@ -1200,23 +1020,6 @@ module Gitlab Gitlab::Git::HookEnv.all(gl_repository).values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).flatten.compact end - def sort_branches(branches, sort_by) - case sort_by - when 'name' - branches.sort_by(&:name) - when 'updated_desc' - branches.sort do |a, b| - b.dereferenced_target.committed_date <=> a.dereferenced_target.committed_date - end - when 'updated_asc' - branches.sort do |a, b| - a.dereferenced_target.committed_date <=> b.dereferenced_target.committed_date - end - else - branches - end - end - # Returns true if the given ref name exists # # Ref names must start with `refs/`. @@ -1231,14 +1034,6 @@ module Gitlab def gitaly_delete_refs(*ref_names) gitaly_ref_client.delete_refs(refs: ref_names) if ref_names.any? end - - def gitlab_projects_error - raise CommandError, @gitlab_projects.output - end - - def rev_list_param(spec) - spec == :all ? ['--all'] : spec - end end end end diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb index cb851b76a23..e0867aeb5a7 100644 --- a/lib/gitlab/git/tree.rb +++ b/lib/gitlab/git/tree.rb @@ -50,51 +50,6 @@ module Gitlab entry[:oid] end end - - def tree_entries_from_rugged(repository, sha, path, recursive) - current_path_entries = get_tree_entries_from_rugged(repository, sha, path) - ordered_entries = [] - - current_path_entries.each do |entry| - ordered_entries << entry - - if recursive && entry.dir? - ordered_entries.concat(tree_entries_from_rugged(repository, sha, entry.path, true)) - end - end - - ordered_entries - end - - def get_tree_entries_from_rugged(repository, sha, path) - commit = repository.lookup(sha) - root_tree = commit.tree - - tree = if path - id = find_id_by_path(repository, root_tree.oid, path) - if id - repository.lookup(id) - else - [] - end - else - root_tree - end - - tree.map do |entry| - new( - id: entry[:oid], - root_id: root_tree.oid, - name: entry[:name], - type: entry[:type], - mode: entry[:filemode].to_s(8), - path: path ? File.join(path, entry[:name]) : entry[:name], - commit_id: sha - ) - end - rescue Rugged::ReferenceError - [] - end end def initialize(options) diff --git a/lib/gitlab/git/version.rb b/lib/gitlab/git/version.rb index 1e14e8b652a..4bd91898457 100644 --- a/lib/gitlab/git/version.rb +++ b/lib/gitlab/git/version.rb @@ -1,8 +1,6 @@ module Gitlab module Git module Version - extend Gitlab::Git::Popen - def self.git_version Gitlab::VersionInfo.parse(Gitaly::Server.all.first.git_binary_version) end diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 9d992be66eb..ae92a624e05 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -163,20 +163,6 @@ module Gitlab Gitlab::Git::WikiPage.new(wiki_page, version) end end - - def committer_with_hooks(commit_details) - Gitlab::Git::CommitterWithHooks.new(self, commit_details.to_h) - end - - def with_committer_with_hooks(commit_details, &block) - committer = committer_with_hooks(commit_details) - - yield committer - - committer.commit - - nil - end end end end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index a17cd27e82d..c8fa50757f6 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -368,15 +368,6 @@ module Gitlab private - def gitlab_projects(shard_name, disk_path) - Gitlab::Git::GitlabProjects.new( - shard_name, - disk_path, - global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, - logger: Rails.logger - ) - end - def gitlab_shell_fast_execute(cmd) output, status = gitlab_shell_fast_execute_helper(cmd) diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 6cd5810325f..65c68277167 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'Import/Export - project import integration test', :js do include Select2Helper + include GitHelpers let(:user) { create(:user) } let(:file) { File.join(Rails.root, 'spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') } @@ -94,12 +95,6 @@ describe 'Import/Export - project import integration test', :js do wiki.repository.exists? && !wiki.repository.empty? end - def project_hook_exists?(project) - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? - end - end - def click_import_project_tab find('#import-project-tab').click end diff --git a/spec/lib/gitlab/git/committer_with_hooks_spec.rb b/spec/lib/gitlab/git/committer_with_hooks_spec.rb deleted file mode 100644 index c7626058acd..00000000000 --- a/spec/lib/gitlab/git/committer_with_hooks_spec.rb +++ /dev/null @@ -1,156 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::CommitterWithHooks, :seed_helper do - # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234 - skip 'needs to be moved to gitaly-ruby test suite' do - shared_examples 'calling wiki hooks' do - let(:project) { create(:project) } - let(:user) { project.owner } - let(:project_wiki) { ProjectWiki.new(project, user) } - let(:wiki) { project_wiki.wiki } - let(:options) do - { - id: user.id, - username: user.username, - name: user.name, - email: user.email, - message: 'commit message' - } - end - - subject { described_class.new(wiki, options) } - - before do - project_wiki.create_page('home', 'test content') - end - - shared_examples 'failing pre-receive hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update') - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') - end - - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end - - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev - - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - - expect(current_rev).to eq find_current_rev - end - end - - shared_examples 'failing update hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') - end - - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end - - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev - - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - - expect(current_rev).to eq find_current_rev - end - end - - shared_examples 'failing post-receive hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, '']) - end - - it 'does not raise exception' do - expect { subject.commit }.not_to raise_error - end - - it 'creates the commit' do - current_rev = find_current_rev - - subject.commit - - expect(current_rev).not_to eq find_current_rev - end - end - - shared_examples 'when hooks call succceeds' do - let(:hook) { double(:hook) } - - it 'calls the three hooks' do - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil]) - - subject.commit - end - - it 'creates the commit' do - current_rev = find_current_rev - - subject.commit - - expect(current_rev).not_to eq find_current_rev - end - end - - context 'when creating a page' do - before do - project_wiki.create_page('index', 'test content') - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - context 'when updating a page' do - before do - project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown) - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - context 'when deleting a page' do - before do - project_wiki.delete_page(find_page('home')) - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - def find_current_rev - wiki.gollum_wiki.repo.commits.first&.sha - end - - def find_page(name) - wiki.page(title: name) - end - end - - context 'when Gitaly is enabled' do - it_behaves_like 'calling wiki hooks' - end - - context 'when Gitaly is disabled', :disable_gitaly do - it_behaves_like 'calling wiki hooks' - end - end -end diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 87d9fcee39e..27d803e0117 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -2,12 +2,24 @@ require "spec_helper" describe Gitlab::Git::Diff, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:gitaly_diff) do + Gitlab::GitalyClient::Diff.new( + from_path: '.gitmodules', + to_path: '.gitmodules', + old_mode: 0100644, + new_mode: 0100644, + from_id: '0792c58905eff3432b721f8c4a64363d8e28d9ae', + to_id: 'efd587ccb47caf5f31fc954edb21f0a713d9ecc3', + overflow_marker: false, + collapsed: false, + too_large: false, + patch: "@@ -4,3 +4,6 @@\n [submodule \"gitlab-shell\"]\n \tpath = gitlab-shell\n \turl = https://github.com/gitlabhq/gitlab-shell.git\n+[submodule \"gitlab-grack\"]\n+\tpath = gitlab-grack\n+\turl = https://gitlab.com/gitlab-org/gitlab-grack.git\n" + ) + end before do @raw_diff_hash = { diff: < 'scriptFile' }, success: true) - - is_expected.to be_truthy - - expect(script.string).to eq("#!/bin/sh\nexec ssh '-oIdentityFile=\"/tmp files/keyFile\"' '-oIdentitiesOnly=\"yes\"' \"$@\"") - expect(key.string).to eq('SSH KEY') - end - end - - describe 'with known_hosts data' do - let(:extra_args) { { known_hosts: 'KNOWN HOSTS' } } - - it 'sets GIT_SSH to a custom script' do - script = stub_tempfile('scriptFile', 'gitlab-shell-ssh-wrapper', chmod: 0o755) - key = stub_tempfile('/tmp files/knownHosts', 'gitlab-shell-known-hosts', chmod: 0o400) - - stub_spawn(cmd, 600, tmp_repo_path, { 'GIT_SSH' => 'scriptFile' }, success: true) - - is_expected.to be_truthy - - expect(script.string).to eq("#!/bin/sh\nexec ssh '-oStrictHostKeyChecking=\"yes\"' '-oUserKnownHostsFile=\"/tmp files/knownHosts\"' \"$@\"") - expect(key.string).to eq('KNOWN HOSTS') - end - end - end - - describe '#import_project' do - let(:project) { create(:project) } - let(:import_url) { TestEnv.factory_repo_path_bare } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} clone --bare -- #{import_url} #{tmp_repo_path}) } - let(:timeout) { 600 } - - subject { gl_projects.import_project(import_url, timeout) } - - shared_examples 'importing repository' do - context 'success import' do - it 'imports a repo' do - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy - - is_expected.to be_truthy - - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_truthy - end - end - - context 'already exists' do - it "doesn't import" do - FileUtils.mkdir_p(tmp_repo_path) - - is_expected.to be_falsy - end - end - end - - describe 'logging' do - it 'imports a repo' do - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}>." - expect(logger).to receive(:info).with(message) - - subject - end - end - - context 'timeout' do - it 'does not import a repo' do - stub_spawn_timeout(cmd, timeout, nil) - - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}> failed." - expect(logger).to receive(:error).with(message) - - is_expected.to be_falsy - - expect(gl_projects.output).to eq("Timed out\n") - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy - end - end - - it_behaves_like 'importing repository' - end - - describe '#fork_repository' do - let(:dest_repos) { TestEnv::REPOS_STORAGE } - let(:dest_repos_path) { tmp_repos_path } - let(:dest_repo_name) { File.join('@hashed', 'aa', 'bb', 'xyz.git') } - let(:dest_repo) { File.join(dest_repos_path, dest_repo_name) } - - subject { gl_projects.fork_repository(dest_repos, dest_repo_name) } - - before do - FileUtils.mkdir_p(dest_repos_path) - end - - after do - FileUtils.rm_rf(dest_repos_path) - end - - shared_examples 'forking a repository' do - it 'forks the repository' do - is_expected.to be_truthy - - expect(File.exist?(dest_repo)).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy - end - - it 'does not fork if a project of the same name already exists' do - # create a fake project at the intended destination - FileUtils.mkdir_p(dest_repo) - - is_expected.to be_falsy - end - end - - it_behaves_like 'forking a repository' - - # We seem to be stuck to having only one working Gitaly storage in tests, changing - # that is not very straight-forward so I'm leaving this test here for now till - # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed. - context 'different storages' do - let(:dest_repos) { 'alternative' } - let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), dest_repos) } - - before do - stub_storage_settings(dest_repos => { 'path' => dest_repos_path }) - end - - it 'forks the repo' do - is_expected.to be_truthy - - expect(File.exist?(dest_repo)).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy - end - end - - describe 'log messages' do - describe 'successful fork' do - it do - message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>." - expect(logger).to receive(:info).with(message) - - subject - end - end - - describe 'failed fork due existing destination' do - it do - FileUtils.mkdir_p(dest_repo) - message = "fork-repository failed: destination repository <#{dest_repo}> already exists." - expect(logger).to receive(:error).with(message) - - subject - end - end - end - end - - def build_gitlab_projects(*args) - described_class.new( - *args, - global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, - logger: logger - ) - end - - def stub_spawn(*args, success: true) - exitstatus = success ? 0 : nil - expect(gl_projects).to receive(:popen_with_timeout).with(*args) - .and_return(["output", exitstatus]) - end - - def stub_spawn_timeout(*args) - expect(gl_projects).to receive(:popen_with_timeout).with(*args) - .and_raise(Timeout::Error) - end -end diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb deleted file mode 100644 index a45c8510b15..00000000000 --- a/spec/lib/gitlab/git/hook_spec.rb +++ /dev/null @@ -1,111 +0,0 @@ -require 'spec_helper' -require 'fileutils' - -describe Gitlab::Git::Hook do - before do - # We need this because in the spec/spec_helper.rb we define it like this: - # allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) - allow_any_instance_of(described_class).to receive(:trigger).and_call_original - end - - around do |example| - # TODO move hook tests to gitaly-ruby. Hook will disappear from gitlab-ce - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - describe "#trigger" do - set(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw_repository } - let(:repo_path) { repository.path } - let(:hooks_dir) { File.join(repo_path, 'hooks') } - let(:user) { create(:user) } - let(:gl_id) { Gitlab::GlId.gl_id(user) } - let(:gl_username) { user.username } - - def create_hook(name) - FileUtils.mkdir_p(hooks_dir) - hook_path = File.join(hooks_dir, name) - File.open(hook_path, 'w', 0755) do |f| - f.write(<<~HOOK) - #!/bin/sh - exit 0 - HOOK - end - end - - def create_failing_hook(name) - FileUtils.mkdir_p(hooks_dir) - hook_path = File.join(hooks_dir, name) - File.open(hook_path, 'w', 0755) do |f| - f.write(<<~HOOK) - #!/bin/sh - echo 'regular message from the hook' - echo 'error message from the hook' 1>&2 - echo 'error message from the hook line 2' 1>&2 - exit 1 - HOOK - end - end - - ['pre-receive', 'post-receive', 'update'].each do |hook_name| - context "when triggering a #{hook_name} hook" do - context "when the hook is successful" do - let(:hook_path) { File.join(hooks_dir, hook_name) } - let(:gl_repository) { Gitlab::GlRepository.gl_repository(project, false) } - let(:env) do - { - 'GL_ID' => gl_id, - 'GL_USERNAME' => gl_username, - 'PWD' => repo_path, - 'GL_PROTOCOL' => 'web', - 'GL_REPOSITORY' => gl_repository - } - end - - it "returns success with no errors" do - create_hook(hook_name) - hook = described_class.new(hook_name, repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - if hook_name != 'update' - expect(Open3).to receive(:popen3) - .with(env, hook_path, chdir: repo_path).and_call_original - end - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be true - expect(errors).to be_blank - end - end - - context "when the hook is unsuccessful" do - it "returns failure with errors" do - create_failing_hook(hook_name) - hook = described_class.new(hook_name, repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be false - expect(errors).to eq("error message from the hook\nerror message from the hook line 2\n") - end - end - end - end - - context "when the hook doesn't exist" do - it "returns success with no errors" do - hook = described_class.new('unknown_hook', repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be true - expect(errors).to be_nil - end - end - end -end diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb deleted file mode 100644 index 55ffced36ac..00000000000 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::HooksService, :seed_helper do - let(:gl_id) { 'user-456' } - let(:gl_username) { 'janedoe' } - let(:user) { Gitlab::Git::User.new(gl_username, 'Jane Doe', 'janedoe@example.com', gl_id) } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } - let(:service) { described_class.new } - let(:blankrev) { Gitlab::Git::BLANK_SHA } - let(:oldrev) { SeedRepo::Commit::PARENT_ID } - let(:newrev) { SeedRepo::Commit::ID } - let(:ref) { 'refs/heads/feature' } - - describe '#execute' do - context 'when receive hooks were successful' do - let(:hook) { double(:hook) } - - it 'calls all three hooks' do - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect(hook).to receive(:trigger).with(gl_id, gl_username, blankrev, newrev, ref) - .exactly(3).times.and_return([true, nil]) - - service.execute(user, repository, blankrev, newrev, ref) { } - end - end - - context 'when pre-receive hook failed' do - it 'does not call post-receive hook' do - expect(service).to receive(:run_hook).with('pre-receive').and_return([false, 'hello world']) - expect(service).not_to receive(:run_hook).with('post-receive') - - expect do - service.execute(user, repository, blankrev, newrev, ref) - end.to raise_error(Gitlab::Git::PreReceiveError, 'hello world') - end - end - - context 'when update hook failed' do - it 'does not call post-receive hook' do - expect(service).to receive(:run_hook).with('pre-receive').and_return([true, nil]) - expect(service).to receive(:run_hook).with('update').and_return([false, 'hello world']) - expect(service).not_to receive(:run_hook).with('post-receive') - - expect do - service.execute(user, repository, blankrev, newrev, ref) - end.to raise_error(Gitlab::Git::PreReceiveError, 'hello world') - end - end - end -end diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb deleted file mode 100644 index c4edd6961e1..00000000000 --- a/spec/lib/gitlab/git/index_spec.rb +++ /dev/null @@ -1,239 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::Index, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:index) { described_class.new(repository) } - - before do - index.read_tree(lookup('master').tree) - end - - around do |example| - # TODO move these specs to gitaly-ruby. The Index class will disappear from gitlab-ce - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - describe '#create' do - let(:options) do - { - content: 'Lorem ipsum...', - file_path: 'documents/story.txt' - } - end - - context 'when no file at that path exists' do - it 'creates the file in the index' do - index.create(options) - - entry = index.get(options[:file_path]) - - expect(entry).not_to be_nil - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - end - - context 'when a file at that path exists' do - before do - options[:file_path] = 'files/executables/ls' - end - - it 'raises an error' do - expect { index.create(options) }.to raise_error('A file with this name already exists') - end - end - - context 'when content is in base64' do - before do - options[:content] = Base64.encode64(options[:content]) - options[:encoding] = 'base64' - end - - it 'decodes base64' do - index.create(options) - - entry = index.get(options[:file_path]) - expect(lookup(entry[:oid]).content).to eq(Base64.decode64(options[:content])) - end - end - - context 'when content contains CRLF' do - before do - repository.autocrlf = :input - options[:content] = "Hello,\r\nWorld" - end - - it 'converts to LF' do - index.create(options) - - entry = index.get(options[:file_path]) - expect(lookup(entry[:oid]).content).to eq("Hello,\nWorld") - end - end - end - - describe '#create_dir' do - let(:options) do - { - file_path: 'newdir' - } - end - - context 'when no file or dir at that path exists' do - it 'creates the dir in the index' do - index.create_dir(options) - - entry = index.get(options[:file_path] + '/.gitkeep') - - expect(entry).not_to be_nil - end - end - - context 'when a file at that path exists' do - before do - options[:file_path] = 'files/executables/ls' - end - - it 'raises an error' do - expect { index.create_dir(options) }.to raise_error('A file with this name already exists') - end - end - - context 'when a directory at that path exists' do - before do - options[:file_path] = 'files/executables' - end - - it 'raises an error' do - expect { index.create_dir(options) }.to raise_error('A directory with this name already exists') - end - end - end - - describe '#update' do - let(:options) do - { - content: 'Lorem ipsum...', - file_path: 'README.md' - } - end - - context 'when no file at that path exists' do - before do - options[:file_path] = 'documents/story.txt' - end - - it 'raises an error' do - expect { index.update(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at that path exists' do - it 'updates the file in the index' do - index.update(options) - - entry = index.get(options[:file_path]) - - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - - it 'preserves file mode' do - options[:file_path] = 'files/executables/ls' - - index.update(options) - - entry = index.get(options[:file_path]) - - expect(entry[:mode]).to eq(0100755) - end - end - end - - describe '#move' do - let(:options) do - { - content: 'Lorem ipsum...', - previous_path: 'README.md', - file_path: 'NEWREADME.md' - } - end - - context 'when no file at that path exists' do - it 'raises an error' do - options[:previous_path] = 'documents/story.txt' - - expect { index.move(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at the new path already exists' do - it 'raises an error' do - options[:file_path] = 'CHANGELOG' - - expect { index.move(options) }.to raise_error("A file with this name already exists") - end - end - - context 'when a file at that path exists' do - it 'removes the old file in the index' do - index.move(options) - - entry = index.get(options[:previous_path]) - - expect(entry).to be_nil - end - - it 'creates the new file in the index' do - index.move(options) - - entry = index.get(options[:file_path]) - - expect(entry).not_to be_nil - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - - it 'preserves file mode' do - options[:previous_path] = 'files/executables/ls' - - index.move(options) - - entry = index.get(options[:file_path]) - - expect(entry[:mode]).to eq(0100755) - end - end - end - - describe '#delete' do - let(:options) do - { - file_path: 'README.md' - } - end - - context 'when no file at that path exists' do - before do - options[:file_path] = 'documents/story.txt' - end - - it 'raises an error' do - expect { index.delete(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at that path exists' do - it 'removes the file in the index' do - index.delete(options) - - entry = index.get(options[:file_path]) - - expect(entry).to be_nil - end - end - end - - def lookup(revision) - repository.rugged.rev_parse(revision) - end -end diff --git a/spec/lib/gitlab/git/popen_spec.rb b/spec/lib/gitlab/git/popen_spec.rb deleted file mode 100644 index 074e66d2a5d..00000000000 --- a/spec/lib/gitlab/git/popen_spec.rb +++ /dev/null @@ -1,179 +0,0 @@ -require 'spec_helper' - -describe 'Gitlab::Git::Popen' do - let(:path) { Rails.root.join('tmp').to_s } - let(:test_string) { 'The quick brown fox jumped over the lazy dog' } - # The pipe buffer is typically 64K. This string is about 440K. - let(:spew_command) { ['bash', '-c', "for i in {1..10000}; do echo '#{test_string}' 1>&2; done"] } - - let(:klass) do - Class.new(Object) do - include Gitlab::Git::Popen - end - end - - context 'popen' do - context 'zero status' do - let(:result) { klass.new.popen(%w(ls), path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to include('tests') } - end - - context 'non-zero status' do - let(:result) { klass.new.popen(%w(cat NOTHING), path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to eq(1) } - it { expect(output).to include('No such file or directory') } - end - - context 'unsafe string command' do - it 'raises an error when it gets called with a string argument' do - expect { klass.new.popen('ls', path) }.to raise_error(RuntimeError) - end - end - - context 'with custom options' do - let(:vars) { { 'foobar' => 123, 'PWD' => path } } - let(:options) { { chdir: path } } - - it 'calls popen3 with the provided environment variables' do - expect(Open3).to receive(:popen3).with(vars, 'ls', options) - - klass.new.popen(%w(ls), path, { 'foobar' => 123 }) - end - end - - context 'use stdin' do - let(:result) { klass.new.popen(%w[cat], path) { |stdin| stdin.write 'hello' } } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to eq('hello') } - end - - context 'with lazy block' do - it 'yields a lazy io' do - expect_lazy_io = lambda do |io| - expect(io).to be_a Enumerator::Lazy - expect(io.inspect).to include('#(io) {}) - end - end - end - - context 'with a process that writes a lot of data to stderr' do - it 'returns zero' do - output, status = klass.new.popen(spew_command, path) - - expect(output).to include(test_string) - expect(status).to eq(0) - end - end - end - - context 'popen_with_timeout' do - let(:timeout) { 1.second } - - context 'no timeout' do - context 'zero status' do - let(:result) { klass.new.popen_with_timeout(%w(ls), timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to include('tests') } - end - - context 'multi-line string' do - let(:test_string) { "this is 1 line\n2nd line\n3rd line\n" } - let(:result) { klass.new.popen_with_timeout(['echo', test_string], timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - # echo adds its own line - it { expect(output).to eq(test_string + "\n") } - end - - context 'non-zero status' do - let(:result) { klass.new.popen_with_timeout(%w(cat NOTHING), timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to eq(1) } - it { expect(output).to include('No such file or directory') } - end - - context 'unsafe string command' do - it 'raises an error when it gets called with a string argument' do - expect { klass.new.popen_with_timeout('ls', timeout, path) }.to raise_error(RuntimeError) - end - end - end - - context 'timeout' do - context 'timeout' do - it "raises a Timeout::Error" do - expect { klass.new.popen_with_timeout(%w(sleep 1000), timeout, path) }.to raise_error(Timeout::Error) - end - - it "handles processes that do not shutdown correctly" do - expect { klass.new.popen_with_timeout(['bash', '-c', "trap -- '' SIGTERM; sleep 1000"], timeout, path) }.to raise_error(Timeout::Error) - end - - it 'handles process that writes a lot of data to stderr' do - output, status = klass.new.popen_with_timeout(spew_command, timeout, path) - - expect(output).to include(test_string) - expect(status).to eq(0) - end - end - - context 'timeout period' do - let(:time_taken) do - begin - start = Time.now - klass.new.popen_with_timeout(%w(sleep 1000), timeout, path) - rescue - Time.now - start - end - end - - it { expect(time_taken).to be >= timeout } - end - - context 'clean up' do - let(:instance) { klass.new } - - it 'kills the child process' do - expect(instance).to receive(:kill_process_group_for_pid).and_wrap_original do |m, *args| - # is the PID, and it should not be running at this point - m.call(*args) - - pid = args.first - begin - Process.getpgid(pid) - raise "The child process should have been killed" - rescue Errno::ESRCH - end - end - - expect { instance.popen_with_timeout(['bash', '-c', "trap -- '' SIGTERM; sleep 1000"], timeout, path) }.to raise_error(Timeout::Error) - end - end - end - end -end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 28c34e234f7..dfe36d7d459 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -322,21 +322,12 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#commit_count' do - shared_examples 'simple commit counting' do - it { expect(repository.commit_count("master")).to eq(25) } - it { expect(repository.commit_count("feature")).to eq(9) } - it { expect(repository.commit_count("does-not-exist")).to eq(0) } - end - - context 'when Gitaly commit_count feature is enabled' do - it_behaves_like 'simple commit counting' - it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do - subject { repository.commit_count('master') } - end - end + it { expect(repository.commit_count("master")).to eq(25) } + it { expect(repository.commit_count("feature")).to eq(9) } + it { expect(repository.commit_count("does-not-exist")).to eq(0) } - context 'when Gitaly commit_count feature is disabled', :skip_gitaly_mock do - it_behaves_like 'simple commit counting' + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do + subject { repository.commit_count('master') } end end @@ -378,118 +369,88 @@ describe Gitlab::Git::Repository, :seed_helper do end describe "#delete_branch" do - shared_examples "deleting a branch" do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - after do - ensure_seeds - end - - it "removes the branch from the repo" do - branch_name = "to-be-deleted-soon" + let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - repository.create_branch(branch_name) - expect(repository_rugged.branches[branch_name]).not_to be_nil + after do + ensure_seeds + end - repository.delete_branch(branch_name) - expect(repository_rugged.branches[branch_name]).to be_nil - end + it "removes the branch from the repo" do + branch_name = "to-be-deleted-soon" - context "when branch does not exist" do - it "raises a DeleteBranchError exception" do - expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError) - end - end - end + repository.create_branch(branch_name) + expect(repository_rugged.branches[branch_name]).not_to be_nil - context "when Gitaly delete_branch is enabled" do - it_behaves_like "deleting a branch" + repository.delete_branch(branch_name) + expect(repository_rugged.branches[branch_name]).to be_nil end - context "when Gitaly delete_branch is disabled", :skip_gitaly_mock do - it_behaves_like "deleting a branch" + context "when branch does not exist" do + it "raises a DeleteBranchError exception" do + expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError) + end end end describe "#create_branch" do - shared_examples 'creating a branch' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - after do - ensure_seeds - end - - it "should create a new branch" do - expect(repository.create_branch('new_branch', 'master')).not_to be_nil - end + let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - it "should create a new branch with the right name" do - expect(repository.create_branch('another_branch', 'master').name).to eq('another_branch') - end + after do + ensure_seeds + end - it "should fail if we create an existing branch" do - repository.create_branch('duplicated_branch', 'master') - expect {repository.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") - end + it "should create a new branch" do + expect(repository.create_branch('new_branch', 'master')).not_to be_nil + end - it "should fail if we create a branch from a non existing ref" do - expect {repository.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") - end + it "should create a new branch with the right name" do + expect(repository.create_branch('another_branch', 'master').name).to eq('another_branch') end - context 'when Gitaly create_branch feature is enabled' do - it_behaves_like 'creating a branch' + it "should fail if we create an existing branch" do + repository.create_branch('duplicated_branch', 'master') + expect {repository.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") end - context 'when Gitaly create_branch feature is disabled', :skip_gitaly_mock do - it_behaves_like 'creating a branch' + it "should fail if we create a branch from a non existing ref" do + expect {repository.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") end end describe '#delete_refs' do - shared_examples 'deleting refs' do - let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - def repo_rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repo.rugged - end - end + let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - after do - ensure_seeds + def repo_rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repo.rugged end + end - it 'deletes the ref' do - repo.delete_refs('refs/heads/feature') - - expect(repo_rugged.references['refs/heads/feature']).to be_nil - end + after do + ensure_seeds + end - it 'deletes all refs' do - refs = %w[refs/heads/wip refs/tags/v1.1.0] - repo.delete_refs(*refs) + it 'deletes the ref' do + repo.delete_refs('refs/heads/feature') - refs.each do |ref| - expect(repo_rugged.references[ref]).to be_nil - end - end + expect(repo_rugged.references['refs/heads/feature']).to be_nil + end - it 'does not fail when deleting an empty list of refs' do - expect { repo.delete_refs(*[]) }.not_to raise_error - end + it 'deletes all refs' do + refs = %w[refs/heads/wip refs/tags/v1.1.0] + repo.delete_refs(*refs) - it 'raises an error if it failed' do - expect { repo.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) + refs.each do |ref| + expect(repo_rugged.references[ref]).to be_nil end end - context 'when Gitaly delete_refs feature is enabled' do - it_behaves_like 'deleting refs' + it 'does not fail when deleting an empty list of refs' do + expect { repo.delete_refs(*[]) }.not_to raise_error end - context 'when Gitaly delete_refs feature is disabled', :disable_gitaly do - it_behaves_like 'deleting refs' + it 'raises an error if it failed' do + expect { repo.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) end end @@ -542,38 +503,28 @@ describe Gitlab::Git::Repository, :seed_helper do Gitlab::Shell.new.remove_repository('default', 'my_project') end - shared_examples 'repository mirror fetching' do - it 'fetches a repository as a mirror remote' do - subject - - expect(refs(new_repository_path)).to eq(refs(repository_path)) - end - - context 'with keep-around refs' do - let(:sha) { SeedRepo::Commit::ID } - let(:keep_around_ref) { "refs/keep-around/#{sha}" } - let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + it 'fetches a repository as a mirror remote' do + subject - before do - repository_rugged.references.create(keep_around_ref, sha, force: true) - repository_rugged.references.create(tmp_ref, sha, force: true) - end + expect(refs(new_repository_path)).to eq(refs(repository_path)) + end - it 'includes the temporary and keep-around refs' do - subject + context 'with keep-around refs' do + let(:sha) { SeedRepo::Commit::ID } + let(:keep_around_ref) { "refs/keep-around/#{sha}" } + let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } - expect(refs(new_repository_path)).to include(keep_around_ref) - expect(refs(new_repository_path)).to include(tmp_ref) - end + before do + repository_rugged.references.create(keep_around_ref, sha, force: true) + repository_rugged.references.create(tmp_ref, sha, force: true) end - end - context 'with gitaly enabled' do - it_behaves_like 'repository mirror fetching' - end + it 'includes the temporary and keep-around refs' do + subject - context 'with gitaly enabled', :skip_gitaly_mock do - it_behaves_like 'repository mirror fetching' + expect(refs(new_repository_path)).to include(keep_around_ref) + expect(refs(new_repository_path)).to include(tmp_ref) + end end def new_repository_path @@ -918,25 +869,15 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#merge_base' do - shared_examples '#merge_base' do - where(:from, :to, :result) do - '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' - '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' - '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil - 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil - end - - with_them do - it { expect(repository.merge_base(from, to)).to eq(result) } - end + where(:from, :to, :result) do + '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil + 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil end - context 'with gitaly' do - it_behaves_like '#merge_base' - end - - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#merge_base' + with_them do + it { expect(repository.merge_base(from, to)).to eq(result) } end end @@ -1028,54 +969,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe '#autocrlf' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.rugged.config['core.autocrlf'] = true - end - - around do |example| - # OK because autocrlf is only used in gitaly-ruby - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it 'return the value of the autocrlf option' do - expect(@repo.autocrlf).to be(true) - end - - after(:all) do - @repo.rugged.config.delete('core.autocrlf') - end - end - - describe '#autocrlf=' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.rugged.config['core.autocrlf'] = false - end - - around do |example| - # OK because autocrlf= is only used in gitaly-ruby - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it 'should set the autocrlf option to the provided option' do - @repo.autocrlf = :input - - File.open(File.join(SEED_STORAGE_PATH, TEST_MUTABLE_REPO_PATH, 'config')) do |config_file| - expect(config_file.read).to match('autocrlf = input') - end - end - - after(:all) do - @repo.rugged.config.delete('core.autocrlf') - end - end - describe '#find_branch' do it 'should return a Branch for master' do branch = repository.find_branch('master') @@ -1175,57 +1068,47 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#merged_branch_names' do - shared_examples 'finding merged branch names' do - context 'when branch names are passed' do - it 'only returns the names we are asking' do - names = repository.merged_branch_names(%w[merge-test]) + context 'when branch names are passed' do + it 'only returns the names we are asking' do + names = repository.merged_branch_names(%w[merge-test]) - expect(names).to contain_exactly('merge-test') - end + expect(names).to contain_exactly('merge-test') + end - it 'does not return unmerged branch names' do - names = repository.merged_branch_names(%w[feature]) + it 'does not return unmerged branch names' do + names = repository.merged_branch_names(%w[feature]) - expect(names).to be_empty - end + expect(names).to be_empty end + end - context 'when no root ref is available' do - it 'returns empty list' do - project = create(:project, :empty_repo) + context 'when no root ref is available' do + it 'returns empty list' do + project = create(:project, :empty_repo) - names = project.repository.merged_branch_names(%w[feature]) + names = project.repository.merged_branch_names(%w[feature]) - expect(names).to be_empty - end + expect(names).to be_empty end + end - context 'when no branch names are specified' do - before do - repository.create_branch('identical', 'master') - end - - after do - ensure_seeds - end - - it 'returns all merged branch names except for identical one' do - names = repository.merged_branch_names + context 'when no branch names are specified' do + before do + repository.create_branch('identical', 'master') + end - expect(names).to include('merge-test') - expect(names).to include('fix-mode') - expect(names).not_to include('feature') - expect(names).not_to include('identical') - end + after do + ensure_seeds end - end - context 'when Gitaly merged_branch_names feature is enabled' do - it_behaves_like 'finding merged branch names' - end + it 'returns all merged branch names except for identical one' do + names = repository.merged_branch_names - context 'when Gitaly merged_branch_names feature is disabled', :disable_gitaly do - it_behaves_like 'finding merged branch names' + expect(names).to include('merge-test') + expect(names).to include('fix-mode') + expect(names).not_to include('feature') + expect(names).not_to include('identical') + end end end @@ -1342,76 +1225,46 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#ref_exists?' do - shared_examples 'checks the existence of refs' do - it 'returns true for an existing tag' do - expect(repository.ref_exists?('refs/heads/master')).to eq(true) - end - - it 'returns false for a non-existing tag' do - expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) - end - - it 'raises an ArgumentError for an empty string' do - expect { repository.ref_exists?('') }.to raise_error(ArgumentError) - end + it 'returns true for an existing tag' do + expect(repository.ref_exists?('refs/heads/master')).to eq(true) + end - it 'raises an ArgumentError for an invalid ref' do - expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) - end + it 'returns false for a non-existing tag' do + expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) end - context 'when Gitaly ref_exists feature is enabled' do - it_behaves_like 'checks the existence of refs' + it 'raises an ArgumentError for an empty string' do + expect { repository.ref_exists?('') }.to raise_error(ArgumentError) end - context 'when Gitaly ref_exists feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of refs' + it 'raises an ArgumentError for an invalid ref' do + expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) end end describe '#tag_exists?' do - shared_examples 'checks the existence of tags' do - it 'returns true for an existing tag' do - tag = repository.tag_names.first - - expect(repository.tag_exists?(tag)).to eq(true) - end - - it 'returns false for a non-existing tag' do - expect(repository.tag_exists?('v9000')).to eq(false) - end - end + it 'returns true for an existing tag' do + tag = repository.tag_names.first - context 'when Gitaly ref_exists_tags feature is enabled' do - it_behaves_like 'checks the existence of tags' + expect(repository.tag_exists?(tag)).to eq(true) end - context 'when Gitaly ref_exists_tags feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of tags' + it 'returns false for a non-existing tag' do + expect(repository.tag_exists?('v9000')).to eq(false) end end describe '#branch_exists?' do - shared_examples 'checks the existence of branches' do - it 'returns true for an existing branch' do - expect(repository.branch_exists?('master')).to eq(true) - end - - it 'returns false for a non-existing branch' do - expect(repository.branch_exists?('kittens')).to eq(false) - end - - it 'returns false when using an invalid branch name' do - expect(repository.branch_exists?('.bla')).to eq(false) - end + it 'returns true for an existing branch' do + expect(repository.branch_exists?('master')).to eq(true) end - context 'when Gitaly ref_exists_branches feature is enabled' do - it_behaves_like 'checks the existence of branches' + it 'returns false for a non-existing branch' do + expect(repository.branch_exists?('kittens')).to eq(false) end - context 'when Gitaly ref_exists_branches feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of branches' + it 'returns false when using an invalid branch name' do + expect(repository.branch_exists?('.bla')).to eq(false) end end @@ -1502,52 +1355,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe '#with_repo_branch_commit' do - context 'when comparing with the same repository' do - let(:start_repository) { repository } - - context 'when the branch exists' do - let(:start_branch_name) { 'master' } - - it 'yields the commit' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) - end - end - - context 'when the branch does not exist' do - let(:start_branch_name) { 'definitely-not-master' } - - it 'yields nil' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(nil) - end - end - end - - context 'when comparing with another repository' do - let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - context 'when the branch exists' do - let(:start_branch_name) { 'master' } - - it 'yields the commit' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) - end - end - - context 'when the branch does not exist' do - let(:start_branch_name) { 'definitely-not-master' } - - it 'yields nil' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(nil) - end - end - end - end - describe '#fetch_source_branch!' do let(:local_ref) { 'refs/merge-requests/1/head' } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } @@ -1598,29 +1405,19 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#rm_branch' do - shared_examples "user deleting a branch" do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw } - let(:branch_name) { "to-be-deleted-soon" } - - before do - project.add_developer(user) - repository.create_branch(branch_name) - end + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw } + let(:branch_name) { "to-be-deleted-soon" } - it "removes the branch from the repo" do - repository.rm_branch(branch_name, user: user) - - expect(repository_rugged.branches[branch_name]).to be_nil - end + before do + project.add_developer(user) + repository.create_branch(branch_name) end - context "when Gitaly user_delete_branch is enabled" do - it_behaves_like "user deleting a branch" - end + it "removes the branch from the repo" do + repository.rm_branch(branch_name, user: user) - context "when Gitaly user_delete_branch is disabled", :skip_gitaly_mock do - it_behaves_like "user deleting a branch" + expect(repository_rugged.branches[branch_name]).to be_nil end end @@ -1744,39 +1541,29 @@ describe Gitlab::Git::Repository, :seed_helper do ensure_seeds end - shared_examples '#merge' do - it 'can perform a merge' do - merge_commit_id = nil - result = repository.merge(user, source_sha, target_branch, 'Test merge') do |commit_id| - merge_commit_id = commit_id - end - - expect(result.newrev).to eq(merge_commit_id) - expect(result.repo_created).to eq(false) - expect(result.branch_created).to eq(false) + it 'can perform a merge' do + merge_commit_id = nil + result = repository.merge(user, source_sha, target_branch, 'Test merge') do |commit_id| + merge_commit_id = commit_id end - it 'returns nil if there was a concurrent branch update' do - concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' - result = repository.merge(user, source_sha, target_branch, 'Test merge') do - # This ref update should make the merge fail - repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) - end - - # This 'nil' signals that the merge was not applied - expect(result).to be_nil + expect(result.newrev).to eq(merge_commit_id) + expect(result.repo_created).to eq(false) + expect(result.branch_created).to eq(false) + end - # Our concurrent ref update should not have been undone - expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) + it 'returns nil if there was a concurrent branch update' do + concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' + result = repository.merge(user, source_sha, target_branch, 'Test merge') do + # This ref update should make the merge fail + repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) end - end - context 'with gitaly' do - it_behaves_like '#merge' - end + # This 'nil' signals that the merge was not applied + expect(result).to be_nil - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#merge' + # Our concurrent ref update should not have been undone + expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) end end @@ -1888,88 +1675,47 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#add_remote' do let(:mirror_refmap) { '+refs/*:refs/*' } - shared_examples 'add_remote' do - it 'added the remote' do - begin - rugged.remotes.delete(remote_name) - rescue Rugged::ConfigError - end - - repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - - expect(rugged.remotes[remote_name]).not_to be_nil - expect(rugged.config["remote.#{remote_name}.mirror"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.prune"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) + it 'added the remote' do + begin + rugged.remotes.delete(remote_name) + rescue Rugged::ConfigError end - end - context 'using Gitaly' do - it_behaves_like 'add_remote' - end + repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'add_remote' + expect(rugged.remotes[remote_name]).not_to be_nil + expect(rugged.config["remote.#{remote_name}.mirror"]).to eq('true') + expect(rugged.config["remote.#{remote_name}.prune"]).to eq('true') + expect(rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) end end describe '#remove_remote' do - shared_examples 'remove_remote' do - it 'removes the remote' do - rugged.remotes.create(remote_name, url) - - repository.remove_remote(remote_name) - - expect(rugged.remotes[remote_name]).to be_nil - end - end + it 'removes the remote' do + rugged.remotes.create(remote_name, url) - context 'using Gitaly' do - it_behaves_like 'remove_remote' - end + repository.remove_remote(remote_name) - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'remove_remote' + expect(rugged.remotes[remote_name]).to be_nil end end end - describe '#gitlab_projects' do - subject { repository.gitlab_projects } - - it do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - expect(subject.shard_path).to eq(storage_path) - end - end - it { expect(subject.repository_relative_path).to eq(repository.relative_path) } - end - describe '#bundle_to_disk' do - shared_examples 'bundling to disk' do - let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } + let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } - after do - FileUtils.rm_rf(save_path) - end - - it 'saves a bundle to disk' do - repository.bundle_to_disk(save_path) - - success = system( - *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), - [:out, :err] => '/dev/null' - ) - expect(success).to be true - end + after do + FileUtils.rm_rf(save_path) end - context 'when Gitaly bundle_to_disk feature is enabled' do - it_behaves_like 'bundling to disk' - end + it 'saves a bundle to disk' do + repository.bundle_to_disk(save_path) - context 'when Gitaly bundle_to_disk feature is disabled', :disable_gitaly do - it_behaves_like 'bundling to disk' + success = system( + *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), + [:out, :err] => '/dev/null' + ) + expect(success).to be true end end @@ -2044,138 +1790,41 @@ describe Gitlab::Git::Repository, :seed_helper do end end - context 'gitlab_projects commands' do - let(:gitlab_projects) { repository.gitlab_projects } - let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } - - describe '#push_remote_branches' do - subject do - repository.push_remote_branches('downstream-remote', ['master']) - end - - it 'executes the command' do - expect(gitlab_projects).to receive(:push_branches) - .with('downstream-remote', timeout, true, ['master']) - .and_return(true) - - is_expected.to be_truthy - end - - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:push_branches) - .with('downstream-remote', timeout, true, ['master']) - .and_return(false) - - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end - end - - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end - - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) - - is_expected.to be_truthy - end + describe '#clean_stale_repository_files' do + let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) + it 'cleans up the files' do + create_worktree = %W[git -C #{repository_path} worktree add --detach #{worktree_path} master] + raise 'preparation failed' unless system(*create_worktree, err: '/dev/null') - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end - end + FileUtils.touch(worktree_path, mtime: Time.now - 8.hours) + # git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object, + # but the HEAD must be 40 characters long or git will ignore it. + File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA) - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end + # git 2.16 fails with "fatal: bad object HEAD" + expect(rev_list_all).to be false - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) + repository.clean_stale_repository_files - is_expected.to be_truthy - end - - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) - - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end + expect(rev_list_all).to be true + expect(File.exist?(worktree_path)).to be_falsey end - describe '#clean_stale_repository_files' do - let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } - - it 'cleans up the files' do - create_worktree = %W[git -C #{repository_path} worktree add --detach #{worktree_path} master] - raise 'preparation failed' unless system(*create_worktree, err: '/dev/null') - - FileUtils.touch(worktree_path, mtime: Time.now - 8.hours) - # git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object, - # but the HEAD must be 40 characters long or git will ignore it. - File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA) - - # git 2.16 fails with "fatal: bad object HEAD" - expect(rev_list_all).to be false - - repository.clean_stale_repository_files - - expect(rev_list_all).to be true - expect(File.exist?(worktree_path)).to be_falsey - end - - def rev_list_all - system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null') - end - - it 'increments a counter upon an error' do - expect(repository.gitaly_repository_client).to receive(:cleanup).and_raise(Gitlab::Git::CommandError) - - counter = double(:counter) - - expect(counter).to receive(:increment) - expect(Gitlab::Metrics).to receive(:counter).with(:failed_repository_cleanup_total, - 'Number of failed repository cleanup events').and_return(counter) - - repository.clean_stale_repository_files - end + def rev_list_all + system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null') end - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end + it 'increments a counter upon an error' do + expect(repository.gitaly_repository_client).to receive(:cleanup).and_raise(Gitlab::Git::CommandError) - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) + counter = double(:counter) - is_expected.to be_truthy - end - - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) + expect(counter).to receive(:increment) + expect(Gitlab::Metrics).to receive(:counter).with(:failed_repository_cleanup_total, + 'Number of failed repository cleanup events').and_return(counter) - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end + repository.clean_stale_repository_files end end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 7ffa84f906d..8a699eb1461 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::ImportExport::RepoRestorer do + include GitHelpers + describe 'bundle a project Git repo' do let(:user) { create(:user) } let!(:project_with_repo) { create(:project, :repository, name: 'test-repo-restorer', path: 'test-repo-restorer') } @@ -36,9 +38,7 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist - end + expect(project_hook_exists?(project)).to be true end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index f8bf896950e..b1b7c427313 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -7,15 +7,10 @@ describe Gitlab::Shell do let(:repository) { project.repository } let(:gitlab_shell) { described_class.new } let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } } - let(:gitlab_projects) { double('gitlab_projects') } let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } before do allow(Project).to receive(:find).and_return(project) - - allow(gitlab_shell).to receive(:gitlab_projects) - .with(project.repository_storage, project.disk_path + '.git') - .and_return(gitlab_projects) end it { is_expected.to respond_to :add_key } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 93898012d34..dffac05152b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1028,194 +1028,6 @@ describe Repository do end end - describe '#update_branch_with_hooks' do - let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature - let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev - let(:updating_ref) { 'refs/heads/feature' } - let(:target_project) { project } - let(:target_repository) { target_project.repository } - - around do |example| - # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - context 'when pre hooks were successful' do - before do - service = Gitlab::Git::HooksService.new - expect(Gitlab::Git::HooksService).to receive(:new).and_return(service) - expect(service).to receive(:execute) - .with(git_user, target_repository.raw_repository, old_rev, new_rev, updating_ref) - .and_yield(service).and_return(true) - end - - it 'runs without errors' do - expect do - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do - new_rev - end - end.not_to raise_error - end - - it 'ensures the autocrlf Git option is set to :input' do - service = Gitlab::Git::OperationService.new(git_user, repository.raw_repository) - - expect(service).to receive(:update_autocrlf_option) - - service.with_branch('feature') { new_rev } - end - - context "when the branch wasn't empty" do - it 'updates the head' do - expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) - - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do - new_rev - end - - expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) - end - end - - context 'when target project does not have the commit' do - let(:target_project) { create(:project, :empty_repo) } - let(:old_rev) { Gitlab::Git::BLANK_SHA } - let(:new_rev) { project.commit('feature').sha } - let(:updating_ref) { 'refs/heads/master' } - - it 'fetch_ref and create the branch' do - expect(target_project.repository.raw_repository).to receive(:fetch_ref) - .and_call_original - - Gitlab::Git::OperationService.new(git_user, target_repository.raw_repository) - .with_branch( - 'master', - start_repository: project.repository.raw_repository, - start_branch_name: 'feature') { new_rev } - - expect(target_repository.branch_names).to contain_exactly('master') - end - end - - context 'when target project already has the commit' do - let(:target_project) { create(:project, :repository) } - - it 'does not fetch_ref and just pass the commit' do - expect(target_repository).not_to receive(:fetch_ref) - - Gitlab::Git::OperationService.new(git_user, target_repository.raw_repository) - .with_branch('feature', start_repository: project.repository.raw_repository) { new_rev } - end - end - end - - context 'when temporary ref failed to be created from other project' do - let(:target_project) { create(:project, :empty_repo) } - - before do - expect(target_project.repository.raw_repository).to receive(:run_git) - end - - it 'raises Rugged::ReferenceError' do - expect do - Gitlab::Git::OperationService.new(git_user, target_project.repository.raw_repository) - .with_branch('feature', - start_repository: project.repository.raw_repository, - &:itself) - end.to raise_error(Gitlab::Git::CommandError) - end - end - - context 'when the update adds more than one commit' do - let(:old_rev) { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' } - - it 'runs without errors' do - # old_rev is an ancestor of new_rev - expect(repository.merge_base(old_rev, new_rev)).to eq(old_rev) - - # old_rev is not a direct ancestor (parent) of new_rev - expect(repository.rugged.lookup(new_rev).parent_ids).not_to include(old_rev) - - branch = 'feature-ff-target' - repository.add_branch(user, branch, old_rev) - - expect do - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch(branch) do - new_rev - end - end.not_to raise_error - end - end - - context 'when the update would remove commits from the target branch' do - let(:branch) { 'master' } - let(:old_rev) { repository.find_branch(branch).dereferenced_target.sha } - - it 'raises an exception' do - # The 'master' branch is NOT an ancestor of new_rev. - expect(repository.merge_base(old_rev, new_rev)).not_to eq(old_rev) - - # Updating 'master' to new_rev would lose the commits on 'master' that - # are not contained in new_rev. This should not be allowed. - expect do - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch(branch) do - new_rev - end - end.to raise_error(Gitlab::Git::CommitError) - end - end - - context 'when pre hooks failed' do - it 'gets an error' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) - - expect do - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do - new_rev - end - end.to raise_error(Gitlab::Git::PreReceiveError) - end - end - - context 'when target branch is different from source branch' do - before do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) - end - - subject do - Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('new-feature') do - new_rev - end - end - - it 'returns branch_created as true' do - expect(subject).not_to be_repo_created - expect(subject).to be_branch_created - end - end - - context 'when repository is empty' do - before do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) - end - - it 'expires creation and branch cache' do - empty_repository = create(:project, :empty_repo).repository - - expect(empty_repository).to receive(:expire_exists_cache) - expect(empty_repository).to receive(:expire_root_ref_cache) - expect(empty_repository).to receive(:expire_emptiness_caches) - expect(empty_repository).to receive(:expire_branches_cache) - - empty_repository.create_file(user, 'CHANGELOG', 'Changelog!', - message: 'Updates file content', - branch_name: 'master') - end - end - end - describe '#exists?' do it 'returns true when a repository exists' do expect(repository.exists?).to be(true) @@ -1298,40 +1110,6 @@ describe Repository do end end - describe '#update_autocrlf_option' do - around do |example| - # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - describe 'when autocrlf is not already set to :input' do - before do - repository.raw_repository.autocrlf = true - end - - it 'sets autocrlf to :input' do - Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_autocrlf_option) - - expect(repository.raw_repository.autocrlf).to eq(:input) - end - end - - describe 'when autocrlf is already set to :input' do - before do - repository.raw_repository.autocrlf = :input - end - - it 'does nothing' do - expect(repository.raw_repository).not_to receive(:autocrlf=) - .with(:input) - - Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_autocrlf_option) - end - end - end - describe '#empty?' do let(:empty_repository) { create(:project_empty_repo).repository } @@ -2025,27 +1803,6 @@ describe Repository do end end - describe '#update_ref' do - around do |example| - # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it 'can create a ref' do - Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) - - expect(repository.find_branch('foobar')).not_to be_nil - end - - it 'raises CommitError when the ref update fails' do - expect do - Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA) - end.to raise_error(Gitlab::Git::CommitError) - end - end - describe '#contribution_guide', :use_clean_rails_memory_store_caching do it 'returns and caches the output' do expect(repository).to receive(:file_on_head) diff --git a/spec/support/helpers/git_helpers.rb b/spec/support/helpers/git_helpers.rb new file mode 100644 index 00000000000..fc92bc38561 --- /dev/null +++ b/spec/support/helpers/git_helpers.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module GitHelpers + def project_hook_exists?(project) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project_path = project.repository.raw_repository.path + + File.exist?(File.join(project_path, 'hooks', 'post-receive')) + end + end +end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 3f8e3ae5190..8e8ec574edb 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -107,10 +107,6 @@ module TestEnv .and_call_original end - def disable_pre_receive - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) - end - # Clean /tmp/tests # # Keeps gitlab-shell and gitlab-test -- cgit v1.2.1