Merge branch 'support-only-changes-on-mr-pipelines' into 'master'

Support `only: changes:` on MR pipelines

See merge request gitlab-org/gitlab-ce!24490

app/models/ci/pipeline.rb

@@ -687,11 +687,20 @@ module Ci
       end
     end
 
+    # Returns the modified paths.
+    #
+    # The returned value is
+    # * Array: List of modified paths that should be evaluated
+    # * nil: Modified path can not be evaluated
     def modified_paths
       strong_memoize(:modified_paths) do
+        if merge_request?
+          merge_request.modified_paths
+        elsif branch_updated?
           push_details.modified_paths
         end
       end
+    end
 
     def default_branch?
       ref == project.default_branch

changelogs/unreleased/support-only-changes-on-mr-pipelines.yml

+---
+title: 'Support `only: changes:` on MR pipelines'
+merge_request: 24490
+author: Hiroyuki Sato
+type: added

doc/ci/yaml/README.md

@@ -423,10 +423,28 @@ connected with merge requests yet, and because GitLab is creating pipelines
 before an user can create a merge request we don't know a target branch at
 this point.
 
-Without a target branch, it is not possible to know what the common ancestor is,
-thus we always create a job in that case. This feature works best for stable
-branches like `master` because in that case GitLab uses the previous commit
-that is present in a branch to compare against the latest SHA that was pushed.
+#### Using `changes` with `merge_requests`
+
+With [pipelines for merge requests](../merge_request_pipelines/index.md),
+make it possible to define if a job should be created base on files modified
+in a merge request.
+
+For example:
+
+```
+docker build service one:
+  script: docker build -t my-service-one-image:$CI_COMMIT_REF_SLUG .
+  only:
+    refs:
+      - merge_requests
+    changes:
+      - Dockerfile
+      - service-one/**/*
+```
+
+In the scenario above, if you create or update a merge request that changes
+either files in `service-one` folder or `Dockerfile`, GitLab creates and triggers
+the `docker build service one` job.
 
 ## `tags`
 

lib/gitlab/ci/build/policy/changes.rb

@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def satisfied_by?(pipeline, seed)
-            return true unless pipeline.branch_updated?
+            return true if pipeline.modified_paths.nil?
 
             pipeline.modified_paths.any? do |path|
               @globs.any? do |glob|

spec/lib/gitlab/ci/build/policy/changes_spec.rb

@@ -73,9 +73,9 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
 
-      context 'when pipelines does not run for a branch update' do
+      context 'when modified paths can not be evaluated' do
         before do
-          pipeline.before_sha = Gitlab::Git::BLANK_SHA
+          allow(pipeline).to receive(:modified_paths) { nil }
         end
 
         it 'is always satisfied' do
@@ -115,5 +115,57 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
     end
+
+    context 'when branch is created' do
+      let(:pipeline) do
+        create(:ci_empty_pipeline, project: project,
+                                   ref: 'feature',
+                                   source: source,
+                                   sha: '0b4bc9a4',
+                                   before_sha: Gitlab::Git::BLANK_SHA,
+                                   merge_request: merge_request)
+      end
+
+      let(:ci_build) do
+        build(:ci_build, pipeline: pipeline, project: project, ref: 'feature')
+      end
+
+      let(:seed) { double('build seed', to_resource: ci_build) }
+
+      context 'when source is merge request' do
+        let(:source) { :merge_request }
+
+        let(:merge_request) do
+          create(:merge_request,
+                 source_project: project,
+                 source_branch: 'feature',
+                 target_project: project,
+                 target_branch: 'master')
+        end
+
+        it 'is satified by changes in the merge request' do
+          policy = described_class.new(%w[files/ruby/feature.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+
+        it 'is not satified by changes not in the merge request' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).not_to be_satisfied_by(pipeline, seed)
+        end
+      end
+
+      context 'when source is push' do
+        let(:source) { :push }
+        let(:merge_request) { nil }
+
+        it 'is always satified' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+      end
+    end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -1172,8 +1172,26 @@ describe Ci::Pipeline, :mailer do
         pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA)
       end
 
-      it 'raises an error' do
-        expect { pipeline.modified_paths }.to raise_error(ArgumentError)
+      it 'returns nil' do
+        expect(pipeline.modified_paths).to be_nil
+      end
+    end
+
+    context 'when source is merge request' do
+      let(:pipeline) do
+        create(:ci_pipeline, source: :merge_request, merge_request: merge_request)
+      end
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: project,
+               source_branch: 'feature',
+               target_project: project,
+               target_branch: 'master')
+      end
+
+      it 'returns merge request modified paths' do
+        expect(pipeline.modified_paths).to match(merge_request.modified_paths)
       end
     end
   end