Bug 213474

Summary: sed behavior change with a/i/c operands.
Product: Base System Reporter: Sergey <kpect>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed Overcome By Events    
Severity: Affects Many People CC: pfg
Priority: --- Keywords: patch, regression
Version: 11.0-STABLE   
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208554
Attachments:
Description Flags
Proof-of-concept: provide a compile-time flag for GNU compatibility
none
Add a CFLAG to re-enable FreeBSD 10 behavior none

Description Sergey 2016-10-14 09:30:00 UTC
Hello.

Al of a sudden sed began to act differently on FreeBSD 11 

    On FreeBSD 11.0 following script (<------> - tab character)

# cat file
SEARCHED_LINE

#!/bin/sh
sed -E -i '.orig' \
<------>-e '/^SEARCHED_LINE/ a\
<------>line1\
line2\
<------>\<------>line3' \
file

performs this output:

SEARCHED_LINE
<------>line1
line2
<------><------>line3

While on FreeBSD 10.3 and previuos versions it performed this:

line1
line2
<------>line3

Why have you changed pretty sane and convenient sed's behaviuor to gsed's one?
My scripts looked very neat and structured with previuos BSD sed.
Do I have to rewrite all my scripts now?
Regards,
Sergey.
Comment 1 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-10-17 19:51:03 UTC
I am not sure this can be classified as a regression.

It is certainly a change in behavior with the idea of being more compatible with GNU sed: some people may be moving from linux to FreeBSD and may appreciate the compatibility.

Standards don't force either behavior though, and I do see it would be somewhat advantageous to do some things different than GNU sed, but I am completely
agnostic here.

If we revert to the previous behavior I would prefer it be done fast but OTOH, the damage is already done for 11.0.
Comment 2 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-10-18 01:34:33 UTC
Created attachment 175892 [details]
Proof-of-concept: provide a compile-time flag for GNU compatibility

An idea that could use polishing is to re-use the WITHOUT_GNU_SUPPORT to decide whether sed should attempt to behave in a GNU compatible way or not.

I am leaning towards defaulting to the previous behavior: as Sergey states, people can always install GNU sed if the want.
Comment 3 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-10-20 16:35:42 UTC
Created attachment 175985 [details]
Add a CFLAG to re-enable FreeBSD 10 behavior

The new behavior in FBSD 11+, is also present in SYSV UNIX. The new behavior should help people migrating from Solaris as well as linux so I think it should be the default.

It still looks convenient and easy to add a CFLAG (LEGACY_BSDSED_COMPAT) to make it easier to re-enable the old behavior.

I am not convinced we should go as far as adding a new build option for this, but at least this should make it easier to custom-build a sed that supports the old behavior.
Comment 4 Sergey 2016-10-27 13:28:19 UTC
(In reply to Pedro F. Giffuni from comment #3)

In my humble opinion it's a bad decision.
In that case i'd rather patch sed's source like this:

	# Restore original BSD sed
	sed -E -i '' \
		-e '/		p = lbuf;/ a\
		\		EATSPACE();' \
	usr.bin/sed/compile.c

I suppose FreeBSD shuld remain authenic and must not rush after Linux, as the ones who like Linux will use Linux, not BSD. FreeBSD should not copy or be 'like' Linux because it will never be on par with it... Why not to import coreutils in this case? FreeBSD has it's own benefits and should keep 'em. Those who migrate from Linux seek for something else/something new and not another linux with different name.

Regards,
Sergey.
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-11-04 20:50:34 UTC
A commit references this bug:

Author: pfg
Date: Fri Nov  4 20:49:59 UTC 2016
New revision: 308314
URL: https://svnweb.freebsd.org/changeset/base/308314

Log:
  sed(1): add LEGACY_BSDSED_COMPAT compile-time flag.

  In r297602, which included a __FreeBSD_version bump to 1100105, we changed
  sed 'i' and 'a' from discarding whitespaces to conform with what GNU and
  sysvish sed do.

  There are arguments in favor of keeping the old behavior but the new
  behavior is also useful for migration purposes. It seems important to at
  least consider the case of developers depending on the previous behavior,
  so add a CFLAG to enable the old behaviour.

  PR:		213474
  MFC after:	5 days

Changes:
  head/usr.bin/sed/compile.c
Comment 6 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-11-05 14:56:03 UTC
(In reply to commit-hook from comment #5)

For the record:

kib@ has suggested, and I agree, that an environment variable may be more convenient for this case. IMHO, the default should remain the current behavior.

The naming of a descriptive environment variable, and perhaps even a patch for both code and manpage are welcome ;).
Comment 7 Sergey 2016-11-06 07:47:44 UTC
IMHO it'd be better to leave BSD behavior as default one as anyone who likes may install GNU sed from ports (/usr/ports/textproc/gsed) together with GNU awk and bash to play with.
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-11-09 18:01:27 UTC
A commit references this bug:

Author: pfg
Date: Wed Nov  9 18:00:50 UTC 2016
New revision: 308472
URL: https://svnweb.freebsd.org/changeset/base/308472

Log:
  MFC r308314:
  sed(1): add LEGACY_BSDSED_COMPAT compile-time flag.

  In r297602, which included a __FreeBSD_version bump to 1100105, we changed
  sed 'i' and 'a' from discarding whitespaces to conform with what GNU and
  sysvish sed do.

  There are arguments in favor of keeping the old behavior but the new
  behavior is also useful for migration purposes. It seems important to at
  least consider the case of developers depending on the previous behavior
  so add a CFLAG to enable the old behavior.

  PR:		213474

Changes:
_U  stable/11/
  stable/11/usr.bin/sed/compile.c