diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 4175746be39..b6449f27034 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -10,7 +10,7 @@ module Gitlab include Gitlab::EncodingHelper def ref_name(ref) - encode! ref.sub(/\Arefs\/(tags|heads)\//, '') + encode! ref.sub(/\Arefs\/(tags|heads|remotes)\//, '') end def branch_name(ref) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index efb4f983cfa..a3bc79109f8 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -82,10 +82,14 @@ module Gitlab end # Returns an Array of Branches - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/389 - def branches(sort_by: nil) - branches_filter(sort_by: sort_by) + def branches + gitaly_migrate(:branches) do |is_enabled| + if is_enabled + gitaly_ref_client.branches + else + branches_filter + end + end end def reload_rugged @@ -471,20 +475,6 @@ module Gitlab end end - # Sets HEAD to the commit specified by +ref+; +ref+ can be a branch or - # tag name or a commit SHA. Valid +reset_type+ values are: - # - # [:soft] - # the head will be moved to the commit. - # [:mixed] - # will trigger a +:soft+ reset, plus the index will be replaced - # with the content of the commit tree. - # [:hard] - # will trigger a +:mixed+ reset and the working directory will be - # replaced with the content of the index. (Untracked and ignored files - # will be left alone) - delegate :reset, to: :rugged - # Mimic the `git clean` command and recursively delete untracked files. # Valid keys that can be passed in the +options+ hash are: # @@ -509,154 +499,6 @@ module Gitlab # TODO: implement this method end - # Check out the specified ref. Valid options are: - # - # :b - Create a new branch at +start_point+ and set HEAD to the new - # branch. - # - # * These options are passed to the Rugged::Repository#checkout method: - # - # :progress :: - # A callback that will be executed for checkout progress notifications. - # Up to 3 parameters are passed on each execution: - # - # - The path to the last updated file (or +nil+ on the very first - # invocation). - # - The number of completed checkout steps. - # - The number of total checkout steps to be performed. - # - # :notify :: - # A callback that will be executed for each checkout notification - # types specified with +:notify_flags+. Up to 5 parameters are passed - # on each execution: - # - # - An array containing the +:notify_flags+ that caused the callback - # execution. - # - The path of the current file. - # - A hash describing the baseline blob (or +nil+ if it does not - # exist). - # - A hash describing the target blob (or +nil+ if it does not exist). - # - A hash describing the workdir blob (or +nil+ if it does not - # exist). - # - # :strategy :: - # A single symbol or an array of symbols representing the strategies - # to use when performing the checkout. Possible values are: - # - # :none :: - # Perform a dry run (default). - # - # :safe :: - # Allow safe updates that cannot overwrite uncommitted data. - # - # :safe_create :: - # Allow safe updates plus creation of missing files. - # - # :force :: - # Allow all updates to force working directory to look like index. - # - # :allow_conflicts :: - # Allow checkout to make safe updates even if conflicts are found. - # - # :remove_untracked :: - # Remove untracked files not in index (that are not ignored). - # - # :remove_ignored :: - # Remove ignored files not in index. - # - # :update_only :: - # Only update existing files, don't create new ones. - # - # :dont_update_index :: - # Normally checkout updates index entries as it goes; this stops - # that. - # - # :no_refresh :: - # Don't refresh index/config/etc before doing checkout. - # - # :disable_pathspec_match :: - # Treat pathspec as simple list of exact match file paths. - # - # :skip_locked_directories :: - # Ignore directories in use, they will be left empty. - # - # :skip_unmerged :: - # Allow checkout to skip unmerged files (NOT IMPLEMENTED). - # - # :use_ours :: - # For unmerged files, checkout stage 2 from index (NOT IMPLEMENTED). - # - # :use_theirs :: - # For unmerged files, checkout stage 3 from index (NOT IMPLEMENTED). - # - # :update_submodules :: - # Recursively checkout submodules with same options (NOT - # IMPLEMENTED). - # - # :update_submodules_if_changed :: - # Recursively checkout submodules if HEAD moved in super repo (NOT - # IMPLEMENTED). - # - # :disable_filters :: - # If +true+, filters like CRLF line conversion will be disabled. - # - # :dir_mode :: - # Mode for newly created directories. Default: +0755+. - # - # :file_mode :: - # Mode for newly created files. Default: +0755+ or +0644+. - # - # :file_open_flags :: - # Mode for opening files. Default: - # IO::CREAT | IO::TRUNC | IO::WRONLY. - # - # :notify_flags :: - # A single symbol or an array of symbols representing the cases in - # which the +:notify+ callback should be invoked. Possible values are: - # - # :none :: - # Do not invoke the +:notify+ callback (default). - # - # :conflict :: - # Invoke the callback for conflicting paths. - # - # :dirty :: - # Invoke the callback for "dirty" files, i.e. those that do not need - # an update but no longer match the baseline. - # - # :updated :: - # Invoke the callback for any file that was changed. - # - # :untracked :: - # Invoke the callback for untracked files. - # - # :ignored :: - # Invoke the callback for ignored files. - # - # :all :: - # Invoke the callback for all these cases. - # - # :paths :: - # A glob string or an array of glob strings specifying which paths - # should be taken into account for the checkout operation. +nil+ will - # match all files. Default: +nil+. - # - # :baseline :: - # A Rugged::Tree that represents the current, expected contents of the - # workdir. Default: +HEAD+. - # - # :target_directory :: - # A path to an alternative workdir directory in which the checkout - # should be performed. - def checkout(ref, options = {}, start_point = "HEAD") - if options[:b] - rugged.branches.create(ref, start_point) - options.delete(:b) - end - default_options = { strategy: [:recreate_missing, :safe] } - rugged.checkout(ref, default_options.merge(options)) - end - # Delete the specified branch from the repository def delete_branch(branch_name) rugged.branches.delete(branch_name) diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 2306fb3cbf5..b0f7548b7dc 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -10,6 +10,19 @@ module Gitlab @storage = repository.storage end + def branches + request = Gitaly::FindAllBranchesRequest.new(repository: @gitaly_repo) + response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request) + + response.flat_map do |message| + message.branches.map do |branch| + gitaly_commit = GitalyClient::Commit.new(@repository, branch.target) + target_commit = Gitlab::Git::Commit.decorate(gitaly_commit) + Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) + end + end + end + def default_branch_name request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo) response = GitalyClient.call(@storage, :ref_service, :find_default_branch_name, request) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 83d067b2c31..50736d353ad 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -378,144 +378,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#reset" do - change_path = File.join(SEED_STORAGE_PATH, TEST_NORMAL_REPO_PATH, "CHANGELOG") - untracked_path = File.join(SEED_STORAGE_PATH, TEST_NORMAL_REPO_PATH, "UNTRACKED") - tracked_path = File.join(SEED_STORAGE_PATH, TEST_NORMAL_REPO_PATH, "files", "ruby", "popen.rb") - - change_text = "New changelog text" - untracked_text = "This file is untracked" - - reset_commit = SeedRepo::LastCommit::ID - - context "--hard" do - before(:all) do - # Modify a tracked file - File.open(change_path, "w") do |f| - f.write(change_text) - end - - # Add an untracked file to the working directory - File.open(untracked_path, "w") do |f| - f.write(untracked_text) - end - - @normal_repo = Gitlab::Git::Repository.new('default', TEST_NORMAL_REPO_PATH) - @normal_repo.reset("HEAD", :hard) - end - - it "should replace the working directory with the content of the index" do - File.open(change_path, "r") do |f| - expect(f.each_line.first).not_to eq(change_text) - end - - File.open(tracked_path, "r") do |f| - expect(f.each_line.to_a[8]).to include('raise RuntimeError, "System commands') - end - end - - it "should not touch untracked files" do - expect(File.exist?(untracked_path)).to be_truthy - end - - it "should move the HEAD to the correct commit" do - new_head = @normal_repo.rugged.head.target.oid - expect(new_head).to eq(reset_commit) - end - - it "should move the tip of the master branch to the correct commit" do - new_tip = @normal_repo.rugged.references["refs/heads/master"] - .target.oid - - expect(new_tip).to eq(reset_commit) - end - - after(:all) do - # Fast-forward to the original HEAD - FileUtils.rm_rf(TEST_NORMAL_REPO_PATH) - ensure_seeds - end - end - end - - describe "#checkout" do - new_branch = "foo_branch" - - context "-b" do - before(:all) do - @normal_repo = Gitlab::Git::Repository.new('default', TEST_NORMAL_REPO_PATH) - @normal_repo.checkout(new_branch, { b: true }, "origin/feature") - end - - it "should create a new branch" do - expect(@normal_repo.rugged.branches[new_branch]).not_to be_nil - end - - it "should move the HEAD to the correct commit" do - expect(@normal_repo.rugged.head.target.oid).to( - eq(@normal_repo.rugged.branches["origin/feature"].target.oid) - ) - end - - it "should refresh the repo's #heads collection" do - head_names = @normal_repo.branches.map { |h| h.name } - expect(head_names).to include(new_branch) - end - - after(:all) do - FileUtils.rm_rf(TEST_NORMAL_REPO_PATH) - ensure_seeds - end - end - - context "without -b" do - context "and specifying a nonexistent branch" do - it "should not do anything" do - normal_repo = Gitlab::Git::Repository.new('default', TEST_NORMAL_REPO_PATH) - - expect { normal_repo.checkout(new_branch) }.to raise_error(Rugged::ReferenceError) - expect(normal_repo.rugged.branches[new_branch]).to be_nil - expect(normal_repo.rugged.head.target.oid).to( - eq(normal_repo.rugged.branches["master"].target.oid) - ) - - head_names = normal_repo.branches.map { |h| h.name } - expect(head_names).not_to include(new_branch) - end - - after(:all) do - FileUtils.rm_rf(TEST_NORMAL_REPO_PATH) - ensure_seeds - end - end - - context "and with a valid branch" do - before(:all) do - @normal_repo = Gitlab::Git::Repository.new('default', TEST_NORMAL_REPO_PATH) - @normal_repo.rugged.branches.create("feature", "origin/feature") - @normal_repo.checkout("feature") - end - - it "should move the HEAD to the correct commit" do - expect(@normal_repo.rugged.head.target.oid).to( - eq(@normal_repo.rugged.branches["feature"].target.oid) - ) - end - - it "should update the working directory" do - File.open(File.join(SEED_STORAGE_PATH, TEST_NORMAL_REPO_PATH, ".gitignore"), "r") do |f| - expect(f.read.each_line.to_a).not_to include(".DS_Store\n") - end - end - - after(:all) do - FileUtils.rm_rf(SEED_STORAGE_PATH, TEST_NORMAL_REPO_PATH) - ensure_seeds - end - end - end - end - describe "#delete_branch" do before(:all) do @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) @@ -1049,19 +911,49 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#branches with deleted branch' do - before(:each) do - ref = double() - allow(ref).to receive(:name) { 'bad-branch' } - allow(ref).to receive(:target) { raise Rugged::ReferenceError } - branches = double() - allow(branches).to receive(:each) { [ref].each } - allow(repository.rugged).to receive(:branches) { branches } + describe '#branches' do + subject { repository.branches } + + context 'with local and remote branches' do + let(:repository) do + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + end + + before do + create_remote_branch(repository, 'joe', 'remote_branch', 'master') + repository.create_branch('local_branch', 'master') + end + + after do + FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH) + ensure_seeds + end + + it 'returns the local and remote branches' do + expect(subject.any? { |b| b.name == 'joe/remote_branch' }).to eq(true) + expect(subject.any? { |b| b.name == 'local_branch' }).to eq(true) + end end - it 'should return empty branches' do - expect(repository.branches).to eq([]) + # With Gitaly enabled, Gitaly just doesn't return deleted branches. + context 'with deleted branch with Gitaly disabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) + end + + it 'returns no results' do + ref = double() + allow(ref).to receive(:name) { 'bad-branch' } + allow(ref).to receive(:target) { raise Rugged::ReferenceError } + branches = double() + allow(branches).to receive(:each) { [ref].each } + allow(repository.rugged).to receive(:branches) { branches } + + expect(subject).to be_empty + end end + + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :branches end describe '#branch_count' do @@ -1208,7 +1100,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end it 'returns the local branches' do - create_remote_branch('joe', 'remote_branch', 'master') + create_remote_branch(@repo, 'joe', 'remote_branch', 'master') @repo.create_branch('local_branch', 'master') expect(@repo.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) @@ -1235,9 +1127,9 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - def create_remote_branch(remote_name, branch_name, source_branch_name) - source_branch = @repo.branches.find { |branch| branch.name == source_branch_name } - rugged = @repo.rugged + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) + source_branch = repository.branches.find { |branch| branch.name == source_branch_name } + rugged = repository.rugged rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) end diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 1e8ed9d645b..0b1c890f956 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -6,6 +6,17 @@ describe Gitlab::GitalyClient::RefService do let(:relative_path) { project.path_with_namespace + '.git' } let(:client) { described_class.new(project.repository) } + describe '#branches' do + it 'sends a find_all_branches message' do + expect_any_instance_of(Gitaly::RefService::Stub) + .to receive(:find_all_branches) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return([]) + + client.branches + end + end + describe '#branch_names' do it 'sends a find_all_branch_names message' do expect_any_instance_of(Gitaly::RefService::Stub)