"sed -i" without an extension argument does not work on SmartOS

[wiedi]

"sed -i" without an extension argument does not work on SmartOS but does on OpenIndiana.

I went through some history and found the following:

• In https://www.illumos.org/issues/586 (sed -i should take an "optional" argument) this was initially fixed and integrated as: https://github.com/joyent/illumos-joyent/commit/e50226eccc6dfcba3cc6f0df38438900e3df225c.

• However it was later reverted in: https://github.com/joyent/illumos-joyent/commit/58474c20a5282db4eba6c3a9f4c028e06988e09e.

• I assume because of https://www.illumos.org/issues/1815 (sed -i "" no longer works) for which @jclulow has proposed a fix for in https://gist.github.com/jclulow/1394562.

Is this correct? Could we revert the revert and get 586 back, maybe with the additional fix for 1815?

[danmcd]

This looks like something people forgot about as a fire burned somewhere. The GIST you cite should be enough to take and run with it. I'd suggest moving said gist into a modern illumos-gate repo and send it through a build/test/review cycle on the public list.

[wiedi]

I've done some tests of the 1815 fix on OI and that seems to work. I'll send it to the list for review later.

If anyone wants to play with this on SmartOS I have the two commits in this branch: https://github.com/joyent/illumos-joyent/compare/master...wiedi:sed - the build is still running, I'll report back when I've done the testing also on SmartOS.

[wiedi]

@jclulow was so helpful to point me to the original review thread [1] for 1815 which I had missed. In that thread a simpler solution was proposed: https://sysmgr.org/~leftwing/files/webrev/1815/ That fix also was already reviewed by two people but the integration never happend.

Just to be sure I tested the simpler fix on SmartOS and OpenIndiana with a small script:

#!/bin/ksh

sed_cmd="$1"
[[ -z "$sed_cmd" ]] && sed_cmd="sed"

f=test.txt

fatal()
{
	typeset msg="$*"
	[[ -z "$msg" ]] && msg="failed"
	echo "TEST FAILED: $sed_cmd: $msg" >&2
	exit 1
}

check_sed()
{
	typeset ret=$1
	typeset val=$2
	typeset msg=$3

	if [[ $ret -ne 0 ]]; then
		fatal "test $val failed, exit code $ret"
	fi

	if [[ $val != $(cat $f) ]]; then
		fatal "test $val failed, content is $val"
	fi
}


echo zero > $f

$sed_cmd -i '' 's/zero/one/' $f
check_sed $? "one"

$sed_cmd -i'' 's/one/two/' $f
check_sed $? "two"

$sed_cmd -i 's/two/three/' $f
check_sed $? "three"

$sed_cmd -i.1 's/three/four/' $f
check_sed $? "four"
if [[ "three" != $(cat $f.1) ]]; then
	fatal "test four failed, backup content is $val"
fi

rm $f $f.1

echo "TEST PASSED: $sed_cmd"

Given that this already has been reviewed would it be ok to go to RTI directly? Is it ok for me to do this?

Would this fix be enough for SmartOS to finally also take the 586 change and unify the behaviour on all illumos distributions (and NetBSD, OpenBSD and GNU)? This would be a flag-day, but in my opinion prevent a lot of frustration and confusion going forward.

[1] https://illumos.topicbox.com/groups/developer/T00e923ce681d4fb0-M2eed482332a8ece152cc0463