From 6fe6c876c4b3ab37fde2280363eb17fbf958c877 Mon Sep 17 00:00:00 2001 From: mkarolin Date: Fri, 31 Jul 2020 11:43:56 -0400 Subject: [PATCH] Fixes l10nUtil to not run when unneeded. Fixes #7800 Replaces brave/brave-browser#8471 (after scripts moved from b-b to b-c). Moved all top level const values consumed by other scripts into functions. --- build/commands/lib/chromiumRebaseL10n.js | 8 +- build/commands/lib/l10nUtil.js | 289 +++++++++++++---------- build/commands/lib/pullL10n.js | 8 +- build/commands/lib/pushL10n.js | 8 +- build/commands/lib/util.js | 3 +- 5 files changed, 173 insertions(+), 143 deletions(-) diff --git a/build/commands/lib/chromiumRebaseL10n.js b/build/commands/lib/chromiumRebaseL10n.js index ccb0b37eb0b..e228c7bb7ba 100644 --- a/build/commands/lib/chromiumRebaseL10n.js +++ b/build/commands/lib/chromiumRebaseL10n.js @@ -5,7 +5,7 @@ const path = require('path') const config = require('../lib/config') const util = require('../lib/util') -const {rebaseBraveStringFilesOnChromiumL10nFiles, braveAutoGeneratedPaths, logRemovedGRDParts} = require('./l10nUtil') +const l10nUtil = require('./l10nUtil') const resetChromeStringFiles = () => { // Revert to originals before string replacement because original grd(p)s are @@ -19,13 +19,13 @@ const resetChromeStringFiles = () => { const chromiumRebaseL10n = async (options) => { resetChromeStringFiles() - const removed = await rebaseBraveStringFilesOnChromiumL10nFiles() - braveAutoGeneratedPaths.forEach((sourceStringPath) => { + const removed = await l10nUtil.rebaseBraveStringFilesOnChromiumL10nFiles() + l10nUtil.getBraveAutoGeneratedPaths().forEach((sourceStringPath) => { const cmdOptions = config.defaultOptions cmdOptions.cwd = config.braveCoreDir util.run('python', ['script/chromium-rebase-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) - logRemovedGRDParts(removed) + l10nUtil.logRemovedGRDParts(removed) } module.exports = chromiumRebaseL10n diff --git a/build/commands/lib/l10nUtil.js b/build/commands/lib/l10nUtil.js index 4a0eb62260b..adc953b12f2 100644 --- a/build/commands/lib/l10nUtil.js +++ b/build/commands/lib/l10nUtil.js @@ -59,8 +59,6 @@ const braveExtensionMessagesPath = path.resolve(path.join(srcDir, 'brave', 'comp const braveRewardsExtensionMessagesPath = path.resolve(path.join(srcDir, 'brave', 'components', 'brave_rewards', 'resources', 'extension', 'brave_rewards', '_locales', 'en_US', 'messages.json')) const braveAndroidBraveStringsPath = path.resolve(path.join(srcDir, 'brave', 'browser', 'ui', 'android', 'strings', 'android_brave_strings.grd')) -const srcGit = path.resolve(path.join(srcDir, '.git')) - // Helper function to find all grdp parts in a grd. function getGrdPartsFromGrd(path) { const grd = new JSDOM(fs.readFileSync(path, 'utf8')) @@ -73,21 +71,17 @@ function getGrdPartsFromGrd(path) { } // Helper function to create a mapping for grd and all of its grdp parts. -function AddGrd(chromiumPath, bravePath, exclude = new Set()) { - if (!fs.existsSync(srcGit)) { - // Chromium repository has not been initialized yet. - return - } +function addGrd(chromiumPath, bravePath, exclude = new Set()) { if (verboseLogFindGrd) console.log("Adding mappings for GRD: " + chromiumPath) + if (!fs.existsSync(chromiumPath)) { + const err = new Error(`addGrd: Error. File not found at path "${chromiumPath}"`) + console.error(err) + throw err + } let mapping = { [chromiumPath]: bravePath } - if (!fs.existsSync(chromiumPath)) { - const err = new Error(`AddGrd: Error. File not found at path "${chromiumPath}"`) - console.error(err) - return - } const grdps = getGrdPartsFromGrd(chromiumPath) if (grdps.length) { const chromiumDir = path.dirname(chromiumPath) @@ -104,102 +98,6 @@ function AddGrd(chromiumPath, bravePath, exclude = new Set()) { return mapping } -// Add all GRD mappings here. -// Brave specific only grd and grdp files should NOT be added. -// Using AddGrd will add GRD and all of its GRDPs. -// TODO(petemill): Do not do this file processing in the module root, do it behind a function. -console.log(chalk.italic('Recursing through GRD to find GRDP files...')) -const grdsWithAutoAddedGrdps = { - ...AddGrd(chromiumComponentsStringsPath, braveComponentsStringsPath), - ...AddGrd(chromiumGeneratedResourcesPath, braveGeneratedResourcesPath, chromiumGeneratedResourcesExcludes), - ...AddGrd(androidChromeStringsPath, braveAndroidChromeStringsPath) -} -console.log(chalk.italic('Done recursing through GRD to find GRDP files.')) - -// When adding new grd or grdp files, never add a grdp part path without a parent grd path. -// Group them with a leading and trailing newline to keep this file organized. -// The first 3 are added explicitly because we change the file names. -const chromiumToAutoGeneratedBraveMapping = { - [chromiumStringsPath]: braveStringsPath, - [chromiumSettingsPartPath]: braveSettingsPartPath, - - [chromiumComponentsChromiumStringsPath]: braveComponentsBraveStringsPath, - - ...grdsWithAutoAddedGrdps -} - -// Same as with chromiumToAutoGeneratedBraveMapping but maps in the opposite direction -module.exports.autoGeneratedBraveToChromiumMapping = Object.keys(chromiumToAutoGeneratedBraveMapping) - .reduce((obj, key) => ({ ...obj, [chromiumToAutoGeneratedBraveMapping[key]]: key }), {}) - -// All paths which are not generated -module.exports.braveNonGeneratedPaths = [ - braveSpecificGeneratedResourcesPath, braveResourcesComponentsStringsPath, braveExtensionMessagesPath, braveRewardsExtensionMessagesPath, braveAndroidBraveStringsPath -] - -// All paths which are generated -module.exports.braveAutoGeneratedPaths = Object.values(chromiumToAutoGeneratedBraveMapping) - -// Brave specific strings and Chromium mapped Brave strings will be here. -// But you only need to add the Brave specific strings manually here. -module.exports.allBravePaths = module.exports.braveNonGeneratedPaths.concat(module.exports.braveAutoGeneratedPaths) - -// Get all GRD and JSON paths whether they are generatd or not -// Push and pull scripts for l10n use this. -// Transifex manages files per grd and not per grd or grdp. -// This is because only 1 xtb is created per grd per locale even if it has multiple grdp files. -module.exports.braveTopLevelPaths = module.exports.allBravePaths.filter((x) => ['grd', 'json'].includes(x.split('.').pop())) - -// Helper function to retrieve ethereum-remote-client paths relative -// to the Brave paths -module.exports.getEthereumRemoteClientPaths = function (extensionPath) { - let basePath = extensionPath - if (!basePath) { - basePath = '../../../ethereum-remote-client' - } - - return [ - `${basePath}/app/_locales/en/messages.json` - ] -} - -// Helper function to retrieve Greaselion script paths relative to the -// Brave paths. -// -// Greaselion.json consists of an array of Greaselion rules, -// specifying scripts to inject into given sites based on certain -// preconditions. If the rule contains a "messages" key, then the -// script contains user-visible strings that require translation. This -// helper function gathers those messages.json files for transmission -// to Transifex. -module.exports.getGreaselionScriptPaths = function (extensionPath) { - let basePath = extensionPath - if (!basePath) { - basePath = '../../../brave-site-specific-scripts' - } - - const jsonContent = fs.readFileSync(`${basePath}/Greaselion.json`, 'utf8') - if (!jsonContent) { - console.error('Missing Greaselion.json') - return [] - } - - const greaselionRules = JSON.parse(jsonContent) - if (!greaselionRules) { - console.error('Malformed Greaselion.json') - return [] - } - - let paths = [] - greaselionRules.forEach((rule) => { - if (rule.messages) { - paths.push(`${basePath}/${rule.messages}/en_US/messages.json`) - } - }) - - return paths -} - // Helper functions that's, for a given pair of chromium to brave GRD mapping // from the supplied map, determines which GRDP parts are no longer present in // the chromium GRD file. @@ -223,30 +121,161 @@ function getRemovedGRDParts(mapping) { return removedMap } -// Helper function to pretty print removed GRDP file names. -module.exports.logRemovedGRDParts = function (mapping) { - if (mapping.size) { - console.log("\n**************************************************************************") - console.log("The following GRDP files are no longer in the corresponding Chromium GRDs:\n") - for (const [grd, grdps] of mapping.entries()) { - console.log(" From " + grd + ":") - for (const grdp of grdps) { - console.log(" - " + grdp) - } +// Add all GRD mappings here. +function getAutoGeneratedGrdMappings() { + if (typeof(getAutoGeneratedGrdMappings.mappings) === 'undefined') { + console.log(chalk.italic('Recursing through GRD to find GRDP files...')) + // Brave specific only grd and grdp files should NOT be added. + // Using AddGrd will add GRD and all of its GRDPs. + getAutoGeneratedGrdMappings.mappings = { + ...addGrd(chromiumComponentsStringsPath, braveComponentsStringsPath), + ...addGrd(chromiumGeneratedResourcesPath, braveGeneratedResourcesPath, chromiumGeneratedResourcesExcludes), + ...addGrd(androidChromeStringsPath, braveAndroidChromeStringsPath) } + console.log(chalk.italic('Done recursing through GRD to find GRDP files.')) } + return getAutoGeneratedGrdMappings.mappings } -// This simply reads Chromium files that are passed to it and replaces branding strings -// with Brave specific branding strings. -// Do not use this for filtering XML, instead use chromium-rebase-l10n.py. -// Only add idempotent replacements here (i.e. don't append replace A with AX here) -module.exports.rebaseBraveStringFilesOnChromiumL10nFiles = async function (path) { - const removedMap = getRemovedGRDParts(grdsWithAutoAddedGrdps) - const ops = Object.entries(chromiumToAutoGeneratedBraveMapping).map(async ([sourcePath, destPath]) => { - let contents = await new Promise(resolve => fs.readFile(sourcePath, 'utf8', (err, data) => resolve(data))) - await new Promise(resolve => fs.writeFile(destPath, contents, 'utf8', resolve)) - }) - await Promise.all(ops) - return removedMap +function getChromiumToAutoGeneratedBraveMapping() { + if (typeof(getChromiumToAutoGeneratedBraveMapping.mapping) === 'undefined') { + // When adding new grd or grdp files, never add a grdp part path without a parent grd path. + // Group them with a leading and trailing newline to keep this file organized. + // The first 3 are added explicitly because we change the file names. + getChromiumToAutoGeneratedBraveMapping.mapping = { + [chromiumStringsPath]: braveStringsPath, + [chromiumSettingsPartPath]: braveSettingsPartPath, + + [chromiumComponentsChromiumStringsPath]: braveComponentsBraveStringsPath, + + ...getAutoGeneratedGrdMappings() + } + } + return getChromiumToAutoGeneratedBraveMapping.mapping } + +const l10nUtil = { + // Same as with chromiumToAutoGeneratedBraveMapping but maps in the opposite direction + getAutoGeneratedBraveToChromiumMapping: () => { + if (typeof(l10nUtil.getAutoGeneratedBraveToChromiumMapping.mapping) === 'undefined') { + const chromiumToAutoGeneratedBraveMapping = getChromiumToAutoGeneratedBraveMapping() + l10nUtil.getAutoGeneratedBraveToChromiumMapping.mapping = Object.keys( + chromiumToAutoGeneratedBraveMapping).reduce((obj, key) => ( + { ...obj, [chromiumToAutoGeneratedBraveMapping[key]]: key }), {}) + } + return l10nUtil.getAutoGeneratedBraveToChromiumMapping.mapping + }, + + // All paths which are generated + getBraveAutoGeneratedPaths: () => { + return Object.values(getChromiumToAutoGeneratedBraveMapping()) + }, + + // All paths which are not generated + getBraveNonGeneratedPaths: () => { + if (typeof(l10nUtil.getBraveNonGeneratedPaths.paths) === 'undefined') { + l10nUtil.getBraveNonGeneratedPaths.paths = [ + braveSpecificGeneratedResourcesPath, + braveResourcesComponentsStringsPath, + braveExtensionMessagesPath, + braveRewardsExtensionMessagesPath, + braveAndroidBraveStringsPath + ] + } + return l10nUtil.getBraveNonGeneratedPaths.paths + }, + + // Brave specific strings and Chromium mapped Brave strings will be here. + // But you only need to add the Brave specific strings manually here. + getAllBravePaths: () => { + return l10nUtil.getBraveNonGeneratedPaths().concat(l10nUtil.getBraveAutoGeneratedPaths()) + }, + + // Get all GRD and JSON paths whether they are generatd or not + // Push and pull scripts for l10n use this. + // Transifex manages files per grd and not per grd or grdp. + // This is because only 1 xtb is created per grd per locale even if it has multiple grdp files. + getBraveTopLevelPaths: () => { + return l10nUtil.getAllBravePaths().filter((x) => ['grd', 'json'].includes(x.split('.').pop())) + }, + +// Helper function to retrieve ethereum-remote-client paths relative +// to the Brave paths + getEthereumRemoteClientPaths: (extensionPath) => { + let basePath = extensionPath + if (!basePath) { + basePath = '../../../ethereum-remote-client' + } + + return [ + `${basePath}/app/_locales/en/messages.json` + ] + }, + +// Helper function to retrieve Greaselion script paths relative to the +// Brave paths. +// +// Greaselion.json consists of an array of Greaselion rules, +// specifying scripts to inject into given sites based on certain +// preconditions. If the rule contains a "messages" key, then the +// script contains user-visible strings that require translation. This +// helper function gathers those messages.json files for transmission +// to Transifex. + getGreaselionScriptPaths: (extensionPath) => { + let basePath = extensionPath + if (!basePath) { + basePath = '../../../brave-site-specific-scripts' + } + + const jsonContent = fs.readFileSync(`${basePath}/Greaselion.json`, 'utf8') + if (!jsonContent) { + console.error('Missing Greaselion.json') + return [] + } + + const greaselionRules = JSON.parse(jsonContent) + if (!greaselionRules) { + console.error('Malformed Greaselion.json') + return [] + } + + let paths = [] + greaselionRules.forEach((rule) => { + if (rule.messages) { + paths.push(`${basePath}/${rule.messages}/en_US/messages.json`) + } + }) + + return paths + }, + + // Helper function to pretty print removed GRDP file names. + logRemovedGRDParts: (mapping) => { + if (mapping.size) { + console.log("\n**************************************************************************") + console.log("The following GRDP files are no longer in the corresponding Chromium GRDs:\n") + for (const [grd, grdps] of mapping.entries()) { + console.log(" From " + grd + ":") + for (const grdp of grdps) { + console.log(" - " + grdp) + } + } + } + }, + + // This simply reads Chromium files that are passed to it and replaces branding strings + // with Brave specific branding strings. + // Do not use this for filtering XML, instead use chromium-rebase-l10n.py. + // Only add idempotent replacements here (i.e. don't append replace A with AX here) + rebaseBraveStringFilesOnChromiumL10nFiles: async (path) => { + const removedMap = getRemovedGRDParts(getAutoGeneratedGrdMappings()) + const ops = Object.entries(getChromiumToAutoGeneratedBraveMapping()).map(async ([sourcePath, destPath]) => { + let contents = await new Promise(resolve => fs.readFile(sourcePath, 'utf8', (err, data) => resolve(data))) + await new Promise(resolve => fs.writeFile(destPath, contents, 'utf8', resolve)) + }) + await Promise.all(ops) + return removedMap + }, +} // const l10nUtil + +module.exports = l10nUtil diff --git a/build/commands/lib/pullL10n.js b/build/commands/lib/pullL10n.js index 3bd7109624f..c5c4873ff91 100644 --- a/build/commands/lib/pullL10n.js +++ b/build/commands/lib/pullL10n.js @@ -1,7 +1,7 @@ const path = require('path') const config = require('../lib/config') const util = require('../lib/util') -const {braveTopLevelPaths, getEthereumRemoteClientPaths, getGreaselionScriptPaths} = require('./l10nUtil') +const l10nUtil = require('./l10nUtil') const pullL10n = (options) => { const cmdOptions = config.defaultOptions @@ -9,12 +9,12 @@ const pullL10n = (options) => { if (options.extension) { const extensionPath = options.extension_path if (options.extension === 'ethereum-remote-client') { - getEthereumRemoteClientPaths(extensionPath).forEach((sourceStringPath) => { + l10nUtil.getEthereumRemoteClientPaths(extensionPath).forEach((sourceStringPath) => { util.run('python', ['script/pull-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) return } else if (options.extension === 'greaselion') { - getGreaselionScriptPaths(extensionPath).forEach((sourceStringPath) => { + l10nUtil.getGreaselionScriptPaths(extensionPath).forEach((sourceStringPath) => { util.run('python', ['script/pull-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) return @@ -31,7 +31,7 @@ const pullL10n = (options) => { util.run('git', ['checkout', '--', targetFile], { cwd: srcDir }) }) - braveTopLevelPaths.forEach((sourceStringPath) => { + l10nUtil.getBraveTopLevelPaths().forEach((sourceStringPath) => { if (!options.grd_path || sourceStringPath.endsWith(path.sep + options.grd_path)) util.run('python', ['script/pull-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) diff --git a/build/commands/lib/pushL10n.js b/build/commands/lib/pushL10n.js index 927f7ad815a..3f726c1a516 100644 --- a/build/commands/lib/pushL10n.js +++ b/build/commands/lib/pushL10n.js @@ -1,7 +1,7 @@ const path = require('path') const config = require('../lib/config') const util = require('../lib/util') -const {braveTopLevelPaths, getEthereumRemoteClientPaths, getGreaselionScriptPaths} = require('./l10nUtil') +const l10nUtil = require('./l10nUtil') const pushL10n = (options) => { const runOptions = { cwd: config.srcDir } @@ -10,12 +10,12 @@ const pushL10n = (options) => { if (options.extension) { const extensionPath = options.extension_path if (options.extension === 'ethereum-remote-client') { - getEthereumRemoteClientPaths(extensionPath).forEach((sourceStringPath) => { + l10nUtil.getEthereumRemoteClientPaths(extensionPath).forEach((sourceStringPath) => { util.run('python', ['script/push-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) return } else if (options.extension === 'greaselion') { - getGreaselionScriptPaths(extensionPath).forEach((sourceStringPath) => { + l10nUtil.getGreaselionScriptPaths(extensionPath).forEach((sourceStringPath) => { util.run('python', ['script/push-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) return @@ -28,7 +28,7 @@ const pushL10n = (options) => { util.run('git', args, runOptions) args = ['checkout', '--', '*.grd*'] util.run('git', args, runOptions) - braveTopLevelPaths.forEach((sourceStringPath) => { + l10nUtil.getBraveTopLevelPaths().forEach((sourceStringPath) => { if (!options.grd_path || sourceStringPath.endsWith(path.sep + options.grd_path)) util.run('python', ['script/push-l10n.py', '--source_string_path', sourceStringPath], cmdOptions) }) diff --git a/build/commands/lib/util.js b/build/commands/lib/util.js index 93287ef533f..857c6a64f73 100755 --- a/build/commands/lib/util.js +++ b/build/commands/lib/util.js @@ -4,7 +4,7 @@ const { spawn, spawnSync } = require('child_process') const config = require('./config') const fs = require('fs-extra') const crypto = require('crypto') -const autoGeneratedBraveToChromiumMapping = Object.assign({}, require('./l10nUtil').autoGeneratedBraveToChromiumMapping) +const l10nUtil = require('./l10nUtil') const Log = require('./sync/logging') const fixPywin32 = (options = {}) => { @@ -221,6 +221,7 @@ const util = { const braveAndroidJavaStringsTranslationsDir = path.join(config.braveCoreDir, 'browser', 'ui', 'android', 'strings', 'translations') let fileMap = new Set(); + const autoGeneratedBraveToChromiumMapping = Object.assign({}, l10nUtil.getAutoGeneratedBraveToChromiumMapping()) // The following 3 entries we map to the same name, not the chromium equivalent name for copying back autoGeneratedBraveToChromiumMapping[path.join(braveAppDir, 'brave_strings.grd')] = path.join(chromeAppDir, 'brave_strings.grd') autoGeneratedBraveToChromiumMapping[path.join(braveAppDir, 'settings_brave_strings.grdp')] = path.join(chromeAppDir, 'settings_brave_strings.grdp')