Created attachment 223048 [details] sed.diff This is one of the most frustrating things in FreeBSD userland for me and can be a PITA when creating new ports or updating them because the rest of the world expects sed to behave differently than our sed. I'm sure this has come up before but I could not find a bug for it. Our sed uses getopt("i:") instead of getopt("i::") like GNU sed or even NetBSD sed or OpenBSD sed. As a consequence something like `sed -i 's/foo/bar/' file` will not work on FreeBSD but works just fine on Linux/NetBSD/OpenBSD (our sed interprets `s/foo/bar/` as the backup file suffix and `file` as the expression, whereas GNU/NetBSD/OpenBSD sed see that we want no backup file). For a concrete example of something in the wild, this here is from the editors/mg configure script which is fine with the other seds sed -i 's,<term.h>,"terminfo_term.h",g' display.c but errors out with ours sed: 1: "display.c": extra characters at the end of d command If have sent a patch for this upstream to "fix" it, but really it is sed(1) that should be fixed and should be made to work like the rest of the world. We should deal with the consequences on our side. I have a hard time sending patches like this upstream without flinching because I'm essentially pushing a fix for a FreeBSD bug onto upstream. It has been like this forever and FWICT it has been copied to macOS too, so maybe it is too late now... Changing this is obviously a POLA violation and will break scripts of many users but it would be nice if we could do it for FreeBSD 14 anyway. I'm attaching a patch that does what I want I think but most of the tests and other scripts in the tree are broken since they all use bad sed calls now. Ironically there also seem to be some scripts in the tree that are obviously broken with FreeBSD sed and expect GNU sed behavior (for example release/tools/gce.conf). I'd be willing to help fix any ports fallout caused by this.
How about also adding a test case src/usr.bin/sed/tests?
(In reply to Li-Wen Hsu from comment #1) What test should be added? There are test cases for -i in the test suite already AFAICT. Obviously all of them are broken now because of the behavior change to -i.
(In reply to Tobias Kortkamp from comment #2) Hmm, there is no usr.bin/sed test failing currently: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/17760/testReport/usr.bin.sed/ So we might need to check if those -i tests are not running, or we also need to modify them along with the fix.
(In reply to Li-Wen Hsu from comment #3) I'm confused. Is that build with the patch applied? That can't be right.
(In reply to Tobias Kortkamp from comment #4) No, that build is what in the -CURRENT now. If there are tests about -i running, then the current sed behavior matches the tests. So if we change (fix) the -i behavior, those tests will fail and we may also need to modify them along with the fix.
(In reply to Li-Wen Hsu from comment #5) Yes, of course. Tests and a lot of other scripts too. But I think preemptively working on that might be a waste of time unless multiple people think this change is generally a good idea and wanted.
This is going to be hard change to make, but we might be able to do it in 14.0
As far as I can tell there is no compatible way to specify no backup extension (between GNU and current FreeBSD sed), but we could have FreeBSD sed handle both -i and -i '' (assuming that there's no legitimate other interpretation of -i ''). Maybe we can do this in a couple of steps, starting with a warning on '-i .bak'? something like: case 'i': if (optarg) { inplace = optarg; } else { if (optind >= argc) errx(1, "no arg for -i"); if (*argv[optind] == '\0') { inplace = ""; } else { inplace = argv[optind]; warnx("-i %s deprecated, use -i%s", inplace, inplace); } optind++; } } and then later: case 'i': if (optarg) { inplace = optarg; } else { inplace = ""; // Backwards compat for historical -i '' if (optind < argc && *argv[optind] == '\0') optind++; } break;
I posted a possible first step in https://reviews.freebsd.org/D29123
Man page update with no functional change, as a first step: https://reviews.freebsd.org/D29128