Remove empty locale folders in gettext:regenerate

When a `locale/*/gitlab.po` file is removed, the folder (*) will remain
on local dev machines as the folder still contains git ignored files.
This slows down the `gettext:regenerate` command, as the task looks for
the folders to determine which `gitlab.po` files need to be generated.

Add a step to remove these 'empty' folders before starting to regenerate
translations.

Add some specs for the gettext rake tasks.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/238208

lib/tasks/gettext.rake

+# frozen_string_literal: true
+
 require "gettext_i18n_rails/tasks"
 
 namespace :gettext do
-  # Customize list of translatable files
-  # See: https://github.com/grosser/gettext_i18n_rails#customizing-list-of-translatable-files
-  def files_to_translate
-    folders = %W(ee app lib config #{locale_path}).join(',')
-    exts = %w(rb erb haml slim rhtml js jsx vue handlebars hbs mustache).join(',')
-
-    Dir.glob(
-      "{#{folders}}/**/*.{#{exts}}"
-    )
-  end
-
-  # Disallow HTML from translatable strings
-  # See: https://docs.gitlab.com/ee/development/i18n/externalization.html#html
-  def html_todolist
-    return @html_todolist if defined?(@html_todolist)
-
-    @html_todolist = YAML.load_file(Rails.root.join('lib/gitlab/i18n/html_todo.yml'))
-  end
-
   task :compile do
     # See: https://gitlab.com/gitlab-org/gitlab-foss/issues/33014#note_31218998
-    FileUtils.touch(File.join(Rails.root, 'locale/gitlab.pot'))
+    FileUtils.touch(pot_file_path)
 
     Rake::Task['gettext:po_to_json'].invoke
   end
 
   desc 'Regenerate gitlab.pot file'
   task :regenerate do
-    pot_file = 'locale/gitlab.pot'
-    # Remove all translated files, this speeds up finding
-    FileUtils.rm Dir['locale/**/gitlab.*']
+    ensure_locale_folder_presence!
+
+    # Clean up folders that do not contain a gitlab.po file
+    Pathname.new(locale_path).children.each do |child|
+      next unless child.directory?
+
+      folder_path = child.to_path
+
+      if File.exist?("#{folder_path}/gitlab.po")
+        # remove all translated files to speed up finding
+        FileUtils.rm Dir["#{folder_path}/gitlab.*"]
+      else
+        # remove empty translation folders so we don't generate un-needed .po files
+        puts "Deleting #{folder_path} as it does not contain a 'gitlab.po' file."
+
+        FileUtils.rm_r folder_path
+      end
+    end
+
     # remove the `pot` file to ensure it's completely regenerated
-    FileUtils.rm_f pot_file
+    FileUtils.rm_f(pot_file_path)
 
     Rake::Task['gettext:find'].invoke
 
@@ -42,10 +41,12 @@ namespace :gettext do
       raise 'failed to cleanup generated locale/*/gitlab.po files'
     end
 
+    raise 'gitlab.pot file not generated' unless File.exist?(pot_file_path)
+
     # Remove timestamps from the pot file
-    pot_content = File.read pot_file
+    pot_content = File.read pot_file_path
     pot_content.gsub!(/^"POT?\-(?:Creation|Revision)\-Date\:.*\n/, '')
-    File.write pot_file, pot_content
+    File.write pot_file_path, pot_content
 
     puts <<~MSG
     All done. Please commit the changes to `locale/gitlab.pot`.
@@ -67,8 +68,7 @@ namespace :gettext do
       Gitlab::I18n::PoLinter.new(po_path: file, html_todolist: html_todolist, locale: locale)
     end
 
-    pot_file = Rails.root.join('locale/gitlab.pot')
-    linters.unshift(Gitlab::I18n::PoLinter.new(po_path: pot_file, html_todolist: html_todolist))
+    linters.unshift(Gitlab::I18n::PoLinter.new(po_path: pot_file_path, html_todolist: html_todolist))
 
     failed_linters = linters.select { |linter| linter.errors.any? }
 
@@ -84,12 +84,11 @@ namespace :gettext do
   end
 
   task :updated_check do
-    pot_file = 'locale/gitlab.pot'
     # Removing all pre-translated files speeds up `gettext:find` as the
     # files don't need to be merged.
     # Having `LC_MESSAGES/gitlab.mo files present also confuses the output.
     FileUtils.rm Dir['locale/**/gitlab.*']
-    FileUtils.rm_f pot_file
+    FileUtils.rm_f pot_file_path
 
     # `gettext:find` writes touches to temp files to `stderr` which would cause
     # `static-analysis` to report failures. We can ignore these.
@@ -97,18 +96,18 @@ namespace :gettext do
       Rake::Task['gettext:find'].invoke
     end
 
-    pot_diff = `git diff -- #{pot_file} | grep -E '^(\\+|-)msgid'`.strip
+    pot_diff = `git diff -- #{pot_file_path} | grep -E '^(\\+|-)msgid'`.strip
 
     # reset the locale folder for potential next tasks
     `git checkout -- locale`
 
     if pot_diff.present?
       raise <<~MSG
-        Changes in translated strings found, please update file `#{pot_file}` by running:
+        Changes in translated strings found, please update file `#{pot_file_path}` by running:
 
           bin/rake gettext:regenerate
 
-        Then commit and push the resulting changes to `#{pot_file}`.
+        Then commit and push the resulting changes to `#{pot_file_path}`.
 
         The diff was:
 
@@ -117,6 +116,27 @@ namespace :gettext do
     end
   end
 
+  private
+
+  # Customize list of translatable files
+  # See: https://github.com/grosser/gettext_i18n_rails#customizing-list-of-translatable-files
+  def files_to_translate
+    folders = %W(ee app lib config #{locale_path}).join(',')
+    exts = %w(rb erb haml slim rhtml js jsx vue handlebars hbs mustache).join(',')
+
+    Dir.glob(
+      "{#{folders}}/**/*.{#{exts}}"
+    )
+  end
+
+  # Disallow HTML from translatable strings
+  # See: https://docs.gitlab.com/ee/development/i18n/externalization.html#html
+  def html_todolist
+    return @html_todolist if defined?(@html_todolist)
+
+    @html_todolist = YAML.safe_load(File.read(Rails.root.join('lib/gitlab/i18n/html_todo.yml')))
+  end
+
   def report_errors_for_file(file, errors_for_file)
     puts "Errors in `#{file}`:"
 
@@ -140,4 +160,21 @@ namespace :gettext do
     $stderr.reopen(old_stderr)
     old_stderr.close
   end
+
+  def ensure_locale_folder_presence!
+    unless Dir.exist?(locale_path)
+      raise <<~MSG
+      Cannot find '#{locale_path}' folder. Please ensure you're running this task from the gitlab repo.
+
+      MSG
+    end
+  end
+
+  def locale_path
+    @locale_path ||= Rails.root.join('locale')
+  end
+
+  def pot_file_path
+    @pot_file_path ||= File.join(locale_path, 'gitlab.pot')
+  end
 end

spec/fixtures/valid.po

@@ -8,6 +8,7 @@ msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
 "PO-Revision-Date: 2017-07-13 12:10-0500\n"
+"PO-Creation-Date: 2016-07-13 12:11-0500\n"
 "Language-Team: Spanish\n"
 "Language: es\n"
 "MIME-Version: 1.0\n"

spec/tasks/gettext_rake_spec.rb

+# frozen_string_literal: true
+
+require "rake_helper"
+
+RSpec.describe 'gettext' do
+  let(:locale_path) { Rails.root.join('tmp/gettext_spec') }
+  let(:pot_file_path) { File.join(locale_path, 'gitlab.pot') }
+
+  before do
+    Rake.application.rake_require('tasks/gettext')
+
+    FileUtils.rm_r(locale_path) if Dir.exist?(locale_path)
+    FileUtils.mkdir_p(locale_path)
+
+    allow(Rails.root).to receive(:join).and_call_original
+    allow(Rails.root).to receive(:join).with('locale').and_return(locale_path)
+  end
+
+  after do
+    FileUtils.rm_r(locale_path) if Dir.exist?(locale_path)
+  end
+
+  describe ':compile' do
+    before do
+      allow(Rake::Task).to receive(:[]).and_call_original
+    end
+
+    it 'creates a pot file and invokes the \'gettext:po_to_json\' task' do
+      expect(Rake::Task).to receive(:[]).with('gettext:po_to_json').and_return(double(invoke: true))
+
+      expect { run_rake_task('gettext:compile') }
+        .to change { File.exist?(pot_file_path) }
+        .to be_truthy
+    end
+  end
+
+  describe ':regenerate' do
+    before do
+      # this task takes a *really* long time to complete, so stub it for the spec
+      allow(Rake::Task['gettext:find']).to receive(:invoke) { invoke_find.call }
+    end
+
+    context 'when the locale folder is not found' do
+      let(:invoke_find) { -> { true } }
+
+      before do
+        FileUtils.rm_r(locale_path) if Dir.exist?(locale_path)
+      end
+
+      it 'raises an error' do
+        expect { run_rake_task('gettext:regenerate') }
+          .to raise_error(/Cannot find '#{locale_path}' folder/)
+      end
+    end
+
+    context 'where there are existing /**/gitlab.po files' do
+      let(:locale_nz_path) { File.join(locale_path, 'en_NZ') }
+      let(:po_file_path) { File.join(locale_nz_path, 'gitlab.po') }
+
+      let(:invoke_find) { -> { File.write pot_file_path, 'pot file test updates' } }
+
+      before do
+        FileUtils.mkdir(locale_nz_path)
+        File.write(po_file_path, fixture_file('valid.po'))
+      end
+
+      it 'does not remove that locale' do
+        expect { run_rake_task('gettext:regenerate') }
+          .not_to change { Dir.exist?(locale_nz_path) }
+      end
+    end
+
+    context 'when there are locale folders without a gitlab.po file' do
+      let(:empty_locale_path) { File.join(locale_path, 'en_NZ') }
+
+      let(:invoke_find) { -> { File.write pot_file_path, 'pot file test updates' } }
+
+      before do
+        FileUtils.mkdir(empty_locale_path)
+      end
+
+      it 'removes those folders' do
+        expect { run_rake_task('gettext:regenerate') }
+          .to change { Dir.exist?(empty_locale_path) }
+          .to eq false
+      end
+    end
+
+    context 'when the gitlab.pot file cannot be generated' do
+      let(:invoke_find) { -> { true } }
+
+      it 'prints an error' do
+        expect { run_rake_task('gettext:regenerate') }
+          .to raise_error(/gitlab.pot file not generated/)
+      end
+    end
+
+    context 'when gettext:find changes the revision dates' do
+      let(:invoke_find) { -> { File.write pot_file_path, fixture_file('valid.po') } }
+
+      before do
+        File.write pot_file_path, fixture_file('valid.po')
+      end
+
+      it 'resets the changes' do
+        pot_file = File.read(pot_file_path)
+        expect(pot_file).to include('PO-Revision-Date: 2017-07-13 12:10-0500')
+        expect(pot_file).to include('PO-Creation-Date: 2016-07-13 12:11-0500')
+
+        run_rake_task('gettext:regenerate')
+
+        pot_file = File.read(pot_file_path)
+        expect(pot_file).not_to include('PO-Revision-Date: 2017-07-13 12:10-0500')
+        expect(pot_file).not_to include('PO-Creation-Date: 2016-07-13 12:11-0500')
+      end
+    end
+  end
+
+  describe ':lint' do
+    before do
+      # make sure we test on the fixture files, not the actual gitlab repo as
+      # this takes a long time
+      allow(Rails.root)
+        .to receive(:join)
+        .with('locale/*/gitlab.po')
+        .and_return(File.join(locale_path, '*/gitlab.po'))
+    end
+
+    context 'when all PO files are valid' do
+      before do
+        nz_locale_path = File.join(locale_path, 'en_NZ')
+        FileUtils.mkdir(nz_locale_path)
+
+        po_file_path = File.join(nz_locale_path, 'gitlab.po')
+        File.write(po_file_path, fixture_file('valid.po'))
+        File.write(pot_file_path, fixture_file('valid.po'))
+      end
+
+      it 'completes without error' do
+        expect { run_rake_task('gettext:lint') }
+          .not_to raise_error
+      end
+    end
+
+    context 'when there are invalid PO files' do
+      before do
+        nz_locale_path = File.join(locale_path, 'en_NZ')
+        FileUtils.mkdir(nz_locale_path)
+
+        po_file_path = File.join(nz_locale_path, 'gitlab.po')
+        File.write(po_file_path, fixture_file('invalid.po'))
+        File.write(pot_file_path, fixture_file('valid.po'))
+      end
+
+      it 'raises an error' do
+        expect { run_rake_task('gettext:lint') }
+          .to raise_error(/Not all PO-files are valid/)
+      end
+    end
+
+    context 'when the .pot file is invalid' do
+      before do
+        nz_locale_path = File.join(locale_path, 'en_NZ')
+        FileUtils.mkdir(nz_locale_path)
+
+        po_file_path = File.join(nz_locale_path, 'gitlab.po')
+        File.write(po_file_path, fixture_file('valid.po'))
+        File.write(pot_file_path, fixture_file('invalid.po'))
+      end
+
+      it 'raises an error' do
+        expect { run_rake_task('gettext:lint') }
+          .to raise_error(/Not all PO-files are valid/)
+      end
+    end
+  end
+end