Merge pull request #280 from jakub-nt/CFE-4560

CFE-4560: Implement Step 3 of `cfbs convert`

Author: olehermanse

cfbs/commands.py

@@ -60,12 +60,15 @@ def search_command(terms: List[str]):
 from cfbs.cfbs_json import CFBSJson
 from cfbs.cfbs_types import CFBSCommandExitCode, CFBSCommandGitResult
 from cfbs.masterfiles.analyze import most_relevant_version
+from cfbs.masterfiles.download import download_single_version
 from cfbs.updates import ModuleUpdates, update_module
 from cfbs.utils import (
     CFBSNetworkError,
     CFBSUserError,
     CFBSValidationError,
+    cfbs_dir,
     cfbs_filename,
+    display_diff,
     is_cfbs_repo,
     read_json,
     CFBSExitError,
@@ -1131,7 +1134,7 @@ def analyze_command(
 @cfbs_command("convert")
 def convert_command(non_interactive=False, offline=False):
     def cfbs_convert_cleanup():
-        os.unlink(cfbs_filename())
+        rm(cfbs_filename(), missing_ok=True)
         rm(".git", missing_ok=True)
 
     def cfbs_convert_git_commit(
@@ -1172,14 +1175,18 @@ def cfbs_convert_git_commit(
     )
 
     print("Analyzing '" + path_string + "'...")
-    analyzed_files, _ = analyze_policyset(
-        path=dir_name,
-        is_parentpath=False,
-        reference_version=None,
-        masterfiles_dir=dir_name,
-        ignored_path_components=None,
-        offline=offline,
-    )
+    try:
+        analyzed_files, _ = analyze_policyset(
+            path=dir_name,
+            is_parentpath=False,
+            reference_version=None,
+            masterfiles_dir=dir_name,
+            ignored_path_components=None,
+            offline=offline,
+        )
+    except:
+        print("Analyzing the policy set failed, aborting conversion.")
+        raise
 
     current_index = CFBSConfig.get_instance().index
     default_version = current_index.get_module_object("masterfiles")["version"]
@@ -1205,7 +1212,20 @@ def cfbs_convert_git_commit(
     print("Initializing a new CFBS project...")
     # since there should be no other files than the masterfiles-name directory, there shouldn't be a .git directory
     assert not is_git_repo()
-    r = init_command(masterfiles="no", non_interactive=non_interactive, use_git=True)
+    try:
+        r = init_command(
+            masterfiles="no", non_interactive=non_interactive, use_git=True
+        )
+    except CFBSGitError:
+        cfbs_convert_cleanup()
+        print(
+            "A Git operation failed during initialization of a new CFBS project, aborting conversion."
+        )
+        raise
+    except:
+        print("Initializing a new CFBS project failed, aborting conversion.")
+        cfbs_convert_cleanup()
+        raise
     # the cfbs-init should've created a Git repository
     assert is_git_repo()
     if r != 0:
@@ -1215,19 +1235,33 @@ def cfbs_convert_git_commit(
 
     print("Adding masterfiles %s to the project..." % masterfiles_version)
     masterfiles_to_add = ["masterfiles@%s" % masterfiles_version]
-    r = add_command(masterfiles_to_add, added_by="cfbs convert")
+    try:
+        r = add_command(masterfiles_to_add, added_by="cfbs convert")
+    except:
+        print(
+            "Adding the masterfiles module to the project failed, aborting conversion."
+        )
+        cfbs_convert_cleanup()
+        raise
     if r != 0:
-        print("Adding the masterfiles module failed, aborting conversion.")
+        print(
+            "Adding the masterfiles module to the project failed, aborting conversion."
+        )
         cfbs_convert_cleanup()
         return r
 
     print("Adding the policy files...")
     local_module_to_add = [path_string]
-    r = add_command(
-        local_module_to_add,
-        added_by="cfbs convert",
-        explicit_build_steps=["copy ./ ./"],
-    )
+    try:
+        r = add_command(
+            local_module_to_add,
+            added_by="cfbs convert",
+            explicit_build_steps=["copy ./ ./"],
+        )
+    except:
+        print("Adding the policy files module failed, aborting conversion.")
+        cfbs_convert_cleanup()
+        raise
     if r != 0:
         print("Adding the policy files module failed, aborting conversion.")
         cfbs_convert_cleanup()
@@ -1285,7 +1319,7 @@ def cfbs_convert_git_commit(
         if prompt_user_yesno(
             non_interactive, "Delete files from other versions? (Recommended)"
         ):
-            print("Deleting %s files." % len(files_to_delete))
+            print("Deleting %s files..." % len(files_to_delete))
             for file_d in files_to_delete:
                 rm(os.path.join(dir_name, file_d))
 
@@ -1299,7 +1333,71 @@ def cfbs_convert_git_commit(
     print(
         "The next conversion step is to handle files which have custom modifications."
     )
-    print("This is not implemented yet.")
+    if not prompt_user_yesno(non_interactive, "Do you want to continue?"):
+        raise CFBSExitError("User did not proceed, exiting.")
+    print("The following files have custom modifications:")
+    modified_files = analyzed_files.modified
+    for modified_file in modified_files:
+        print("-", modified_file)
+    for i, modified_file in enumerate(modified_files, start=1):
+        # program failures in the middle of this loop would be very user-unfriendly,
+        # so we will catch exceptions and continue the conversion when handling errors
+        print("\nFile", i, "diff -", modified_file + ":")
+        mpf_dir_path = os.path.join(cfbs_dir(), "masterfiles")
+        mpf_version_dir_path = os.path.join(
+            mpf_dir_path, masterfiles_version, "tarball", "masterfiles"
+        )
+        mpf_filepath = os.path.join(mpf_version_dir_path, modified_file)
+        display_diffs = True
+        if not os.path.exists(mpf_version_dir_path):
+            try:
+                download_single_version(mpf_dir_path, masterfiles_version)
+            except Exception as e:
+                print(
+                    "Downloading original masterfiles failed (%s), continuing conversion without displaying file diffs."
+                    % str(e)
+                )
+                display_diffs = False
+        if display_diffs:
+            try:
+                display_diff(mpf_filepath, os.path.join(dir_name, modified_file))
+            except:
+                log.warning(
+                    "Displaying a diff between your file and the default file failed, continuing without displaying a diff..."
+                )
+        if i == 1:
+            if display_diffs:
+                print(
+                    "Above you can see the differences between your file and the default file."
+                )
+            print(
+                "As much as possible, we recommend getting rid of these custom modifications."
+            )
+            print(
+                "Usually, the same thing can be achieved by adding a variable to def.json, or through adding your own policy file (inside 'services/')."
+            )
+        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"
+
+        response = prompt_user(non_interactive, prompt_str, ["1", "2"], "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)
+            try:
+                cfbs_convert_git_commit(commit_message)
+            except:
+                log.warning("Git commit failed, continuing without committing...")
+        if response == "2":
+            print("Keeping file as is, nothing to do.")
+
+    print("Conversion finished successfully.")
+
     return 0
 
 

cfbs/git.py

@@ -49,7 +49,7 @@ def ls_remote(remote, branch):
 
 
 def is_git_repo(path=None):
-    """Is the given path a Git repository?)
+    """Is the given path a Git repository?
 
     :param:`path` defaults to CWD (if `None`)
 

cfbs/main.py

@@ -145,6 +145,7 @@ def _main() -> int:
         "clean",
         "update",
         "input",
+        "convert",
     ):
         raise CFBSUserError(
             "The option --non-interactive is not for cfbs %s" % (args.command)