From 47c844bf34cff269c4c31c2327f1684f5265ebd4 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Wed, 12 Jul 2017 16:31:36 +0200
Subject: Add Gitaly notes and annotations to Gitlab::Git

---
 lib/gitlab/git/attributes.rb        | 5 +++++
 lib/gitlab/git/blame.rb             | 3 +++
 lib/gitlab/git/blob.rb              | 4 ++++
 lib/gitlab/git/blob_snippet.rb      | 2 ++
 lib/gitlab/git/branch.rb            | 2 ++
 lib/gitlab/git/commit.rb            | 4 ++++
 lib/gitlab/git/commit_stats.rb      | 4 ++++
 lib/gitlab/git/compare.rb           | 2 ++
 lib/gitlab/git/diff_collection.rb   | 2 ++
 lib/gitlab/git/env.rb               | 2 ++
 lib/gitlab/git/gitmodules_parser.rb | 2 ++
 lib/gitlab/git/hook.rb              | 4 ++++
 lib/gitlab/git/index.rb             | 4 ++++
 lib/gitlab/git/path_helper.rb       | 2 ++
 lib/gitlab/git/popen.rb             | 2 ++
 lib/gitlab/git/ref.rb               | 3 +++
 lib/gitlab/git/repository.rb        | 5 +++++
 lib/gitlab/git/rev_list.rb          | 4 ++++
 lib/gitlab/git/tag.rb               | 2 ++
 lib/gitlab/git/tree.rb              | 6 ++++++
 lib/gitlab/git/util.rb              | 2 ++
 21 files changed, 66 insertions(+)

diff --git a/lib/gitlab/git/attributes.rb b/lib/gitlab/git/attributes.rb
index 42140ecc993..2d20cd473a7 100644
--- a/lib/gitlab/git/attributes.rb
+++ b/lib/gitlab/git/attributes.rb
@@ -1,3 +1,8 @@
+# Gitaly note: JV: not sure what to make of this class. Why does it use
+# the full disk path of the repository to look up attributes This is
+# problematic in Gitaly, because Gitaly hides the full disk path to the
+# repository from gitlab-ce.
+
 module Gitlab
   module Git
     # Class for parsing Git attribute files and extracting the attributes for
diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb
index 66829a03c2e..0deaab01b5b 100644
--- a/lib/gitlab/git/blame.rb
+++ b/lib/gitlab/git/blame.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: needs 1 RPC for #load_blame.
+
 module Gitlab
   module Git
     class Blame
@@ -24,6 +26,7 @@ module Gitlab
 
       private
 
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/376
       def load_blame
         cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path})
         # Read in binary mode to ensure ASCII-8BIT
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index ea386c2ddcb..b6dd3cd20e0 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: seems to be completely migrated (behind feature flags).
+
 module Gitlab
   module Git
     class Blob
@@ -107,6 +109,8 @@ module Gitlab
           detect && detect[:type] == :binary
         end
 
+        private
+
         # Recursive search of blob id by path
         #
         # Ex.
diff --git a/lib/gitlab/git/blob_snippet.rb b/lib/gitlab/git/blob_snippet.rb
index d7975f88aaa..68116e775c6 100644
--- a/lib/gitlab/git/blob_snippet.rb
+++ b/lib/gitlab/git/blob_snippet.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class BlobSnippet
diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb
index 124526e4b59..e2be9d784b9 100644
--- a/lib/gitlab/git/branch.rb
+++ b/lib/gitlab/git/branch.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class Branch < Ref
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 9c0606d780a..8ab3b0498ff 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -48,6 +48,7 @@ module Gitlab
         #
         #   Commit.find(repo, 'master')
         #
+        # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/321
         def find(repo, commit_id = "HEAD")
           return commit_id if commit_id.is_a?(Gitlab::Git::Commit)
           return decorate(commit_id) if commit_id.is_a?(Rugged::Commit)
@@ -124,6 +125,7 @@ module Gitlab
         #        are documented here:
         #        http://www.rubydoc.info/github/libgit2/rugged/Rugged#SORT_NONE-constant)
         #
+        # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/326
         def find_all(repo, options = {})
           actual_options = options.dup
 
@@ -243,6 +245,8 @@ module Gitlab
       # Shows the diff between the commit's parent and the commit.
       #
       # Cuts out the header and stats from #to_patch and returns only the diff.
+      #
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324
       def to_diff
         diff_from_parent.patch
       end
diff --git a/lib/gitlab/git/commit_stats.rb b/lib/gitlab/git/commit_stats.rb
index e9118bbed0e..57c29ad112c 100644
--- a/lib/gitlab/git/commit_stats.rb
+++ b/lib/gitlab/git/commit_stats.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: 1 RPC, migration in progress.
+
 # Gitlab::Git::CommitStats counts the additions, deletions, and total changes
 # in a commit.
 module Gitlab
@@ -6,6 +8,8 @@ module Gitlab
       attr_reader :id, :additions, :deletions, :total
 
       # Instantiate a CommitStats object
+      #
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/323
       def initialize(commit)
         @id = commit.id
         @additions = 0
diff --git a/lib/gitlab/git/compare.rb b/lib/gitlab/git/compare.rb
index 78e440395a5..7cb842256d0 100644
--- a/lib/gitlab/git/compare.rb
+++ b/lib/gitlab/git/compare.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class Compare
diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb
index 555894907cc..0d8fe185ac5 100644
--- a/lib/gitlab/git/diff_collection.rb
+++ b/lib/gitlab/git/diff_collection.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class DiffCollection
diff --git a/lib/gitlab/git/env.rb b/lib/gitlab/git/env.rb
index 0fdc57ec954..f80193ac553 100644
--- a/lib/gitlab/git/env.rb
+++ b/lib/gitlab/git/env.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     # Ephemeral (per request) storage for environment variables that some Git
diff --git a/lib/gitlab/git/gitmodules_parser.rb b/lib/gitlab/git/gitmodules_parser.rb
index f4e3b5e5129..4a43b9b444d 100644
--- a/lib/gitlab/git/gitmodules_parser.rb
+++ b/lib/gitlab/git/gitmodules_parser.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class GitmodulesParser
diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb
index 5042916343b..8f0c377ef4f 100644
--- a/lib/gitlab/git/hook.rb
+++ b/lib/gitlab/git/hook.rb
@@ -1,3 +1,7 @@
+# Gitaly note: JV: looks like this is only used by GitHooksService in
+# app/services. We shouldn't bother migrating this until we know how
+# GitHooksService will be migrated.
+
 module Gitlab
   module Git
     class Hook
diff --git a/lib/gitlab/git/index.rb b/lib/gitlab/git/index.rb
index 666743006e5..db532600d1b 100644
--- a/lib/gitlab/git/index.rb
+++ b/lib/gitlab/git/index.rb
@@ -1,3 +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
diff --git a/lib/gitlab/git/path_helper.rb b/lib/gitlab/git/path_helper.rb
index 0148cd8df05..42c80aabd0a 100644
--- a/lib/gitlab/git/path_helper.rb
+++ b/lib/gitlab/git/path_helper.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     class PathHelper
diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb
index df9ca3ee5ac..25fa62ce4bd 100644
--- a/lib/gitlab/git/popen.rb
+++ b/lib/gitlab/git/popen.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 require 'open3'
 
 module Gitlab
diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb
index ebf7393dc61..c1a10688285 100644
--- a/lib/gitlab/git/ref.rb
+++ b/lib/gitlab/git/ref.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: probably no RPC's here (just one interaction with Rugged).
+
 module Gitlab
   module Git
     class Ref
@@ -24,6 +26,7 @@ module Gitlab
         str.gsub(/\Arefs\/heads\//, '')
       end
 
+      # Gitaly: this method will probably be migrated indirectly via its call sites.
       def self.dereference_object(object)
         object = object.target while object.is_a?(Rugged::Tag::Annotation)
 
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index e51966313d4..b79e28cb150 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -553,6 +553,7 @@ module Gitlab
       #   @repository.submodule_url_for('master', 'rack')
       #   # => git@localhost:rack.git
       #
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/329
       def submodule_url_for(ref, path)
         Gitlab::GitalyClient.migrate(:submodule_url_for) do |is_enabled|
           if is_enabled
@@ -567,6 +568,8 @@ module Gitlab
       end
 
       # Return total commits count accessible from passed ref
+      #
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/330
       def commit_count(ref)
         gitaly_migrate(:commit_count) do |is_enabled|
           if is_enabled
@@ -838,6 +841,7 @@ module Gitlab
       # Ex.
       #   repo.ls_files('master')
       #
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/327
       def ls_files(ref)
         actual_ref = ref || root_ref
 
@@ -864,6 +868,7 @@ module Gitlab
         raw_output.compact
       end
 
+      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/328
       def copy_gitattributes(ref)
         begin
           commit = lookup(ref)
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
index a16b0ed76f4..2b5785a1f08 100644
--- a/lib/gitlab/git/rev_list.rb
+++ b/lib/gitlab/git/rev_list.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: will probably be migrated indirectly by migrating the call sites.
+
 module Gitlab
   module Git
     class RevList
@@ -15,6 +17,8 @@ module Gitlab
       end
 
       # This methods returns an array of missed references
+      #
+      # Should become obsolete after https://gitlab.com/gitlab-org/gitaly/issues/348.
       def missed_ref
         execute([*base_args, '--max-count=1', oldrev, "^#{newrev}"])
       end
diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb
index b5342c3d310..9a39de6ad07 100644
--- a/lib/gitlab/git/tag.rb
+++ b/lib/gitlab/git/tag.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+#
 module Gitlab
   module Git
     class Tag < Ref
diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb
index b6d4e6cfe46..8122ff0e81f 100644
--- a/lib/gitlab/git/tree.rb
+++ b/lib/gitlab/git/tree.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: needs 1 RPC, migration is in progress.
+
 module Gitlab
   module Git
     class Tree
@@ -10,6 +12,8 @@ module Gitlab
         # Get list of tree objects
         # for repository based on commit sha and path
         # Uses rugged for raw objects
+        #
+        # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/320
         def where(repository, sha, path = nil)
           path = nil if path == '' || path == '/'
 
@@ -40,6 +44,8 @@ module Gitlab
           end
         end
 
+        private
+
         # Recursive search of tree id for path
         #
         # Ex.
diff --git a/lib/gitlab/git/util.rb b/lib/gitlab/git/util.rb
index 7973da2e8f8..4708f22dcb3 100644
--- a/lib/gitlab/git/util.rb
+++ b/lib/gitlab/git/util.rb
@@ -1,3 +1,5 @@
+# Gitaly note: JV: no RPC's here.
+
 module Gitlab
   module Git
     module Util
-- 
cgit v1.2.1


From 0b0e89ef7b5708cb2ac488d4641e071ce842ca23 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Wed, 12 Jul 2017 17:06:35 +0200
Subject: Try to eliminate unused diff options

---
 lib/gitlab/git/diff.rb           | 100 ++-------------------------------------
 spec/lib/gitlab/git/diff_spec.rb |  10 ++--
 2 files changed, 9 insertions(+), 101 deletions(-)

diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index cf7829a583b..cf95f673667 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -1,3 +1,5 @@
+# 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
@@ -81,110 +83,16 @@ module Gitlab
         # Return a copy of the +options+ hash containing only keys that can be
         # passed to Rugged.  Allowed options are:
         #
-        #  :max_size ::
-        #    An integer specifying the maximum byte size of a file before a it
-        #    will be treated as binary. The default value is 512MB.
-        #
-        #  :context_lines ::
-        #    The number of unchanged lines that define the boundary of a hunk
-        #    (and to display before and after the actual changes). The default is
-        #    3.
-        #
-        #  :interhunk_lines ::
-        #    The maximum number of unchanged lines between hunk boundaries before
-        #    the hunks will be merged into a one. The default is 0.
-        #
-        #  :old_prefix ::
-        #    The virtual "directory" to prefix to old filenames in hunk headers.
-        #    The default is "a".
-        #
-        #  :new_prefix ::
-        #    The virtual "directory" to prefix to new filenames in hunk headers.
-        #    The default is "b".
-        #
-        #  :reverse ::
-        #    If true, the sides of the diff will be reversed.
-        #
-        #  :force_text ::
-        #    If true, all files will be treated as text, disabling binary
-        #    attributes & detection.
-        #
-        #  :ignore_whitespace ::
-        #    If true, all whitespace will be ignored.
-        #
         #  :ignore_whitespace_change ::
         #    If true, changes in amount of whitespace will be ignored.
         #
-        #  :ignore_whitespace_eol ::
-        #    If true, whitespace at end of line will be ignored.
-        #
-        #  :ignore_submodules ::
-        #    if true, submodules will be excluded from the diff completely.
-        #
-        #  :patience ::
-        #    If true, the "patience diff" algorithm will be used (currenlty
-        #    unimplemented).
-        #
-        #  :include_ignored ::
-        #    If true, ignored files will be included in the diff.
-        #
-        #  :include_untracked ::
-        #   If true, untracked files will be included in the diff.
-        #
-        #  :include_unmodified ::
-        #    If true, unmodified files will be included in the diff.
-        #
-        #  :recurse_untracked_dirs ::
-        #    Even if +:include_untracked+ is true, untracked directories will
-        #    only be marked with a single entry in the diff. If this flag is set
-        #    to true, all files under ignored directories will be included in the
-        #    diff, too.
-        #
         #  :disable_pathspec_match ::
         #    If true, the given +*paths+ will be applied as exact matches,
         #    instead of as fnmatch patterns.
         #
-        #  :deltas_are_icase ::
-        #    If true, filename comparisons will be made with case-insensitivity.
-        #
-        #  :include_untracked_content ::
-        #    if true, untracked content will be contained in the the diff patch
-        #    text.
-        #
-        #  :skip_binary_check ::
-        #    If true, diff deltas will be generated without spending time on
-        #    binary detection. This is useful to improve performance in cases
-        #    where the actual file content difference is not needed.
-        #
-        #  :include_typechange ::
-        #    If true, type changes for files will not be interpreted as deletion
-        #    of the "old file" and addition of the "new file", but will generate
-        #    typechange records.
-        #
-        #  :include_typechange_trees ::
-        #    Even if +:include_typechange+ is true, blob -> tree changes will
-        #    still usually be handled as a deletion of the blob. If this flag is
-        #    set to true, blob -> tree changes will be marked as typechanges.
-        #
-        #  :ignore_filemode ::
-        #    If true, file mode changes will be ignored.
-        #
-        #  :recurse_ignored_dirs ::
-        #    Even if +:include_ignored+ is true, ignored directories will only be
-        #    marked with a single entry in the diff. If this flag is set to true,
-        #    all files under ignored directories will be included in the diff,
-        #    too.
         def filter_diff_options(options, default_options = {})
-          allowed_options = [:max_size, :context_lines, :interhunk_lines,
-                             :old_prefix, :new_prefix, :reverse, :force_text,
-                             :ignore_whitespace, :ignore_whitespace_change,
-                             :ignore_whitespace_eol, :ignore_submodules,
-                             :patience, :include_ignored, :include_untracked,
-                             :include_unmodified, :recurse_untracked_dirs,
-                             :disable_pathspec_match, :deltas_are_icase,
-                             :include_untracked_content, :skip_binary_check,
-                             :include_typechange, :include_typechange_trees,
-                             :ignore_filemode, :recurse_ignored_dirs, :paths,
+          allowed_options = [:ignore_whitespace_change,
+                             :disable_pathspec_match, :paths,
                              :max_files, :max_lines, :limits, :expanded]
 
           if default_options
diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb
index d97e85364c2..0d712f39aea 100644
--- a/spec/lib/gitlab/git/diff_spec.rb
+++ b/spec/lib/gitlab/git/diff_spec.rb
@@ -241,7 +241,7 @@ EOT
   end
 
   describe '.filter_diff_options' do
-    let(:options) { { max_size: 100, invalid_opt: true } }
+    let(:options) { { max_files: 100, invalid_opt: true } }
 
     context "without default options" do
       let(:filtered_options) { described_class.filter_diff_options(options) }
@@ -253,7 +253,7 @@ EOT
 
     context "with default options" do
       let(:filtered_options) do
-        default_options = { max_size: 5, bad_opt: 1, ignore_whitespace: true }
+        default_options = { max_files: 5, bad_opt: 1, ignore_whitespace_change: true }
         described_class.filter_diff_options(options, default_options)
       end
 
@@ -263,12 +263,12 @@ EOT
       end
 
       it "should merge with default options" do
-        expect(filtered_options).to have_key(:ignore_whitespace)
+        expect(filtered_options).to have_key(:ignore_whitespace_change)
       end
 
       it "should override default options" do
-        expect(filtered_options).to have_key(:max_size)
-        expect(filtered_options[:max_size]).to eq(100)
+        expect(filtered_options).to have_key(:max_files)
+        expect(filtered_options[:max_files]).to eq(100)
       end
     end
   end
-- 
cgit v1.2.1