From 162d8235dec46e3311f5a373d666c234685ebc3f Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Wed, 21 Aug 2019 22:20:05 -0700 Subject: [PATCH] Command to reset non-patched chromium files Shows a diff of each modified chromium file and asks if the user wants to reset that file. This makes sure only files that are modified are reset, if desired, and also gives an opportunity for the developer to not lose work unintentionally. This fixes a couple of different scenarios: 1. Developer wants to reset all chromium files to the current brave-core patched state. Right now, they could run `git reset --hard` in `src` directory (or run `npm run init` which will run that same git reset command), but that will result in a very long subsequent build time. Running both `npm run sync` and this new command, `npm run reset_non_patched`, will make sure only files that are patched by brave-core, and files that were otherwise modified are reset, but not the entire chromium repository. 2. A scenario whereby a developer can be editing chromium files in an attempt to debug an issue or create a feature, but doesn't know exactly which changes they wish to keep. Instead of running `npm run update_patches` and having to decide which patches to keep and dismiss and then resetting the other changes manually, this command will allow them to make the decision about which changes to keep or discard first, before committing to create patches. --- lib/chromiumPathFilter.js | 11 +++++++ lib/gitPatcher.js | 59 ++++++++++++++++++++++++++++++++++++ lib/updatePatches.js | 8 ++--- lib/util.js | 6 ++++ package.json | 3 +- scripts/reset-non-patched.js | 55 +++++++++++++++++++++++++++++++++ scripts/updatePatches.js | 11 +------ 7 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 lib/chromiumPathFilter.js create mode 100644 scripts/reset-non-patched.js diff --git a/lib/chromiumPathFilter.js b/lib/chromiumPathFilter.js new file mode 100644 index 0000000000..e2f4a004f6 --- /dev/null +++ b/lib/chromiumPathFilter.js @@ -0,0 +1,11 @@ +module.exports = function chromiumPathFilter (s) { + return s.length > 0 && + !s.startsWith('chrome/app/theme/default') && + !s.startsWith('chrome/app/theme/brave') && + !s.startsWith('chrome/app/theme/chromium') && + !s.endsWith('.png') && !s.endsWith('.xtb') && + !s.endsWith('.grd') && !s.endsWith('.grdp') && + !s.includes('google_update_idl') && + !s.endsWith('new_tab_page_view.xml') && + !s.endsWith('channel_constants.xml') +} \ No newline at end of file diff --git a/lib/gitPatcher.js b/lib/gitPatcher.js index f735a93417..e9a46c89af 100644 --- a/lib/gitPatcher.js +++ b/lib/gitPatcher.js @@ -56,6 +56,65 @@ module.exports = class GitPatcher { } } + async resetNonPatchedChanges (onChangeFound, repoPathFilter) { + // 1. iterate changes in working dir + // 2. is there a corresponding patch file + // - yes? do nothing + // - no? ask to reset + + // Validate + if (typeof onChangeFound !== 'function') { + throw new Error('onChangeFound must be a function') + } + const patchDirExists = await fs.exists(this.patchDirPath) + if (!patchDirExists) { + return + } + + // Get files changes, filtered to non-patched and also files we care about + this.logProgressLine('Getting modified paths from git...') + let modifiedPaths = await util.gitGetModifiedPaths(this.repoPath) + this.logProgressLine(`Got ${modifiedPaths.length} modified paths.`) + if (typeof repoPathFilter === 'function') { + // ignore paths requested in param + modifiedPaths = modifiedPaths.filter(repoPathFilter) + } + // ignore patched paths + const patchInfoPaths = (await fs.readdir(this.patchDirPath)) + .filter(s => s.endsWith(`.${extPatchInfo}`)) + .map(p => path.join(this.patchDirPath, p)) + this.logProgress(os.EOL + 'Getting all patched paths...') + const ops = [] + const patchedPaths = [] + for (const patchInfoPath of patchInfoPaths) { + const op = this.getPatchInfo(patchInfoPath).then(patchInfo => { + this.logProgress('.') + patchedPaths.push(...patchInfo.appliesTo.map(a => a.path)) + }) + ops.push(op) + } + await Promise.all(ops) + this.logProgressLine(`Got ${patchedPaths.length} patched paths.`) + if (patchedPaths.length) { + modifiedPaths = modifiedPaths.filter(p => !patchedPaths.includes(p)) + this.logProgressLine(`Filtered to ${modifiedPaths.length} modified non-patched paths to reset.`) + } + // Ask for reset for each + const filesToReset = [] + for (const p of modifiedPaths) { + const diffContent = await util.runGitAsync(this.repoPath, ['diff', '--color=always', '--', p]) + const shouldReset = await onChangeFound(p, diffContent) + if (shouldReset) { + filesToReset.push(p) + } + } + this.logProgressLine(`Resetting ${filesToReset.length} files:`, filesToReset) + if (filesToReset.length) { + await this.resetRepoFiles(filesToReset) + } + return + } + async applyPatches () { // STRATEGY: // 1. iterate .patch files in dir diff --git a/lib/updatePatches.js b/lib/updatePatches.js index 07ffb85ab7..d9029b4d51 100644 --- a/lib/updatePatches.js +++ b/lib/updatePatches.js @@ -5,11 +5,7 @@ const util = require('../lib/util') const desiredReplacementSeparator = '-' const patchExtension = '.patch' -async function getModifiedPaths (gitRepoPath) { - const modifiedDiffArgs = ['diff', '--diff-filter=M', '--name-only', '--ignore-space-at-eol'] - const cmdOutput = await util.runAsync('git', modifiedDiffArgs, { cwd: gitRepoPath }) - return cmdOutput.split('\n').filter(s => s) -} + async function writePatchFiles (modifiedPaths, gitRepoPath, patchDirPath) { // replacing forward slashes and adding the patch extension to get nice filenames @@ -95,7 +91,7 @@ async function removeStalePatchFiles (patchFilenames, patchDirPath, keepPatchFil * @param {*} [keepPatchFilenames=[]] Patch filenames to never delete */ async function updatePatches (gitRepoPath, patchDirPath, repoPathFilter, keepPatchFilenames = []) { - let modifiedPaths = await getModifiedPaths(gitRepoPath) + let modifiedPaths = await util.gitGetModifiedPaths(gitRepoPath) if (typeof repoPathFilter === 'function') { modifiedPaths = modifiedPaths.filter(repoPathFilter) } diff --git a/lib/util.js b/lib/util.js index 612677b115..ccaebc0911 100755 --- a/lib/util.js +++ b/lib/util.js @@ -89,6 +89,12 @@ const util = { }) }, + gitGetModifiedPaths: async function (gitRepoPath, verbose = false, logError = false) { + const modifiedDiffArgs = ['diff', '--diff-filter=M', '--name-only', '--ignore-space-at-eol'] + const cmdOutput = await util.runGitAsync(gitRepoPath, modifiedDiffArgs, verbose, logError) + return cmdOutput.split('\n').filter(s => s) + }, + buildGClientConfig: () => { function replacer(key, value) { return value; diff --git a/package.json b/package.json index 9a5678da63..7f5a582c3a 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "versions": "node ./scripts/commands.js versions", "upload": "node ./scripts/commands.js upload", "update_patches": "node ./scripts/commands.js update_patches", - "apply_patches": "node ./scripts/sync.js --run_hooks", + "apply_patches": "npm run sync", + "reset_non_patched": "node ./scripts/reset-non-patched.js", "start": "node ./scripts/commands.js start", "network-audit": "node ./scripts/commands.js start --enable_brave_update --network_log --user_data_dir_name=brave-network-test", "push_l10n": "node ./scripts/commands.js push_l10n", diff --git a/scripts/reset-non-patched.js b/scripts/reset-non-patched.js new file mode 100644 index 0000000000..a06679d618 --- /dev/null +++ b/scripts/reset-non-patched.js @@ -0,0 +1,55 @@ +// Copyright (c) 2019 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. + +const path = require('path') +const os = require('os') +const readline = require('readline') +const program = require('commander') +const fs = require('fs-extra') +const chalk = require('chalk') +const config = require('../lib/config') +const util = require('../lib/util') +const GitPatcher = require('../lib/gitPatcher') +const chromiumRepoPathFilter = require('../lib/chromiumPathFilter') +program + .version(process.env.npm_package_version) + +async function RunCommand () { + program.parse(process.argv) + config.update(program) + + const coreRepoPath = config.projects['brave-core'].dir + const patchesPath = path.join(coreRepoPath, 'patches') + const v8PatchesPath = path.join(patchesPath, 'v8') + const chromiumRepoPath = config.projects['chrome'].dir + const v8RepoPath = path.join(chromiumRepoPath, 'v8') + const chromiumPatcher = new GitPatcher(patchesPath, chromiumRepoPath) + const v8Patcher = new GitPatcher(v8PatchesPath, v8RepoPath) + + const rl = readline.createInterface(process.stdin, process.stdout) + function onChangeFound(modifiedPath, diffContent) { + console.log(chalk.blue('---------')) + console.log(diffContent) + return new Promise(resolve => { + rl.question(`Reset file at: ${modifiedPath}? y/[n]`, answer => { + resolve(answer.length && answer.toLowerCase()[0] === 'y') + }) + }) + } + + const chromiumPatchStatus = await chromiumPatcher.resetNonPatchedChanges(onChangeFound, chromiumRepoPathFilter) + const v8PatchStatus = await v8Patcher.resetNonPatchedChanges(onChangeFound) +} + +console.log('Brave reset non-patched files starting') +RunCommand() +.then(() => { + console.log('Brave reset complete') + process.exit(0) +}) +.catch((err) => { + console.error('Brave reset ERROR:') + console.error(err) +}) diff --git a/scripts/updatePatches.js b/scripts/updatePatches.js index 0a4c4b7cb2..8a97597630 100644 --- a/scripts/updatePatches.js +++ b/scripts/updatePatches.js @@ -1,16 +1,7 @@ const path = require('path') const config = require('../lib/config') const updatePatches = require('../lib/updatePatches') - -const chromiumPathFilter = (s) => s.length > 0 && - !s.startsWith('chrome/app/theme/default') && - !s.startsWith('chrome/app/theme/brave') && - !s.startsWith('chrome/app/theme/chromium') && - !s.endsWith('.png') && !s.endsWith('.xtb') && - !s.endsWith('.grd') && !s.endsWith('.grdp') && - !s.endsWith('new_tab_page_view.xml') && - !s.endsWith('channel_constants.xml') && - !s.includes('google_update_idl') +const chromiumPathFilter = require('../lib/chromiumPathFilter') module.exports = function RunCommand (options) { config.update(options)