Resolve "Create a new job to check that EE-specific lines are minimal"

.gitlab-ci.yml

@@ -459,6 +459,18 @@ ee-files-location-check:
     - branches
     - //@gitlab-org/gitlab-ee
 
+ee-specific-lines-check:
+  <<: *dedicated-runner
+  stage: test
+  before_script: []
+  cache: {}
+  retry: 0
+  script:
+    - scripts/ee-specific-lines-check
+  only:
+    - branches
+    - //@gitlab-org/gitlab-ee
+
 update-tests-metadata:
   <<: *tests-metadata-state
   <<: *only-canonical-masters

scripts/ee-files-location-check

 #!/usr/bin/env ruby
 
-WHITELIST = [
-  'CHANGELOG-EE.md',
-  'config/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4946
-  'doc/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4948#note_59945483
-  'qa/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4997#note_59764702
-  'scripts/*',
-  'spec/javascripts/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/3871
-  'vendor/assets/javascripts/jasmine-jquery.js'
-].freeze
+require_relative 'ee_specific_check/ee_specific_check'
 
-def run_git_command(cmd)
-  puts "=> Running `git #{cmd}`"
-  `git #{cmd}`
-end
+include EESpecificCheck # rubocop:disable Style/MixinUsage
 
-run_git_command("remote add canonical-ee https://gitlab.com/gitlab-org/gitlab-ee.git")
-run_git_command("remote add canonical-ce https://gitlab.com/gitlab-org/gitlab-ce.git")
-run_git_command("fetch canonical-ee master --quiet")
+base = find_compare_base
 
 new_files_in_this_branch_not_at_the_ee_top_level =
-  run_git_command("diff canonical-ee/master...HEAD --name-status --diff-filter=A -- ./ ':!ee' | cut -f2").lines.map(&:strip)
-
-ce_repo_url = ENV.fetch('CI_REPOSITORY_URL', 'https://gitlab.com/gitlab-org/gitlab-ce.git').sub('gitlab-ee', 'gitlab-ce')
-current_branch = ENV.fetch('CI_COMMIT_REF_NAME', `git rev-parse --abbrev-ref HEAD`).strip
-minimal_ce_branch_name = current_branch.sub(/(\Aee\-|\-ee\z)/, '')
-
-ls_remote_output = run_git_command("ls-remote #{ce_repo_url} \"*#{minimal_ce_branch_name}*\"")
-remote_to_fetch = 'canonical-ce'
-branch_to_fetch = 'master'
-
-if ls_remote_output.include?(minimal_ce_branch_name)
-  remote_to_fetch = ce_repo_url
-  branch_to_fetch = ls_remote_output.scan(%r{(?<=refs/heads/).+}).sort_by(&:size).first
-  puts
-  puts "💪 We found the branch '#{branch_to_fetch}' in the #{ce_repo_url} repository. We will fetch it."
-else
-  puts "⚠️ We did not find a branch that would match the current '#{current_branch}' branch in the #{ce_repo_url} repository. We will fetch 'master' instead."
-  puts "ℹ️ If you have a CE branch for the current branch, make sure that its name includes '#{minimal_ce_branch_name}'."
-end
-
-run_git_command("fetch #{remote_to_fetch} #{branch_to_fetch} --quiet")
+  run_git_command("diff #{base.ee_merge_base}...HEAD --name-status --diff-filter=A -- ./ ':!ee' | cut -f2").lines.map(&:strip)
 
 ee_specific_files_in_ce_master_not_at_the_ee_top_level =
-  run_git_command("diff FETCH_HEAD..HEAD --name-status --diff-filter=A -- ./ ':!ee' | cut -f2").lines.map(&:strip)
+  run_git_command("diff #{base.ce_updated_base}..HEAD --name-status --diff-filter=A -- ./ ':!ee' | cut -f2").lines.map(&:strip)
+
 new_ee_specific_files_not_at_the_ee_top_level =
   new_files_in_this_branch_not_at_the_ee_top_level & ee_specific_files_in_ce_master_not_at_the_ee_top_level
 
@@ -59,15 +27,11 @@ new_ee_specific_files_not_at_the_ee_top_level.each do |file|
 end
 
 if status.zero?
-  puts
-  puts "🎉 All good, congrats! 🎉"
+  say "🎉 All good, congrats! 🎉"
 end
 
-run_git_command("remote remove canonical-ee")
-run_git_command("remote remove canonical-ce")
+remove_remotes
 
-puts
-puts "ℹ️ For more information on the why and how of this job, see https://docs.gitlab.com/ee/development/ee_features.html#detection-of-ee-only-files"
-puts
+say "ℹ️ For more information on the why and how of this job, see https://docs.gitlab.com/ee/development/ee_features.html#detection-of-ee-only-files"
 
 exit(status)

scripts/ee-specific-lines-check

+#!/usr/bin/env ruby
+
+require_relative 'ee_specific_check/ee_specific_check'
+
+include EESpecificCheck # rubocop:disable Style/MixinUsage
+
+base = find_compare_base
+
+current_numstat = updated_diff_numstat(base.ce_merge_base, base.ee_merge_base)
+updated_numstat = updated_diff_numstat(base.ce_updated_base, 'HEAD')
+
+offenses = updated_numstat.select do |file, updated_delta|
+  current_delta = current_numstat[file]
+
+  more_lines =
+    current_delta &&
+    updated_delta > current_delta &&
+    WHITELIST.all? { |pattern| !Dir.glob(pattern).include?(file) }
+
+  # Don't complain if we're just adding one more line, which could be
+  # `prepend EE::Module` that we want.
+  more_lines && !(current_delta == 0 && updated_delta == 1)
+end
+
+if offenses.empty?
+  say "🎉 All good, congrats! 🎉"
+else
+  puts
+
+  offenses.each do |(file, delta)|
+    puts "* 💥 #{file} has #{delta - current_numstat[file]} more different lines than before! 💥"
+  end
+
+  say <<~MESSAGE
+    ℹ️ Consider using an EE module to add the features you want.
+    ℹ️ See this for detail: https://docs.gitlab.com/ee/development/ee_features.html#ee-features-based-on-ce-features
+  MESSAGE
+end
+
+remove_remotes
+
+say "ℹ️ For more information on why, see https://gitlab.com/gitlab-org/gitlab-ee/issues/2952"
+
+exit(offenses.size)

scripts/ee_specific_check/ee_specific_check.rb

+module EESpecificCheck
+  WHITELIST = [
+    'CHANGELOG-EE.md',
+    'config/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4946
+    'doc/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4948#note_59945483
+    'qa/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/4997#note_59764702
+    'scripts/**/*',
+    'spec/javascripts/**/*', # https://gitlab.com/gitlab-org/gitlab-ee/issues/3871
+    'vendor/assets/javascripts/jasmine-jquery.js',
+    '.gitlab-ci.yml'
+  ].freeze
+
+  CompareBase = Struct.new(:ce_merge_base, :ee_merge_base, :ce_updated_base)
+
+  module_function
+
+  def say(message)
+    puts "\n#{message}", "\n" # puts would eat trailing newline
+  end
+
+  def find_compare_base
+    setup_canonical_remotes
+
+    fetch_head_ce = fetch_remote_ce_branch
+    ce_merge_base = run_git_command("merge-base #{fetch_head_ce} HEAD").strip
+    ee_merge_base = run_git_command("merge-base canonical-ee/master HEAD").strip
+
+    ce_updated_base =
+      if fetch_head_ce.start_with?('canonical-ce')
+        ce_merge_base # Compare with merge-base if no specific CE branch
+      else
+        fetch_head_ce # Compare with the new HEAD if we do have one
+      end
+
+    CompareBase.new(ce_merge_base, ee_merge_base, ce_updated_base)
+  end
+
+  def setup_canonical_remotes
+    run_git_command(
+      "remote add canonical-ee https://gitlab.com/gitlab-org/gitlab-ee.git",
+      "remote add canonical-ce https://gitlab.com/gitlab-org/gitlab-ce.git")
+
+    run_git_command("fetch canonical-ee master --quiet")
+  end
+
+  def fetch_remote_ce_branch
+    remote_to_fetch, branch_to_fetch = find_remote_ce_branch
+
+    run_git_command("fetch #{remote_to_fetch} #{branch_to_fetch} --quiet")
+
+    "#{remote_to_fetch}/#{branch_to_fetch}"
+  end
+
+  def remove_remotes
+    run_git_command(
+      "remote remove canonical-ee",
+      "remote remove canonical-ce",
+      "remote remove target-ce")
+  end
+
+  def updated_diff_numstat(from, to)
+    scan_diff_numstat(
+      run_git_command("diff #{from}..#{to} --numstat -- . '!ee'"))
+  end
+
+  def find_remote_ce_branch
+    ls_remote_output = run_git_command("ls-remote #{ce_repo_url} \"*#{minimal_ce_branch_name}*\"")
+
+    if ls_remote_output.include?(minimal_ce_branch_name)
+      branch_to_fetch = ls_remote_output.scan(%r{(?<=refs/heads/).+}).sort_by(&:size).first
+
+      say "💪 We found the branch '#{branch_to_fetch}' in the #{ce_repo_url} repository. We will fetch it."
+
+      run_git_command("remote add target-ce #{ce_repo_url}")
+
+      ['target-ce', branch_to_fetch]
+    else
+      say <<~MESSAGE
+        ⚠️ We did not find a branch that would match the current '#{current_branch}' branch in the #{ce_repo_url} repository. We will fetch 'master' instead.
+        ℹ️ If you have a CE branch for the current branch, make sure that its name includes '#{minimal_ce_branch_name}'.
+      MESSAGE
+
+      %w[canonical-ce master]
+    end
+  end
+
+  def ce_repo_url
+    @ce_repo_url ||= ENV.fetch('CI_REPOSITORY_URL', 'https://gitlab.com/gitlab-org/gitlab-ce.git').sub('gitlab-ee', 'gitlab-ce')
+  end
+
+  def current_branch
+    @current_branch ||= ENV.fetch('CI_COMMIT_REF_NAME', `git rev-parse --abbrev-ref HEAD`).strip
+  end
+
+  def minimal_ce_branch_name
+    @minimal_ce_branch_name ||= current_branch.sub(/(\Aee\-|\-ee\z)/, '')
+  end
+
+  def scan_diff_numstat(numstat)
+    numstat.scan(/(\d+)\s+(\d+)\s+(.+)/)
+      .each_with_object({}) do |(added, deleted, file), result|
+        result[file] = added.to_i + deleted.to_i
+      end
+  end
+
+  def run_git_command(*commands)
+    cmds = commands.map { |cmd| "git #{cmd}" }
+
+    output = run_command(*cmds)
+
+    if commands.size == 1
+      output.first
+    else
+      output
+    end
+  end
+
+  def run_command(*commands)
+    commands.map do |cmd|
+      puts "=> Running `#{cmd}`"
+
+      `#{cmd}`
+    end
+  end
+end
+
+if $0 == __FILE__
+  require 'rspec/autorun'
+
+  RSpec.describe EESpecificCheck do
+    include EESpecificCheck
+
+    before do
+      allow(self).to receive(:puts)
+    end
+
+    describe '.run_git_command' do
+      it 'returns the single output when there is a single command' do
+        output = run_git_command('status')
+
+        expect(output).to be_kind_of(String)
+        expect(self).to have_received(:puts).with(/git status/)
+      end
+
+      it 'returns an array of output for more commands' do
+        output = run_git_command('status', 'help')
+
+        expect(output).to all(be_a(String))
+        expect(self).to have_received(:puts).with(/git status/)
+        expect(self).to have_received(:puts).with(/git help/)
+      end
+    end
+  end
+end