[CodeExtractor] Fix "transferred from" in reinsert

This could be a "[FIXUP]" commit, but I think it is worth pointing out
what I did here.

Since there is multiple `git-filter-branch` calls, the original commit
SHAs that existed are re-written after each.  So delaying writing the
updated commit message will only produce references to commits that
don't exist since they were temporary.

So moving `--msg-filter` to the first `git-filter-branch` was the first
logical step.  However, this caused another issue.  Since we allow a
fallback for finding the commit messages that existed in the previous
repository, that would now be incorrect (in it's previous form) since it
relied on matching the exact messages created.

To handle this in most cases, and lines of `(transferred from ...@` are
filtered out from the commit messages before comparing.

* * *

The remaining changes are just to support this change.

  - Adjust some method signatures to support this change.
  - Update `assert_commits` to confirm the commit messages are pointing
    to a proper commit
  - Add a `reference_repo_dir` and `reference_repo` to help with the
    `assert_commits` changes (set in one of the helper methods)

Author: NickLaMuro

lib/code_extractor.rb

@@ -104,14 +104,16 @@ def extract_commits extractions, upstream_name
     # Note:  We don't use `--subdirectory-filter` here as it will remove merge
     # commits, which we don't want.
     #
-    def prune_commits extractions
+    def prune_commits extractions, upstream_name
       puts "Pruning commits…"
 
+      @upstream_name ||= upstream_name
+
       build_prune_script extractions
 
       Dir.chdir git_dir do
         `git checkout -b #{prune_branch} #{@source_branch}`
-        `git filter-branch -f --prune-empty --tree-filter #{@prune_script} HEAD`
+        `git filter-branch -f --prune-empty --tree-filter #{@prune_script} #{msg_filter upstream_name} HEAD`
         `git filter-branch -f --prune-empty --index-filter '
             git read-tree --empty
             git reset $GIT_COMMIT -- #{@keep_directory}
@@ -156,12 +158,10 @@ def add_target_remote target_name, target_remote
     # branch, and the "root" commit assumes the parent of the current HEAD of
     # the target remote's (master) branch
     #
-    def inject_commits target_base_branch, upstream_name
+    def inject_commits target_base_branch
       puts "Injecting commits…"
 
-      @upstream_name     ||= upstream_name
-      target_base_branch ||= 'master'
-
+      target_base_branch     ||= 'master'
       @reference_target_branch = "#{target_remote_name}/#{target_base_branch}"
 
       Dir.chdir git_dir do
@@ -211,9 +211,6 @@ def inject_commits target_base_branch, upstream_name
           else
             cat -
           fi
-          echo
-          echo
-          echo "(transferred from #{upstream_name}@$GIT_COMMIT)"
         ' -- #{inject_branch}`
 
         # Old (bad:  doesn't handle merges)
@@ -383,13 +380,15 @@ def previously_extracted_commits
               false
             else
               upstream_full_msg = `git show -s --format="%s%n%n%b" #{commit}`
+              upstream_full_msg.gsub! /^\(transferred from #{upstream_name}@.*$/, ''
 
               # TODO:  Change this to a `.one?` check, and if this fails, and
               # there is more than one, then compare changed file base names...
               # which is harder still...
               target_commits.any? do |target_commit|
                 target_full_msg = `git show -s --format="%s%n%n%b" #{target_commit}`
-                upstream_full_msg == target_full_msg
+                target_full_msg.gsub! /^\(transferred from #{upstream_name}@.*$/, ''
+                upstream_full_msg.strip == target_full_msg.strip
               end
             end
           end
@@ -443,10 +442,10 @@ def extract
     def reinsert
       return unless @config[:reinsert]
 
-      @source_project.prune_commits @config[:extractions]
+      @source_project.prune_commits @config[:extractions], @config[:upstream_name]
       @source_project.run_extra_cmds @config[:extra_cmds]
       @source_project.add_target_remote @config[:target_name], @config[:target_remote]
-      @source_project.inject_commits @config[:target_base_branch], @config[:upstream_name]
+      @source_project.inject_commits @config[:target_base_branch]
 
       true
     end

test/code_extractor_reinsert_test.rb

@@ -240,6 +240,7 @@ def apply_new_commits_on_extracted_repo &block
       end
     end
 
+    self.reference_repo_dir = @cloned_extractions_dir
     CodeExtractor::TestRepo.clone_at(extracted_dir, @cloned_extractions_dir, &block)
   end
 

test/test_helper.rb

@@ -21,7 +21,7 @@ module CodeExtractor
   # test, and clean up when it is done.
   #
   class TestCase < Minitest::Test
-    attr_writer :extractions, :extractions_hash
+    attr_writer :extractions, :extractions_hash, :reference_repo_dir
     attr_reader :extracted_dir, :repo_dir, :sandbox_dir
 
     # Create a sandbox for the given test, and name it after the test class and
@@ -61,14 +61,30 @@ def assert_no_tags
       assert tags.empty?, "Expected there to be no tags, but got `#{tags.join ', '}'"
     end
 
+    TRANSFERRED_FROM_REGEXP = /\(transferred from (?<UPSTREAM>[^@]*)@(?<SHA>[^\)]*)\)/
     def assert_commits expected_commits
       start_commit   = destination_repo.last_commit
       sorting        = Rugged::SORT_TOPO # aka:  sort like git-log
-      actual_commits = destination_repo.walk(start_commit, sorting).map(&:message)
-
-      actual_commits.map! { |msg| msg.lines.first.chomp }
-
-      assert_equal expected_commits, actual_commits
+      actual_commits = destination_repo.walk(start_commit, sorting).map {|c| c }
+      commit_msgs    = actual_commits.map { |commit| commit.message.lines.first.chomp }
+
+      assert_equal expected_commits, commit_msgs
+
+      # Check that the "transferred from ..." reference line is correct
+      actual_commits.map do |commit|
+        [
+          commit,
+          commit.message.lines.last.match(TRANSFERRED_FROM_REGEXP)
+        ]
+      end.each do |commit, transfered|
+        next unless transfered
+
+        transferred_commit = reference_repo.lookup(transfered[:SHA])
+        assert transferred_commit.is_a?(Rugged::Commit),
+          "'transfered from' of #{transfered[:SHA]} from #{commit} is not a valid commit"
+        assert_equal commit.message.lines.first.chomp,
+          transferred_commit.message.lines.first.chomp
+      end
     end
 
     # Helper methods
@@ -81,6 +97,10 @@ def destination_repo
       @destination_repo ||= Rugged::Repository.new @extracted_dir
     end
 
+    def reference_repo
+      @reference_repo ||= Rugged::Repository.new @reference_repo_dir
+    end
+
     def current_commit_message
       destination_repo.head.target.message
     end