"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Christian Weisgerber <naddy@mips.inka.de>
Subject:
regress: Replace sed -i with ed for portability
To:
gameoftrees@openbsd.org
Date:
Sat, 4 Mar 2023 21:34:05 +0100

Download raw body.

Thread
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;
	}
}