Merge branch 'mr-on-fork' of https://github.com/karlhungus/gitlabhq into karlhungus-mr-on-fork

Conflicts:
	app/views/projects/commit/show.html.haml
	app/views/projects/compare/show.html.haml
	app/views/projects/merge_requests/branch_from.js.haml

4 files changed, 168 insertions, 38 deletions

diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 861a4f4d159..d690f1d07e7 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -14,6 +14,14 @@ module API
           end
           not_found!
         end
+
+        def not_fork?(target_project_id, user_project)
+          target_project_id.nil? || target_project_id == user_project.id.to_s
+        end
+
+        def target_matches_fork(target_project_id,user_project)
+          user_project.forked? && user_project.forked_from_project.id.to_s == target_project_id
+        end
       end
 
       # List merge requests
@@ -51,9 +59,10 @@ module API
       #
       # Parameters:
       #
-      #   id (required)            - The ID of a project
+      #   id (required)            - The ID of a project - this will be the source of the merge request
       #   source_branch (required) - The source branch
       #   target_branch (required) - The target branch
+      #   target_project           - The target project of the merge request defaults to the :id of the project
       #   assignee_id              - Assignee user ID
       #   title (required)         - Title of MR
       #
@@ -63,10 +72,20 @@ module API
       post ":id/merge_requests" do
         authorize! :write_merge_request, user_project
         required_attributes! [:source_branch, :target_branch, :title]
-
-        attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title]
+        attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id]
         merge_request = user_project.merge_requests.new(attrs)
         merge_request.author = current_user
+        merge_request.source_project = user_project
+        target_project_id = attrs[:target_project_id]
+        if not_fork?(target_project_id, user_project)
+          merge_request.target_project = user_project
+        else
+          if target_matches_fork(target_project_id,user_project)
+            merge_request.target_project = Project.find_by_id(attrs[:target_project_id])
+          else
+            render_api_error!('(Bad Request) Specified target project that is not the source project, or the source fork of the project.', 400)
+          end
+        end
 
         if merge_request.save
           merge_request.reload_code
diff --git a/lib/gitlab/satellite/action.rb b/lib/gitlab/satellite/action.rb
index 63303ca3de1..5ea6f956765 100644
--- a/lib/gitlab/satellite/action.rb
+++ b/lib/gitlab/satellite/action.rb
@@ -25,25 +25,31 @@ module Gitlab
           end
         end
       rescue Errno::ENOMEM => ex
-        Gitlab::GitLogger.error(ex.message)
-        return false
+        return handle_exception(ex)
       rescue Grit::Git::GitTimeout => ex
-        Gitlab::GitLogger.error(ex.message)
-        return false
+        return handle_exception(ex)
       ensure
         Gitlab::ShellEnv.reset_env
       end
 
-      # * Clears the satellite
-      # * Updates the satellite from Gitolite
+      # * Recreates the satellite
       # * Sets up Git variables for the user
       #
       # Note: use this within #in_locked_and_timed_satellite
       def prepare_satellite!(repo)
         project.satellite.clear_and_update!
 
-        repo.git.config({}, "user.name", user.name)
-        repo.git.config({}, "user.email", user.email)
+        repo.config['user.name'] = user.name
+        repo.config['user.email'] = user.email
+      end
+
+      def default_options(options = {})
+        {raise: true, timeout: true}.merge(options)
+      end
+
+      def handle_exception(exception)
+        Gitlab::GitLogger.error(exception.message)
+        false
       end
     end
   end
diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb
index 832db6621c4..6f402e80a63 100644
--- a/lib/gitlab/satellite/merge_action.rb
+++ b/lib/gitlab/satellite/merge_action.rb
@@ -5,48 +5,118 @@ module Gitlab
       attr_accessor :merge_request
 
       def initialize(user, merge_request)
-        super user, merge_request.project
+        super user, merge_request.target_project
         @merge_request = merge_request
       end
 
       # Checks if a merge request can be executed without user interaction
       def can_be_merged?
         in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
           merge_in_satellite!(merge_repo)
         end
       end
 
       # Merges the source branch into the target branch in the satellite and
-      # pushes it back to Gitolite.
-      # It also removes the source branch if requested in the merge request.
+      # pushes it back to the repository.
+      # It also removes the source branch if requested in the merge request (and this is permitted by the merge request).
       #
       # Returns false if the merge produced conflicts
-      # Returns false if pushing from the satellite to Gitolite failed or was rejected
+      # Returns false if pushing from the satellite to the repository failed or was rejected
       # Returns true otherwise
       def merge!
         in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
           if merge_in_satellite!(merge_repo)
             # push merge back to Gitolite
             # will raise CommandFailed when push fails
-            merge_repo.git.push({raise: true, timeout: true}, :origin, merge_request.target_branch)
-
+            merge_repo.git.push(default_options, :origin, merge_request.target_branch)
             # remove source branch
             if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch)
               # will raise CommandFailed when push fails
-              merge_repo.git.push({raise: true, timeout: true}, :origin, ":#{merge_request.source_branch}")
+              merge_repo.git.push(default_options, :origin, ":#{merge_request.source_branch}")
             end
-
             # merge, push and branch removal successful
             true
           end
         end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
 
-      private
+      # Get a raw diff of the source to the target
+      def diff_in_satellite
+        in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
+
+          update_satellite_source_and_target!(merge_repo)
+          if merge_request.for_fork?
+            diff = merge_repo.git.native(:diff, default_options, "origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}")
+          else
+            diff = merge_repo.git.native(:diff, default_options, "#{merge_request.target_branch}", "#{merge_request.source_branch}")
+
+          end
+          return diff
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
+
+      # Only show what is new in the source branch compared to the target branch, not the other way around.
+      # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2)
+      # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B"
+      def diffs_between_satellite
+        in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
+          update_satellite_source_and_target!(merge_repo)
+          if merge_request.for_fork?
+            common_commit = merge_repo.git.native(:merge_base, default_options, ["origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}"]).strip
+            #this method doesn't take default options
+            diffs = merge_repo.diff(common_commit, "source/#{merge_request.source_branch}")
+          else
+            raise "Attempt to determine diffs between for a non forked merge request in satellite MergeRequest.id:[#{merge_request.id}]"
+          end
+          diffs = diffs.map { |diff| Gitlab::Git::Diff.new(diff) }
+          return diffs
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
+
+      # Get commit as an email patch
+      def format_patch
+        in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
+          update_satellite_source_and_target!(merge_repo)
+          if (merge_request.for_fork?)
+            patch = merge_repo.git.format_patch(default_options({stdout: true}), "origin/#{merge_request.target_branch}...source/#{merge_request.source_branch}")
+          else
+            patch = merge_repo.git.format_patch(default_options({stdout: true}), "#{merge_request.target_branch}...#{merge_request.source_branch}")
+          end
+          return patch
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
+
+      # Retrieve an array of commits between the source and the target
+      def commits_between
+        in_locked_and_timed_satellite do |merge_repo|
+          prepare_satellite!(merge_repo)
+          update_satellite_source_and_target!(merge_repo)
+          if (merge_request.for_fork?)
+            commits = merge_repo.commits_between("origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}")
+          else
+            raise "Attempt to determine commits between for a non forked merge request in satellite MergeRequest.id:[#{merge_request.id}]"
+          end
+          commits = commits.map { |commit| Gitlab::Git::Commit.new(commit, nil) }
+          return commits
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
 
+      private
       # Merges the source_branch into the target_branch in the satellite.
       #
       # Note: it will clear out the satellite before doing anything
@@ -54,18 +124,35 @@ module Gitlab
       # Returns false if the merge produced conflicts
       # Returns true otherwise
       def merge_in_satellite!(repo)
-        prepare_satellite!(repo)
-
-        # create target branch in satellite at the corresponding commit from Gitolite
-        repo.git.checkout({raise: true, timeout: true, b: true}, merge_request.target_branch, "origin/#{merge_request.target_branch}")
+        update_satellite_source_and_target!(repo)
 
-        # merge the source branch from Gitolite into the satellite
+        # merge the source branch into the satellite
         # will raise CommandFailed when merge fails
-        repo.git.pull({raise: true, timeout: true, no_ff: true}, :origin, merge_request.source_branch)
+        if merge_request.for_fork?
+          repo.git.pull(default_options({no_ff: true}), 'source', merge_request.source_branch)
+        else
+          repo.git.pull(default_options({no_ff: true}), 'origin', merge_request.source_branch)
+        end
       rescue Grit::Git::CommandFailed => ex
-        Gitlab::GitLogger.error(ex.message)
-        false
+        handle_exception(ex)
       end
+
+      # Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for merges, diffs etc
+      def update_satellite_source_and_target!(repo)
+        if  merge_request.for_fork?
+          repo.remote_add('source', merge_request.source_project.repository.path_to_repo)
+          repo.remote_fetch('source')
+          repo.git.checkout(default_options({b: true}), merge_request.target_branch, "origin/#{merge_request.target_branch}")
+        else
+          # We can't trust the input here being branch names, we can't always check it out because it could be a relative ref i.e. HEAD~3
+          # we could actually remove the if true, because it should never ever happen (as long as the satellite has been prepared)
+          repo.git.checkout(default_options, "#{merge_request.source_branch}")
+          repo.git.checkout(default_options, "#{merge_request.target_branch}")
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
+
     end
   end
 end
diff --git a/lib/gitlab/satellite/satellite.rb b/lib/gitlab/satellite/satellite.rb
index 69f8551661b..6cb7814fae5 100644
--- a/lib/gitlab/satellite/satellite.rb
+++ b/lib/gitlab/satellite/satellite.rb
@@ -1,5 +1,5 @@
 module Gitlab
-  class SatelliteNotExistError < StandardError; end
+  class SatelliteNotExistError < StandardError;  end
 
   module Satellite
     class Satellite
@@ -24,8 +24,11 @@ module Gitlab
       def clear_and_update!
         raise_no_satellite unless exists?
 
+        File.exists? path
+        @repo = nil
         clear_working_dir!
         delete_heads!
+        remove_remotes!
         update_from_source!
       end
 
@@ -55,10 +58,11 @@ module Gitlab
         raise_no_satellite unless exists?
 
         File.open(lock_file, "w+") do |f|
-          f.flock(File::LOCK_EX)
-
-          Dir.chdir(path) do
-            return yield
+          begin
+            f.flock File::LOCK_EX
+            Dir.chdir(path) { return yield }
+          ensure
+            f.flock File::LOCK_UN
           end
         end
       end
@@ -100,20 +104,34 @@ module Gitlab
         if heads.include? PARKING_BRANCH
           repo.git.checkout({}, PARKING_BRANCH)
         else
-          repo.git.checkout({b: true}, PARKING_BRANCH)
+          repo.git.checkout(default_options({b: true}), PARKING_BRANCH)
         end
 
         # remove the parking branch from the list of heads ...
         heads.delete(PARKING_BRANCH)
         # ... and delete all others
-        heads.each { |head| repo.git.branch({D: true}, head) }
+        heads.each { |head| repo.git.branch(default_options({D: true}), head) }
+      end
+
+      # Deletes all remotes except origin
+      #
+      # This ensures we have no remote name clashes or issues updating branches when
+      # working with the satellite.
+      def remove_remotes!
+        remotes = repo.git.remote.split(' ')
+        remotes.delete('origin')
+        remotes.each { |name| repo.git.remote(default_options,'rm', name)}
       end
 
       # Updates the satellite from Gitolite
       #
       # Note: this will only update remote branches (i.e. origin/*)
       def update_from_source!
-        repo.git.fetch({timeout: true}, :origin)
+        repo.git.fetch(default_options, :origin)
+      end
+
+      def default_options(options = {})
+        {raise: true, timeout: true}.merge(options)
       end
 
       # Create directory for storing