Bug 1641184, use only the merge dir for repacks, r=nalexander
authorAxel Hecht <axel@pike.org>
Wed, 27 May 2020 17:04:06 +0000
changeset 532684 9c9f8c859ff332396cacb97e55be9b65854a09ba
parent 532683 bb1cee2cf6c560b801509e8e3215113024e83e5a
child 532685 d7f46110bc8b1de57b95f748b556dc4faacf8302
push id37457
push usernerli@mozilla.com
push dateThu, 28 May 2020 15:51:15 +0000
treeherdermozilla-central@272e3c98d002 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1641184
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1641184, use only the merge dir for repacks, r=nalexander l10n-merge creates a full merge dir for a while now, let's simplify the build logic to only read from that directory during repacks and langpacks. Differential Revision: https://phabricator.services.mozilla.com/D77023
browser/build.mk
build/docs/locales.rst
config/config.mk
python/mozbuild/mozbuild/jar.py
python/mozbuild/mozbuild/test/test_jarmaker.py
--- a/browser/build.mk
+++ b/browser/build.mk
@@ -30,20 +30,18 @@ source-upload::
 	@$(MAKE) -C browser/installer source-upload
 
 hg-bundle::
 	@$(MAKE) -C browser/installer hg-bundle
 
 wget-en-US:
 	@$(MAKE) -C browser/locales $@
 
-# make -j1 because dependencies in l10n build targets don't work
-# with parallel builds
 merge-% installers-% langpack-% chrome-%:
-	$(MAKE) -j1 -C browser/locales $@
+	$(MAKE) -C browser/locales $@
 
 ifdef ENABLE_TESTS
 # Implemented in testing/testsuite-targets.mk
 
 mochitest-browser-chrome:
 	$(RUN_MOCHITEST) --flavor=browser
 	$(CHECK_TEST_ERROR)
 
--- a/build/docs/locales.rst
+++ b/build/docs/locales.rst
@@ -294,26 +294,19 @@ compare against.
 
 The process can be manually triggered via
 
 .. code-block:: bash
 
     $> ./mach build merge-$AB_CD
 
 It creates another directory in the object dir, :file:`browser/locales/merge-dir/$AB_CD`, in
-which the modified files are stored. The actual repackaging process looks for
-the localized files in the merge dir first, then the localized file, and then
-in ``en-US``. Thus, for the ``de`` localization of
-:file:`browser/locales/en-US/chrome/browser/browser.dtd`, it checks
-
-1. :file:`$objdir/browser/locales/merge-de/browser/chrome/browser/browser.dtd`
-2. :file:`$(LOCALE_BASEDIR)/de/browser/chrome/browser/browser.dtd`
-3. :file:`browser/locales/en-US/chrome/browser/browser.dtd`
-
-and will include the first of those files it finds.
+which the sanitized files are stored. The actual repackaging process only looks
+in the merged directory, so the preparation steps of l10n-merge need to ensure
+that all files are generated or copied.
 
 l10n-merge modifies a file if it supports the particular file type, and there
 are missing strings which are not filtered out, or if an existing string
 shows an error. See the Checks section below for details. If the files are
 not modified, l10n-merge copies them over to the respective location in the
 merge dir.
 
 Checks
--- a/config/config.mk
+++ b/config/config.mk
@@ -367,62 +367,41 @@ sysinstall_cmd = install_cmd
 # overridden by the command line. (Besides, AB_CD is prettier).
 AB_CD = $(MOZ_UI_LOCALE)
 
 include $(MOZILLA_DIR)/config/AB_rCD.mk
 
 # Many locales directories want this definition.
 ACDEFINES += -DAB_CD=$(AB_CD)
 
-EXPAND_LOCALE_SRCDIR = $(if $(filter en-US,$(AB_CD)),$(LOCALE_TOPDIR)/$(1)/en-US,$(or $(realpath $(L10NBASEDIR)),$(abspath $(L10NBASEDIR)))/$(AB_CD)/$(subst /locales,,$(1)))
+EXPAND_LOCALE_SRCDIR = $(if $(filter en-US,$(AB_CD)),$(LOCALE_TOPDIR)/$(1)/en-US,$(REAL_LOCALE_MERGEDIR)/$(subst /locales,,$(1)))
 
 ifdef relativesrcdir
 LOCALE_RELATIVEDIR ?= $(relativesrcdir)
 endif
 
 ifdef LOCALE_RELATIVEDIR
 LOCALE_SRCDIR ?= $(call EXPAND_LOCALE_SRCDIR,$(LOCALE_RELATIVEDIR))
 endif
 
 ifdef relativesrcdir
 MAKE_JARS_FLAGS += --relativesrcdir=$(LOCALE_RELATIVEDIR)
 ifneq (en-US,$(AB_CD))
 ifdef IS_LANGUAGE_REPACK
-MAKE_JARS_FLAGS += --locale-mergedir=$(REAL_LOCALE_MERGEDIR)
-endif
-ifdef IS_LANGUAGE_REPACK
-MAKE_JARS_FLAGS += --l10n-base=$(L10NBASEDIR)/$(AB_CD)
+MAKE_JARS_FLAGS += --l10n-base=$(REAL_LOCALE_MERGEDIR)
 endif
 else
 MAKE_JARS_FLAGS += -c $(LOCALE_SRCDIR)
 endif # en-US
 else
 MAKE_JARS_FLAGS += -c $(LOCALE_SRCDIR)
 endif # ! relativesrcdir
 
-ifdef IS_LANGUAGE_REPACK
-MERGE_FILE = $(firstword \
-  $(wildcard $(REAL_LOCALE_MERGEDIR)/$(subst /locales,,$(LOCALE_RELATIVEDIR))/$(1)) \
-  $(wildcard $(LOCALE_SRCDIR)/$(1)) \
-  $(srcdir)/en-US/$(1) )
-# Like MERGE_FILE, but with the specified relative source directory
-# $(2) replacing $(srcdir).  It's expected that $(2) will include
-# '/locales' but not '/locales/en-US'.
-#
-# MERGE_RELATIVE_FILE and MERGE_FILE could be -- ahem -- merged by
-# making the second argument optional, but that expression makes for
-# difficult to read Make.
-MERGE_RELATIVE_FILE = $(firstword \
-  $(wildcard $(REAL_LOCALE_MERGEDIR)/$(subst /locales,,$(2))/$(1)) \
-  $(wildcard $(call EXPAND_LOCALE_SRCDIR,$(2))/$(1)) \
-  $(topsrcdir)/$(2)/en-US/$(1) )
-else
 MERGE_FILE = $(LOCALE_SRCDIR)/$(1)
 MERGE_RELATIVE_FILE = $(call EXPAND_LOCALE_SRCDIR,$(2))/$(1)
-endif
 
 ifneq (WINNT,$(OS_ARCH))
 RUN_TEST_PROGRAM = $(DIST)/bin/run-mozilla.sh
 endif # ! WINNT
 
 # autoconf.mk sets OBJ_SUFFIX to an error to avoid use before including
 # this file
 OBJ_SUFFIX := $(_OBJ_SUFFIX)
--- a/python/mozbuild/mozbuild/jar.py
+++ b/python/mozbuild/mozbuild/jar.py
@@ -206,17 +206,16 @@ class JarMaker(object):
         self.outputFormat = outputFormat
         self.useJarfileManifest = useJarfileManifest
         self.useChromeManifest = useChromeManifest
         self.pp = Preprocessor()
         self.topsourcedir = None
         self.sourcedirs = []
         self.localedirs = None
         self.l10nbase = None
-        self.l10nmerge = None
         self.relativesrcdir = None
         self.rootManifestAppId = None
         self._seen_output = set()
 
     def getCommandLineParser(self):
         '''Get a optparse.OptionParser for jarmaker.
 
         This OptionParser has the options for jarmaker as well as
@@ -240,21 +239,17 @@ class JarMaker(object):
                      help='create chrome.manifest instead of jarfile.manifest'
                      )
         p.add_option('-s', type='string', action='append', default=[],
                      help='source directory')
         p.add_option('-t', type='string', help='top source directory')
         p.add_option('-c', '--l10n-src', type='string',
                      action='append', help='localization directory')
         p.add_option('--l10n-base', type='string', action='store',
-                     help='base directory to be used for localization (requires relativesrcdir)'
-                     )
-        p.add_option('--locale-mergedir', type='string', action='store',
-                     help='base directory to be used for l10n-merge '
-                     '(requires l10n-base and relativesrcdir)'
+                     help='merged directory to be used for localization (requires relativesrcdir)'
                      )
         p.add_option('--relativesrcdir', type='string',
                      help='relativesrcdir to be used for localization')
         p.add_option('-d', type='string', help='base directory')
         p.add_option('--root-manifest-entry-appid', type='string',
                      help='add an app id specific root chrome manifest entry.'
                      )
         return p
@@ -339,23 +334,21 @@ class JarMaker(object):
     def generateLocaleDirs(self, relativesrcdir):
         if os.path.basename(relativesrcdir) == 'locales':
             # strip locales
             l10nrelsrcdir = os.path.dirname(relativesrcdir)
         else:
             l10nrelsrcdir = relativesrcdir
         locdirs = []
 
-        # generate locales dirs, merge, l10nbase, en-US
-        if self.l10nmerge:
-            locdirs.append(os.path.join(self.l10nmerge, l10nrelsrcdir))
+        # generate locales merge or en-US
         if self.l10nbase:
             locdirs.append(os.path.join(self.l10nbase, l10nrelsrcdir))
-        if self.l10nmerge or not self.l10nbase:
-            # add en-US if we merge, or if it's not l10n
+        else:
+            # add en-US if it's not l10n
             locdirs.append(os.path.join(self.topsourcedir,
                                         relativesrcdir, 'en-US'))
         return locdirs
 
     def processJarSection(self, jarinfo, jardir):
         '''Internal method called by makeJar to actually process a section
         of a jar.mn file.
         '''
@@ -584,22 +577,16 @@ def main(args=None):
         jm.useJarfileManifest = False
     if options.l10n_base:
         if not options.relativesrcdir:
             p.error('relativesrcdir required when using l10n-base')
         if options.l10n_src:
             p.error('both l10n-src and l10n-base are not supported')
         jm.l10nbase = options.l10n_base
         jm.relativesrcdir = options.relativesrcdir
-        jm.l10nmerge = options.locale_mergedir
-        if jm.l10nmerge and not os.path.isdir(jm.l10nmerge):
-            logging.warning("WARNING: --locale-mergedir passed, but '%s' does not exist. "
-                            "Ignore this message if the locale is complete." % jm.l10nmerge)
-    elif options.locale_mergedir:
-        p.error('l10n-base required when using locale-mergedir')
     jm.localedirs = options.l10n_src
     if options.root_manifest_entry_appid:
         jm.rootManifestAppId = options.root_manifest_entry_appid
     noise = logging.INFO
     if options.verbose is not None:
         noise = options.verbose and logging.DEBUG or logging.WARN
     if sys.version_info[:2] > (2, 3):
         logging.basicConfig(format='%(message)s')
--- a/python/mozbuild/mozbuild/test/test_jarmaker.py
+++ b/python/mozbuild/mozbuild/test/test_jarmaker.py
@@ -340,24 +340,20 @@ class Test_relativesrcdir(unittest.TestC
     def test_l10n_no_merge(self):
         jm = self.jm
         jm.l10nbase = '/L10N_BASE'
         jm.makeJar(self.fake_empty_file, '/NO_OUTPUT_REQUIRED')
         self.assertEquals(jm.localedirs, [os.path.join('/L10N_BASE', 'browser')])
 
     def test_l10n_merge(self):
         jm = self.jm
-        jm.l10nbase = '/L10N_BASE'
-        jm.l10nmerge = '/L10N_MERGE'
+        jm.l10nbase = '/L10N_MERGE'
         jm.makeJar(self.fake_empty_file, '/NO_OUTPUT_REQUIRED')
         self.assertEquals(jm.localedirs,
                           [os.path.join('/L10N_MERGE', 'browser'),
-                           os.path.join('/L10N_BASE', 'browser'),
-                           os.path.join(os.path.abspath('/TOPSOURCEDIR'),
-                                        'browser/locales', 'en-US')
                            ])
 
     def test_override(self):
         jm = self.jm
         jm.outputFormat = 'flat'  # doesn't touch chrome dir without files
         jarcontents = StringIO('''en-US.jar:
 relativesrcdir dom/locales:
 ''')