Implemented an option to write file overwrite diffs during a copy build step to a file, and warnings on identical file overwrites

Ticket: ENT-13116

Ticket: ENT-13117
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>

Author: jakub-nt

cfbs/args.py

@@ -27,6 +27,7 @@ class ArgsTypesNamespace(argparse.Namespace):
     git_user_email = None  # type: Optional[str]
     git_commit_message = None  # type: Optional[str]
     ignore_versions_json = False  # type: bool
+    diffs = None  # type: Optional[str]
     omit_download = False  # type: bool
     check_against_git = False  # type: bool
     minimum_version = None  # type: Optional[str]
@@ -161,6 +162,13 @@ def get_arg_parser(whitespace_for_manual=False):
         help="Ignore versions.json. Necessary in case of a custom index or testing changes to the default index.",
         action="store_true",
     )
+    parser.add_argument(
+        "--diffs",
+        help="Write diffs of files overwritten with a copy build step during 'cfbs build' to the specified file",
+        nargs="?",
+        const="diffs.txt",
+        default=None,
+    )
     parser.add_argument(
         "--omit-download",
         help="Use existing masterfiles instead of downloading in 'cfbs generate-release-information'",

cfbs/build.py

@@ -17,13 +17,16 @@
 from cfbs.utils import (
     canonify,
     cp,
+    cp_dry_itemize,
     deduplicate_def_json,
+    file_diff_text,
     find,
     merge_json,
     mkdir,
     pad_right,
     read_json,
     rm,
+    save_file,
     sh,
     strip_left,
     touch,
@@ -129,8 +132,68 @@ def _perform_copy_step(args, source, destination, prefix):
         dst = ""
     print("%s copy '%s' 'masterfiles/%s'" % (prefix, src, dst))
     src, dst = os.path.join(source, src), os.path.join(destination, dst)
+
+    step_diffs_data = ""
+
+    itemization = cp_dry_itemize(src, dst)
+    noop_overwrites_relpaths = []
+    actual_overwrites_relpaths = []
+    for item_string, file_relpath in itemization:
+        if item_string[1] != "f":
+            # only consider regular files
+            continue
+        if item_string[0] == "." or len(item_string) < 3:
+            # the first character being a dot means that it's a no-op overwrite (possibly except file attributes)
+            # explanation for the `< 3` comparison:
+            # if all attributes are unchanged, the rsync item string will use spaces instead of dots and they will have been parsed away earlier
+            noop_overwrites_relpaths.append(file_relpath)
+            continue
+        if item_string[2] == "+":
+            # the copied file is new
+            continue
+        if item_string[2] == "c":
+            # the copied file has a different checksum
+            actual_overwrites_relpaths.append(file_relpath)
+            continue
+        elif item_string[2] == ".":
+            # the copied regular file doesn't have a different checksum
+            noop_overwrites_relpaths.append(file_relpath)
+        log.debug("Novel item string: %s %s" % (item_string, file_relpath))
+    for file_relpath in actual_overwrites_relpaths:
+        if os.path.isfile(src):
+            fileA = src
+        else:
+            fileA = os.path.join(src, file_relpath)
+        if os.path.isfile(dst):
+            fileB = dst
+        else:
+            fileB = os.path.join(dst, file_relpath)
+        file_diff_data = file_diff_text(fileA, fileB)
+        step_diffs_data += file_diff_data
+    if len(noop_overwrites_relpaths) > 0:
+        warning_message = (
+            "Identical file overwrites occured during copy.\n"
+            + " Check your modules and their build steps to ascertain whether this is intentional.\n"
+            + " In most cases, the cause is a file from a latter module already being provided by an earlier module (commonly stock masterfiles).\n"
+            + " In that case, the file is best deleted from the latter module(s).\n"
+            + " Identical overwrites count: %s\n" % len(noop_overwrites_relpaths)
+        )
+        # display affected files, without flooding the output
+        if len(noop_overwrites_relpaths) < 20:
+            for overwrite_noop in noop_overwrites_relpaths:
+                warning_message += "  " + overwrite_noop + "\n"
+        else:
+            for overwrite_noop in noop_overwrites_relpaths[:9]:
+                warning_message += "  " + overwrite_noop + "\n"
+            warning_message += "   ...\n"
+            for overwrite_noop in noop_overwrites_relpaths[-9:]:
+                warning_message += "  " + overwrite_noop + "\n"
+        # display all the messages as one warning
+        log.warning(warning_message)
     cp(src, dst)
 
+    return step_diffs_data
+
 
 def _perform_run_step(args, source, prefix):
     shell_command = " ".join(args)
@@ -316,7 +379,7 @@ def _perform_replace_version_step(module, i, args, name, destination, prefix):
     _perform_replacement(n, to_replace, version, filename)
 
 
-def perform_build(config: CFBSConfig) -> int:
+def perform_build(config: CFBSConfig, diffs_filename=None) -> int:
     if not config.get("build"):
         raise CFBSExitError("No 'build' key found in the configuration")
 
@@ -349,6 +412,8 @@ def perform_build(config: CFBSConfig) -> int:
                         % (step, expected, actual)
                     )
 
+    diffs_data = ""
+
     print("\nSteps:")
     max_length = config.longest_module_key_length("name")
     for module in config["build"]:
@@ -362,7 +427,8 @@ def perform_build(config: CFBSConfig) -> int:
             prefix = "%03d %s :" % (counter, pad_right(name, max_length))
 
             if operation == "copy":
-                _perform_copy_step(args, source, destination, prefix)
+                step_diffs_data = _perform_copy_step(args, source, destination, prefix)
+                diffs_data += step_diffs_data
             elif operation == "run":
                 _perform_run_step(args, source, prefix)
             elif operation == "delete":
@@ -386,6 +452,14 @@ def perform_build(config: CFBSConfig) -> int:
                     module, i, args, name, destination, prefix
                 )
 
+    try:
+        save_file(diffs_filename, diffs_data)
+    except IsADirectoryError:
+        print("")
+        log.warning(
+            "An existing directory was provided as the '--diffs' file path - writing the diffs file for the build failed - continuing build..."
+        )
+
     assert os.path.isdir("./out/masterfiles/")
     shutil.copyfile("./cfbs.json", "./out/masterfiles/cfbs.json")
     if os.path.isfile("out/masterfiles/def.json"):

cfbs/cfbs.1

@@ -1,9 +1,9 @@
-.TH CFBS "1" "2025\-08\-14" "cfbs" "CFEngine Build System manual"
+.TH CFBS "1" "2025\-10\-03" "cfbs" "CFEngine Build System manual"
 .SH NAME
 cfbs \- combines multiple modules into 1 policy set to deploy on your infrastructure. Modules can be custom promise types, JSON files which enable certain functionality, or reusable CFEngine policy. The modules you use can be written by the CFEngine team, others in the community, your colleagues, or yourself.
 .SH SYNOPSIS
 .B cfbs
-[-h] [--loglevel LOGLEVEL] [-M] [--version] [--force] [--non-interactive] [--index INDEX] [--check] [--checksum CHECKSUM] [--keep-order] [--git {yes,no}] [--git-user-name GIT_USER_NAME] [--git-user-email GIT_USER_EMAIL] [--git-commit-message GIT_COMMIT_MESSAGE] [--ignore-versions-json] [--omit-download] [--check-against-git] [--from MINIMUM_VERSION] [--to-json [TO_JSON]] [--reference-version REFERENCE_VERSION] [--masterfiles-dir MASTERFILES_DIR] [--ignored-path-components [IGNORED_PATH_COMPONENTS ...]] [--offline] [--masterfiles MASTERFILES] [cmd] [args ...]
+[-h] [--loglevel LOGLEVEL] [-M] [--version] [--force] [--non-interactive] [--index INDEX] [--check] [--checksum CHECKSUM] [--keep-order] [--git {yes,no}] [--git-user-name GIT_USER_NAME] [--git-user-email GIT_USER_EMAIL] [--git-commit-message GIT_COMMIT_MESSAGE] [--ignore-versions-json] [--diffs [DIFFS]] [--omit-download] [--check-against-git] [--from MINIMUM_VERSION] [--to-json [TO_JSON]] [--reference-version REFERENCE_VERSION] [--masterfiles-dir MASTERFILES_DIR] [--ignored-path-components [IGNORED_PATH_COMPONENTS ...]] [--offline] [--masterfiles MASTERFILES] [cmd] [args ...]
 .SH DESCRIPTION
 CFEngine Build System.
 
@@ -72,6 +72,10 @@ Specify git commit message
 \fB\-\-ignore\-versions\-json\fR
 Ignore versions.json. Necessary in case of a custom index or testing changes to the default index.
 
+.TP
+\fB\-\-diffs\fR \fI\,[DIFFS]\/\fR
+Write diffs of files overwritten with a copy build step during 'cfbs build' to the specified file
+
 .TP
 \fB\-\-omit\-download\fR
 Use existing masterfiles instead of downloading in 'cfbs generate\-release\-information'

cfbs/commands.py

@@ -902,7 +902,7 @@ def download_command(force, ignore_versions=False):
 
 
 @cfbs_command("build")
-def build_command(ignore_versions=False):
+def build_command(ignore_versions=False, diffs_filename=None):
     config = CFBSConfig.get_instance()
     r = validate_config(config)
     if r != 0:
@@ -915,7 +915,7 @@ def build_command(ignore_versions=False):
         # so we try building anyway and don't return error(s)
     init_out_folder()
     _download_dependencies(config, ignore_versions=ignore_versions)
-    r = perform_build(config)
+    r = perform_build(config, diffs_filename)
     return r
 
 

cfbs/main.py

@@ -138,6 +138,11 @@ def _main() -> int:
             % args.command
         )
 
+    if args.diffs and args.command != "build":
+        raise CFBSUserError(
+            "The option --diffs is only for 'cfbs build', not 'cfbs %s'" % args.command
+        )
+
     if args.non_interactive and args.command not in (
         "init",
         "add",
@@ -213,7 +218,9 @@ def _main() -> int:
     if args.command == "download":
         return commands.download_command(args.force)
     if args.command == "build":
-        return commands.build_command(ignore_versions=args.ignore_versions_json)
+        return commands.build_command(
+            ignore_versions=args.ignore_versions_json, diffs_filename=args.diffs
+        )
     if args.command == "install":
         return commands.install_command(args.args)
     if args.command == "update":

cfbs/utils.py

@@ -1,3 +1,5 @@
+import difflib
+import logging as log
 import os
 import re
 import sys
@@ -10,7 +12,7 @@
 import urllib.error
 from collections import OrderedDict
 from shutil import rmtree
-from typing import Iterable, Union
+from typing import Iterable, List, Tuple, Union
 
 from cfbs.pretty import pretty
 
@@ -111,6 +113,24 @@ def display_diff(path_A, path_B):
         raise
 
 
+def file_diff(filepath_A, filepath_B):
+    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)
+
+    return u_diff
+
+
+def file_diff_text(filepath_A, filepath_B):
+    u_diff = file_diff(filepath_A, filepath_B)
+    diff_text = "".join(u_diff)
+
+    return diff_text
+
+
 def mkdir(path: str, exist_ok=True):
     os.makedirs(path, exist_ok=exist_ok)
 
@@ -147,6 +167,40 @@ def cp(src, dst):
     sh("rsync -r %s/ %s" % (src, dst))
 
 
+def cp_dry_itemize(src: str, dst: str) -> List[Tuple[str, str]]:
+    above = os.path.dirname(dst)
+    if not os.path.exists(above):
+        mkdir(above)
+    if dst.endswith("/") and not os.path.exists(dst):
+        mkdir(dst)
+    if os.path.isfile(src):
+        result = subprocess.run(
+            "rsync -rniic %s %s" % (src, dst),
+            shell=True,
+            check=True,
+            stdout=subprocess.PIPE,
+        )
+    else:
+        result = subprocess.run(
+            "rsync -rniic %s/ %s" % (src, dst),
+            shell=True,
+            check=True,
+            stdout=subprocess.PIPE,
+        )
+    lines = result.stdout.decode("utf-8").split("\n")
+    itemization = []
+    for line in lines:
+        terms = line.split()
+        if len(terms) == 2:
+            item_string = terms[0]
+            file_relpath = terms[1]
+            itemization.append((item_string, file_relpath))
+        elif len(terms) > 0:
+            log.debug("Abnormal rsync output line: '%s'" % terms)
+
+    return itemization
+
+
 def pad_left(s, n):
     return s.rjust(n)