From a4056a4a81b4075ff2749a3075377a32bcf1a47d Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 12 Nov 2019 13:48:38 +0200
Subject: [PATCH 01/32] Pass artifacts in CI DAG needs

Implementation idea for adding the artifacts keyword
into CI DAG needs.
---
 app/models/ci/build.rb                        |  2 +-
 ...12090226_add_artifacts_to_ci_build_need.rb | 21 +++++++++++++++
 db/schema.rb                                  |  1 +
 ee/lib/ee/gitlab/ci/config/entry/need.rb      |  2 +-
 lib/gitlab/ci/config/entry/job.rb             |  2 +-
 lib/gitlab/ci/config/entry/need.rb            | 26 ++++++++++++++++++-
 lib/gitlab/ci/config/entry/needs.rb           | 11 +++++++-
 7 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4679e8b74d7c9c..225e17e2768113 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -764,7 +764,7 @@ def dependencies
 
       # find all jobs that are needed
       if Feature.enabled?(:ci_dag_support, project, default_enabled: true) && needs.exists?
-        depended_jobs = depended_jobs.where(name: needs.select(:name))
+        depended_jobs = depended_jobs.where(name: needs.where(artifacts: true).select(:name))
       end
 
       # find all jobs that are dependent on
diff --git a/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb
new file mode 100644
index 00000000000000..46f01a740b5157
--- /dev/null
+++ b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddArtifactsToCiBuildNeed < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # TODO add partial index on name and artifacts=true
+  def up
+    add_column_with_default(:ci_build_needs, :artifacts,
+                            :boolean,
+                            default: true,
+                            allow_null: false)
+  end
+
+  def down
+    remove_column(:ci_build_needs, :artifacts)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9dccceb79f0745..7f380c969e21f9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -595,6 +595,7 @@
   create_table "ci_build_needs", id: :serial, force: :cascade do |t|
     t.integer "build_id", null: false
     t.text "name", null: false
+    t.boolean "artifacts", default: true, null: false
     t.index ["build_id", "name"], name: "index_ci_build_needs_on_build_id_and_name", unique: true
   end
 
diff --git a/ee/lib/ee/gitlab/ci/config/entry/need.rb b/ee/lib/ee/gitlab/ci/config/entry/need.rb
index 1f34412364ff5b..799b1278d1a977 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/need.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/need.rb
@@ -9,7 +9,7 @@ module Need
             extend ActiveSupport::Concern
 
             prepended do
-              strategy :Bridge, class: EE::Gitlab::Ci::Config::Entry::Need::Bridge, if: -> (config) { config.is_a?(Hash) }
+              strategy :Bridge, class: EE::Gitlab::Ci::Config::Entry::Need::Bridge, if: -> (config) { config.is_a?(Hash) && config.key?(:pipeline) }
             end
 
             class Bridge < ::Gitlab::Config::Entry::Node
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 7c01e6ffbe8705..e2aa1162b4b92e 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -127,7 +127,7 @@ class Job < ::Gitlab::Config::Entry::Node
 
           entry :needs, Entry::Needs,
             description: 'Needs configuration for this job.',
-            metadata: { allowed_needs: %i[job] },
+            metadata: { allowed_needs: %i[job dependency] },
             inherit: false
 
           entry :variables, Entry::Variables,
diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index b6db546d8ff42e..5fd00351c40df3 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -6,6 +6,7 @@ class Config
       module Entry
         class Need < ::Gitlab::Config::Entry::Simplifiable
           strategy :Job, if: -> (config) { config.is_a?(String) }
+          strategy :Dependency, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
 
           class Job < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
@@ -20,7 +21,30 @@ def type
             end
 
             def value
-              { name: @config }
+              { name: @config, artifacts: true }
+            end
+          end
+
+          class Dependency < ::Gitlab::Config::Entry::Node
+            include ::Gitlab::Config::Entry::Validatable
+            include ::Gitlab::Config::Entry::Attributable
+
+            ALLOWED_KEYS = %i[job artifacts].freeze
+            attributes :job, :artifacts
+
+            validations do
+              validates :config, presence: true
+              validates :config, allowed_keys: ALLOWED_KEYS
+              validates :job, type: String, presence: true
+              validates :artifacts, boolean: true
+            end
+
+            def type
+              :dependency
+            end
+
+            def value
+              { name: job, artifacts: artifacts }
             end
           end
 
diff --git a/lib/gitlab/ci/config/entry/needs.rb b/lib/gitlab/ci/config/entry/needs.rb
index 28452aaaa16b48..3d13584064e496 100644
--- a/lib/gitlab/ci/config/entry/needs.rb
+++ b/lib/gitlab/ci/config/entry/needs.rb
@@ -44,9 +44,18 @@ def compose!(deps = nil)
 
           def value
             values = @entries.values.select(&:type)
-            values.group_by(&:type).transform_values do |values|
+            values = values.group_by(&:type).transform_values do |values|
               values.map(&:value)
             end
+
+            dependencies = values.delete(:dependency)
+
+            if dependencies
+              values[:job] ||= []
+              values[:job].concat(dependencies)
+            end
+
+            values
           end
         end
       end
-- 
GitLab


From 1b3afd2f7cc32cc9d2cff6f0fd3f2940cb2892c7 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 14 Nov 2019 18:27:43 +0200
Subject: [PATCH 02/32] Refactor passing CI DAG artifacts

Update config Normalizer to leave other keys in the job definition
and change only the name.
---
 ...-dependencies-and-artifacts-with-needs.yml |   5 +
 ee/lib/ee/gitlab/ci/config/entry/need.rb      |   4 +-
 lib/gitlab/ci/config/entry/job.rb             |   2 +-
 lib/gitlab/ci/config/entry/need.rb            |   8 +-
 lib/gitlab/ci/config/entry/needs.rb           |  13 +-
 lib/gitlab/ci/config/normalizer.rb            |   2 +-
 spec/lib/gitlab/ci/config/entry/need_spec.rb  | 161 ++++++++++++++++--
 spec/lib/gitlab/ci/config/entry/needs_spec.rb |  85 ++++++++-
 spec/lib/gitlab/ci/config/normalizer_spec.rb  |  10 +-
 spec/lib/gitlab/ci/yaml_processor_spec.rb     |  54 +++++-
 10 files changed, 305 insertions(+), 39 deletions(-)
 create mode 100644 changelogs/unreleased/ci-merge-dependencies-and-artifacts-with-needs.yml

diff --git a/changelogs/unreleased/ci-merge-dependencies-and-artifacts-with-needs.yml b/changelogs/unreleased/ci-merge-dependencies-and-artifacts-with-needs.yml
new file mode 100644
index 00000000000000..d9ba35b44c4f22
--- /dev/null
+++ b/changelogs/unreleased/ci-merge-dependencies-and-artifacts-with-needs.yml
@@ -0,0 +1,5 @@
+---
+title: Control passing artifacts from CI DAG needs
+merge_request: 19943
+author:
+type: added
diff --git a/ee/lib/ee/gitlab/ci/config/entry/need.rb b/ee/lib/ee/gitlab/ci/config/entry/need.rb
index 799b1278d1a977..d91abdcb91ae73 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/need.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/need.rb
@@ -9,7 +9,9 @@ module Need
             extend ActiveSupport::Concern
 
             prepended do
-              strategy :Bridge, class: EE::Gitlab::Ci::Config::Entry::Need::Bridge, if: -> (config) { config.is_a?(Hash) && config.key?(:pipeline) }
+              strategy :Bridge,
+                class: EE::Gitlab::Ci::Config::Entry::Need::Bridge,
+                if: -> (config) { config.is_a?(Hash) && !config.key?(:job) }
             end
 
             class Bridge < ::Gitlab::Config::Entry::Node
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index e2aa1162b4b92e..7c01e6ffbe8705 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -127,7 +127,7 @@ class Job < ::Gitlab::Config::Entry::Node
 
           entry :needs, Entry::Needs,
             description: 'Needs configuration for this job.',
-            metadata: { allowed_needs: %i[job dependency] },
+            metadata: { allowed_needs: %i[job] },
             inherit: false
 
           entry :variables, Entry::Variables,
diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index 5fd00351c40df3..c7963a823fca03 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -6,7 +6,7 @@ class Config
       module Entry
         class Need < ::Gitlab::Config::Entry::Simplifiable
           strategy :Job, if: -> (config) { config.is_a?(String) }
-          strategy :Dependency, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
+          strategy :ComplexJob, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
 
           class Job < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
@@ -25,7 +25,7 @@ def value
             end
           end
 
-          class Dependency < ::Gitlab::Config::Entry::Node
+          class ComplexJob < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
             include ::Gitlab::Config::Entry::Attributable
 
@@ -36,11 +36,11 @@ class Dependency < ::Gitlab::Config::Entry::Node
               validates :config, presence: true
               validates :config, allowed_keys: ALLOWED_KEYS
               validates :job, type: String, presence: true
-              validates :artifacts, boolean: true
+              validates :artifacts, boolean: true, allow_nil: false
             end
 
             def type
-              :dependency
+              :complex_job
             end
 
             def value
diff --git a/lib/gitlab/ci/config/entry/needs.rb b/lib/gitlab/ci/config/entry/needs.rb
index 3d13584064e496..8e1f55ef3ab1a6 100644
--- a/lib/gitlab/ci/config/entry/needs.rb
+++ b/lib/gitlab/ci/config/entry/needs.rb
@@ -48,13 +48,16 @@ def value
               values.map(&:value)
             end
 
-            dependencies = values.delete(:dependency)
+            merge_jobs(values)
+          end
 
-            if dependencies
-              values[:job] ||= []
-              values[:job].concat(dependencies)
-            end
+          private
+
+          def merge_jobs(values)
+            jobs = values.delete(:job).to_a
+            jobs = jobs.concat values.delete(:complex_job).to_a
 
+            values[:job] = jobs if jobs.any?
             values
           end
         end
diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb
index e714ef225f5938..1139efee9e8336 100644
--- a/lib/gitlab/ci/config/normalizer.rb
+++ b/lib/gitlab/ci/config/normalizer.rb
@@ -44,7 +44,7 @@ def expand_needs(job_needs)
 
             if all_job_names = parallelized_jobs[job_need_name]
               all_job_names.map do |job_name|
-                { name: job_name }
+                job_need.merge(name: job_name)
               end
             else
               job_need
diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
index d119e60490085b..6dfe28827525f8 100644
--- a/spec/lib/gitlab/ci/config/entry/need_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -5,31 +5,170 @@
 describe ::Gitlab::Ci::Config::Entry::Need do
   subject(:need) { described_class.new(config) }
 
-  context 'when job is specified' do
-    let(:config) { 'job_name' }
+  shared_examples 'complex job type' do
+    describe '#type' do
+      subject(:need_type) { need.type }
 
-    describe '#valid?' do
-      it { is_expected.to be_valid }
+      it { is_expected.to eq(:complex_job) }
+    end
+  end
+
+  shared_examples 'simple job type' do
+    describe '#type' do
+      subject(:need_type) { need.type }
+
+      it { is_expected.to eq(:job) }
+    end
+  end
+
+  context 'with simple config' do
+    context 'when job is specified' do
+      let(:config) { 'job_name' }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true)
+        end
+      end
+
+      it_behaves_like 'simple job type'
+    end
+
+    context 'when need is empty' do
+      let(:config) { '' }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'is returns an error about an empty config' do
+          expect(need.errors)
+            .to contain_exactly("job config can't be blank")
+        end
+      end
+
+      it_behaves_like 'simple job type'
+    end
+  end
+
+  context 'with complex config' do
+    context 'with job name and artifacts true' do
+      let(:config) { { job: 'job_name', artifacts: true } }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true)
+        end
+      end
+
+      it_behaves_like 'complex job type'
+    end
+
+    context 'with job name and artifacts false' do
+      let(:config) { { job: 'job_name', artifacts: false } }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: false)
+        end
+      end
+
+      it_behaves_like 'complex job type'
+    end
+
+    context 'with job name and artifacts nil' do
+      let(:config) { { job: 'job_name', artifacts: nil } }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'is returns an error about artifacts type' do
+          expect(need.errors)
+            .to contain_exactly('complex job artifacts should be a boolean value')
+        end
+      end
+
+      it_behaves_like 'complex job type'
+    end
+
+    context 'when job name is empty' do
+      let(:config) { { job: '', artifacts: true } }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'is returns an error about an empty config' do
+          expect(need.errors)
+            .to contain_exactly("complex job job can't be blank")
+        end
+      end
+
+      it_behaves_like 'complex job type'
     end
 
-    describe '#value' do
-      it 'returns job needs configuration' do
-        expect(need.value).to eq(name: 'job_name')
+    context 'when job name is not a string' do
+      let(:config) { { job: :job_name, artifacts: false } }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'is returns an error about job type' do
+          expect(need.errors)
+            .to contain_exactly('complex job job should be a string')
+        end
       end
+
+      it_behaves_like 'complex job type'
+    end
+
+    context 'when job has unknown keys' do
+      let(:config) { { job: 'job_name', artifacts: false, some: :key } }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'is returns an error about job type' do
+          expect(need.errors)
+            .to contain_exactly('complex job config contains unknown keys: some')
+        end
+      end
+
+      it_behaves_like 'complex job type'
     end
   end
 
-  context 'when need is empty' do
-    let(:config) { '' }
+  context 'when need config is not a string or a hash' do
+    let(:config) { :job_name }
 
     describe '#valid?' do
       it { is_expected.not_to be_valid }
     end
 
     describe '#errors' do
-      it 'is returns an error about an empty config' do
+      it 'is returns an error about job type' do
         expect(need.errors)
-          .to contain_exactly("job config can't be blank")
+          .to contain_exactly('unknown strategy has an unsupported type')
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/config/entry/needs_spec.rb b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
index f4a76b52d30c0e..d234653bc80dee 100644
--- a/spec/lib/gitlab/ci/config/entry/needs_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
@@ -51,9 +51,34 @@
         end
       end
     end
+
+    context 'when wrong needs type is used' do
+      let(:config) { [{ job: 'job_name', artifacts: true, some: :key }] }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'returns error about incorrect type' do
+          expect(needs.errors).to contain_exactly(
+            'need config contains unknown keys: some')
+        end
+      end
+    end
   end
 
   describe '.compose!' do
+    shared_examples 'entry with descendant nodes' do
+      describe '#descendants' do
+        it 'creates valid descendant nodes' do
+          expect(needs.descendants.count).to eq 2
+          expect(needs.descendants)
+            .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
+        end
+      end
+    end
+
     context 'when valid job entries composed' do
       let(:config) { %w[first_job_name second_job_name] }
 
@@ -65,20 +90,66 @@
         it 'returns key value' do
           expect(needs.value).to eq(
             job: [
-              { name: 'first_job_name' },
-              { name: 'second_job_name' }
+              { name: 'first_job_name',  artifacts: true },
+              { name: 'second_job_name', artifacts: true }
             ]
           )
         end
       end
 
-      describe '#descendants' do
-        it 'creates valid descendant nodes' do
-          expect(needs.descendants.count).to eq 2
-          expect(needs.descendants)
-            .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
+      it_behaves_like 'entry with descendant nodes'
+    end
+
+    context 'with complex job entries composed' do
+      let(:config) do
+        [
+          { job: 'first_job_name',  artifacts: true },
+          { job: 'second_job_name', artifacts: false }
+        ]
+      end
+
+      before do
+        needs.compose!
+      end
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(needs.value).to eq(
+            job: [
+              { name: 'first_job_name',  artifacts: true },
+              { name: 'second_job_name', artifacts: false }
+            ]
+          )
         end
       end
+
+      it_behaves_like 'entry with descendant nodes'
+    end
+
+    context 'with mixed job entries composed' do
+      let(:config) do
+        [
+          'first_job_name',
+          { job: 'second_job_name', artifacts: false }
+        ]
+      end
+
+      before do
+        needs.compose!
+      end
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(needs.value).to eq(
+            job: [
+              { name: 'first_job_name',  artifacts: true },
+              { name: 'second_job_name', artifacts: false }
+            ]
+          )
+        end
+      end
+
+      it_behaves_like 'entry with descendant nodes'
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/config/normalizer_spec.rb b/spec/lib/gitlab/ci/config/normalizer_spec.rb
index bf88047838755a..db62fb7524dfa7 100644
--- a/spec/lib/gitlab/ci/config/normalizer_spec.rb
+++ b/spec/lib/gitlab/ci/config/normalizer_spec.rb
@@ -105,7 +105,7 @@
     context 'for needs' do
       let(:expanded_job_attributes) do
         expanded_job_names.map do |job_name|
-          { name: job_name }
+          { name: job_name, extra: :key }
         end
       end
 
@@ -117,7 +117,7 @@
               script: 'echo 1',
               needs: {
                 job: [
-                  { name: job_name.to_s }
+                  { name: job_name.to_s, extra: :key }
                 ]
               }
             }
@@ -140,8 +140,8 @@
               script: 'echo 1',
               needs: {
                 job: [
-                  { name: job_name.to_s },
-                  { name: "other_job" }
+                  { name: job_name.to_s, extra: :key },
+                  { name: "other_job", extra: :key }
                 ]
               }
             }
@@ -153,7 +153,7 @@
         end
 
         it "includes the regular job in dependencies" do
-          expect(subject.dig(:final_job, :needs, :job)).to include(name: 'other_job')
+          expect(subject.dig(:final_job, :needs, :job)).to include(name: 'other_job', extra: :key)
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 5dc51f83b3cfc7..af30dbd8691870 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1525,8 +1525,48 @@ module Ci
               name: "test1",
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "build1" },
-                { name: "build2" }
+                { name: "build1", artifacts: true },
+                { name: "build2", artifacts: true }
+              ],
+              when: "on_success",
+              allow_failure: false,
+              yaml_variables: []
+            )
+          end
+        end
+
+        context 'needs two builds' do
+          let(:needs) do
+            [
+              { job: 'parallel', artifacts: false },
+              { job: 'build1',   artifacts: true },
+              'build2'
+            ]
+          end
+
+          it "does create jobs with valid specification" do
+            expect(subject.builds.size).to eq(7)
+            expect(subject.builds[0]).to eq(
+              stage: "build",
+              stage_idx: 1,
+              name: "build1",
+              options: {
+                script: ["test"]
+              },
+              when: "on_success",
+              allow_failure: false,
+              yaml_variables: []
+            )
+            expect(subject.builds[4]).to eq(
+              stage: "test",
+              stage_idx: 2,
+              name: "test1",
+              options: { script: ["test"] },
+              needs_attributes: [
+                { name: "build2", artifacts: true },
+                { name: "parallel 1/2", artifacts: false },
+                { name: "parallel 2/2", artifacts: false },
+                { name: "build1", artifacts: true }
               ],
               when: "on_success",
               allow_failure: false,
@@ -1546,8 +1586,8 @@ module Ci
               name: "test1",
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "parallel 1/2" },
-                { name: "parallel 2/2" }
+                { name: "parallel 1/2", artifacts: true },
+                { name: "parallel 2/2", artifacts: true }
               ],
               when: "on_success",
               allow_failure: false,
@@ -1574,6 +1614,12 @@ module Ci
 
           it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies the build2 should be part of needs') }
         end
+
+        context 'complex needs syntax without artifacts' do
+          let(:needs) { [{ job: 'build1' }] }
+
+          it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1:needs:need artifacts should be a boolean value') }
+        end
       end
 
       context 'with when/rules conflict' do
-- 
GitLab


From 9ab28cb93b207aadaa5883d415b5c84bc590abc9 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 15 Nov 2019 10:52:53 +0200
Subject: [PATCH 03/32] Update YamlProcessor EE specs to include artifacts

---
 ee/spec/lib/gitlab/ci/yaml_processor_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb
index e782c762daacd7..ab053a76014f62 100644
--- a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -67,7 +67,7 @@
             bridge_needs: { pipeline: 'some/project' }
           },
           needs_attributes: [
-            { name: "build" }
+            { name: "build", artifacts: true }
           ],
           when: "on_success",
           allow_failure: false,
-- 
GitLab


From fae909341e2d3687d642063d0837482786c39761 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 15 Nov 2019 10:53:50 +0200
Subject: [PATCH 04/32] Update Needs EE specs to include DAG artifacts

---
 .../lib/ee/gitlab/ci/config/entry/needs_spec.rb   | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
index 2df14ae0b4cb12..c7272adaef5f99 100644
--- a/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
+++ b/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
@@ -48,7 +48,13 @@
 
   describe '.compose!' do
     context 'when valid job entries composed' do
-      let(:config) { ['first_job_name', pipeline: 'some/project'] }
+      let(:config) do
+        [
+          'first_job_name',
+          { job: 'second_job_name', artifacts: false },
+          { pipeline: 'some/project' }
+        ]
+      end
 
       before do
         needs.compose!
@@ -61,7 +67,10 @@
       describe '#value' do
         it 'returns key value' do
           expect(needs.value).to eq(
-            job: [{ name: 'first_job_name' }],
+            job: [
+              { name: 'first_job_name',  artifacts: true },
+              { name: 'second_job_name', artifacts: false }
+            ],
             bridge: [{ pipeline: 'some/project' }]
           )
         end
@@ -69,7 +78,7 @@
 
       describe '#descendants' do
         it 'creates valid descendant nodes' do
-          expect(needs.descendants.count).to eq 2
+          expect(needs.descendants.count).to eq(3)
           expect(needs.descendants)
             .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
         end
-- 
GitLab


From 525359dacb4a50709b8c2a33308fdb94ce050fbd Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 15 Nov 2019 11:57:48 +0200
Subject: [PATCH 05/32] Remove partial index todo for ci_build_needs

A Ci::Build can have at most 50 ci_build_needs
Adding a partial index for this type of query would not
save too much time because there is already an index
that covers the build_id column:
index_ci_build_needs_on_build_id_and_name
---
 db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb
index 46f01a740b5157..2fbd003b2e5495 100644
--- a/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb
+++ b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb
@@ -7,7 +7,6 @@ class AddArtifactsToCiBuildNeed < ActiveRecord::Migration[5.2]
 
   disable_ddl_transaction!
 
-  # TODO add partial index on name and artifacts=true
   def up
     add_column_with_default(:ci_build_needs, :artifacts,
                             :boolean,
-- 
GitLab


From de265474c2f677b320244d9d35d94d4bc6796de5 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 15 Nov 2019 17:01:02 +0200
Subject: [PATCH 06/32] Add needs specs for CreatePipelineService

Test DAG needs artifact keyword in the context of
CreatePipelineService
---
 .../ci/create_pipeline_service/needs_spec.rb  | 149 ++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 spec/services/ci/create_pipeline_service/needs_spec.rb

diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb
new file mode 100644
index 00000000000000..042312be9d0865
--- /dev/null
+++ b/spec/services/ci/create_pipeline_service/needs_spec.rb
@@ -0,0 +1,149 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CreatePipelineService do
+  context 'needs' do
+    let(:user)     { create(:admin) }
+    let(:ref)      { 'refs/heads/master' }
+    let(:source)   { :push }
+    let(:service)  { described_class.new(project, user, { ref: ref }) }
+    let(:project)  { create(:project, :repository, creator: user) }
+    let(:pipeline) { service.execute(source) }
+
+    before do
+      stub_ci_pipeline_yaml_file(config)
+    end
+
+    context 'with a valid config' do
+      let(:config) do
+        <<~YAML
+          build_a:
+            stage: build
+            script:
+              - make
+            artifacts:
+              paths:
+                - binaries/
+          build_b:
+            stage: build
+            script:
+              - make
+            artifacts:
+              paths:
+                - other_binaries/
+          build_c:
+            stage: build
+            script:
+              - make
+          build_d:
+            stage: build
+            script:
+              - make
+            parallel: 3
+
+          test_a:
+            stage: test
+            script:
+              - ls
+            needs:
+              - build_a
+              - job: build_b
+                artifacts: true
+              - job: build_c
+                artifacts: false
+            dependencies:
+              - build_a
+
+          test_b:
+            stage: test
+            script:
+              - ls
+            parallel: 2
+            needs:
+              - build_a
+              - job: build_b
+                artifacts: true
+              - job: build_d
+                artifacts: false
+        YAML
+      end
+
+      it 'creates a pipeline with builds' do
+        expected_builds = [
+          'build_a', 'build_b', 'build_c', 'build_d 1/3', 'build_d 2/3',
+          'build_d 3/3', 'test_a', 'test_b 1/2', 'test_b 2/2'
+        ]
+
+        expect(pipeline).to be_persisted
+        expect(pipeline.builds.pluck(:name)).to contain_exactly(*expected_builds)
+      end
+
+      it 'saves needs and dependencies' do
+        test_a = pipeline.builds.find_by(name: 'test_a')
+
+        expect(test_a.needs.map(&:attributes))
+          .to contain_exactly(
+            a_hash_including('name' => 'build_a', 'artifacts' => true),
+            a_hash_including('name' => 'build_b', 'artifacts' => true),
+            a_hash_including('name' => 'build_c', 'artifacts' => false)
+          )
+
+        expect(test_a.options)
+          .to match(a_hash_including('dependencies' => ['build_a']))
+      end
+
+      it 'saves parallel jobs' do
+        ['1/2', '2/2'].each do |part|
+          test_job = pipeline.builds.find_by(name: "test_b #{part}")
+
+          expect(test_job.needs.map(&:attributes))
+            .to contain_exactly(
+              a_hash_including('name' => 'build_a',     'artifacts' => true),
+              a_hash_including('name' => 'build_b',     'artifacts' => true),
+              a_hash_including('name' => 'build_d 1/3', 'artifacts' => false),
+              a_hash_including('name' => 'build_d 2/3', 'artifacts' => false),
+              a_hash_including('name' => 'build_d 3/3', 'artifacts' => false)
+            )
+        end
+      end
+    end
+
+    context 'with an invalid config' do
+      let(:config) do
+        <<~YAML
+          build_a:
+            stage: build
+            script:
+              - make
+            artifacts:
+              paths:
+                - binaries/
+
+          build_b:
+            stage: build
+            script:
+              - make
+            artifacts:
+              paths:
+                - other_binaries/
+
+          test_a:
+            stage: test
+            script:
+              - ls
+            needs:
+              - build_a
+              - job: build_b
+                artifacts: string
+        YAML
+      end
+
+      it 'creates a pipeline without builds' do
+        expect(pipeline).to be_persisted
+        expect(pipeline.builds.any?).to be_falsey
+        expect(pipeline.yaml_errors).to eq('jobs:test_a:needs:need artifacts should be a boolean value')
+      end
+    end
+  end
+end
-- 
GitLab


From a331cb1bcc2242e6aef6c671392bed5c564ca7e5 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 15 Nov 2019 18:33:21 +0200
Subject: [PATCH 07/32] Add Ci::Build#dependencies specs

Add specs to cover the Ci::BuildNeed artifacts column
---
 spec/models/ci/build_spec.rb | 45 ++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index d491300add2791..916f5536ebda08 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -741,20 +741,26 @@
 
       before do
         needs.to_a.each do |need|
-          create(:ci_build_need, build: final, name: need)
+          create(:ci_build_need, build: final, **need)
         end
       end
 
       subject { final.dependencies }
 
-      context 'when depedencies are defined' do
+      context 'when dependencies are defined' do
         let(:dependencies) { %w(rspec staging) }
 
         it { is_expected.to contain_exactly(rspec_test, staging) }
       end
 
       context 'when needs are defined' do
-        let(:needs) { %w(build rspec staging) }
+        let(:needs) do
+          [
+            { name: 'build',   artifacts: true },
+            { name: 'rspec',   artifacts: true },
+            { name: 'staging', artifacts: true }
+          ]
+        end
 
         it { is_expected.to contain_exactly(build, rspec_test, staging) }
 
@@ -767,13 +773,44 @@
         end
       end
 
+      context 'when need artifacts are defined' do
+        let(:needs) do
+          [
+            { name: 'build',   artifacts: true },
+            { name: 'rspec',   artifacts: false },
+            { name: 'staging', artifacts: true }
+          ]
+        end
+
+        it { is_expected.to contain_exactly(build, staging) }
+      end
+
       context 'when needs and dependencies are defined' do
         let(:dependencies) { %w(rspec staging) }
-        let(:needs) { %w(build rspec staging) }
+        let(:needs) do
+          [
+            { name: 'build',   artifacts: true },
+            { name: 'rspec',   artifacts: true },
+            { name: 'staging', artifacts: true }
+          ]
+        end
 
         it { is_expected.to contain_exactly(rspec_test, staging) }
       end
 
+      context 'when needs and dependencies contradict' do
+        let(:dependencies) { %w(rspec staging) }
+        let(:needs) do
+          [
+            { name: 'build',   artifacts: true },
+            { name: 'rspec',   artifacts: false },
+            { name: 'staging', artifacts: true }
+          ]
+        end
+
+        it { is_expected.to contain_exactly(staging) }
+      end
+
       context 'when nor dependencies or needs are defined' do
         it { is_expected.to contain_exactly(build, rspec_test, rubocop_test, staging) }
       end
-- 
GitLab


From 2f48bca7f8c841a1e5524e15e20faab4c1fc2970 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 14:07:52 +0200
Subject: [PATCH 08/32] Add docs for needs:job

---
 doc/ci/yaml/README.md | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index ce3d6247d86618..d0d38fd42eea10 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2232,6 +2232,32 @@ This example creates three paths of execution:
 - Related to the above, stages must be explicitly defined for all jobs
   that have the keyword `needs:` or are referred to by one.
 
+#### Complex `needs` syntax
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
+
+It is possible to manage the artifacts download for a job directly from the
+`needs` keyword, without using `dependencies`.
+
+```yaml
+build_job:
+  stage: build
+
+rspec:
+  stage: test
+  needs:
+    - job: build_job
+      artifacts: true
+
+rubocop:
+  stage: test
+  needs:
+    - job: build_job
+      artifacts: false
+```
+
+`artifacts` defaults to `true` when using the simple syntax.
+
 ### `coverage`
 
 > [Introduced][ce-7447] in GitLab 8.17.
-- 
GitLab


From 05d90bdc25953b8d44b58f6d46c9e24a03de600d Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 15:55:37 +0200
Subject: [PATCH 09/32] Refactor needs specs

Replace let with let_it_be for project and user
Split specs into smaller specs
---
 .../ci/create_pipeline_service/needs_spec.rb  | 27 +++++++++++--------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb
index 042312be9d0865..c5022974688080 100644
--- a/spec/services/ci/create_pipeline_service/needs_spec.rb
+++ b/spec/services/ci/create_pipeline_service/needs_spec.rb
@@ -4,11 +4,12 @@
 
 describe Ci::CreatePipelineService do
   context 'needs' do
-    let(:user)     { create(:admin) }
+    let_it_be(:user)    { create(:admin) }
+    let_it_be(:project) { create(:project, :repository, creator: user) }
+
     let(:ref)      { 'refs/heads/master' }
     let(:source)   { :push }
     let(:service)  { described_class.new(project, user, { ref: ref }) }
-    let(:project)  { create(:project, :repository, creator: user) }
     let(:pipeline) { service.execute(source) }
 
     before do
@@ -69,6 +70,8 @@
         YAML
       end
 
+      let(:test_a_build) { pipeline.builds.find_by(name: 'test_a') }
+
       it 'creates a pipeline with builds' do
         expected_builds = [
           'build_a', 'build_b', 'build_c', 'build_d 1/3', 'build_d 2/3',
@@ -79,17 +82,17 @@
         expect(pipeline.builds.pluck(:name)).to contain_exactly(*expected_builds)
       end
 
-      it 'saves needs and dependencies' do
-        test_a = pipeline.builds.find_by(name: 'test_a')
-
-        expect(test_a.needs.map(&:attributes))
+      it 'saves needs' do
+        expect(test_a_build.needs.map(&:attributes))
           .to contain_exactly(
             a_hash_including('name' => 'build_a', 'artifacts' => true),
             a_hash_including('name' => 'build_b', 'artifacts' => true),
             a_hash_including('name' => 'build_c', 'artifacts' => false)
           )
+      end
 
-        expect(test_a.options)
+      it 'saves dependencies' do
+        expect(test_a_build.options)
           .to match(a_hash_including('dependencies' => ['build_a']))
       end
 
@@ -139,10 +142,12 @@
         YAML
       end
 
-      it 'creates a pipeline without builds' do
-        expect(pipeline).to be_persisted
-        expect(pipeline.builds.any?).to be_falsey
-        expect(pipeline.yaml_errors).to eq('jobs:test_a:needs:need artifacts should be a boolean value')
+      it { expect(pipeline).to be_persisted }
+      it { expect(pipeline.builds.any?).to be_falsey }
+
+      it 'assigns an error to the pipeline' do
+        expect(pipeline.yaml_errors)
+          .to eq('jobs:test_a:needs:need artifacts should be a boolean value')
       end
     end
   end
-- 
GitLab


From 9b332d8fffc51190d7d1ea15be795d24b86dd1d6 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 15:57:29 +0200
Subject: [PATCH 10/32] Rename Need Job strategy to SimpleJob

Rename Need Job strategy to SimpleJob to maintain consistency
with ComplexJob
---
 lib/gitlab/ci/config/entry/need.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index c7963a823fca03..4972a0f86c3fba 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -5,10 +5,10 @@ module Ci
     class Config
       module Entry
         class Need < ::Gitlab::Config::Entry::Simplifiable
-          strategy :Job, if: -> (config) { config.is_a?(String) }
+          strategy :SimpleJob, if: -> (config) { config.is_a?(String) }
           strategy :ComplexJob, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
 
-          class Job < ::Gitlab::Config::Entry::Node
+          class SimpleJob < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
 
             validations do
-- 
GitLab


From d1a21770fc535a611515ac9555acc16008d6c7a0 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 17:31:45 +0200
Subject: [PATCH 11/32] Refactor merge_jobs for Needs entry

Use a more descriptive name: merge_job_needs_types
Alter the needs values only if complex jobs are defined
---
 lib/gitlab/ci/config/entry/needs.rb           | 13 +++++++------
 spec/lib/gitlab/ci/config/entry/needs_spec.rb | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/needs.rb b/lib/gitlab/ci/config/entry/needs.rb
index 8e1f55ef3ab1a6..ab5c5c118fdea2 100644
--- a/lib/gitlab/ci/config/entry/needs.rb
+++ b/lib/gitlab/ci/config/entry/needs.rb
@@ -48,17 +48,18 @@ def value
               values.map(&:value)
             end
 
-            merge_jobs(values)
+            merge_job_needs_types(values)
           end
 
           private
 
-          def merge_jobs(values)
-            jobs = values.delete(:job).to_a
-            jobs = jobs.concat values.delete(:complex_job).to_a
+          def merge_job_needs_types(values)
+            return values unless values.key?(:complex_job)
 
-            values[:job] = jobs if jobs.any?
-            values
+            values.tap do |values_hash|
+              values_hash[:job] =
+                values_hash[:job].to_a + values.delete(:complex_job).to_a
+            end
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/entry/needs_spec.rb b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
index d234653bc80dee..b8b84b5efd20cd 100644
--- a/spec/lib/gitlab/ci/config/entry/needs_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
@@ -151,5 +151,21 @@
 
       it_behaves_like 'entry with descendant nodes'
     end
+
+    context 'with empty config' do
+      let(:config) do
+        []
+      end
+
+      before do
+        needs.compose!
+      end
+
+      describe '#value' do
+        it 'returns empty value' do
+          expect(needs.value).to eq({})
+        end
+      end
+    end
   end
 end
-- 
GitLab


From aec72f09ae8b621c731ad251a0130d5298cf61c1 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 17:45:12 +0200
Subject: [PATCH 12/32] Explain how Build#dependencies works

---
 app/models/ci/build.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 225e17e2768113..05ffb9b7fa7b65 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -772,6 +772,8 @@ def dependencies
         depended_jobs = depended_jobs.where(name: options[:dependencies])
       end
 
+      # if both needs and dependencies are used,
+      # the end result will be an intersection between them
       depended_jobs
     end
 
-- 
GitLab


From 8718306b6065a843d5981eafdbc325d6c35a4e59 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 18 Nov 2019 18:06:06 +0200
Subject: [PATCH 13/32] Update need spec validation messages

This change was introduced by changing the name of Job strategy
to SimpleJob in the Need entry.
---
 spec/lib/gitlab/ci/config/entry/need_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
index 6dfe28827525f8..afb16f205a5529 100644
--- a/spec/lib/gitlab/ci/config/entry/need_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -48,7 +48,7 @@
       describe '#errors' do
         it 'is returns an error about an empty config' do
           expect(need.errors)
-            .to contain_exactly("job config can't be blank")
+            .to contain_exactly("simple job config can't be blank")
         end
       end
 
-- 
GitLab


From ca3830c429c90eab23ecb7e05d0a2e32aba08224 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 19 Nov 2019 18:50:13 +0200
Subject: [PATCH 14/32] Add artifacts scope on BuildNeeds

---
 app/models/ci/build.rb            | 2 +-
 app/models/ci/build_need.rb       | 1 +
 spec/models/ci/build_need_spec.rb | 7 +++++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 05ffb9b7fa7b65..7d3cb62e4eee46 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -764,7 +764,7 @@ def dependencies
 
       # find all jobs that are needed
       if Feature.enabled?(:ci_dag_support, project, default_enabled: true) && needs.exists?
-        depended_jobs = depended_jobs.where(name: needs.where(artifacts: true).select(:name))
+        depended_jobs = depended_jobs.where(name: needs.artifacts.select(:name))
       end
 
       # find all jobs that are dependent on
diff --git a/app/models/ci/build_need.rb b/app/models/ci/build_need.rb
index 6531dfd332fee6..0b243c20e67bbc 100644
--- a/app/models/ci/build_need.rb
+++ b/app/models/ci/build_need.rb
@@ -10,5 +10,6 @@ class BuildNeed < ApplicationRecord
     validates :name, presence: true, length: { maximum: 128 }
 
     scope :scoped_build, -> { where('ci_builds.id=ci_build_needs.build_id') }
+    scope :artifacts, -> { where(artifacts: true) }
   end
 end
diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb
index 450dd550a8f338..d1186fa981d1c3 100644
--- a/spec/models/ci/build_need_spec.rb
+++ b/spec/models/ci/build_need_spec.rb
@@ -10,4 +10,11 @@
   it { is_expected.to validate_presence_of(:build) }
   it { is_expected.to validate_presence_of(:name) }
   it { is_expected.to validate_length_of(:name).is_at_most(128) }
+
+  describe '.artifacts' do
+    let_it_be(:with_artifacts)    { create(:ci_build_need, artifacts: true) }
+    let_it_be(:without_artifacts) { create(:ci_build_need, artifacts: false) }
+
+    it { expect(described_class.artifacts).to contain_exactly(with_artifacts) }
+  end
 end
-- 
GitLab


From a6c9957ed859b14a34a3e6fe216987bd4bca1254 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 19 Nov 2019 19:35:18 +0200
Subject: [PATCH 15/32] Rename Need job strategies

---
 lib/gitlab/ci/config/entry/need.rb           |  8 ++++----
 spec/lib/gitlab/ci/config/entry/need_spec.rb | 10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index 4972a0f86c3fba..49126b32ae0485 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -5,10 +5,10 @@ module Ci
     class Config
       module Entry
         class Need < ::Gitlab::Config::Entry::Simplifiable
-          strategy :SimpleJob, if: -> (config) { config.is_a?(String) }
-          strategy :ComplexJob, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
+          strategy :JobString, if: -> (config) { config.is_a?(String) }
+          strategy :JobHash, if: -> (config) { config.is_a?(Hash) && config.key?(:job) }
 
-          class SimpleJob < ::Gitlab::Config::Entry::Node
+          class JobString < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
 
             validations do
@@ -25,7 +25,7 @@ def value
             end
           end
 
-          class ComplexJob < ::Gitlab::Config::Entry::Node
+          class JobHash < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
             include ::Gitlab::Config::Entry::Attributable
 
diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
index afb16f205a5529..d8e15499294868 100644
--- a/spec/lib/gitlab/ci/config/entry/need_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -48,7 +48,7 @@
       describe '#errors' do
         it 'is returns an error about an empty config' do
           expect(need.errors)
-            .to contain_exactly("simple job config can't be blank")
+            .to contain_exactly("job string config can't be blank")
         end
       end
 
@@ -99,7 +99,7 @@
       describe '#errors' do
         it 'is returns an error about artifacts type' do
           expect(need.errors)
-            .to contain_exactly('complex job artifacts should be a boolean value')
+            .to contain_exactly('job hash artifacts should be a boolean value')
         end
       end
 
@@ -116,7 +116,7 @@
       describe '#errors' do
         it 'is returns an error about an empty config' do
           expect(need.errors)
-            .to contain_exactly("complex job job can't be blank")
+            .to contain_exactly("job hash job can't be blank")
         end
       end
 
@@ -133,7 +133,7 @@
       describe '#errors' do
         it 'is returns an error about job type' do
           expect(need.errors)
-            .to contain_exactly('complex job job should be a string')
+            .to contain_exactly('job hash job should be a string')
         end
       end
 
@@ -150,7 +150,7 @@
       describe '#errors' do
         it 'is returns an error about job type' do
           expect(need.errors)
-            .to contain_exactly('complex job config contains unknown keys: some')
+            .to contain_exactly('job hash config contains unknown keys: some')
         end
       end
 
-- 
GitLab


From 35b5c0f11e1dd744dc991ac2bcd5361c9ac48481 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 19 Nov 2019 19:37:44 +0200
Subject: [PATCH 16/32] Remove Needs complex job type

---
 lib/gitlab/ci/config/entry/need.rb           |  2 +-
 lib/gitlab/ci/config/entry/needs.rb          | 15 +----------
 spec/lib/gitlab/ci/config/entry/need_spec.rb | 26 +++++++-------------
 spec/lib/gitlab/ci/yaml_processor_spec.rb    |  4 +--
 4 files changed, 13 insertions(+), 34 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index 49126b32ae0485..505d4d6385cbcd 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -40,7 +40,7 @@ class JobHash < ::Gitlab::Config::Entry::Node
             end
 
             def type
-              :complex_job
+              :job
             end
 
             def value
diff --git a/lib/gitlab/ci/config/entry/needs.rb b/lib/gitlab/ci/config/entry/needs.rb
index ab5c5c118fdea2..28452aaaa16b48 100644
--- a/lib/gitlab/ci/config/entry/needs.rb
+++ b/lib/gitlab/ci/config/entry/needs.rb
@@ -44,22 +44,9 @@ def compose!(deps = nil)
 
           def value
             values = @entries.values.select(&:type)
-            values = values.group_by(&:type).transform_values do |values|
+            values.group_by(&:type).transform_values do |values|
               values.map(&:value)
             end
-
-            merge_job_needs_types(values)
-          end
-
-          private
-
-          def merge_job_needs_types(values)
-            return values unless values.key?(:complex_job)
-
-            values.tap do |values_hash|
-              values_hash[:job] =
-                values_hash[:job].to_a + values.delete(:complex_job).to_a
-            end
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
index d8e15499294868..e20f6756dd1662 100644
--- a/spec/lib/gitlab/ci/config/entry/need_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -5,15 +5,7 @@
 describe ::Gitlab::Ci::Config::Entry::Need do
   subject(:need) { described_class.new(config) }
 
-  shared_examples 'complex job type' do
-    describe '#type' do
-      subject(:need_type) { need.type }
-
-      it { is_expected.to eq(:complex_job) }
-    end
-  end
-
-  shared_examples 'simple job type' do
+  shared_examples 'job type' do
     describe '#type' do
       subject(:need_type) { need.type }
 
@@ -35,7 +27,7 @@
         end
       end
 
-      it_behaves_like 'simple job type'
+      it_behaves_like 'job type'
     end
 
     context 'when need is empty' do
@@ -52,7 +44,7 @@
         end
       end
 
-      it_behaves_like 'simple job type'
+      it_behaves_like 'job type'
     end
   end
 
@@ -70,7 +62,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
 
     context 'with job name and artifacts false' do
@@ -86,7 +78,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
 
     context 'with job name and artifacts nil' do
@@ -103,7 +95,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
 
     context 'when job name is empty' do
@@ -120,7 +112,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
 
     context 'when job name is not a string' do
@@ -137,7 +129,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
 
     context 'when job has unknown keys' do
@@ -154,7 +146,7 @@
         end
       end
 
-      it_behaves_like 'complex job type'
+      it_behaves_like 'job type'
     end
   end
 
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index af30dbd8691870..07602090e56bf8 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1563,10 +1563,10 @@ module Ci
               name: "test1",
               options: { script: ["test"] },
               needs_attributes: [
-                { name: "build2", artifacts: true },
                 { name: "parallel 1/2", artifacts: false },
                 { name: "parallel 2/2", artifacts: false },
-                { name: "build1", artifacts: true }
+                { name: "build1", artifacts: true },
+                { name: "build2", artifacts: true }
               ],
               when: "on_success",
               allow_failure: false,
-- 
GitLab


From a4b9c6c8de4052e0029a653b50930035a1835dc9 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Wed, 20 Nov 2019 11:25:19 +0200
Subject: [PATCH 17/32] Rename Bridge strategy to BridgeHash

Rename Bridge strategy to BridgeHash to include
the object type that is expected
---
 ee/lib/ee/gitlab/ci/config/entry/need.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ee/lib/ee/gitlab/ci/config/entry/need.rb b/ee/lib/ee/gitlab/ci/config/entry/need.rb
index d91abdcb91ae73..844d7eb9fb7ab4 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/need.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/need.rb
@@ -9,12 +9,12 @@ module Need
             extend ActiveSupport::Concern
 
             prepended do
-              strategy :Bridge,
-                class: EE::Gitlab::Ci::Config::Entry::Need::Bridge,
+              strategy :BridgeHash,
+                class: EE::Gitlab::Ci::Config::Entry::Need::BridgeHash,
                 if: -> (config) { config.is_a?(Hash) && !config.key?(:job) }
             end
 
-            class Bridge < ::Gitlab::Config::Entry::Node
+            class BridgeHash < ::Gitlab::Config::Entry::Node
               include ::Gitlab::Config::Entry::Validatable
               include ::Gitlab::Config::Entry::Attributable
 
-- 
GitLab


From 5718deacb1b1abf13f357ccda0734427347faaf3 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Wed, 20 Nov 2019 12:15:41 +0200
Subject: [PATCH 18/32] Deprecate mixing DAG needs with dependencies

Document what happens when mixing needs with dependencies
---
 doc/ci/yaml/README.md | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index d0d38fd42eea10..876b26a4bd1a7c 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2258,6 +2258,39 @@ rubocop:
 
 `artifacts` defaults to `true` when using the simple syntax.
 
+Mixing `needs` with `dependencies` is now deprecated. It might lead to
+surprising results because the artifacts list is the intersection between
+`artifacts: true` and `dependencies`.
+
+```yaml
+build_job:
+  stage: build
+
+rspec:
+  stage: test
+  needs:
+    - job: build_job
+      artifacts: false
+  dependencies:
+    - build_job
+```
+
+We don't download artifacts because they are removed by `artifacts: false`
+
+```yaml
+build_job:
+  stage: build
+
+rspec:
+  stage: test
+  needs:
+    - job: build_job
+      artifacts: true
+  dependencies: []
+```
+
+We don't download artifacts because they are removed by `dependencies: []`
+
 ### `coverage`
 
 > [Introduced][ce-7447] in GitLab 8.17.
-- 
GitLab


From de55515133ce2a8e7505a79b9d734189585ba4dc Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Wed, 20 Nov 2019 12:20:18 +0200
Subject: [PATCH 19/32] Update expected error message for Bridge

Update expected error message for Bridge because
the Bridge strategy was renamed to BridgeHash
---
 ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb
index 2883851ce33e6e..b14f80e0aec496 100644
--- a/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb
+++ b/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb
@@ -29,7 +29,7 @@
     describe '#errors' do
       it 'is returns an error about an empty config' do
         expect(subject.errors)
-          .to include("bridge config can't be blank")
+          .to include("bridge hash config can't be blank")
       end
     end
   end
-- 
GitLab


From 33a7a5c250015c092420055b57ebe3b00a65bc9d Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 10:53:30 +0000
Subject: [PATCH 20/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 876b26a4bd1a7c..0547ec7ba9af8c 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2232,7 +2232,7 @@ This example creates three paths of execution:
 - Related to the above, stages must be explicitly defined for all jobs
   that have the keyword `needs:` or are referred to by one.
 
-#### Complex `needs` syntax
+#### Artifact downloads with `needs`
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
 
-- 
GitLab


From 833d55179fdb5dac83c087a798c69d0f778c8601 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 10:54:14 +0000
Subject: [PATCH 21/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 0547ec7ba9af8c..342991b303d3e3 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2236,8 +2236,10 @@ This example creates three paths of execution:
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
 
-It is possible to manage the artifacts download for a job directly from the
-`needs` keyword, without using `dependencies`.
+When using `needs`, you manage artifacts downloads with the `artifacts`
+keyword, and should not use `dependencies`.
+
+For example:
 
 ```yaml
 build_job:
-- 
GitLab


From a231c1d68fe3d5b34f8137cb92c967f1b18833ec Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 11:39:50 +0000
Subject: [PATCH 22/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 342991b303d3e3..5262a8a9e44d27 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2244,6 +2244,9 @@ For example:
 ```yaml
 build_job:
   stage: build
+  artifacts:
+    paths:
+      - binaries/
 
 rspec:
   stage: test
-- 
GitLab


From 61a6a31bc37179a1cc1fe14a0be58e481dcc44ad Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 14:53:31 +0200
Subject: [PATCH 23/32] Make artifacts true by default for DAG needs

artifacts key can be omitted from the needs job definition
and it will default to true.
---
 lib/gitlab/ci/config/entry/need.rb           |  4 ++--
 spec/lib/gitlab/ci/config/entry/need_spec.rb | 25 ++++++++++++++++----
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
index 505d4d6385cbcd..61bd09fd5f3bb9 100644
--- a/lib/gitlab/ci/config/entry/need.rb
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -36,7 +36,7 @@ class JobHash < ::Gitlab::Config::Entry::Node
               validates :config, presence: true
               validates :config, allowed_keys: ALLOWED_KEYS
               validates :job, type: String, presence: true
-              validates :artifacts, boolean: true, allow_nil: false
+              validates :artifacts, boolean: true, allow_nil: true
             end
 
             def type
@@ -44,7 +44,7 @@ def type
             end
 
             def value
-              { name: job, artifacts: artifacts }
+              { name: job, artifacts: artifacts || artifacts.nil? }
             end
           end
 
diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
index e20f6756dd1662..92b71c5f6cc091 100644
--- a/spec/lib/gitlab/ci/config/entry/need_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -85,13 +85,28 @@
       let(:config) { { job: 'job_name', artifacts: nil } }
 
       describe '#valid?' do
-        it { is_expected.not_to be_valid }
+        it { is_expected.to be_valid }
       end
 
-      describe '#errors' do
-        it 'is returns an error about artifacts type' do
-          expect(need.errors)
-            .to contain_exactly('job hash artifacts should be a boolean value')
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true)
+        end
+      end
+
+      it_behaves_like 'job type'
+    end
+
+    context 'without artifacts key' do
+      let(:config) { { job: 'job_name' } }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'returns job needs configuration' do
+          expect(need.value).to eq(name: 'job_name', artifacts: true)
         end
       end
 
-- 
GitLab


From 495bc21a153fb22ad659f1bdf4484a624ad9c4ae Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 16:45:17 +0200
Subject: [PATCH 24/32] Add more specs for DAG needs artifacts

---
 spec/lib/gitlab/ci/yaml_processor_spec.rb     | 35 +++++++++++++++----
 .../ci/create_pipeline_service/needs_spec.rb  | 25 +++++++++++--
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 07602090e56bf8..ed2d97b1a38237 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1596,6 +1596,35 @@ module Ci
           end
         end
 
+        context 'needs dependencies artifacts' do
+          let(:needs) do
+            [
+              "build1",
+              { job: "build2" },
+              { job: "parallel", artifacts: true }
+            ]
+          end
+
+          it "does create jobs with valid specification" do
+            expect(subject.builds.size).to eq(7)
+            expect(subject.builds[4]).to eq(
+              stage: "test",
+              stage_idx: 2,
+              name: "test1",
+              options: { script: ["test"] },
+              needs_attributes: [
+                { name: "build1", artifacts: true },
+                { name: "build2", artifacts: true },
+                { name: "parallel 1/2", artifacts: true },
+                { name: "parallel 2/2", artifacts: true }
+              ],
+              when: "on_success",
+              allow_failure: false,
+              yaml_variables: []
+            )
+          end
+        end
+
         context 'undefined need' do
           let(:needs) { ['undefined'] }
 
@@ -1614,12 +1643,6 @@ module Ci
 
           it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies the build2 should be part of needs') }
         end
-
-        context 'complex needs syntax without artifacts' do
-          let(:needs) { [{ job: 'build1' }] }
-
-          it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1:needs:need artifacts should be a boolean value') }
-        end
       end
 
       context 'with when/rules conflict' do
diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb
index c5022974688080..5ef7e592b36169 100644
--- a/spec/services/ci/create_pipeline_service/needs_spec.rb
+++ b/spec/services/ci/create_pipeline_service/needs_spec.rb
@@ -67,15 +67,25 @@
                 artifacts: true
               - job: build_d
                 artifacts: false
+
+          test_c:
+            stage: test
+            script:
+              - ls
+            needs:
+              - build_a
+              - job: build_b
+              - job: build_c
+                artifacts: true
         YAML
       end
 
-      let(:test_a_build) { pipeline.builds.find_by(name: 'test_a') }
+      let(:test_a_build) { pipeline.builds.find_by!(name: 'test_a') }
 
       it 'creates a pipeline with builds' do
         expected_builds = [
           'build_a', 'build_b', 'build_c', 'build_d 1/3', 'build_d 2/3',
-          'build_d 3/3', 'test_a', 'test_b 1/2', 'test_b 2/2'
+          'build_d 3/3', 'test_a', 'test_b 1/2', 'test_b 2/2', 'test_c'
         ]
 
         expect(pipeline).to be_persisted
@@ -96,6 +106,17 @@
           .to match(a_hash_including('dependencies' => ['build_a']))
       end
 
+      it 'artifacts default to true' do
+        test_job = pipeline.builds.find_by!(name: 'test_c')
+
+        expect(test_job.needs.map(&:attributes))
+          .to contain_exactly(
+            a_hash_including('name' => 'build_a', 'artifacts' => true),
+            a_hash_including('name' => 'build_b', 'artifacts' => true),
+            a_hash_including('name' => 'build_c', 'artifacts' => true)
+          )
+      end
+
       it 'saves parallel jobs' do
         ['1/2', '2/2'].each do |part|
           test_job = pipeline.builds.find_by(name: "test_b #{part}")
-- 
GitLab


From 45b9fa13b39fcbba5a26276170ec55e9fdcaf50e Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Thu, 21 Nov 2019 15:03:22 +0000
Subject: [PATCH 25/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 5262a8a9e44d27..f19ffed52c4436 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2261,7 +2261,9 @@ rubocop:
       artifacts: false
 ```
 
-`artifacts` defaults to `true` when using the simple syntax.
+In the example, the `rspec` job will download the `build_job` artifacts, while the
+`rubocop` job will not. Note that if `artifacts` is not included with `needs`, it
+will default to `true` and the artifacts will be downloaded.
 
 Mixing `needs` with `dependencies` is now deprecated. It might lead to
 surprising results because the artifacts list is the intersection between
-- 
GitLab


From dc19baee0f55c777fe9dde0b5785868a89b40bdc Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 22 Nov 2019 11:32:57 +0200
Subject: [PATCH 26/32] Update docs for needs artifacts

Add more examples for artifacts true
Remove bad examples
---
 doc/ci/yaml/README.md | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index f19ffed52c4436..fbd1a051f0d8f2 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2262,41 +2262,22 @@ rubocop:
 ```
 
 In the example, the `rspec` job will download the `build_job` artifacts, while the
-`rubocop` job will not. Note that if `artifacts` is not included with `needs`, it
-will default to `true` and the artifacts will be downloaded.
-
-Mixing `needs` with `dependencies` is now deprecated. It might lead to
-surprising results because the artifacts list is the intersection between
-`artifacts: true` and `dependencies`.
+`rubocop` job will not.
 
 ```yaml
-build_job:
-  stage: build
-
-rspec:
-  stage: test
-  needs:
-    - job: build_job
-      artifacts: false
-  dependencies:
-    - build_job
-```
-
-We don't download artifacts because they are removed by `artifacts: false`
-
-```yaml
-build_job:
-  stage: build
-
-rspec:
-  stage: test
+job:
   needs:
-    - job: build_job
+    - job: build_job_1
       artifacts: true
-  dependencies: []
+    - job: build_job_2
+    - build_job_3
 ```
 
-We don't download artifacts because they are removed by `dependencies: []`
+The job above will download the artifacts from all three jobs, as the artifacts
+will default to true for both `build_job_2` and `build_job_3`.
+
+Mixing `artifacts` with `dependencies` is not supported, and it can result in
+undesired behavior.
 
 ### `coverage`
 
-- 
GitLab


From 13929a3aa8eab47072bb64d554e72df29144d32a Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 22 Nov 2019 15:23:15 +0000
Subject: [PATCH 27/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index fbd1a051f0d8f2..41795d810a0cb1 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2236,8 +2236,8 @@ This example creates three paths of execution:
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
 
-When using `needs`, you manage artifacts downloads with the `artifacts`
-keyword, and should not use `dependencies`.
+When using `needs`, artifact downloads are controlled with the `artifacts`
+keyword, and should not use `dependencies` any more. This is deprecated since GitLab 12.6.
 
 For example:
 
-- 
GitLab


From 3c8e51452854d69cdcdd05aa6b1c89449ef82df9 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Fri, 22 Nov 2019 17:29:22 +0200
Subject: [PATCH 28/32] Update default value example

---
 doc/ci/yaml/README.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 41795d810a0cb1..8fd6ec2fc891ce 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2263,9 +2263,12 @@ rubocop:
 
 In the example, the `rspec` job will download the `build_job` artifacts, while the
 `rubocop` job will not.
+Additionally, in the three syntax examples below, the `rspec` job will download the artifacts
+from all three `build_jobs`, as `artifacts` is true for `build_job_1`, and will
+**default** to true for both `build_job_2` and `build_job_3`.
 
 ```yaml
-job:
+rspec:
   needs:
     - job: build_job_1
       artifacts: true
@@ -2273,9 +2276,6 @@ job:
     - build_job_3
 ```
 
-The job above will download the artifacts from all three jobs, as the artifacts
-will default to true for both `build_job_2` and `build_job_3`.
-
 Mixing `artifacts` with `dependencies` is not supported, and it can result in
 undesired behavior.
 
-- 
GitLab


From 13a80af388675eeaa5ad151c87f7f530865f1900 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Mon, 25 Nov 2019 10:50:21 +0200
Subject: [PATCH 29/32] Remove duplicate deprecation message

---
 doc/ci/yaml/README.md | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 8fd6ec2fc891ce..4e2b3f593c8a34 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2276,9 +2276,6 @@ rspec:
     - build_job_3
 ```
 
-Mixing `artifacts` with `dependencies` is not supported, and it can result in
-undesired behavior.
-
 ### `coverage`
 
 > [Introduced][ce-7447] in GitLab 8.17.
-- 
GitLab


From 56bf1bc70574e6c19d7ed7f47622586e5df05e80 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 26 Nov 2019 08:21:23 +0000
Subject: [PATCH 30/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 4e2b3f593c8a34..adde030e8e2a55 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2236,8 +2236,8 @@ This example creates three paths of execution:
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/14311) in GitLab v12.6.
 
-When using `needs`, artifact downloads are controlled with the `artifacts`
-keyword, and should not use `dependencies` any more. This is deprecated since GitLab 12.6.
+When using `needs`, artifact downloads are controlled with `artifacts: true` or `artifacts: false`.
+The `dependencies` keyword should not be used with `needs`, as this is deprecated since GitLab 12.6.
 
 For example:
 
-- 
GitLab


From a9f48bd62d800c551c4bc5be7d532bfb576a53f4 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 26 Nov 2019 08:22:06 +0000
Subject: [PATCH 31/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index adde030e8e2a55..9c507fe468390e 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2239,7 +2239,8 @@ This example creates three paths of execution:
 When using `needs`, artifact downloads are controlled with `artifacts: true` or `artifacts: false`.
 The `dependencies` keyword should not be used with `needs`, as this is deprecated since GitLab 12.6.
 
-For example:
+In the example below, the `rspec` job will download the `build_job` artifacts, while the
+`rubocop` job will not:
 
 ```yaml
 build_job:
-- 
GitLab


From baddbbcbe9fb79334daaa645b42b5e0f3c241d38 Mon Sep 17 00:00:00 2001
From: Marius Bobin <mbobin@gitlab.com>
Date: Tue, 26 Nov 2019 08:22:34 +0000
Subject: [PATCH 32/32] Apply suggestion to doc/ci/yaml/README.md

---
 doc/ci/yaml/README.md | 2 --
 1 file changed, 2 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 9c507fe468390e..8dd7e2f76f3eb8 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -2262,8 +2262,6 @@ rubocop:
       artifacts: false
 ```
 
-In the example, the `rspec` job will download the `build_job` artifacts, while the
-`rubocop` job will not.
 Additionally, in the three syntax examples below, the `rspec` job will download the artifacts
 from all three `build_jobs`, as `artifacts` is true for `build_job_1`, and will
 **default** to true for both `build_job_2` and `build_job_3`.
-- 
GitLab