build: export matching patches (#41174)

* build: make patches/config.json an array of objects

This file was previously an object of patch_dir keys to repo values;
Now is an array of objects containing `patch_dir` and `repo` properties.
This makes other per-target properties (e.g. `grep`) possible.

* build: include Note metadata when exporting patches

* build: support keyword filtering in export_patches()

* build: add optional `--grep` arg to git-export-patches script

* build: update export_all_patches to understand new config file

* fixup! build: update export_all_patches to understand new config file

chore: make lint happy

* fixup! build: make patches/config.json an array of objects

chore: fix oops

* refactor: remove support for the old file format

There is more code using config.json than I thought, so the
effort-to-reward of supporting the old format is not worth it.

* build: update apply_all_patches to understand new config file

* build: update lint.js to understand new config file

* build: update patches-mtime-cache.py to understand new config file

* fixup! build: update apply_all_patches to understand new config file

fix: oops

* fixup! build: update apply_all_patches to understand new config file

fix minor syntax wart

* fixup! build: support keyword filtering in export_patches()

refactor: use idiomatic python

* refactor: warn if config.json has an invalid repo

patches/config.json

@@ -1,27 +1,15 @@
-{
-  "src/electron/patches/chromium": "src",
-
-  "src/electron/patches/boringssl": "src/third_party/boringssl/src",
-
-  "src/electron/patches/devtools_frontend": "src/third_party/devtools-frontend/src",
-
-  "src/electron/patches/ffmpeg": "src/third_party/ffmpeg",
-
-  "src/electron/patches/v8":  "src/v8",
-
-  "src/electron/patches/node": "src/third_party/electron_node",
-
-  "src/electron/patches/nan": "src/third_party/nan",
-
-  "src/electron/patches/perfetto": "src/third_party/perfetto",
-
-  "src/electron/patches/squirrel.mac": "src/third_party/squirrel.mac",
-
-  "src/electron/patches/Mantle": "src/third_party/squirrel.mac/vendor/Mantle",
-
-  "src/electron/patches/ReactiveObjC": "src/third_party/squirrel.mac/vendor/ReactiveObjC",
-
-  "src/electron/patches/webrtc": "src/third_party/webrtc",
-
-  "src/electron/patches/reclient-configs": "src/third_party/engflow-reclient-configs"
-}
+[
+  { "patch_dir": "src/electron/patches/chromium", "repo": "src" },
+  { "patch_dir": "src/electron/patches/boringssl", "repo": "src/third_party/boringssl/src" },
+  { "patch_dir": "src/electron/patches/devtools_frontend", "repo": "src/third_party/devtools-frontend/src" },
+  { "patch_dir": "src/electron/patches/ffmpeg", "repo": "src/third_party/ffmpeg" },
+  { "patch_dir": "src/electron/patches/v8", "repo": "src/v8" },
+  { "patch_dir": "src/electron/patches/node", "repo": "src/third_party/electron_node" },
+  { "patch_dir": "src/electron/patches/nan", "repo": "src/third_party/nan" },
+  { "patch_dir": "src/electron/patches/perfetto", "repo": "src/third_party/perfetto" },
+  { "patch_dir": "src/electron/patches/squirrel.mac", "repo": "src/third_party/squirrel.mac" },
+  { "patch_dir": "src/electron/patches/Mantle", "repo": "src/third_party/squirrel.mac/vendor/Mantle" },
+  { "patch_dir": "src/electron/patches/ReactiveObjC", "repo": "src/third_party/squirrel.mac/vendor/ReactiveObjC" },
+  { "patch_dir": "src/electron/patches/webrtc", "repo": "src/third_party/webrtc" },
+  { "patch_dir": "src/electron/patches/reclient-configs", "repo": "src/third_party/engflow-reclient-configs" }
+]

script/apply_all_patches.py

@@ -3,19 +3,30 @@
 import argparse
 import json
 import os
+import warnings
 
 from lib import git
 from lib.patches import patch_from_dir
 
+THREEWAY = "ELECTRON_USE_THREE_WAY_MERGE_FOR_PATCHES" in os.environ
 
-def apply_patches(dirs):
-  threeway = os.environ.get("ELECTRON_USE_THREE_WAY_MERGE_FOR_PATCHES")
-  for patch_dir, repo in dirs.items():
-    if os.path.exists(repo):
-      git.import_patches(repo=repo, patch_data=patch_from_dir(patch_dir),
-        threeway=threeway is not None,
-        committer_name="Electron Scripts", committer_email="scripts@electron")
+def apply_patches(target):
+  repo = target.get('repo')
+  if not os.path.exists(repo):
+    warnings.warn('repo not found: %s' % repo)
+    return
+  patch_dir = target.get('patch_dir')
+  git.import_patches(
+    committer_email="scripts@electron",
+    committer_name="Electron Scripts",
+    patch_data=patch_from_dir(patch_dir),
+    repo=repo,
+    threeway=THREEWAY,
+  )
 
+def apply_config(config):
+  for target in config:
+    apply_patches(target)
 
 def parse_args():
   parser = argparse.ArgumentParser(description='Apply Electron patches')
@@ -26,9 +37,8 @@ def parse_args():
 
 
 def main():
-  configs = parse_args().config
-  for config_json in configs:
-    apply_patches(json.load(config_json))
+  for config_json in parse_args().config:
+    apply_config(json.load(config_json))
 
 
 if __name__ == '__main__':

script/export_all_patches.py

@@ -3,14 +3,27 @@
 import argparse
 import json
 import os
+import warnings
 
 from lib import git
 
 
-def export_patches(dirs, dry_run):
-  for patch_dir, repo in dirs.items():
-    if os.path.exists(repo):
-      git.export_patches(repo=repo, out_dir=patch_dir, dry_run=dry_run)
+def export_patches(target, dry_run):
+  repo = target.get('repo')
+  if not os.path.exists(repo):
+    warnings.warn('repo not found: %s' % repo)
+    return
+  git.export_patches(
+    dry_run=dry_run,
+    grep=target.get('grep'),
+    out_dir=target.get('patch_dir'),
+    repo=repo
+  )
+
+
+def export_config(config, dry_run):
+  for target in config:
+    export_patches(target, dry_run)
 
 
 def parse_args():
@@ -28,7 +41,7 @@ def main():
   configs = parse_args().config
   dry_run = parse_args().dry_run
   for config_json in configs:
-    export_patches(json.load(config_json), dry_run)
+    export_config(json.load(config_json), dry_run)
 
 
 if __name__ == '__main__':

script/git-export-patches

@@ -10,13 +10,15 @@ def main(argv):
   parser.add_argument("-o", "--output",
       help="directory into which exported patches will be written",
       required=True)
+  parser.add_argument("--grep",
+      help="only export patches matching a keyword")
   parser.add_argument("patch_range",
       nargs='?',
       help="range of patches to export. Defaults to all commits since the "
            "most recent tag or remote branch.")
   args = parser.parse_args(argv)
 
-  git.export_patches('.', args.output, patch_range=args.patch_range)
+  git.export_patches('.', args.output, patch_range=args.patch_range, grep=args.grep)
 
 
 if __name__ == '__main__':

script/lib/git.py

@@ -149,6 +149,7 @@ def format_patch(repo, since):
     'format-patch',
     '--keep-subject',
     '--no-stat',
+    '--notes',
     '--stdout',
 
     # Per RFC 3676 the signature is separated from the body by a line with
@@ -181,6 +182,16 @@ def split_patches(patch_data):
     patches[-1].append(line)
   return patches
 
+def filter_patches(patches, key):
+  """Return patches that include the specified key"""
+  if key is None:
+    return patches
+  matches = []
+  for patch in patches:
+    if any(key in line for line in patch):
+      matches.append(patch)
+      continue
+  return matches
 
 def munge_subject_to_filename(subject):
   """Derive a suitable filename from a commit's subject"""
@@ -227,7 +238,7 @@ def remove_patch_filename(patch):
     force_keep_next_line = l.startswith('Subject: ')
 
 
-def export_patches(repo, out_dir, patch_range=None, dry_run=False):
+def export_patches(repo, out_dir, patch_range=None, dry_run=False, grep=None):
   if not os.path.exists(repo):
     sys.stderr.write(
       "Skipping patches in {} because it does not exist.\n".format(repo)
@@ -239,6 +250,8 @@ def export_patches(repo, out_dir, patch_range=None, dry_run=False):
         num_patches, repo, patch_range[0:7]))
   patch_data = format_patch(repo, patch_range)
   patches = split_patches(patch_data)
+  if grep:
+    patches = filter_patches(patches, grep)
 
   try:
     os.mkdir(out_dir)

script/lint.js

@@ -201,10 +201,9 @@ const LINTERS = [{
       process.exit(1);
     }
 
-    const config = JSON.parse(fs.readFileSync(patchesConfig, 'utf8'));
-    for (const key of Object.keys(config)) {
+    for (const target of JSON.parse(fs.readFileSync(patchesConfig, 'utf8'))) {
       // The directory the config points to should exist
-      const targetPatchesDir = path.resolve(__dirname, '../../..', key);
+      const targetPatchesDir = path.resolve(__dirname, '../../..', target.patch_dir);
       if (!fs.existsSync(targetPatchesDir)) {
         console.error(`target patch directory: "${targetPatchesDir}" does not exist`);
         process.exit(1);

script/patches-mtime-cache.py

@@ -12,7 +12,9 @@ from lib.patches import patch_from_dir
 
 
 def patched_file_paths(patches_config):
-    for patch_dir, repo in patches_config.items():
+    for target in patches_config:
+        patch_dir = target.get('patch_dir')
+        repo = target.get('repo')
         for line in patch_from_dir(patch_dir).split("\n"):
             if line.startswith("+++"):
                 yield posixpath.join(repo, line[6:])