0.11.0 --explain flag and some minor improvements

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 0.11.0 2022-04-24
+
+- Fix `instater_dir` template variable to be a proper absolute path
+  (previously, relative paths could appear like `/path/to/cwd/../another_dir/setup.yml`)
+- Add `--explain` flag to show reasoning for each changed/skipped task
+- Do not run the pacman package comparison when specifying `--tags`,
+  since in that situation the comparison is not complete
+
 ## 0.10.0 2022-04-13
 
 - Add `fetch_tags` argument to `git` task

instater/VERSION

@@ -1 +1 @@
-0.10.0
+0.11.0

instater/cli.py

@@ -48,6 +48,11 @@ def main():
         action="store_true",
         help="Display operations that would be performed without actually running them",
     )
+    parser.add_argument(
+        "--explain",
+        action="store_true",
+        help="Include messages for each task explaining why the task was changed or skipped",
+    )
     parser.add_argument("--version", action="store_true", help="Display the version of instater")
 
     args = parser.parse_args()
@@ -60,7 +65,14 @@ def main():
 
     try:
         variables = _parse_variables(args.vars)
-        run_tasks(args.setup_file, variables, tags, args.dry_run, skip_tasks=args.skip_tasks)
+        run_tasks(
+            setup_file=args.setup_file,
+            override_variables=variables,
+            tags=tags,
+            dry_run=args.dry_run,
+            explain=args.explain,
+            skip_tasks=args.skip_tasks,
+        )
     except InstaterError as e:
         console = Console()
         console.print(e, style="red")

instater/context.py

@@ -24,12 +24,20 @@ def _jinja_environment(root_directory: Path) -> Environment:
 
 
 class Context:
-    def __init__(self, root_directory: Path, extra_vars: dict, tags: Iterable[str], dry_run: bool = False):
+    def __init__(
+        self,
+        root_directory: Path,
+        extra_vars: dict,
+        tags: Iterable[str],
+        dry_run: bool = False,
+        explain: bool = False,
+    ):
         self.root_directory = root_directory
         self.tags = set(tags)
         self.dry_run = dry_run
+        self.explain = explain
 
-        extra_vars["instater_dir"] = str(root_directory.absolute())
+        extra_vars["instater_dir"] = str(root_directory.resolve())
         self.variables = extra_vars
 
         self.jinja_env = _jinja_environment(root_directory)
@@ -79,3 +87,22 @@ def print_summary(self):
         self.print(f"  skipped: {skipped}", style="blue")
         # if this used style="yellow", the integer count would be turned blue by rich
         self.print(f"  [yellow]changed: {changed}[/yellow]")
+
+    def explain_skip(self, message: str):
+        if self.explain:
+            self.print(message + "\n", style="blue")
+
+    def explain_change(self, message: str):
+        if self.explain:
+            if self.dry_run:
+                message = "[dry_run] " + message
+
+            self.print(message + "\n", style="yellow bold")
+
+    def explain_change_diff(self, a: str, b: str, file_a: str, file_b: str):
+        if self.explain:
+            diff = util.diff_lines(a, b, file_a, file_b)
+            if self.dry_run:
+                diff = "[dry_run]\n" + diff
+
+            self.print(diff + "\n")

instater/importer.py

@@ -17,7 +17,7 @@ def import_submodules(module_name: str):
 
     module = sys.modules[module_name]
 
-    path = Path(module.__file__).parent.absolute()
+    path = Path(module.__file__).parent.resolve()
     prefix = module.__name__
     paths = [str(path)]
 

instater/main.py

@@ -191,35 +191,6 @@ def _include(context: Context, parent_tags: List[str], include: str, tags: Union
     _load_tasks(tasks, context, tags)
 
 
-def _load_context(
-    setup_file,
-    override_variables: dict = None,
-    tags: Iterable[str] = None,
-    dry_run: bool = False,
-) -> Context:
-    setup_file = Path(setup_file)
-    context = Context(setup_file.parent, override_variables or {}, tags or (), dry_run)
-
-    if not setup_file.exists():
-        raise InstaterError(f"Setup file does not exist: {setup_file}")
-
-    _print_start(context, setup_file)
-
-    with setup_file.open() as f:
-        setup_data = yaml.safe_load(f)
-
-    if isinstance(setup_data, list):
-        if len(setup_data) > 1:
-            raise InstaterError(f"Cannot specify multiple root list items in {setup_file}")
-        setup_data = setup_data[0]
-
-    _prompt_variables(setup_data.get("vars_prompt"), context)
-    _file_variables(setup_data.get("vars_files"), context)
-    _load_tasks(setup_data.get("tasks"), context)
-
-    return context
-
-
 def _alert_pacman_manually_installed(context: Context):
     packages = set()
 
@@ -246,9 +217,28 @@ def run_tasks(
     override_variables: dict = None,
     tags: Iterable[str] = None,
     dry_run: bool = False,
+    explain: bool = False,
     skip_tasks: bool = False,
 ):
-    context = _load_context(setup_file, override_variables, tags, dry_run)
+    setup_file = Path(setup_file)
+    context = Context(setup_file.parent, override_variables or {}, tags or (), dry_run, explain)
+
+    if not setup_file.exists():
+        raise InstaterError(f"Setup file does not exist: {setup_file}")
+
+    _print_start(context, setup_file)
+
+    with setup_file.open() as f:
+        setup_data = yaml.safe_load(f)
+
+    if isinstance(setup_data, list):
+        if len(setup_data) > 1:
+            raise InstaterError(f"Cannot specify multiple root list items in {setup_file}")
+        setup_data = setup_data[0]
+
+    _prompt_variables(setup_data.get("vars_prompt"), context)
+    _file_variables(setup_data.get("vars_files"), context)
+    _load_tasks(setup_data.get("tasks"), context)
 
     if not skip_tasks:
         for task in context.tasks:
@@ -257,7 +247,8 @@ def run_tasks(
 
     context.print_summary()
 
-    if shutil.which("pacman"):
+    # Don't run this check when a subset of tags were passed in, since not all tasks are loaded
+    if not tags and shutil.which("pacman"):
         _alert_pacman_manually_installed(context)
 
     return context

instater/tasks/_task.py

@@ -27,6 +27,7 @@ def run_task(self, context: Context) -> bool:
         start = time.time()
 
         if self.when and context.jinja_string("{{ (" + self.when + ") | bool }}") == "False":
+            context.explain_skip(f"when condition failed: {self.when}")
             changed = False
         else:
             changed = self.run_action(context)

instater/tasks/command.py

@@ -28,13 +28,24 @@ def __init__(
 
     def run_action(self, context: Context) -> bool:
         if self.condition:
-            # TODO: only consider specific return codes as valid?
             result = util.shell(self.condition, self.directory, valid_return_codes=None)
             if result.return_code != self.condition_code:
+                context.explain_skip(
+                    f"Condition command returned with code {result.return_code}, "
+                    f"required return code {self.condition_code}"
+                )
                 return False
 
-        if not context.dry_run:
-            for command in self.commands:
-                util.shell(command, self.directory, become=self.become)
+        for command in self.commands:
+            explain_message = f"Running command {command}"
+            if self.directory:
+                explain_message += f" from directory '{self.directory}'"
+            if self.become:
+                explain_message += f" as user '{self.become}'"
+            context.explain_change(explain_message)
+
+            if not context.dry_run:
+                result = util.shell(command, self.directory, become=self.become)
+                context.explain_change(f"  -> {result.stdout}")
 
         return True

instater/tasks/copy.py

@@ -50,27 +50,51 @@ def __init__(
         self.validate = validate
 
     def _update_metadata(self, file: Path, context: Context) -> bool:
-        return util.update_file_metadata(file, self.owner, self.group, self.mode, context.dry_run)
+        return util.update_file_metadata(file, self.owner, self.group, self.mode, context)
 
     def _validate(self, path: Path):
         if not self.validate:
             return
 
-        validate_command = shlex.split(self.validate % path)
         # util.shell will raise an error on exit codes > 0
-        util.shell(validate_command)
+        util.shell(shlex.split(self.validate % path))
+
+    def _read_for_diff(self, path: Path):
+        if not path.exists():
+            return ""
+
+        try:
+            return _read(path).decode("utf-8")
+        except UnicodeDecodeError:
+            return None
+
+    def _explain_diff(self, src: Union[Path, str], dest: Path, context: Context, src_file=None):
+        # extra condition so we only read the file content if necessary
+        if context.explain:
+            src_content = self._read_for_diff(src) if isinstance(src, Path) else src
+            dest_content = self._read_for_diff(dest)
+
+            if src_content is None or dest_content is None:
+                context.explain_change(f"Binary files differ: {src} and {dest}")
+            else:
+                context.explain_change_diff(dest_content, src_content, str(dest), src_file or str(src))
 
     def _update_file_direct(self, src: Path, dest: Path, context: Context) -> bool:
         updated = False
 
         self._validate(src)
 
         if not dest.exists():
+            context.explain_change(f"Destination file does not exist: {dest}")
+            self._explain_diff(src, dest, context)
             if not context.dry_run:
                 dest.parent.mkdir(parents=True, exist_ok=True)
                 shutil.copy(src, dest)
+
             updated = True
         elif not src.samefile(dest) and _read(src) != _read(dest):
+            context.explain_change(f"Source file ({src}) differs from destination file ({dest})")
+            self._explain_diff(src, dest, context)
             if not context.dry_run:
                 shutil.copy(src, dest)
             updated = True
@@ -91,12 +115,14 @@ def _update_file_content(self, content: str, dest: Path, context: Context) -> bo
                 self._validate(Path(temp_file.name))
 
         if not dest.exists():
+            self._explain_diff(content, dest, context, src_file="Template")
             if not context.dry_run:
                 dest.parent.mkdir(parents=True, exist_ok=True)
                 with dest.open("w") as f:
                     f.write(content)
             updated = True
         elif content.encode("utf-8") != _read(dest):
+            self._explain_diff(content, dest, context, src_file="Template")
             if not context.dry_run:
                 with dest.open("w") as f:
                     f.write(content)
@@ -113,9 +139,14 @@ def _update_file_template(self, src: Path, dest: Path, context: Context) -> bool
 
     def _update_file(self, src: Path, dest: Path, context: Context):
         if self.is_template:
-            return self._update_file_template(src, dest, context)
+            updated = self._update_file_template(src, dest, context)
         else:
-            return self._update_file_direct(src, dest, context)
+            updated = self._update_file_direct(src, dest, context)
+
+        if not updated:
+            context.explain_skip(f"File {dest} already has the correct content and metadata")
+
+        return updated
 
     def _update_dir(self, context: Context) -> bool:
         updated = False
@@ -156,7 +187,10 @@ def run_action(self, context: Context) -> bool:
                 if self.is_template:
                     content_str = context.jinja_string(content_str)
 
-                return self._update_file_content(content_str, dest, context)
+                updated = self._update_file_content(content_str, dest, context)
+                if not updated:
+                    context.explain_skip(f"File {dest} already has the correct content and metadata")
+                return updated
         else:
             if dest.exists() and not dest.is_dir():
                 raise InstaterError(f"Destination is a file, expected directory: {dest}")

instater/tasks/file.py

@@ -65,6 +65,7 @@ def run_action(self, context: Context):
         updated = False
 
         if not self.path.exists():
+            context.explain_change(f"Path does not exist: {self.path}")
             if not context.dry_run:
                 if self.directory:
                     self._create_directory()
@@ -87,7 +88,10 @@ def run_action(self, context: Context):
             if not self.path.is_file():
                 raise InstaterError(f"Path exists but is not a file: {self.path}")
 
-        updated |= util.update_file_metadata(self.path, self.owner, self.group, self.mode, context.dry_run)
+        updated |= util.update_file_metadata(self.path, self.owner, self.group, self.mode, context)
+
+        if not updated:
+            context.explain_skip(f"Path {self.path} already is in the correct state")
 
         return updated
 

instater/tasks/git.py

@@ -30,16 +30,42 @@ def _get_remote(self) -> str:
         result = util.shell(["git", "config", "--get", "remote.origin.url"], directory=self.dest, become=self.become)
         return result.stdout
 
-    def _should_pull(self) -> bool:
-        result = util.shell(["git", "fetch", "--dry-run", self.tags_flag], directory=self.dest, become=self.become)
-        return result.stdout != "" or result.stderr != ""
+    def _should_pull(self, context: Context) -> bool:
+        # fetch_result captures when remote has changed relative to local
+        fetch_result = util.shell(
+            ["git", "fetch", "--dry-run", self.tags_flag],
+            directory=self.dest,
+            become=self.become,
+        )
+
+        if fetch_result.stdout != "":
+            context.explain_change(f"Local git repository {self.dest} is not up to date: {fetch_result.stdout}")
+            return True
+
+        # log_result captures when the remote has not changed, but
+        # a local branch needs to be fast forwarded (e.g. if the
+        # local repository has been manually reset to a previous
+        # commit, log_result will indicate that a pull should occur)
+        log_result = util.shell(
+            ["git", "log", "-1", "--format=oneline", "@..@{push}"],
+            directory=self.dest,
+            become=self.become,
+        )
+        if log_result.stdout != "":
+            commits = "\n".join(f"  - {line}" for line in log_result.stdout.splitlines())
+            commits_str = f"[white]{commits}[/white]"
+            context.explain_change(f"Local git repository {self.dest} is not up to date. New commits:\n{commits_str}")
+            return True
+
+        return False
 
     def _pull(self):
         branch = util.shell(["git", "branch", "--show-current"], directory=self.dest, become=self.become).stdout
         util.shell(["git", "pull", "origin", branch, self.tags_flag], directory=self.dest, become=self.become)
 
     def run_action(self, context: Context):
         if not self.dest.exists():
+            context.explain_change("Git repository has not yet been cloned")
             if not context.dry_run:
                 self._clone()
             return True
@@ -50,9 +76,10 @@ def run_action(self, context: Context):
         if self._get_remote() != self.repo:
             raise InstaterError(f"Git remote does not match current local git repo: {self.dest}")
 
-        if self._should_pull():
+        if self._should_pull(context):
             if not context.dry_run:
                 self._pull()
             return True
-
-        return False
+        else:
+            context.explain_skip(f"Local git repository {self.dest} is already up to date")
+            return False