Bug 1729456 - Don't remove files listed in keep or include when vendoring updates; r=tjr

Implements keep key in moz.yaml for keeping pre-existing files in the vendor directory
Implements include key in moz.yaml for forcing inclusion of files and folders from upstream

Differential Revision: https://phabricator.services.mozilla.com/D124799

python/mozbuild/mozbuild/vendor/host_base.py

@@ -16,6 +16,7 @@ class BaseHost:
     def upstream_tag(self, revision):
         """Temporarily clone the repo to get the latest tag and timestamp"""
         with tempfile.TemporaryDirectory() as temp_repo_clone:
+            starting_directory = os.getcwd()
             os.chdir(temp_repo_clone)
             subprocess.run(
                 [
@@ -51,6 +52,7 @@ class BaseHost:
                 universal_newlines=True,
                 check=True,
             ).stdout.splitlines()[-1]
+            os.chdir(starting_directory)
             return (latest_tag, latest_tag_timestamp)
 
     def upstream_snapshot(self, revision):

python/mozbuild/mozbuild/vendor/mach_commands.py

@@ -29,18 +29,24 @@ from mozbuild.vendor.moz_yaml import load_moz_yaml, MozYamlVerifyError
 @CommandArgument(
     "--add-to-exports",
     action="store_true",
-    help="Will attempt to add new header files into any relevant EXPORTS block",
+    help="Will attempt to add new header files into any relevant EXPORTS block.",
     default=False,
 )
 @CommandArgument(
     "--ignore-modified",
     action="store_true",
-    help="Ignore modified files in current checkout",
+    help="Ignore modified files in current checkout.",
     default=False,
 )
 @CommandArgument("-r", "--revision", help="Repository tag or commit to update to.")
 @CommandArgument(
-    "--verify", "-v", action="store_true", help="(Only) verify the manifest"
+    "--verify", "-v", action="store_true", help="(Only) verify the manifest."
+)
+@CommandArgument(
+    "--patch-mode",
+    help="Select how vendored patches will be imported. 'none' skips patch import, and"
+    "'only' imports patches and skips library vendoring.",
+    default="",
 )
 @CommandArgument("library", nargs=1, help="The moz.yaml file of the library to vendor.")
 def vendor(
@@ -51,6 +57,7 @@ def vendor(
     check_for_update=False,
     add_to_exports=False,
     verify=False,
+    patch_mode="",
 ):
     """
     Vendor third-party dependencies into the source repository.
@@ -75,6 +82,28 @@ def vendor(
         print(e)
         sys.exit(1)
 
+    if patch_mode and patch_mode not in ["none", "only"]:
+        print(
+            "Unknown patch mode given '%s'. Please use one of: 'none' or 'only'."
+            % patch_mode
+        )
+        sys.exit(1)
+    if (
+        manifest["vendoring"].get("patches", [])
+        and not patch_mode
+        and not check_for_update
+    ):
+        print(
+            "Patch mode was not given when required. Please use one of: 'none' or 'only'"
+        )
+        sys.exit(1)
+    if patch_mode == "only" and not manifest["vendoring"].get("patches", []):
+        print(
+            "Patch import was specified for %s but there are no vendored patches defined."
+            % library
+        )
+        sys.exit(1)
+
     if not ignore_modified and not check_for_update:
         check_modified_files(command_context)
     if not revision:
@@ -83,7 +112,14 @@ def vendor(
     from mozbuild.vendor.vendor_manifest import VendorManifest
 
     vendor_command = command_context._spawn(VendorManifest)
-    vendor_command.vendor(library, manifest, revision, check_for_update, add_to_exports)
+    vendor_command.vendor(
+        library,
+        manifest,
+        revision,
+        check_for_update,
+        add_to_exports,
+        patch_mode,
+    )
 
     sys.exit(0)
 

python/mozbuild/mozbuild/vendor/moz_yaml.py

@@ -169,16 +169,22 @@ vendoring:
 
   # List of patch files to apply after vendoring. Applied in the order
   # specified, and alphabetically if globbing is used. Patches must apply
-  # cleanly before changes are pushed
+  # cleanly before changes are pushed.
+  # Patch files should be relative to the vendor-directory rather than the gecko
+  # root directory.
   # All patch files are implicitly added to the keep file list.
   # optional
   patches:
     - file
     - path/to/file
     - path/*.patch
+    - path/**  # Captures all files and subdirectories below path
+    - path/*   # Captures all files but _not_ subdirectories below path. Equivalent to `path/`
 
-  # List of files that are not deleted while vendoring
-  # Implicitly contains "moz.yaml", any files referenced as patches
+  # List of files that are not removed from the destination directory while vendoring
+  # in a new version of the library. Intended for mozilla files not present in upstream.
+  # Implicitly contains "moz.yaml", "moz.build", and any files referenced in
+  # "patches"
   # optional
   keep:
     - file
@@ -186,7 +192,7 @@ vendoring:
     - another/path
     - *.mozilla
 
-  # Files/paths that will not be vendored from source repository
+  # Files/paths that will not be vendored from the upstream repository
   # Implicitly contains ".git", and ".gitignore"
   # optional
   exclude:
@@ -196,8 +202,8 @@ vendoring:
     - docs
     - src/*.test
 
-  # Files/paths that will always be vendored, even if they would
-  # otherwise be excluded by "exclude".
+  # Files/paths that will always be vendored from source repository, even if
+  # they would otherwise be excluded by "exclude".
   # optional
   include:
     - file

python/mozbuild/mozbuild/vendor/rewrite_mozbuild.py

@@ -862,7 +862,7 @@ def validate_directory_parameters(moz_yaml_dir, vendoring_dir):
         moz_yaml_dir and vendoring_dir
     ), "If either moz_yaml_dir or vendoring_dir are specified, they both must be"
 
-    if moz_yaml_dir is not None:
+    if moz_yaml_dir is not None and vendoring_dir is not None:
         # Ensure they are provided with trailing slashes
         moz_yaml_dir += "/" if moz_yaml_dir[-1] != "/" else ""
         vendoring_dir += "/" if vendoring_dir[-1] != "/" else ""

python/mozbuild/mozbuild/vendor/vendor_manifest.py

@@ -11,6 +11,7 @@ import glob
 import shutil
 import logging
 import tarfile
+import tempfile
 import requests
 
 import mozfile
@@ -24,10 +25,23 @@ from mozbuild.vendor.rewrite_mozbuild import (
 )
 
 DEFAULT_EXCLUDE_FILES = [".git*"]
+DEFAULT_KEEP_FILES = ["moz.build", "moz.yaml"]
+DEFAULT_INCLUDE_FILES = []
 
 
 class VendorManifest(MozbuildObject):
-    def vendor(self, yaml_file, manifest, revision, check_for_update, add_to_exports):
+    def should_perform_step(self, step):
+        return step not in self.manifest["vendoring"].get("skip-vendoring-steps", [])
+
+    def vendor(
+        self,
+        yaml_file,
+        manifest,
+        revision,
+        check_for_update,
+        add_to_exports,
+        patch_mode,
+    ):
         self.manifest = manifest
         if "vendor-directory" not in self.manifest["vendoring"]:
             self.manifest["vendoring"]["vendor-directory"] = os.path.dirname(yaml_file)
@@ -65,35 +79,41 @@ class VendorManifest(MozbuildObject):
             print("%s %s" % (ref, timestamp))
             return
 
-        def perform_step(step):
-            return step not in self.manifest["vendoring"].get(
-                "skip-vendoring-steps", []
-            )
+        if "patches" in self.manifest["vendoring"]:
+            if patch_mode == "only":
+                self.import_local_patches(
+                    self.manifest["vendoring"]["patches"],
+                    self.manifest["vendoring"]["vendor-directory"],
+                )
+                return
+            else:
+                self.log(
+                    logging.INFO,
+                    "vendor",
+                    {},
+                    "Patches present in manifest please run "
+                    "'./mach vendor --patch-mode only' after commits from upstream "
+                    "have been vendored.",
+                )
 
-        if perform_step("fetch"):
+        if self.should_perform_step("fetch"):
             self.fetch_and_unpack(ref)
         else:
             self.log(logging.INFO, "vendor", {}, "Skipping fetching upstream source.")
 
-        if perform_step("exclude"):
-            self.log(logging.INFO, "vendor", {}, "Removing unnecessary files.")
-            self.clean_upstream()
-        else:
-            self.log(logging.INFO, "vendor", {}, "Skipping removing excluded files.")
-
-        if perform_step("update-moz-yaml"):
+        if self.should_perform_step("update-moz-yaml"):
             self.log(logging.INFO, "vendor", {}, "Updating moz.yaml.")
             self.update_yaml(yaml_file, ref, timestamp)
         else:
             self.log(logging.INFO, "vendor", {}, "Skipping updating the moz.yaml file.")
 
-        if perform_step("update-actions"):
+        if self.should_perform_step("update-actions"):
             self.log(logging.INFO, "vendor", {}, "Updating files")
             self.update_files(ref, yaml_file)
         else:
             self.log(logging.INFO, "vendor", {}, "Skipping running the update actions.")
 
-        if perform_step("hg-add"):
+        if self.should_perform_step("hg-add"):
             self.log(
                 logging.INFO, "vendor", {}, "Registering changes with version control."
             )
@@ -109,7 +129,7 @@ class VendorManifest(MozbuildObject):
                 "Skipping registering changes with version control.",
             )
 
-        if perform_step("update-moz-build"):
+        if self.should_perform_step("update-moz-build"):
             self.log(logging.INFO, "vendor", {}, "Updating moz.build files")
             self.update_moz_build(
                 self.manifest["vendoring"]["vendor-directory"],
@@ -148,6 +168,27 @@ class VendorManifest(MozbuildObject):
                 "Unknown source host: " + self.manifest["vendoring"]["source-hosting"]
             )
 
+    def convert_patterns_to_paths(self, directory, patterns):
+        # glob.iglob uses shell-style wildcards for path name completion.
+        # "recursive=True" enables the double asterisk "**" wildcard which matches
+        # for nested directories as well as the directory we're searching in.
+        paths = []
+        for pattern in patterns:
+            pattern_full_path = mozpath.join(directory, pattern)
+            # If pattern is a directory recursively add contents of directory
+            if os.path.isdir(pattern_full_path):
+                # Append double asterisk to the end to make glob.iglob recursively match
+                # contents of directory
+                paths.extend(
+                    glob.iglob(mozpath.join(pattern_full_path, "**"), recursive=True)
+                )
+            # Otherwise pattern is a file or wildcard expression so add it without altering it
+            else:
+                paths.extend(glob.iglob(pattern_full_path, recursive=True))
+        # Remove folder names from list of paths in order to avoid prematurely
+        # truncating directories elsewhere
+        return [path for path in paths if not os.path.isdir(path)]
+
     def fetch_and_unpack(self, revision):
         """Fetch and unpack upstream source"""
         url = self.source_host.upstream_snapshot(revision)
@@ -158,66 +199,127 @@ class VendorManifest(MozbuildObject):
             "Fetching code archive from {revision_url}",
         )
 
-        prefix = self.manifest["origin"]["name"] + "-" + revision
         with mozfile.NamedTemporaryFile() as tmptarfile:
-            req = requests.get(url, stream=True)
-            for data in req.iter_content(4096):
-                tmptarfile.write(data)
-            tmptarfile.seek(0)
+            with tempfile.TemporaryDirectory() as tmpextractdir:
+                req = requests.get(url, stream=True)
+                for data in req.iter_content(4096):
+                    tmptarfile.write(data)
+                tmptarfile.seek(0)
 
-            tar = tarfile.open(tmptarfile.name)
+                tar = tarfile.open(tmptarfile.name)
 
-            if any(
-                [
-                    name
-                    for name in tar.getnames()
-                    if name.startswith("/") or ".." in name
-                ]
-            ):
-                raise Exception(
-                    "Tar archive contains non-local paths," "e.g. '%s'" % bad_paths[0]
+                for name in tar.getnames():
+                    if name.startswith("/") or ".." in name:
+                        raise Exception(
+                            "Tar archive contains non-local paths, e.g. '%s'" % name
+                        )
+
+                vendor_dir = self.manifest["vendoring"]["vendor-directory"]
+                if self.should_perform_step("keep"):
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {},
+                        "Retaining wanted in-tree files.",
+                    )
+                    to_keep = self.convert_patterns_to_paths(
+                        vendor_dir,
+                        self.manifest["vendoring"].get("keep", [])
+                        + DEFAULT_KEEP_FILES
+                        + self.manifest["vendoring"].get("patches", []),
+                    )
+                else:
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {},
+                        "Skipping retention of included files.",
+                    )
+                    to_keep = []
+
+                self.log(
+                    logging.INFO,
+                    "vendor",
+                    {"vendor_dir": vendor_dir},
+                    "Cleaning {vendor_dir} to import changes.",
                 )
+                # We use double asterisk wildcard here to get complete list of recursive contents
+                for file in self.convert_patterns_to_paths(vendor_dir, "**"):
+                    if file not in to_keep:
+                        mozfile.remove(file)
 
-            vendor_dir = self.manifest["vendoring"]["vendor-directory"]
-            self.log(logging.INFO, "rm_vendor_dir", {}, "rm -rf %s" % vendor_dir)
-            mozfile.remove(vendor_dir)
+                self.log(
+                    logging.INFO,
+                    "vendor",
+                    {"vendor_dir": vendor_dir},
+                    "Unpacking upstream files for {vendor_dir}.",
+                )
+                tar.extractall(tmpextractdir)
 
-            self.log(
-                logging.INFO,
-                "vendor",
-                {"vendor_dir": vendor_dir},
-                "Unpacking upstream files from {vendor_dir}.",
-            )
-            tar.extractall(vendor_dir)
+                prefix = self.manifest["origin"]["name"] + "-" + revision
+                has_prefix = all(
+                    map(lambda name: name.startswith(prefix), tar.getnames())
+                )
+                tar.close()
 
-            has_prefix = all(map(lambda name: name.startswith(prefix), tar.getnames()))
-            tar.close()
+                # GitLab puts everything down a directory; move it up.
+                if has_prefix:
+                    tardir = mozpath.join(tmpextractdir, prefix)
+                    mozfile.copy_contents(tardir, tmpextractdir)
+                    mozfile.remove(tardir)
 
-            # GitLab puts everything properly down a directory; move it up.
-            if has_prefix:
-                tardir = mozpath.join(vendor_dir, prefix)
-                mozfile.copy_contents(tardir, vendor_dir)
-                mozfile.remove(tardir)
+                if self.should_perform_step("include"):
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {},
+                        "Retaining wanted files from upstream changes.",
+                    )
+                    to_include = self.convert_patterns_to_paths(
+                        tmpextractdir,
+                        self.manifest["vendoring"].get("include", [])
+                        + DEFAULT_INCLUDE_FILES,
+                    )
+                else:
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {},
+                        "Skipping retention of included files.",
+                    )
+                    to_include = []
 
-    def clean_upstream(self):
-        """Remove files we don't want to import."""
-        to_exclude = []
-        vendor_dir = self.manifest["vendoring"]["vendor-directory"]
-        for pattern in (
-            self.manifest["vendoring"].get("exclude", []) + DEFAULT_EXCLUDE_FILES
-        ):
-            if "*" in pattern:
-                to_exclude.extend(glob.iglob(mozpath.join(vendor_dir, pattern)))
-            else:
-                to_exclude.append(mozpath.join(vendor_dir, pattern))
-        self.log(
-            logging.INFO,
-            "vendor",
-            {"files": to_exclude},
-            "Removing: " + str(to_exclude),
-        )
-        for f in to_exclude:
-            mozfile.remove(f)
+                if self.should_perform_step("exclude"):
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {},
+                        "Removing unwanted files from upstream changes.",
+                    )
+                    to_exclude = self.convert_patterns_to_paths(
+                        tmpextractdir,
+                        self.manifest["vendoring"].get("exclude", [])
+                        + DEFAULT_EXCLUDE_FILES,
+                    )
+                else:
+                    self.log(
+                        logging.INFO, "vendor", {}, "Skipping removing excluded files."
+                    )
+                    to_exclude = []
+
+                to_exclude = list(set(to_exclude) - set(to_include))
+
+                if to_exclude:
+                    self.log(
+                        logging.INFO,
+                        "vendor",
+                        {"files": to_exclude},
+                        "Removing: " + str(to_exclude),
+                    )
+                    for exclusion in to_exclude:
+                        mozfile.remove(exclusion)
+
+                mozfile.copy_contents(tmpextractdir, vendor_dir)
 
     def update_yaml(self, yaml_file, revision, timestamp):
         with open(yaml_file) as f:
@@ -440,3 +542,20 @@ class VendorManifest(MozbuildObject):
             )
             # Exit with -1 to distinguish this from the Exception case of exiting with 1
             sys.exit(-1)
+
+    def import_local_patches(self, patches, vendor_dir):
+        self.log(logging.INFO, "vendor", {}, "Importing local patches.")
+        for patch in self.convert_patterns_to_paths(vendor_dir, patches):
+            script = [
+                "patch",
+                "-p1",
+                "--directory",
+                vendor_dir,
+                "--input",
+                os.path.abspath(patch),
+                "--no-backup-if-mismatch",
+            ]
+            self.run_process(
+                args=script,
+                log_name=script,
+            )