diff --git a/JSON.md b/JSON.md index 332ebe26..4333293e 100644 --- a/JSON.md +++ b/JSON.md @@ -281,6 +281,10 @@ In `cfbs.json`'s `"steps"`, the build step name must be separated from the rest - `replace_version ` - Replace the string inside the file with the version number of that module. - Works identically to `replace`, except the string to replace with, i.e. the version number, is found automatically. +- `patch ` + - Apply a unified diff patch file. + The patches are applied to files in `out/masterfiles` during build. + Paths inside the .patch file should be given relative to `out/masterfiles`. When `def.json` is modified during a `json`, `input`, `directory`, `bundles`, or `policy_files` build step, the values of some lists of strings are deduplicated, when this does not make any difference in behavior. These cases are: diff --git a/cfbs/build.py b/cfbs/build.py index 03276f30..06f4db89 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -13,9 +13,12 @@ import os import logging as log import shutil +import subprocess from cfbs.cfbs_config import CFBSConfig from cfbs.utils import ( + CFBSUserError, canonify, + cli_tool_present, cp, cp_dry_overwrites, deduplicate_def_json, @@ -126,6 +129,27 @@ def _perform_replacement(n, a, b, filename): raise CFBSExitError("Failed to write to '%s'" % (filename,)) +def _apply_masterfiles_patch(patch_path): + if not cli_tool_present("patch"): + raise CFBSUserError("Working with .patch files requires the 'patch' utility") + + if not os.path.isfile(patch_path): + raise CFBSExitError("Patch at path '%s' not found" % patch_path) + + patch_path = os.path.relpath(patch_path, "out/masterfiles") + + # reasoning for used flags: + # * `-t`: do not interactively ask the user for another path if the patch fails to apply due to the path not being found + # * `-p0`: use paths in the form specified in the .patch files + cmd = "patch -u -t -p0 -i" + patch_path + # the cwd needs to be the base path of the relative paths specified in the .patch files + # currently, the output of the patch command is displayed + cp = subprocess.run(cmd, shell=True, cwd="out/masterfiles") + + if cp.returncode != 0: + raise CFBSExitError("Failed to apply patch '%s'" % patch_path) + + def _perform_copy_step(args, source, destination, prefix): src, dst = args if dst in [".", "./"]: @@ -358,6 +382,19 @@ def _perform_replace_version_step(module, i, args, name, destination, prefix): _perform_replacement(n, to_replace, version, filename) +def _perform_patch_step(module, i, args, name, source, prefix): + assert len(args) == 1 + + patch_relpath = args[0] + print("%s patch '%s'" % (prefix, patch_relpath)) + # New build step so let's be a bit strict about validating it: + validate_build_step(name, module, i, "patch", args, strict=True) + + patch_path = os.path.join(source, patch_relpath) + + _apply_masterfiles_patch(patch_path) + + def perform_build(config: CFBSConfig, diffs_filename=None) -> int: if not config.get("build"): raise CFBSExitError("No 'build' key found in the configuration") @@ -430,6 +467,8 @@ def perform_build(config: CFBSConfig, diffs_filename=None) -> int: _perform_replace_version_step( module, i, args, name, destination, prefix ) + elif operation == "patch": + _perform_patch_step(module, i, args, name, source, prefix) if diffs_filename is not None: try: diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 7d0e5565..bec6b3e1 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -24,6 +24,7 @@ from cfbs.cfbs_types import CFBSCommandGitResult from cfbs.utils import ( CFBSExitError, + CFBSProgrammerError, CFBSUserError, is_a_commit_hash, read_file, @@ -264,6 +265,25 @@ def _find_dependencies(self, modules, exclude): dependencies += self._find_dependencies(dependencies, exclude) return dependencies + def _module_by_name(self, name): + for module in self["build"]: + if module["name"] == name: + return module + + raise CFBSProgrammerError("Module of name %s not found" % name) + + def add_patch_step(self, module_name, patch_filepath): + """Adds a patch step to a module's `"steps"` list, and saves the `CFBSConfig`. + + API note: currently only used for a local module. + + Error handling: + * excepts when module of `module_name` is not present.""" + module = self._module_by_name(module_name) + step = "patch " + patch_filepath + module["steps"].append(step) + self.save() + def _add_policy_files_build_step(self, module): name = module["name"] step = "policy_files services/cfbs/" + ( diff --git a/cfbs/commands.py b/cfbs/commands.py index b98a797f..5ed4ed09 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -69,9 +69,12 @@ def search_command(terms: List[str]): cfbs_dir, cfbs_filename, display_diff, + file_diff_text, is_cfbs_repo, + mkdir, read_json, CFBSExitError, + save_file, strip_right, pad_right, CFBSProgrammerError, @@ -1127,6 +1130,7 @@ def cfbs_convert_cleanup(): def cfbs_convert_git_commit( commit_message: str, add_scope: Union[str, Iterable[str]] = "all" ): + print("Creating Git commit...") try: git_commit_maybe_prompt(commit_message, non_interactive, scope=add_scope) except CFBSGitError: @@ -1263,7 +1267,6 @@ def cfbs_convert_git_commit( for unmodified_mpf_file in analyzed_files.unmodified: rm(os.path.join(dir_name, unmodified_mpf_file)) - print("Creating Git commit...") cfbs_convert_git_commit("Deleted unmodified policy files") print( @@ -1310,9 +1313,6 @@ def cfbs_convert_git_commit( for file_d in files_to_delete: rm(os.path.join(dir_name, file_d)) - print( - "Creating Git commit with deletion of policy files from other versions..." - ) cfbs_convert_git_commit("Deleted policy files from other versions") print("Done.", end=" ") else: @@ -1322,6 +1322,8 @@ def cfbs_convert_git_commit( ) if not prompt_user_yesno(non_interactive, "Do you want to continue?"): raise CFBSExitError("User did not proceed, exiting.") + + first_patch_conversion = True print("The following files have custom modifications:") modified_files = analyzed_files.modified for modified_file in modified_files: @@ -1335,6 +1337,7 @@ def cfbs_convert_git_commit( mpf_dir_path, masterfiles_version, "tarball", "masterfiles" ) mpf_filepath = os.path.join(mpf_version_dir_path, modified_file) + modified_file_path = os.path.join(dir_name, modified_file) display_diffs = True if not os.path.exists(mpf_version_dir_path): try: @@ -1347,7 +1350,7 @@ def cfbs_convert_git_commit( display_diffs = False if display_diffs: try: - display_diff(mpf_filepath, os.path.join(dir_name, modified_file)) + display_diff(mpf_filepath, modified_file_path) except: log.warning( "Displaying a diff between your file and the default file failed, continuing without displaying a diff..." @@ -1366,22 +1369,85 @@ def cfbs_convert_git_commit( prompt_str = "\nChoose an option:\n" prompt_str += "1) Drop modifications - They are not important, or can be achieved in another way.\n" prompt_str += "2) Keep modified file - File is kept as is, and can be handled later. Can make future upgrades more complicated.\n" - prompt_str += "3) (Not implemented yet) Keep patch file - " - prompt_str += "File is converted into a patch file (diff) that hopefully will apply to future versions as well.\n" + prompt_str += "3) Keep patch file - File is converted into a patch file (diff) that hopefully will apply to future versions as well.\n" - response = prompt_user(non_interactive, prompt_str, ["1", "2"], "1") + response = prompt_user(non_interactive, prompt_str, ["1", "2", "3"], "1") if response == "1": print("Deleting './%s'..." % modified_file) - rm(os.path.join(dir_name, modified_file)) - commit_message = "Deleted './%s'" % modified_file - print("Creating Git commit - %s..." % commit_message) + rm(modified_file_path) try: - cfbs_convert_git_commit(commit_message) + cfbs_convert_git_commit("Deleted './%s'" % modified_file) except: log.warning("Git commit failed, continuing without committing...") if response == "2": print("Keeping file as is, nothing to do.") + if response == "3": + print("Converting './%s' into a patch file..." % modified_file) + patches_dir = "custom-masterfiles-patches" + patches_module = "./" + patches_dir + "/" + + file_diff_data = file_diff_text( + mpf_filepath, + modified_file_path, + modified_file, + modified_file, + ) + + patch_filename = modified_file.replace("/", "_") + ".patch" + patch_path = os.path.join(patches_dir, patch_filename) + # append a number if there are multiple files with the same filename + i = 1 + while os.path.exists(patch_path): + patch_filename = ( + modified_file.replace("/", "_") + "-" + str(i) + ".patch" + ) + patch_path = os.path.join(patches_dir, patch_filename) + i += 1 + + # saving the .patch file should be done before creating the patches module + try: + save_file(patch_path, file_diff_data) + except: + log.warning( + "Saving the patch file failed - keeping the modified file as is instead and continuing..." + ) + continue + + # make the patches local module on first use + if first_patch_conversion: + print("Adding patches local module...") + mkdir(patches_dir) + + config = CFBSConfig.get_instance() + # `explicit_build_steps=[]` would fail validation, therefore also add the first build step during the module's creation + config.add_command( + [patches_module], + added_by="cfbs convert", + explicit_build_steps=["patch " + patch_filename], + ) + config.save() + else: + config = CFBSConfig.get_instance() + config.add_patch_step(patches_module, patch_filename) + + print("Deleting './%s'..." % modified_file) + rm(modified_file_path) + + try: + if first_patch_conversion: + cfbs_convert_git_commit( + "Added patches local module, converted './%s' into a .patch file" + % modified_file + ) + else: + cfbs_convert_git_commit( + "Converted './%s' into a .patch file" % modified_file + ) + except: + log.warning("Git commit failed, continuing without committing...") + + first_patch_conversion = False print("Conversion finished successfully.") diff --git a/cfbs/git.py b/cfbs/git.py index d9ea07a3..d79b1205 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -15,7 +15,7 @@ from typing import Iterable, Union from cfbs.prompts import prompt_user -from cfbs.utils import CFBSExitError, are_paths_equal +from cfbs.utils import CFBSExitError, are_paths_equal, cli_tool_present class CFBSGitError(Exception): @@ -23,11 +23,7 @@ class CFBSGitError(Exception): def git_exists(): - try: - check_call(["git", "--version"], stdout=DEVNULL) - return True - except FileNotFoundError: - return False + return cli_tool_present("git") def ls_remote(remote, branch): diff --git a/cfbs/utils.py b/cfbs/utils.py index abb9afc7..615b2b03 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -103,6 +103,17 @@ def sh(cmd: str, directory=None): return _sh(cmd) +def cli_tool_present(name: str): + """Returns whether the CLI tool `name` exists on the system.""" + try: + subprocess.check_call( + ["command -v " + name], stdout=subprocess.DEVNULL, shell=True + ) + return True + except subprocess.CalledProcessError: + return False + + def display_diff(path_A, path_B): """Also displays `stderr`.""" cmd = "diff -u " + path_A + " " + path_B @@ -112,19 +123,24 @@ def display_diff(path_A, path_B): raise -def file_diff(filepath_A, filepath_B): +def file_diff(filepath_A, filepath_B, target_A=None, target_B=None): with open(filepath_A) as f: lines_A = f.readlines() with open(filepath_B) as f: lines_B = f.readlines() - u_diff = difflib.unified_diff(lines_A, lines_B, filepath_A, filepath_B) + if target_A is None: + target_A = filepath_A + if target_B is None: + target_B = filepath_B + + u_diff = difflib.unified_diff(lines_A, lines_B, target_A, target_B) return u_diff -def file_diff_text(filepath_A, filepath_B): - u_diff = file_diff(filepath_A, filepath_B) +def file_diff_text(filepath_A, filepath_B, target_A=None, target_B=None): + u_diff = file_diff(filepath_A, filepath_B, target_A, target_B) diff_text = "".join(u_diff) return diff_text diff --git a/cfbs/validate.py b/cfbs/validate.py index 099d5ccd..15ff5755 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -57,6 +57,7 @@ "bundles": "1+", "replace": 4, # n, a, b, filename "replace_version": 3, # n, string to replace, filename + "patch": 1, } # Constants / regexes / limits for validating build steps: @@ -356,6 +357,12 @@ def validate_build_step(name, module, i, operation, args, strict=False): validate_build_step( name, module, i, "replace", [n, to_replace, version, filename], strict ) + elif operation == "patch": + # TODO: consider what this should validate (CFE-4607): + # * perhaps check that the patch file exists here instead of in `apply_patch`? + # * what kinds of paths should be legal? + # * more validation? + pass else: # TODO: Add more validation of other build steps. pass