From 119d1ddbaaac37edd544334dfdf4d17bf8717ced Mon Sep 17 00:00:00 2001 From: Erik Flodin Date: Sun, 15 Dec 2024 15:10:02 +0100 Subject: [PATCH] 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_unit_choose_template_processor.py} | 12 +- test/test_unit_copy_perms.py | 2 +- test/test_unit_record_score.py | 34 +-- test/test_unit_score_file.py | 12 +- test/test_unit_template_default.py | 10 +- test/test_unit_template_esh.py | 4 +- test/test_unit_template_j2.py | 4 +- yadm | 193 ++++++++---------- 8 files changed, 120 insertions(+), 151 deletions(-) rename test/{test_unit_choose_template_cmd.py => test_unit_choose_template_processor.py} (86%) diff --git a/test/test_unit_choose_template_cmd.py b/test/test_unit_choose_template_processor.py similarity index 86% rename from test/test_unit_choose_template_cmd.py rename to test/test_unit_choose_template_processor.py index 9948f80..3997e72 100644 --- a/test/test_unit_choose_template_cmd.py +++ b/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) diff --git a/test/test_unit_copy_perms.py b/test/test_unit_copy_perms.py index bde0d63..80331b1 100644 --- a/test/test_unit_copy_perms.py +++ b/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 diff --git a/test/test_unit_record_score.py b/test/test_unit_record_score.py index 9049b1b..a6879ff 100644 --- a/test/test_unit_record_score.py +++ b/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_two" "src_two" "cmd_two" - record_score 0 "tgt_three" "src_three" "cmd_three" + record_score 0 "tgt_one" "src_one" "processor_one" + record_score 0 "tgt_two" "src_two" "processor_two" + 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 diff --git a/test/test_unit_score_file.py b/test/test_unit_score_file.py index af881fd..9952c0c 100644 --- a/test/test_unit_score_file.py +++ b/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"]) -def test_template_recording(runner, yadm, cmd_generated): - """Template should be recorded if choose_template_cmd outputs a command""" +@pytest.mark.parametrize("processor_generated", [True, False], ids=["supported-template", "unsupported-template"]) +def test_template_recording(runner, yadm, processor_generated): + """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: - mock = 'function choose_template_cmd() { echo "test_cmd"; }' + if processor_generated: + mock = 'function choose_template_processor() { echo "test_processor"; }' expected = "template recorded" script = f""" diff --git a/test/test_unit_template_default.py b/test/test_unit_template_default.py index 12a493f..3c071c5 100644 --- a/test/test_unit_template_default.py +++ b/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 diff --git a/test/test_unit_template_esh.py b/test/test_unit_template_esh.py index 8583b59..8115100 100644 --- a/test/test_unit_template_esh.py +++ b/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 diff --git a/test/test_unit_template_j2.py b/test/test_unit_template_j2.py index 3dbb8a7..53cb88c 100644 --- a/test/test_unit_template_j2.py +++ b/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 diff --git a/yadm b/yadm index 932312e..8a55bf1 100755 --- a/yadm +++ b/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") - if [ -n "$template_cmd" ]; then + template_processor=$(choose_template_processor "$value") + 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 [[ -z "$template_cmd" || "$score" -lt "${alt_scores[$index]}" ]]; then - # No template command, or template command but lower score + if [[ -n "${alt_template_processors[$index]}" ]]; then + if [[ -z "$template_processor" || "$score" -lt "${alt_scores[$index]}" ]]; then + # Not template, or template but lower score return fi - elif [[ -z "$template_cmd" && "$score" -le "${alt_scores[$index]}" ]]; then - # No template command and too low score + elif [[ -z "$template_processor" && "$score" -le "${alt_scores[$index]}" ]]; then + # 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() { - input="$1" - output="$2" +function template() { + local processor="$1" + 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 - content=$("${AWK_PROGRAM[0]}" \ + "${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[@]}") - - move_file "$input" "$output" "$content" + "$input" "${local_classes[@]}" } 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") - - move_file "$input" "$output" "$content" "$?" + "$J2CLI_PROGRAM" "$input" } 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 -) - - move_file "$input" "$output" "$content" "$?" + "$ENVTPL_PROGRAM" -o - --keep-template "$input" } 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") - - 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_SOURCE="$input" } # ****** 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 - "$template_cmd" "$source" "$target" + if [[ -n "$template_processor" ]]; then + 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" - mode=$(get_mode "$source") - if [[ -z "$mode" ]]; then - debug "Unable to get mode for '$source'" - return 0 # to allow tests to pass - fi - if ! chmod "$mode" "$dest"; then - debug "Unable to set mode to '$mode' on '$dest'" - return 0 # to allow tests to pass + local target="$2" + + local mode + if ! mode=$(get_mode "$source") || ! chmod "$mode" "$target"; then + debug "Unable to copy perms '$mode' from '$source' to '$target'" fi + return 0 }