From 1a30322421fce4e4ef7128f9d3e5fd84312e62cd Mon Sep 17 00:00:00 2001 From: Richard Freeman Date: Sat, 18 Jul 2026 07:28:34 +0000 Subject: [PATCH 1/3] feature: Stage 3 merge tools use /tmp views of ancestor and marker (issue #65) Pass disposable copies of the 3-way ancestor and Portage ._cfg* marker to interactive merge tools so accidental saves on non-live panes cannot corrupt permanent backups or markers. Live file and *.merge output paths stay real. Also extends Tier D mocks/tests (hostile input overwrite, temp path checks) and clarifies tool_intro for 3-way tools. --- cfg-update | 99 ++++++++++++++++++++++++++++++++++++++- docs/ARCHITECTURE.md | 2 + test/run-tests.sh | 109 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 207 insertions(+), 3 deletions(-) diff --git a/cfg-update b/cfg-update index dddb870..96a812b 100755 --- a/cfg-update +++ b/cfg-update @@ -167,6 +167,8 @@ $Term::ANSIColor::AUTORESET = 1; my $path_merged; # /path/file.merge <-- merge result (*.merge) my $path_temp_old; # /path/._temp_old-cfg_file <-- temp copy of live config during merge my $path_temp_new; # /path/._temp_new-cfg_file <-- temp copy of marker during merge + my $path_view_ancestor; # /tmp/cfg-update-$$-ancestor-* <-- Stage 3 merge-tool view (disposable) + my $path_view_new; # /tmp/cfg-update-$$-new-* <-- Stage 3 merge-tool view (disposable) my $is_executable; my $ancestor; my $log_pkg = ""; @@ -955,6 +957,85 @@ sub make_temp_backups{ #ARGS# ("pretend|execute") if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } } +# Disposable /tmp copies of ancestor + marker for Stage 3 merge tools so accidental +# saves on middle/right panes cannot corrupt $path_backup_new or $path_new (issue #65). +# Orthogonal to make_temp_backups (which stages permanent backup promotion). +sub make_merge_view_temps { + if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } + $path_view_ancestor = ""; + $path_view_new = ""; + my $tmpdir = $ENV{TMPDIR}; + if (!defined $tmpdir || $tmpdir eq "") { $tmpdir = "/tmp"; } + $tmpdir =~ s|/+$||; + if (!-d $tmpdir) { + if ($opt_d >= 1) { print "$tab"." mkdir -p \"$tmpdir\"\n"; } + `mkdir -p "$tmpdir" $debug`; + } + my $safe = $cfg_basename; + $safe =~ s/[^A-Za-z0-9._-]/_/g; + if ($safe eq "") { $safe = "file"; } + if (-e $path_backup_new) { + $path_view_ancestor = "$tmpdir/cfg-update-$$-ancestor-$safe"; + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$path_backup_new\" \"$path_view_ancestor\"\n"; } + `cp -pP "$path_backup_new" "$path_view_ancestor" $debug`; + if (!-e $path_view_ancestor) { + if ($opt_d >= 1) { print "$tab"." failed to create $path_view_ancestor; using real ancestor\n"; } + $path_view_ancestor = ""; + } + } + if (-e $path_new) { + $path_view_new = "$tmpdir/cfg-update-$$-new-$safe"; + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$path_new\" \"$path_view_new\"\n"; } + `cp -pP "$path_new" "$path_view_new" $debug`; + if (!-e $path_view_new) { + if ($opt_d >= 1) { print "$tab"." failed to create $path_view_new; using real marker\n"; } + $path_view_new = ""; + } + } + if ($opt_d >= 1) { + print "$tab"." path_view_ancestor = $path_view_ancestor\n"; + print "$tab"." path_view_new = $path_view_new\n"; + $tab =~ s/ //; print "$tab"."\n"; + } +} + +sub cleanup_merge_view_temps { + if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } + if (defined $path_view_ancestor && $path_view_ancestor ne "" && -e $path_view_ancestor) { + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$path_view_ancestor\"\n"; } + `rm -f "$path_view_ancestor" $debug`; + } + if (defined $path_view_new && $path_view_new ne "" && -e $path_view_new) { + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$path_view_new\"\n"; } + `rm -f "$path_view_new" $debug`; + } + $path_view_ancestor = ""; + $path_view_new = ""; + if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } +} + +# Launch merge tool for Stage 3 with disposable /tmp views of ancestor + marker. +# Restores real $path_backup_new / $path_new before return so complete/cancel paths stay correct. +sub launch_tool_with_merge_view_temps { #ARGS# ("pretend|execute","mergetool") + if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } + my $mode = $_[0]; + my $tool = $_[1]; + my $real_backup_new = $path_backup_new; + my $real_new = $path_new; + if ($mode =~ /execute/) { + &make_merge_view_temps; + if ($path_view_ancestor ne "") { $path_backup_new = $path_view_ancestor; } + if ($path_view_new ne "") { $path_new = $path_view_new; } + } + &launch_tool($mode, $tool); + $path_backup_new = $real_backup_new; + $path_new = $real_new; + if ($mode =~ /execute/) { + &cleanup_merge_view_temps; + } + if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } +} + sub update_stage1{ #ARGS# ("pretend|execute") if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } if ($enable_stage1 !~ /^yes$|^true$|^on$/i) { @@ -1101,7 +1182,9 @@ sub update_stage3{ #ARGS# ("pretend|execute") if ($key =~ /2/) { &update_keep_complete($_[0]); $key="s"; } if ($key =~ /v|y/) { &tool_intro($merge_tool_name); - &launch_tool($_[0],$merge_tool); + # Stage 3 only: pass /tmp copies of ancestor + marker so accidental + # saves cannot corrupt permanent backups or Portage markers (issue #65). + &launch_tool_with_merge_view_temps($_[0],$merge_tool); if (-e $path_merged) { if ($tool_saves_mergefile_when_aborted =~ "no") { print "$tab"." Interactive merging completed... (or aborted)\n"; @@ -1810,10 +1893,14 @@ sub tool_intro{ #ARGS# ("mergetoolname") print "$tab"." In $_[0] you select the lines that you want to keep by simply\n"; print "$tab"." clicking on the colored lines. They will appear in the merge-pane.\n"; print "$tab"." When done, click the M-button to save the result, then exit $_[0]!\n"; + print "$tab"." In 3-way mode the ancestor and new-file panes are temporary copies;\n"; + print "$tab"." accidental edits there are discarded. Save the merge output only.\n"; print "$tab"." When you exit $_[0], $progname will finish the update.\n"; } elsif ($_[0] =~ /^kdiff3$/) { print "$tab"." In $_[0] you select the lines that you want to keep.\n"; print "$tab"." When done, click the save button and exit $_[0]!\n"; + print "$tab"." In 3-way mode the ancestor and new-file panes are temporary copies;\n"; + print "$tab"." accidental edits there are discarded. Save the merge output only.\n"; print "$tab"." When you exit $_[0], $progname will finish the update...\n"; } elsif ($_[0] =~ /^kompare$/) { print "$tab"." In $_[0] you select the lines that you want to keep.\n"; @@ -1836,10 +1923,20 @@ sub tool_intro{ #ARGS# ("mergetoolname") print "$tab"." In $_[0] you select the lines that you want to keep.\n"; print "$tab"." When done, save the merged result over the current configfile by\n"; print "$tab"." right-clicking on the left pane and chosing \"Save\"!\n"; + print "$tab"." In 3-way mode the middle (ancestor) and right (new) panes are\n"; + print "$tab"." temporary copies; accidental edits there are discarded.\n"; print "$tab"." When you exit $_[0], $progname will finish the update...\n"; } elsif ($_[0] =~ /^tkdiff$/) { print "$tab"." In $_[0] you select the lines that you want to keep.\n"; print "$tab"." When done, save the merged result with the \"Save & Exit\" button!\n"; + print "$tab"." In 3-way mode the ancestor and new-file panes are temporary copies;\n"; + print "$tab"." accidental edits there are discarded. Save the merge output only.\n"; + print "$tab"." When you exit $_[0], $progname will finish the update...\n"; + } elsif ($_[0] =~ /^imediff$/) { + print "$tab"." In $_[0] you select the lines that you want to keep.\n"; + print "$tab"." When done, the merged result is written to the *.merge output file.\n"; + print "$tab"." In 3-way mode the ancestor and new-file inputs are temporary copies;\n"; + print "$tab"." accidental edits there are discarded.\n"; print "$tab"." When you exit $_[0], $progname will finish the update...\n"; } else { print "$tab"." In $_[0] you select the lines that you want to keep.\n"; diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ea62aa4..287f3e2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -176,6 +176,8 @@ No conflict markers → apply merged result. Conflict → defer to stage 3. Launches the configured merge tool with ancestor, live, and new files. Requires a tool with 3-way support (meld, kdiff3, xxdiff, tkdiff, imediff). +For Stage 3 only, the ancestor (`$path_backup_new`) and Portage marker (`$path_new`) are copied to disposable files under `/tmp` (or `$TMPDIR`) before the tool runs, so accidental saves on non-live panes cannot corrupt permanent backups or markers. The live file and `*.merge` output paths remain real. See issue #65. + ### Stage 4 — Manual 2-way merge (`update_stage4`) Merges live file and `._cfg*` update when no backup exists. Works with all supported tools. diff --git a/test/run-tests.sh b/test/run-tests.sh index 3c5f412..80cfebf 100755 --- a/test/run-tests.sh +++ b/test/run-tests.sh @@ -267,8 +267,11 @@ setup_multi_config_protect_sandbox() { run_cfg_update() { local extra_args=("$@") + # Isolate disposable Stage 3 merge-view temps under the sandbox (issue #65). + mkdir -p "$SANDBOX/tmp" CFG_UPDATE_CONF="$SANDBOX/etc/cfg-update.conf" \ PATH="$SANDBOX/bin:$PATH" \ + TMPDIR="$SANDBOX/tmp" \ perl "$CFG_UPDATE" --ebuild --testsandbox "${extra_args[@]}" } @@ -288,15 +291,31 @@ echo "$*" >>"$log" outfile="" ancestor="" threeway="no" +files=() while [[ $# -gt 0 ]]; do case "$1" in -o) outfile="$2"; shift 2 ;; -b) ancestor="$2"; threeway="yes"; shift 2 ;; -m) shift ;; - *) shift ;; + *) files+=("$1"); shift ;; esac done echo "THREE_WAY=$threeway" >>"$log" +if [[ -n "$ancestor" ]]; then + echo "ANCESTOR=$ancestor" >>"$log" +fi +if [[ ${#files[@]} -ge 1 ]]; then + echo "LIVE=${files[0]}" >>"$log" +fi +if [[ ${#files[@]} -ge 2 ]]; then + echo "NEW=${files[1]}" >>"$log" +fi +# Hostile mode: overwrite tool inputs to simulate accidental pane saves (issue #65). +if [[ "${CFG_UPDATE_MOCK_TRASH_INPUTS:-}" == "1" ]]; then + [[ -n "$ancestor" && -f "$ancestor" ]] && echo "TRASHED-ANCESTOR" >"$ancestor" + [[ ${#files[@]} -ge 2 && -f "${files[1]}" ]] && echo "TRASHED-NEW" >"${files[1]}" + echo "TRASHED_INPUTS=yes" >>"$log" +fi if [[ -n "$outfile" && -f "${CFG_UPDATE_TEST_SANDBOX}/golden.merge" ]]; then cp "${CFG_UPDATE_TEST_SANDBOX}/golden.merge" "$outfile" fi @@ -368,9 +387,20 @@ if [[ "$use_a" == "yes" ]]; then echo "USE_A=yes" >>"$log" fi case "${#files[@]}" in - 3) echo "THREE_WAY=yes" >>"$log" ;; + 3) + echo "THREE_WAY=yes" >>"$log" + echo "LIVE=${files[0]}" >>"$log" + echo "ANCESTOR=${files[1]}" >>"$log" + echo "NEW=${files[2]}" >>"$log" + ;; 2) echo "TWO_WAY=yes" >>"$log" ;; esac +# Hostile mode: overwrite tool inputs to simulate accidental pane saves (issue #65). +if [[ "${CFG_UPDATE_MOCK_TRASH_INPUTS:-}" == "1" && ${#files[@]} -eq 3 ]]; then + [[ -f "${files[1]}" ]] && echo "TRASHED-ANCESTOR" >"${files[1]}" + [[ -f "${files[2]}" ]] && echo "TRASHED-NEW" >"${files[2]}" + echo "TRASHED_INPUTS=yes" >>"$log" +fi if [[ -n "$outfile" && -f "${CFG_UPDATE_TEST_SANDBOX}/golden.merge" ]]; then cp "${CFG_UPDATE_TEST_SANDBOX}/golden.merge" "$outfile" fi @@ -780,6 +810,77 @@ tier_d_execute_manual() { "$SANDBOX/etc/test/test_auto_3way_conflict" \ "$FIXTURES/stage2-3way-merge-conflict/expected/test_auto_3way_conflict.after_replace" + # Stage 3: ancestor/new args are disposable /tmp views (issue #65), not real paths + local real_ancestor real_marker + real_ancestor="$SANDBOX/var/lib/cfg-update/backups${SANDBOX}/etc/test/._new-cfg_test_auto_3way_conflict" + real_marker="$SANDBOX/etc/test/._cfg0000_test_auto_3way_conflict" + assert_file_contains "stage3 kdiff3 ancestor arg is merge-view temp" \ + "$SANDBOX/mock-kdiff3.log" "ANCESTOR=${SANDBOX}/tmp/cfg-update-" + assert_file_contains "stage3 kdiff3 new arg is merge-view temp" \ + "$SANDBOX/mock-kdiff3.log" "NEW=${SANDBOX}/tmp/cfg-update-" + if grep -q "ANCESTOR=${real_ancestor}" "$SANDBOX/mock-kdiff3.log" 2>/dev/null; then + fail "stage3 kdiff3 must not pass real ancestor path" + else + pass "stage3 kdiff3 did not pass real ancestor path" + fi + if grep -q "NEW=${real_marker}" "$SANDBOX/mock-kdiff3.log" 2>/dev/null; then + fail "stage3 kdiff3 must not pass real marker path" + else + pass "stage3 kdiff3 did not pass real marker path" + fi + local leftover + leftover="$(find "$SANDBOX/tmp" -name 'cfg-update-*' 2>/dev/null | wc -l)" + if [[ "$leftover" -eq 0 ]]; then + pass "stage3 cleaned up merge-view temps" + else + fail "stage3 left $leftover merge-view temp(s) under TMPDIR" + fi + + # Stage 3: hostile mock trashes tool inputs; real ancestor/marker must survive cancel + setup_sandbox stage2-3way-merge-conflict stage3_only + install_mock_kdiff3 # no golden → no $path_merged → cancel/finish prompt + sed -i "s|^MERGE_TOOL = .*|MERGE_TOOL = $SANDBOX/bin/kdiff3|" "$SANDBOX/etc/cfg-update.conf" + real_ancestor="$SANDBOX/var/lib/cfg-update/backups${SANDBOX}/etc/test/._new-cfg_test_auto_3way_conflict" + real_marker="$SANDBOX/etc/test/._cfg0000_test_auto_3way_conflict" + local ancestor_before marker_before + ancestor_before="$(md5sum "$real_ancestor" | awk '{print $1}')" + marker_before="$(md5sum "$real_marker" | awk '{print $1}')" + output="$(CFG_UPDATE_MOCK_TRASH_INPUTS=1 run_cfg_update_stdin $'y\ns\n' -u 2>&1)" || true + assert_file_contains "stage3 hostile mock trashed inputs" \ + "$SANDBOX/mock-kdiff3.log" "TRASHED_INPUTS=yes" + local ancestor_after marker_after + ancestor_after="$(md5sum "$real_ancestor" | awk '{print $1}')" + marker_after="$(md5sum "$real_marker" | awk '{print $1}')" + if [[ "$ancestor_before" == "$ancestor_after" ]]; then + pass "stage3 hostile cancel left real ancestor intact" + else + fail "stage3 hostile cancel corrupted real ancestor" + fi + if [[ "$marker_before" == "$marker_after" ]]; then + pass "stage3 hostile cancel left real marker intact" + else + fail "stage3 hostile cancel corrupted real marker" + fi + assert_file_exists "stage3 hostile cancel kept cfg marker" "$real_marker" + + # Stage 3: hostile mock trashes inputs; successful merge still uses real paths for complete + setup_sandbox stage2-3way-merge-conflict stage3_only + install_mock_kdiff3 \ + "$FIXTURES/stage2-3way-merge-conflict/expected/test_auto_3way_conflict.after_replace" + sed -i "s|^MERGE_TOOL = .*|MERGE_TOOL = $SANDBOX/bin/kdiff3|" "$SANDBOX/etc/cfg-update.conf" + # Sandbox --ebuild sets tool_saves_mergefile_when_aborted=no, so confirm with [1]. + output="$(CFG_UPDATE_MOCK_TRASH_INPUTS=1 run_cfg_update_stdin $'y\n1\n' -u 2>&1)" || true + assert_stage_output "stage3 hostile complete" 3 "$output" + assert_file_equals "stage3 hostile complete matches golden" \ + "$SANDBOX/etc/test/test_auto_3way_conflict" \ + "$FIXTURES/stage2-3way-merge-conflict/expected/test_auto_3way_conflict.after_replace" + assert_missing "stage3 hostile complete removed cfg marker" \ + "$SANDBOX/etc/test/._cfg0000_test_auto_3way_conflict" + # After complete, ancestor is replaced by promoted path_temp_new (pre-merge marker), + # not by the trashed view copy. The important check: trash never touched real files mid-run. + assert_file_contains "stage3 hostile complete used merge-view temps" \ + "$SANDBOX/mock-kdiff3.log" "ANCESTOR=${SANDBOX}/tmp/cfg-update-" + # Stage 3: mock imediff must receive 3-way (-a -o live ancestor new) setup_sandbox stage2-3way-merge-conflict stage3_only install_mock_imediff \ @@ -794,6 +895,10 @@ tier_d_execute_manual() { assert_file_equals "stage3 mock imediff merge matches golden" \ "$SANDBOX/etc/test/test_auto_3way_conflict" \ "$FIXTURES/stage2-3way-merge-conflict/expected/test_auto_3way_conflict.after_replace" + assert_file_contains "stage3 imediff ancestor arg is merge-view temp" \ + "$SANDBOX/mock-imediff.log" "ANCESTOR=${SANDBOX}/tmp/cfg-update-" + assert_file_contains "stage3 imediff new arg is merge-view temp" \ + "$SANDBOX/mock-imediff.log" "NEW=${SANDBOX}/tmp/cfg-update-" # Stage 4: mock sdiff must run 2-way merge (no -b ancestor) setup_sandbox stage4-manual-2way stage4_only From 78db55ab0dfc2f698e4d933624dd1f6ab7d7fa84 Mon Sep 17 00:00:00 2001 From: Richard Freeman Date: Sat, 18 Jul 2026 10:45:17 +0000 Subject: [PATCH 2/3] refactor: pass Stage 3 merge-view temps via parameters and returns Drop file-scope $path_view_* globals. make_merge_view_temps takes basename and real paths and returns the two view paths; cleanup takes those paths; launch_tool_with_merge_view_temps keeps them as my locals and only rebinds existing $path_backup_new/$path_new around launch_tool. --- cfg-update | 80 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/cfg-update b/cfg-update index 96a812b..55f2e30 100755 --- a/cfg-update +++ b/cfg-update @@ -167,8 +167,6 @@ $Term::ANSIColor::AUTORESET = 1; my $path_merged; # /path/file.merge <-- merge result (*.merge) my $path_temp_old; # /path/._temp_old-cfg_file <-- temp copy of live config during merge my $path_temp_new; # /path/._temp_new-cfg_file <-- temp copy of marker during merge - my $path_view_ancestor; # /tmp/cfg-update-$$-ancestor-* <-- Stage 3 merge-tool view (disposable) - my $path_view_new; # /tmp/cfg-update-$$-new-* <-- Stage 3 merge-tool view (disposable) my $is_executable; my $ancestor; my $log_pkg = ""; @@ -960,10 +958,12 @@ sub make_temp_backups{ #ARGS# ("pretend|execute") # Disposable /tmp copies of ancestor + marker for Stage 3 merge tools so accidental # saves on middle/right panes cannot corrupt $path_backup_new or $path_new (issue #65). # Orthogonal to make_temp_backups (which stages permanent backup promotion). -sub make_merge_view_temps { +# Takes basename + real paths; returns ($view_ancestor, $view_new) path strings ("" if unused). +sub make_merge_view_temps { #ARGS# ($basename, $path_backup_new, $path_new) if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } - $path_view_ancestor = ""; - $path_view_new = ""; + my ($basename, $src_backup_new, $src_new) = @_; + my $view_ancestor = ""; + my $view_new = ""; my $tmpdir = $ENV{TMPDIR}; if (!defined $tmpdir || $tmpdir eq "") { $tmpdir = "/tmp"; } $tmpdir =~ s|/+$||; @@ -971,67 +971,69 @@ sub make_merge_view_temps { if ($opt_d >= 1) { print "$tab"." mkdir -p \"$tmpdir\"\n"; } `mkdir -p "$tmpdir" $debug`; } - my $safe = $cfg_basename; + my $safe = $basename; + $safe = "" unless defined $safe; $safe =~ s/[^A-Za-z0-9._-]/_/g; if ($safe eq "") { $safe = "file"; } - if (-e $path_backup_new) { - $path_view_ancestor = "$tmpdir/cfg-update-$$-ancestor-$safe"; - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$path_backup_new\" \"$path_view_ancestor\"\n"; } - `cp -pP "$path_backup_new" "$path_view_ancestor" $debug`; - if (!-e $path_view_ancestor) { - if ($opt_d >= 1) { print "$tab"." failed to create $path_view_ancestor; using real ancestor\n"; } - $path_view_ancestor = ""; + if (defined $src_backup_new && -e $src_backup_new) { + $view_ancestor = "$tmpdir/cfg-update-$$-ancestor-$safe"; + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$src_backup_new\" \"$view_ancestor\"\n"; } + `cp -pP "$src_backup_new" "$view_ancestor" $debug`; + if (!-e $view_ancestor) { + if ($opt_d >= 1) { print "$tab"." failed to create $view_ancestor; using real ancestor\n"; } + $view_ancestor = ""; } } - if (-e $path_new) { - $path_view_new = "$tmpdir/cfg-update-$$-new-$safe"; - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$path_new\" \"$path_view_new\"\n"; } - `cp -pP "$path_new" "$path_view_new" $debug`; - if (!-e $path_view_new) { - if ($opt_d >= 1) { print "$tab"." failed to create $path_view_new; using real marker\n"; } - $path_view_new = ""; + if (defined $src_new && -e $src_new) { + $view_new = "$tmpdir/cfg-update-$$-new-$safe"; + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$src_new\" \"$view_new\"\n"; } + `cp -pP "$src_new" "$view_new" $debug`; + if (!-e $view_new) { + if ($opt_d >= 1) { print "$tab"." failed to create $view_new; using real marker\n"; } + $view_new = ""; } } if ($opt_d >= 1) { - print "$tab"." path_view_ancestor = $path_view_ancestor\n"; - print "$tab"." path_view_new = $path_view_new\n"; + print "$tab"." path_view_ancestor = $view_ancestor\n"; + print "$tab"." path_view_new = $view_new\n"; $tab =~ s/ //; print "$tab"."\n"; } + return ($view_ancestor, $view_new); } -sub cleanup_merge_view_temps { +sub cleanup_merge_view_temps { #ARGS# ($view_ancestor, $view_new) if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } - if (defined $path_view_ancestor && $path_view_ancestor ne "" && -e $path_view_ancestor) { - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$path_view_ancestor\"\n"; } - `rm -f "$path_view_ancestor" $debug`; + my ($view_ancestor, $view_new) = @_; + if (defined $view_ancestor && $view_ancestor ne "" && -e $view_ancestor) { + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$view_ancestor\"\n"; } + `rm -f "$view_ancestor" $debug`; } - if (defined $path_view_new && $path_view_new ne "" && -e $path_view_new) { - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$path_view_new\"\n"; } - `rm -f "$path_view_new" $debug`; + if (defined $view_new && $view_new ne "" && -e $view_new) { + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$view_new\"\n"; } + `rm -f "$view_new" $debug`; } - $path_view_ancestor = ""; - $path_view_new = ""; if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } } # Launch merge tool for Stage 3 with disposable /tmp views of ancestor + marker. -# Restores real $path_backup_new / $path_new before return so complete/cancel paths stay correct. +# Temporarily rebinds existing $path_backup_new / $path_new for launch_tool only, then +# restores them so complete/cancel paths still use the real marker and ancestor. sub launch_tool_with_merge_view_temps { #ARGS# ("pretend|execute","mergetool") if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } - my $mode = $_[0]; - my $tool = $_[1]; + my ($mode, $tool) = @_; my $real_backup_new = $path_backup_new; my $real_new = $path_new; + my ($view_ancestor, $view_new) = ("", ""); if ($mode =~ /execute/) { - &make_merge_view_temps; - if ($path_view_ancestor ne "") { $path_backup_new = $path_view_ancestor; } - if ($path_view_new ne "") { $path_new = $path_view_new; } + ($view_ancestor, $view_new) = make_merge_view_temps($cfg_basename, $path_backup_new, $path_new); + if ($view_ancestor ne "") { $path_backup_new = $view_ancestor; } + if ($view_new ne "") { $path_new = $view_new; } } - &launch_tool($mode, $tool); + launch_tool($mode, $tool); $path_backup_new = $real_backup_new; $path_new = $real_new; if ($mode =~ /execute/) { - &cleanup_merge_view_temps; + cleanup_merge_view_temps($view_ancestor, $view_new); } if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } } From dcd39a56689e62b27bd8cf57fa21bc4d5e23962b Mon Sep 17 00:00:00 2001 From: Richard Freeman Date: Sat, 18 Jul 2026 10:52:54 +0000 Subject: [PATCH 3/3] refactor: single-file merge-view temp make/cleanup helpers Replace the Stage-3-only pair API with make_merge_view_temp / cleanup_merge_view_temp operating on one path. Stage 3 calls each twice. Keeps a thin cleanup_merge_view_temps(@paths) convenience for 0-N temps. Prepares for Stage 4 marker-only views (issue #68). --- cfg-update | 89 ++++++++++++++++++++++---------------------- docs/ARCHITECTURE.md | 2 +- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/cfg-update b/cfg-update index 55f2e30..a0b0bea 100755 --- a/cfg-update +++ b/cfg-update @@ -955,15 +955,18 @@ sub make_temp_backups{ #ARGS# ("pretend|execute") if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } } -# Disposable /tmp copies of ancestor + marker for Stage 3 merge tools so accidental -# saves on middle/right panes cannot corrupt $path_backup_new or $path_new (issue #65). +# Disposable /tmp view of one non-live merge input so accidental tool saves cannot +# corrupt permanent backups or Portage markers (issue #65; Stage 4: issue #68). # Orthogonal to make_temp_backups (which stages permanent backup promotion). -# Takes basename + real paths; returns ($view_ancestor, $view_new) path strings ("" if unused). -sub make_merge_view_temps { #ARGS# ($basename, $path_backup_new, $path_new) - if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } - my ($basename, $src_backup_new, $src_new) = @_; - my $view_ancestor = ""; - my $view_new = ""; +# $role is a filename tag only (e.g. "ancestor", "new"). Returns view path or "". +sub make_merge_view_temp { #ARGS# ($src_path, $role, $basename) + if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } + my ($src, $role, $basename) = @_; + my $view = ""; + if (!defined $src || $src eq "" || !-e $src) { + if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } + return $view; + } my $tmpdir = $ENV{TMPDIR}; if (!defined $tmpdir || $tmpdir eq "") { $tmpdir = "/tmp"; } $tmpdir =~ s|/+$||; @@ -971,48 +974,42 @@ sub make_merge_view_temps { #ARGS# ($basename, $path_backup_new, $path_new) if ($opt_d >= 1) { print "$tab"." mkdir -p \"$tmpdir\"\n"; } `mkdir -p "$tmpdir" $debug`; } - my $safe = $basename; - $safe = "" unless defined $safe; + my $safe_role = defined $role ? $role : "view"; + $safe_role =~ s/[^A-Za-z0-9._-]/_/g; + if ($safe_role eq "") { $safe_role = "view"; } + my $safe = defined $basename ? $basename : ""; $safe =~ s/[^A-Za-z0-9._-]/_/g; if ($safe eq "") { $safe = "file"; } - if (defined $src_backup_new && -e $src_backup_new) { - $view_ancestor = "$tmpdir/cfg-update-$$-ancestor-$safe"; - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$src_backup_new\" \"$view_ancestor\"\n"; } - `cp -pP "$src_backup_new" "$view_ancestor" $debug`; - if (!-e $view_ancestor) { - if ($opt_d >= 1) { print "$tab"." failed to create $view_ancestor; using real ancestor\n"; } - $view_ancestor = ""; - } - } - if (defined $src_new && -e $src_new) { - $view_new = "$tmpdir/cfg-update-$$-new-$safe"; - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$src_new\" \"$view_new\"\n"; } - `cp -pP "$src_new" "$view_new" $debug`; - if (!-e $view_new) { - if ($opt_d >= 1) { print "$tab"." failed to create $view_new; using real marker\n"; } - $view_new = ""; - } + $view = "$tmpdir/cfg-update-$$-$safe_role-$safe"; + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." cp -pP \"$src\" \"$view\"\n"; } + `cp -pP "$src" "$view" $debug`; + if (!-e $view) { + if ($opt_d >= 1) { print "$tab"." failed to create $view; using real path\n"; } + $view = ""; } if ($opt_d >= 1) { - print "$tab"." path_view_ancestor = $view_ancestor\n"; - print "$tab"." path_view_new = $view_new\n"; - $tab =~ s/ //; print "$tab"."\n"; + print "$tab"." path_view = $view\n"; + $tab =~ s/ //; print "$tab"."\n"; } - return ($view_ancestor, $view_new); + return $view; } -sub cleanup_merge_view_temps { #ARGS# ($view_ancestor, $view_new) - if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } - my ($view_ancestor, $view_new) = @_; - if (defined $view_ancestor && $view_ancestor ne "" && -e $view_ancestor) { - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$view_ancestor\"\n"; } - `rm -f "$view_ancestor" $debug`; - } - if (defined $view_new && $view_new ne "" && -e $view_new) { - if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$view_new\"\n"; } - `rm -f "$view_new" $debug`; - } - if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } +sub cleanup_merge_view_temp { #ARGS# ($view_path) + if ($opt_d >= 1) { print "$tab"."\n"; $tab = $tab." "; } + my ($view) = @_; + if (defined $view && $view ne "" && -e $view) { + if (($opt_v >= 1) || ($opt_d >= 1)) { print "$tab"." rm -f \"$view\"\n"; } + `rm -f "$view" $debug`; + } + if ($opt_d >= 1) { $tab =~ s/ //; print "$tab"."\n"; } +} + +# Unlink zero or more disposable merge-view paths (convenience wrapper). +sub cleanup_merge_view_temps { + my @views = @_; + foreach my $view (@views) { + cleanup_merge_view_temp($view); + } } # Launch merge tool for Stage 3 with disposable /tmp views of ancestor + marker. @@ -1023,9 +1020,11 @@ sub launch_tool_with_merge_view_temps { #ARGS# ("pretend|execute","mergetool") my ($mode, $tool) = @_; my $real_backup_new = $path_backup_new; my $real_new = $path_new; - my ($view_ancestor, $view_new) = ("", ""); + my $view_ancestor = ""; + my $view_new = ""; if ($mode =~ /execute/) { - ($view_ancestor, $view_new) = make_merge_view_temps($cfg_basename, $path_backup_new, $path_new); + $view_ancestor = make_merge_view_temp($path_backup_new, "ancestor", $cfg_basename); + $view_new = make_merge_view_temp($path_new, "new", $cfg_basename); if ($view_ancestor ne "") { $path_backup_new = $view_ancestor; } if ($view_new ne "") { $path_new = $view_new; } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 287f3e2..bd2790d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -176,7 +176,7 @@ No conflict markers → apply merged result. Conflict → defer to stage 3. Launches the configured merge tool with ancestor, live, and new files. Requires a tool with 3-way support (meld, kdiff3, xxdiff, tkdiff, imediff). -For Stage 3 only, the ancestor (`$path_backup_new`) and Portage marker (`$path_new`) are copied to disposable files under `/tmp` (or `$TMPDIR`) before the tool runs, so accidental saves on non-live panes cannot corrupt permanent backups or markers. The live file and `*.merge` output paths remain real. See issue #65. +For Stage 3, the ancestor (`$path_backup_new`) and Portage marker (`$path_new`) are each copied via `make_merge_view_temp` to disposable files under `/tmp` (or `$TMPDIR`) before the tool runs, so accidental saves on non-live panes cannot corrupt permanent backups or markers. The live file and `*.merge` output paths remain real. See issue #65. Stage 4 marker-only views are tracked in issue #68. ### Stage 4 — Manual 2-way merge (`update_stage4`)