Refactor template handling

Move common template logic out to a new template() function that calls one of
the existing template processors and then handles writing the result and
copying permissions.

test/test_unit_choose_template_processor.py

@@ -1,4 +1,4 @@
-"""Unit tests: choose_template_cmd"""
+"""Unit tests: choose_template_processor"""
 
 import pytest
 
@@ -8,7 +8,7 @@ import pytest
 def test_kind_default(runner, yadm, awk, label):
     """Test kind: default"""
 
-    expected = "template_default"
+    expected = "default"
     awk_avail = "true"
 
     if not awk:
@@ -21,7 +21,7 @@ def test_kind_default(runner, yadm, awk, label):
     script = f"""
         YADM_TEST=1 source {yadm}
         function awk_available {{ {awk_avail}; }}
-        template="$(choose_template_cmd "{label}")"
+        template="$(choose_template_processor "{label}")"
         echo "TEMPLATE:$template"
     """
     run = runner(command=["bash"], inp=script)
@@ -43,9 +43,9 @@ def test_kind_j2cli_envtpl(runner, yadm, envtpl, j2cli, label):
     j2cli_avail = "true" if j2cli else "false"
 
     if label in ("j2cli", "j2") and j2cli:
-        expected = "template_j2cli"
+        expected = "j2cli"
     elif label in ("envtpl", "j2") and envtpl:
-        expected = "template_envtpl"
+        expected = "envtpl"
     else:
         expected = ""
 
@@ -53,7 +53,7 @@ def test_kind_j2cli_envtpl(runner, yadm, envtpl, j2cli, label):
         YADM_TEST=1 source {yadm}
         function envtpl_available {{ {envtpl_avail}; }}
         function j2cli_available {{ {j2cli_avail}; }}
-        template="$(choose_template_cmd "{label}")"
+        template="$(choose_template_processor "{label}")"
         echo "TEMPLATE:$template"
     """
     run = runner(command=["bash"], inp=script)

test/test_unit_copy_perms.py

@@ -8,7 +8,7 @@ OCTAL = "7654"
 NON_OCTAL = "9876"
 
 
-@pytest.mark.parametrize("stat_broken", [True, False], ids=["normal", "stat broken"])
+@pytest.mark.parametrize("stat_broken", [False, True], ids=["normal", "stat broken"])
 def test_copy_perms(runner, yadm, tmpdir, stat_broken):
     """Test function copy_perms"""
     src_mode = 0o754

test/test_unit_record_score.py

@@ -11,7 +11,7 @@ INIT_VARS = """
     alt_scores=()
     alt_targets=()
     alt_sources=()
-    alt_template_cmds=()
+    alt_template_processors=()
 """
 
 REPORT_RESULTS = """
@@ -19,7 +19,7 @@ REPORT_RESULTS = """
     echo "SCORES:${alt_scores[@]}"
     echo "TARGETS:${alt_targets[@]}"
     echo "SOURCES:${alt_sources[@]}"
-    echo "TEMPLATE_CMDS:${alt_template_cmds[@]}"
+    echo "TEMPLATE_PROCESSORS:${alt_template_processors[@]}"
 """
 
 
@@ -39,7 +39,7 @@ def test_dont_record_zeros(runner, yadm):
     assert "SCORES:\n" in run.out
     assert "TARGETS:\n" in run.out
     assert "SOURCES:\n" in run.out
-    assert "TEMPLATE_CMDS:\n" in run.out
+    assert "TEMPLATE_PROCESSORS:\n" in run.out
 
 
 def test_new_scores(runner, yadm):
@@ -60,7 +60,7 @@ def test_new_scores(runner, yadm):
     assert "SCORES:1 2 4\n" in run.out
     assert "TARGETS:tgt_one tgt_two tgt_three\n" in run.out
     assert "SOURCES:src_one src_two src_three\n" in run.out
-    assert "TEMPLATE_CMDS:  \n" in run.out
+    assert "TEMPLATE_PROCESSORS:  \n" in run.out
 
 
 @pytest.mark.parametrize("difference", ["lower", "equal", "higher"])
@@ -84,7 +84,7 @@ def test_existing_scores(runner, yadm, difference):
         alt_scores=(2)
         alt_targets=("testtgt")
         alt_sources=("existing_src")
-        alt_template_cmds=("")
+        alt_template_processors=("")
         record_score "{score}" "testtgt" "new_src" ""
         {REPORT_RESULTS}
     """
@@ -95,7 +95,7 @@ def test_existing_scores(runner, yadm, difference):
     assert f"SCORES:{expected_score}\n" in run.out
     assert "TARGETS:testtgt\n" in run.out
     assert f"SOURCES:{expected_src}\n" in run.out
-    assert "TEMPLATE_CMDS:\n" in run.out
+    assert "TEMPLATE_PROCESSORS:\n" in run.out
 
 
 def test_existing_template(runner, yadm):
@@ -107,7 +107,7 @@ def test_existing_template(runner, yadm):
         alt_scores=(1)
         alt_targets=("testtgt")
         alt_sources=("src")
-        alt_template_cmds=("existing_template")
+        alt_template_processors=("existing_template")
         record_score "2" "testtgt" "new_src" ""
         {REPORT_RESULTS}
     """
@@ -118,7 +118,7 @@ def test_existing_template(runner, yadm):
     assert "SCORES:1\n" in run.out
     assert "TARGETS:testtgt\n" in run.out
     assert "SOURCES:src\n" in run.out
-    assert "TEMPLATE_CMDS:existing_template\n" in run.out
+    assert "TEMPLATE_PROCESSORS:existing_template\n" in run.out
 
 
 def test_config_first(runner, yadm):
@@ -130,7 +130,7 @@ def test_config_first(runner, yadm):
         {INIT_VARS}
         YADM_CONFIG={config}
         record_score "1" "tgt_before" "src_before" ""
-        record_score "1" "tgt_tmp"    "src_tmp"    "cmd_tmp"
+        record_score "1" "tgt_tmp"    "src_tmp"    "processor_tmp"
         record_score "2" "{config}"   "src_config" ""
         record_score "3" "tgt_after"  "src_after"  ""
         {REPORT_RESULTS}
@@ -142,7 +142,7 @@ def test_config_first(runner, yadm):
     assert "SCORES:2 1 1 3\n" in run.out
     assert f"TARGETS:{config} tgt_before tgt_tmp tgt_after\n" in run.out
     assert "SOURCES:src_config src_before src_tmp src_after\n" in run.out
-    assert "TEMPLATE_CMDS:  cmd_tmp \n" in run.out
+    assert "TEMPLATE_PROCESSORS:  processor_tmp \n" in run.out
 
 
 def test_new_template(runner, yadm):
@@ -151,9 +151,9 @@ def test_new_template(runner, yadm):
     script = f"""
         YADM_TEST=1 source {yadm}
         {INIT_VARS}
-        record_score 0 "tgt_one"   "src_one"   "cmd_one"
+        record_score 0 "tgt_one"   "src_one"   "processor_one"
-        record_score 0 "tgt_two"   "src_two"   "cmd_two"
+        record_score 0 "tgt_two"   "src_two"   "processor_two"
-        record_score 0 "tgt_three" "src_three" "cmd_three"
+        record_score 0 "tgt_three" "src_three" "processor_three"
         {REPORT_RESULTS}
     """
     run = runner(command=["bash"], inp=script)
@@ -163,7 +163,7 @@ def test_new_template(runner, yadm):
     assert "SCORES:0 0 0\n" in run.out
     assert "TARGETS:tgt_one tgt_two tgt_three\n" in run.out
     assert "SOURCES:src_one src_two src_three\n" in run.out
-    assert "TEMPLATE_CMDS:cmd_one cmd_two cmd_three\n" in run.out
+    assert "TEMPLATE_PROCESSORS:processor_one processor_two processor_three\n" in run.out
 
 
 def test_overwrite_existing_template(runner, yadm):
@@ -174,9 +174,9 @@ def test_overwrite_existing_template(runner, yadm):
         {INIT_VARS}
         alt_scores=(0)
         alt_targets=("testtgt")
-        alt_template_cmds=("existing_cmd")
+        alt_template_processors=("existing_processor")
         alt_sources=("existing_src")
-        record_score 0 "testtgt" "new_src" "new_cmd"
+        record_score 0 "testtgt" "new_src" "new_processor"
         {REPORT_RESULTS}
     """
     run = runner(command=["bash"], inp=script)
@@ -186,4 +186,4 @@ def test_overwrite_existing_template(runner, yadm):
     assert "SCORES:0\n" in run.out
     assert "TARGETS:testtgt\n" in run.out
     assert "SOURCES:new_src\n" in run.out
-    assert "TEMPLATE_CMDS:new_cmd\n" in run.out
+    assert "TEMPLATE_PROCESSORS:new_processor\n" in run.out

test/test_unit_score_file.py

@@ -267,14 +267,14 @@ def test_score_values_templates(runner, yadm):
     assert run.out == expected
 
 
-@pytest.mark.parametrize("cmd_generated", [True, False], ids=["supported-template", "unsupported-template"])
+@pytest.mark.parametrize("processor_generated", [True, False], ids=["supported-template", "unsupported-template"])
-def test_template_recording(runner, yadm, cmd_generated):
+def test_template_recording(runner, yadm, processor_generated):
-    """Template should be recorded if choose_template_cmd outputs a command"""
+    """Template should be recorded if choose_template_processor outputs a command"""
 
-    mock = "function choose_template_cmd() { return; }"
+    mock = "function choose_template_processor() { return; }"
     expected = ""
-    if cmd_generated:
+    if processor_generated:
-        mock = 'function choose_template_cmd() { echo "test_cmd"; }'
+        mock = 'function choose_template_processor() { echo "test_processor"; }'
         expected = "template recorded"
 
     script = f"""

test/test_unit_template_default.py

@@ -231,7 +231,7 @@ def test_template_default(runner, yadm, tmpdir):
         local_user="{LOCAL_USER}"
         local_distro="{LOCAL_DISTRO}"
         local_distro_family="{LOCAL_DISTRO_FAMILY}"
-        template_default "{input_file}" "{output_file}"
+        template default "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script, env={"VAR": ENV_VAR})
     assert run.success
@@ -251,7 +251,7 @@ def test_source(runner, yadm, tmpdir):
     script = f"""
         YADM_TEST=1 source {yadm}
         set_awk
-        template_default "{input_file}" "{output_file}"
+        template default "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success
@@ -285,7 +285,7 @@ def test_include(runner, yadm, tmpdir):
         set_awk
         local_class="{LOCAL_CLASS}"
         local_system="{LOCAL_SYSTEM}"
-        template_default "{input_file}" "{output_file}"
+        template default "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success
@@ -305,7 +305,7 @@ def test_nested_ifs(runner, yadm, tmpdir):
         YADM_TEST=1 source {yadm}
         set_awk
         local_user="me"
-        template_default "{input_file}" "{output_file}"
+        template default "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success
@@ -323,7 +323,7 @@ def test_env(runner, yadm, tmpdir):
     script = f"""
         YADM_TEST=1 source {yadm}
         set_awk
-        template_default "{input_file}" "{output_file}"
+        template default "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success

test/test_unit_template_esh.py

@@ -140,7 +140,7 @@ def test_template_esh(runner, yadm, tmpdir):
         local_user="{LOCAL_USER}"
         local_distro="{LOCAL_DISTRO}"
         local_distro_family="{LOCAL_DISTRO_FAMILY}"
-        template_esh "{input_file}" "{output_file}"
+        template esh "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success
@@ -159,7 +159,7 @@ def test_source(runner, yadm, tmpdir):
 
     script = f"""
         YADM_TEST=1 source {yadm}
-        template_esh "{input_file}" "{output_file}"
+        template esh "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success

test/test_unit_template_j2.py

@@ -146,7 +146,7 @@ def test_template_j2(runner, yadm, tmpdir, processor):
         local_user="{LOCAL_USER}"
         local_distro="{LOCAL_DISTRO}"
         local_distro_family="{LOCAL_DISTRO_FAMILY}"
-        template_{processor} "{input_file}" "{output_file}"
+        template {processor} "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success
@@ -166,7 +166,7 @@ def test_source(runner, yadm, tmpdir, processor):
 
     script = f"""
         YADM_TEST=1 source {yadm}
-        template_{processor} "{input_file}" "{output_file}"
+        template {processor} "{input_file}" "{output_file}"
     """
     run = runner(command=["bash"], inp=script)
     assert run.success

yadm

@@ -170,7 +170,7 @@ function score_file() {
   local conditions="${source#*##}"
 
   score=0
-  local template_cmd=""
+  local template_processor=""
 
   IFS=',' read -ra fields <<<"$conditions"
   for field in "${fields[@]}"; do
@@ -213,12 +213,13 @@ function score_file() {
         if [ -d "$source" ]; then
           INVALID_ALT+=("$source")
         else
-          template_cmd=$(choose_template_cmd "$value")
+          template_processor=$(choose_template_processor "$value")
-          if [ -n "$template_cmd" ]; then
+          if [ -n "$template_processor" ]; then
             delta=0
+          elif [ -n "$loud" ]; then
+            echo "No supported template processor for template $source"
           else
             debug "No supported template processor for template $source"
-            [ -n "$loud" ] && echo "No supported template processor for template $source"
           fi
         fi
         ;;
@@ -235,17 +236,17 @@ function score_file() {
     score=$((score + 1000 + delta))
   done
 
-  record_score "$score" "$target" "$source" "$template_cmd"
+  record_score "$score" "$target" "$source" "$template_processor"
 }
 
 function record_score() {
   local score="$1"
   local target="$2"
   local source="$3"
-  local template_cmd="$4"
+  local template_processor="$4"
 
   # record nothing if the score is zero
-  [ "$score" -eq 0 ] && [ -z "$template_cmd" ] && return
+  [ "$score" -eq 0 ] && [ -z "$template_processor" ] && return
 
   # search for the index of this target, to see if we already are tracking it
   local -i index=$((${#alt_targets[@]} - 1))
@@ -262,57 +263,94 @@ function record_score() {
 
       alt_sources=("$source" "${alt_sources[@]}")
       alt_scores=("$score" "${alt_scores[@]}")
-      alt_template_cmds=("$template_cmd" "${alt_template_cmds[@]}")
+      alt_template_processors=("$template_processor" "${alt_template_processors[@]}")
     else
       alt_targets+=("$target")
 
       alt_sources+=("$source")
       alt_scores+=("$score")
-      alt_template_cmds+=("$template_cmd")
+      alt_template_processors+=("$template_processor")
     fi
     return
   fi
 
-  if [[ -n "${alt_template_cmds[$index]}" ]]; then
+  if [[ -n "${alt_template_processors[$index]}" ]]; then
-    if [[ -z "$template_cmd" || "$score" -lt "${alt_scores[$index]}" ]]; then
+    if [[ -z "$template_processor" || "$score" -lt "${alt_scores[$index]}" ]]; then
-      # No template command, or template command but lower score
+      # Not template, or template but lower score
       return
     fi
-  elif [[ -z "$template_cmd" && "$score" -le "${alt_scores[$index]}" ]]; then
+  elif [[ -z "$template_processor" && "$score" -le "${alt_scores[$index]}" ]]; then
-    # No template command and too low score
+    # Not template and too low score
     return
   fi
 
   # Record new alt
   alt_sources[index]="$source"
   alt_scores[index]="$score"
-  alt_template_cmds[index]="$template_cmd"
+  alt_template_processors[index]="$template_processor"
 }
 
-function choose_template_cmd() {
+function choose_template_processor() {
   local kind="$1"
 
   if [[ "${kind:-default}" = "default" ]]; then
-    awk_available && echo "template_default"
+    awk_available && echo "default"
   elif [[ "$kind" = "esh" ]]; then
-    esh_available && echo "template_esh"
+    esh_available && echo "esh"
   elif [[ "$kind" = "j2cli" || "$kind" = "j2" ]] && j2cli_available; then
-    echo "template_j2cli"
+    echo "j2cli"
   elif [[ "$kind" = "envtpl" || "$kind" = "j2" ]] && envtpl_available; then
-    echo "template_envtpl"
+    echo "envtpl"
   fi
 
 }
 
 # ****** Template Processors ******
 
-function template_default() {
+function template() {
-  input="$1"
+  local processor="$1"
-  output="$2"
+  local input="$2"
+  local output="$3"
+
+  local content
+  if ! content=$("template_$processor" "$input"); then
+    echo "Error: failed to process template '$input'" >&2
+    return
+  fi
+
+  if [ -r "$output" ] && [ "$content" = "$(<"$output")" ]; then
+    debug "Template output '$output' is unchanged"
+    return
+  fi
+
+  # If the output file already exists as read-only, change it to be writable.
+  # There are some environments in which a read-only file will prevent the move
+  # from being successful.
+  if [ ! -w "$output" ] && [ -e "$output" ]; then
+    chmod u+w "$output"
+  fi
+
+  if [ -n "$loud" ]; then
+    echo "Creating $output from template $input"
+  else
+    debug "Creating $output from template $input"
+  fi
+
+  local temp_file="${output}.$$.$RANDOM"
+  if cat >"$temp_file" <<<"$content" && mv -f "$temp_file" "$output"; then
+    copy_perms "$input" "$output"
+  else
+    echo "Error: failed to create template output '$output'"
+    rm -f "$temp_file"
+  fi
+}
+
+function template_default() {
+  local input="$1"
 
-  local awk_pgm
   # the explicit "space + tab" character class used below is used because not
   # all versions of awk seem to support the POSIX character classes [[:blank:]]
+  local awk_pgm
   read -r -d '' awk_pgm <<"EOF"
 BEGIN {
   classes = ARGV[2]
@@ -425,8 +463,7 @@ function replace_vars(input) {
 }
 EOF
 
-  local content
+  "${AWK_PROGRAM[0]}" \
-  content=$("${AWK_PROGRAM[0]}" \
     -v class="$local_class" \
     -v arch="$local_arch" \
     -v os="$local_system" \
@@ -437,17 +474,13 @@ EOF
     -v source="$input" \
     -v source_dir="$(builtin_dirname "$input")" \
     "$awk_pgm" \
-    "$input" "${local_classes[@]}")
+    "$input" "${local_classes[@]}"
-
-  move_file "$input" "$output" "$content"
 }
 
 function template_j2cli() {
   local input="$1"
-  local output="$2"
-  local content
 
-  content=$(YADM_CLASS="$local_class" \
+  YADM_CLASS="$local_class" \
     YADM_ARCH="$local_arch" \
     YADM_OS="$local_system" \
     YADM_HOSTNAME="$local_host" \
@@ -456,17 +489,13 @@ function template_j2cli() {
     YADM_DISTRO_FAMILY="$local_distro_family" \
     YADM_SOURCE="$input" \
     YADM_CLASSES="$(join_string $'\n' "${local_classes[@]}")" \
-    "$J2CLI_PROGRAM" "$input")
+    "$J2CLI_PROGRAM" "$input"
-
-  move_file "$input" "$output" "$content" "$?"
 }
 
 function template_envtpl() {
   local input="$1"
-  local output="$2"
-  local content
 
-  content=$(YADM_CLASS="$local_class" \
+  YADM_CLASS="$local_class" \
     YADM_ARCH="$local_arch" \
     YADM_OS="$local_system" \
     YADM_HOSTNAME="$local_host" \
@@ -475,17 +504,13 @@ function template_envtpl() {
     YADM_DISTRO_FAMILY="$local_distro_family" \
     YADM_SOURCE="$input" \
     YADM_CLASSES="$(join_string $'\n' "${local_classes[@]}")" \
-    "$ENVTPL_PROGRAM" --keep-template "$input" -o -)
+    "$ENVTPL_PROGRAM" -o - --keep-template "$input"
-
-  move_file "$input" "$output" "$content" "$?"
 }
 
 function template_esh() {
   local input="$1"
-  local output="$2"
-  local content
 
-  content=$(YADM_CLASSES="$(join_string $'\n' "${local_classes[@]}")" \
+  YADM_CLASSES="$(join_string $'\n' "${local_classes[@]}")" \
     "$ESH_PROGRAM" "$input" \
     YADM_CLASS="$local_class" \
     YADM_ARCH="$local_arch" \
@@ -494,60 +519,7 @@ function template_esh() {
     YADM_USER="$local_user" \
     YADM_DISTRO="$local_distro" \
     YADM_DISTRO_FAMILY="$local_distro_family" \
-    YADM_SOURCE="$input")
+    YADM_SOURCE="$input"
-
-  move_file "$input" "$output" "$content" "$?"
-}
-
-function move_file() {
-  local input="$1"
-  local output="$2"
-  local content="$3"
-  local err="${4:-}"
-
-  if [[ -s "$input" && -z "$content" ]]; then
-    debug "Failed to create $output from template $input: error $err"
-    return 1
-  fi
-
-  if [[ -r "$output" ]]; then
-    local old_content
-    old_content=$(< "$output")
-
-    if [[ "$old_content" == "$content" ]]; then
-      debug "Not rewriting file as contents have not changed: $output"
-      return 0
-    fi
-
-    # if the output files already exists as read-only, change it to be writable.
-    # there are some environments in which a read-only file will prevent the move
-    # from being successful.
-
-    if [[ ! -w "$output" ]]; then
-      if ! chmod u+w "$output"; then
-        debug "Unable to make '$output' writeable"
-      fi
-    fi
-  fi
-
-  if [ -n "$loud" ]; then
-    echo "Creating $output from template $input"
-  else
-    debug "Creating $output from template $input"
-  fi
-
-  local temp_file="${output}.$$.$RANDOM"
-  if printf '%s\n' "$content" >"$temp_file"; then
-    if mv -f "$temp_file" "$output"; then
-      copy_perms "$input" "$output"
-      return
-    fi
-    debug "Failed to rename '$temp_file' to '$output'"
-  else
-    debug "Failed to create '$temp_file' to generate '$output'"
-  fi
-  rm -f "$temp_file" &>/dev/null
-  return 1
 }
 
 # ****** yadm Commands ******
@@ -588,7 +560,7 @@ function alt() {
   local alt_targets=()
   local alt_sources=()
   local alt_scores=()
-  local alt_template_cmds=()
+  local alt_template_processors=()
 
   # For removing stale links
   local possible_alt_targets=()
@@ -721,7 +693,7 @@ function alt_linking() {
   for ((index = 0; index < ${#alt_targets[@]}; ++index)); do
     local target="${alt_targets[$index]}"
     local source="${alt_sources[$index]}"
-    local template_cmd="${alt_template_cmds[$index]}"
+    local template_processor="${alt_template_processors[$index]}"
 
     if [[ -L "$target" ]]; then
       rm -f "$target"
@@ -732,8 +704,8 @@ function alt_linking() {
       assert_parent "$target"
     fi
 
-    if [[ -n "$template_cmd" ]]; then
+    if [[ -n "$template_processor" ]]; then
-      "$template_cmd" "$source" "$target"
+      template "$template_processor" "$source" "$target"
     elif [[ "$do_copy" -eq 1 ]]; then
       debug "Copying $source to $target"
       [[ -n "$loud" ]] && echo "Copying $source to $target"
@@ -2099,7 +2071,7 @@ function get_mode {
 
   # only accept results if they are octal
   if [[ ! $mode =~ ^[0-7]+$ ]]; then
-    return
+    return 1
   fi
 
   echo "$mode"
@@ -2107,16 +2079,13 @@ function get_mode {
 
 function copy_perms {
   local source="$1"
-  local dest="$2"
+  local target="$2"
-  mode=$(get_mode "$source")
+
-  if [[ -z "$mode" ]]; then
+  local mode
-    debug "Unable to get mode for '$source'"
+  if ! mode=$(get_mode "$source") || ! chmod "$mode" "$target"; then
-    return 0 # to allow tests to pass
+    debug "Unable to copy perms '$mode' from '$source' to '$target'"
-  fi
-  if ! chmod "$mode" "$dest"; then
-    debug "Unable to set mode to '$mode' on '$dest'"
-    return 0 # to allow tests to pass
   fi
+
   return 0
 }