regress: Replace sed -i with ed for portability

Thomas and I would like to replace the ubiquitous use of "sed -i" in the regression tests with ed(1). "sed -i" is fundamentally unportable. The implementations that support the -i option for in-place editing differ in a crucial aspect: GNU and OpenBSD sed(1) treat the extension for the backup file as an optional argument and use "sed -i" for no backup file. FreeBSD sed(1) treats the extension as an obligatory argument and uses "sed -i ''" for no backup file. There is no single syntax that works for both. Handling this in -portable is painful. Apart from calling "sed -i" directly, a few regression tests also create shell scripts that contain "sed -i" and are executed later. I'm attaching a model diff that replaces all instances of "sed -i" with ed. Take a look, see if you like it. For the script use, check commit.sh or histedit.sh. If the use of "EOF" to terminate nested here documents is too confusing, we can use a different delimiter for the inner one. (Any suggestions?) We can tweak this to taste, because I used a Perl script to generate the diff, which I'm also attaching. Testing this has turned up an oddity: test_commit_prepared_logmsg() in commit.sh produces a script that performs s/foo/bar/ on a file, but that file doesn't contain the string "foo", so no replacement happens. Unlike sed(1), ed(1) treats this as an error. What is that part of the test supposed to accomplish? -- Christian "naddy" Weisgerber naddy@mips.inka.de diff /home/naddy/got commit - 99a97f809268a7ae2d24198c3c71fea8c884e6d2 path + /home/naddy/got blob - 719bac3298df1e16197cba688737ead5ea2508cd file + regress/cmdline/blame.sh --- regress/cmdline/blame.sh +++ regress/cmdline/blame.sh @@ -248,7 +248,10 @@ test_blame_lines_shifted_up() { local short_commit1=`trim_obj_id 32 $commit1` local author_time=`git_show_author_time $testroot/repo` - sed -i -e '/^[345]$/d' $testroot/wt/alpha + ed -s $testroot/wt/alpha <<-\EOF + g/^[345]$/d + w + EOF (cd $testroot/wt && got commit -m "change 2" > /dev/null) local commit2=`git_show_head $testroot/repo` local short_commit2=`trim_obj_id 32 $commit2` @@ -304,7 +307,10 @@ test_blame_lines_shifted_down() { local short_commit1=`trim_obj_id 32 $commit1` local author_time=`git_show_author_time $testroot/repo` - sed -i -e '/^8$/d' $testroot/wt/alpha + ed -s $testroot/wt/alpha <<-\EOF + g/^8$/d + w + EOF (cd $testroot/wt && got commit -m "change 2" > /dev/null) local commit2=`git_show_head $testroot/repo` local short_commit2=`trim_obj_id 32 $commit2` blob - 53011f6f6351ec5e36a888fac308c8d4f0f0002e file + regress/cmdline/commit.sh --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -1591,7 +1591,10 @@ sed -i 's/foo/bar/' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/foo/bar/' "\$1" +ed -s "\$1" <<-EOF + ,s/foo/bar/ + w + EOF EOF chmod +x $testroot/editor.sh @@ -1854,7 +1857,10 @@ sed -i 's/# l/l/' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/# l/l/' "\$1" +ed -s "\$1" <<-EOF + ,s/# l/l/ + w + EOF EOF chmod +x $testroot/editor.sh blob - cfcbb7402cba3671e0519ad680092846ffd36a21 file + regress/cmdline/diff.sh --- regress/cmdline/diff.sh +++ regress/cmdline/diff.sh @@ -443,14 +443,26 @@ test_diff_shows_conflict() { return 1 fi - sed -i 's/2/22/' $testroot/repo/numbers - sed -i 's/8/33/' $testroot/repo/numbers + ed -s $testroot/repo/numbers <<-\EOF + ,s/2/22/ + w + EOF + ed -s $testroot/repo/numbers <<-\EOF + ,s/8/33/ + w + EOF git_commit $testroot/repo -m "modified line 2" local head_rev=`git_show_head $testroot/repo` # modify lines 2 and 8 in conflicting ways - sed -i 's/2/77/' $testroot/wt/numbers - sed -i 's/8/88/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/2/77/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/8/88/ + w + EOF echo "C numbers" > $testroot/stdout.expected echo -n "Updated to refs/heads/master: $head_rev" \ blob - 737b0cb81e316d7acd423ca57c68f090d5f33d9f file + regress/cmdline/fetch.sh --- regress/cmdline/fetch.sh +++ regress/cmdline/fetch.sh @@ -360,7 +360,10 @@ test_fetch_branch() { fi # remove default branch information from got.conf - sed -i -e "/branch {/d" $testroot/repo-clone/got.conf + ed -s $testroot/repo-clone/got.conf <<-EOF + g/branch {/d + w + EOF # make another change on 'foo' and fetch it without got.conf (cd $testroot/repo && git checkout -q foo) @@ -1610,7 +1613,10 @@ test_fetch_honor_wt_conf_bflag() { # from repo: fetch got.conf branch which doesn't exist, so fallback # to repo HEAD "boo" # change default branch in got.conf from "master" to "foo" - sed -i "s/master/foo/" $testroot/repo-clone/got.conf + ed -s $testroot/repo-clone/got.conf <<-EOF + ,s/master/foo/ + w + EOF got fetch -q -r $testroot/repo-clone > $testroot/stdout ret=$? @@ -1708,7 +1714,10 @@ test_fetch_honor_wt_conf_bflag() { # from wt: fetch got.conf "master", wt "boo", and the repo's new HEAD # "hoo" as it no longer matches our remote HEAD symref target "master" - sed -i "s/foo/master/" $testroot/repo-clone/got.conf + ed -s $testroot/repo-clone/got.conf <<-EOF + ,s/foo/master/ + w + EOF echo "modified delta on master" > $testroot/repo/gamma/delta git_commit $testroot/repo -m "modified delta on master" local commit_id5=`git_show_head $testroot/repo` blob - b7f9e922f4dd32f8ae9e742d6701a371efa9fc58 file + regress/cmdline/histedit.sh --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -131,7 +131,10 @@ test_histedit_no_op() { got diff -r $testroot/repo $orig_commit $new_commit2 \ > $testroot/diff - sed -i -e "s/$old_commit2/$new_commit2/" $testroot/diff.expected + ed -s $testroot/diff.expected <<-EOF + ,s/$old_commit2/$new_commit2/ + w + EOF cmp -s $testroot/diff.expected $testroot/diff ret=$? if [ $ret -ne 0 ]; then @@ -340,7 +343,10 @@ test_histedit_swap() { got diff -r $testroot/repo $orig_commit $new_commit1 \ > $testroot/diff - sed -i -e "s/$old_commit2/$new_commit1/" $testroot/diff.expected + ed -s $testroot/diff.expected <<-EOF + ,s/$old_commit2/$new_commit1/ + w + EOF cmp -s $testroot/diff.expected $testroot/diff ret=$? if [ $ret -ne 0 ]; then @@ -445,8 +451,14 @@ test_histedit_drop() { got diff -r $testroot/repo $orig_commit $new_commit2 \ > $testroot/diff - sed -i -e "s/$old_commit1/$orig_commit/" $testroot/diff.expected - sed -i -e "s/$old_commit2/$new_commit2/" $testroot/diff.expected + ed -s $testroot/diff.expected <<-EOF + ,s/$old_commit1/$orig_commit/ + w + EOF + ed -s $testroot/diff.expected <<-EOF + ,s/$old_commit2/$new_commit2/ + w + EOF cmp -s $testroot/diff.expected $testroot/diff ret=$? if [ $ret -ne 0 ]; then @@ -1149,7 +1161,10 @@ test_histedit_path_prefix_edit() { got diff -r $testroot/repo $orig_commit $new_commit1 \ > $testroot/diff - sed -i -e "s/$old_commit1/$new_commit1/" $testroot/diff.expected + ed -s $testroot/diff.expected <<-EOF + ,s/$old_commit1/$new_commit1/ + w + EOF cmp -s $testroot/diff.expected $testroot/diff ret=$? if [ $ret -ne 0 ]; then @@ -1564,7 +1579,10 @@ sed -i 's/.*/committing folded changes/' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/.*/committing folded changes/' "\$1" +ed -s "\$1" <<-EOF + ,s/.*/committing folded changes/ + w + EOF EOF chmod +x $testroot/editor.sh @@ -1680,7 +1698,10 @@ sed -i 'd' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 'd' "\$1" +ed -s "\$1" <<-EOF + ,d + w + EOF EOF chmod +x $testroot/editor.sh @@ -1814,7 +1835,10 @@ sed -i 's/.*/committing edited changes 1/' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/.*/committing edited changes 1/' "\$1" +ed -s "\$1" <<-EOF + ,s/.*/committing edited changes 1/ + w + EOF EOF chmod +x $testroot/editor.sh @@ -1843,7 +1867,10 @@ sed -i 's/.*/committing edited changes 2/' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/.*/committing edited changes 2/' "\$1" +ed -s "\$1" <<-EOF + ,s/.*/committing edited changes 2/ + w + EOF EOF chmod +x $testroot/editor.sh @@ -2239,7 +2266,10 @@ sed -i 's/ x bit / executable bit /' "\$1" cat > $testroot/editor.sh <<EOF #!/bin/sh -sed -i 's/ x bit / executable bit /' "\$1" +ed -s "\$1" <<-EOF + ,s/ x bit / executable bit / + w + EOF EOF chmod +x $testroot/editor.sh blob - fceaf181cd74c8b1810cfe97234af037bb77561c file + regress/cmdline/rebase.sh --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -891,8 +891,14 @@ test_rebase_preserves_logmsg() { (cd $testroot/wt && got log -c newbranch -l2 | grep -v ^date: \ > $testroot/log) - sed -i -e "s/$orig_commit1/$new_commit1/" $testroot/log.expected - sed -i -e "s/$orig_commit2/$new_commit2/" $testroot/log.expected + ed -s $testroot/log.expected <<-EOF + ,s/$orig_commit1/$new_commit1/ + w + EOF + ed -s $testroot/log.expected <<-EOF + ,s/$orig_commit2/$new_commit2/ + w + EOF cmp -s $testroot/log.expected $testroot/log ret=$? if [ $ret -ne 0 ]; then blob - b568329b09e4e5ae44e43459bc4b32b4d3050b36 file + regress/cmdline/revert.sh --- regress/cmdline/revert.sh +++ regress/cmdline/revert.sh @@ -384,9 +384,18 @@ test_revert_patch() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF (cd $testroot/wt && got diff > $testroot/numbers.diff) @@ -564,7 +573,10 @@ EOF fi # put first hunk back - sed -i -e 's/^2$/a/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF # revert middle hunk printf "n\ny\nn\n" > $testroot/patchscript @@ -866,7 +878,10 @@ test_revert_patch_one_change() { # Ensure file size is changed. Avoids race condition causing test # failures where 'got revert' does not see changes to revert if # timestamps and size in stat info remain unchanged. - sed -i -e 's/^2$/aa/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/aa/ + w + EOF # revert change with -p printf "y\n" > $testroot/patchscript blob - 01dd0b976f6f7b5d840e72398cadf29728591229 file + regress/cmdline/stage.sh --- regress/cmdline/stage.sh +++ regress/cmdline/stage.sh @@ -1452,9 +1452,18 @@ test_stage_patch() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF # don't stage any hunks printf "n\nn\nn\n" > $testroot/patchscript @@ -1739,9 +1748,18 @@ test_stage_patch_twice() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF # stage middle hunk printf "n\ny\nn\n" > $testroot/patchscript @@ -2235,9 +2253,18 @@ test_stage_patch_quit() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF (cd $testroot/wt && got rm zzz > /dev/null) # stage first hunk and quit; and don't pass a path argument to @@ -2344,9 +2371,18 @@ test_stage_patch_incomplete_script() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF # stage first hunk and then stop responding; got should error out printf "y\n" > $testroot/patchscript blob - 6098eb8ab154c97c655c6d305d0d01b1db1c71a7 file + regress/cmdline/status.sh --- regress/cmdline/status.sh +++ regress/cmdline/status.sh @@ -187,11 +187,17 @@ test_status_shows_local_mods_after_update() { return 1 fi - sed -i 's/2/22/' $testroot/repo/numbers + ed -s $testroot/repo/numbers <<-\EOF + ,s/2/22/ + w + EOF git_commit $testroot/repo -m "modified line 2" # modify line 7; both changes should merge cleanly - sed -i 's/7/77/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/7/77/ + w + EOF echo "G numbers" > $testroot/stdout.expected echo -n "Updated to refs/heads/master: " >> $testroot/stdout.expected @@ -342,11 +348,17 @@ test_status_shows_no_mods_after_complete_merge() { return 1 fi - sed -i 's/2/22/' $testroot/repo/numbers + ed -s $testroot/repo/numbers <<-\EOF + ,s/2/22/ + w + EOF git_commit $testroot/repo -m "modified line 2" # modify line 2 again; no local changes are left after merge - sed -i 's/2/22/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/2/22/ + w + EOF echo "G numbers" > $testroot/stdout.expected echo -n "Updated to refs/heads/master: " >> $testroot/stdout.expected @@ -396,11 +408,17 @@ test_status_shows_conflict() { return 1 fi - sed -i 's/2/22/' $testroot/repo/numbers + ed -s $testroot/repo/numbers <<-\EOF + ,s/2/22/ + w + EOF git_commit $testroot/repo -m "modified line 2" # modify line 2 in a conflicting way - sed -i 's/2/77/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/2/77/ + w + EOF echo "C numbers" > $testroot/stdout.expected echo -n "Updated to refs/heads/master: " >> $testroot/stdout.expected blob - 768bc40e938998a7e2da71aaab91d5477bcc2725 file + regress/cmdline/unstage.sh --- regress/cmdline/unstage.sh +++ regress/cmdline/unstage.sh @@ -198,9 +198,18 @@ test_unstage_patch() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF (cd $testroot/wt && got stage > /dev/null) ret=$? @@ -829,9 +838,18 @@ test_unstage_patch_quit() { return 1 fi - sed -i -e 's/^2$/a/' $testroot/wt/numbers - sed -i -e 's/^7$/b/' $testroot/wt/numbers - sed -i -e 's/^16$/c/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/^2$/a/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^7$/b/ + w + EOF + ed -s $testroot/wt/numbers <<-\EOF + ,s/^16$/c/ + w + EOF (cd $testroot/wt && got rm zzz > /dev/null) (cd $testroot/wt && got stage > /dev/null) blob - 0c032a1c4d45adc6a488c656983e61f0effc8e54 file + regress/cmdline/update.sh --- regress/cmdline/update.sh +++ regress/cmdline/update.sh @@ -684,12 +684,18 @@ test_update_merges_file_edits() { echo "modified alpha" > $testroot/repo/alpha echo "modified beta" > $testroot/repo/beta - sed -i 's/2/22/' $testroot/repo/numbers + ed -s $testroot/repo/numbers <<-\EOF + ,s/2/22/ + w + EOF git_commit $testroot/repo -m "modified 3 files" echo "modified alpha, too" > $testroot/wt/alpha touch $testroot/wt/beta - sed -i 's/7/77/' $testroot/wt/numbers + ed -s $testroot/wt/numbers <<-\EOF + ,s/7/77/ + w + EOF echo "C alpha" > $testroot/stdout.expected echo "U beta" >> $testroot/stdout.expected #!/usr/bin/perl -w while(<>) { if (/^\tsed -i( -e)? (['"])([^'"]*)['"]\s(.*)/) { chomp; my ($quote, $expr, $file) = ($2, $3, $4); if ($quote eq "'") { print "\ted -s $file <<-\\EOF\n"; } else { print "\ted -s $file <<-EOF\n"; } if ($expr =~ /^\//) { print "\tg$expr\n"; } else { print "\t,$expr\n"; } print "\tw\n"; print "\tEOF\n"; } elsif (/^sed -i( -e)? (['"])([^'"]*)['"]\s(.*)/) { chomp; my ($quote, $expr, $file) = ($2, $3, $4); print "ed -s $file <<-EOF\n"; if ($expr =~ /^\//) { print "\tg$expr\n"; } else { print "\t,$expr\n"; } print "\tw\n"; print "\tEOF\n"; } else { print; } }

2023-03-04 20:34 Christian Weisgerber:
regress: Replace sed -i with ed for portability
- 2023-03-05 07:53 Stefan Sperling:
  regress: Replace sed -i with ed for portability
- 2023-03-05 08:10 Omar Polo:
  regress: Replace sed -i with ed for portability