On Thu, Apr 21, 2022 at 5:10 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > On Thu, Apr 21 2022, Junio C Hamano wrote: > > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> + sed -i -e 's/\(sha256.\).*/\1:no_check/' "$path" && > > > > "sed -i" is not POSIX and without macOS box I do not know if it > > works there. FreeBSD sed manual seems to indicate they have > > > > sed -i <extension> > > > > Ah, no, I'd say we should NOT use "sed -i" here, not in the form in > > this patch, after seeing > > https://unix.stackexchange.com/questions/401905/bsd-sed-vs-gnu-sed-and-i > > but that is 4-year old information, so... > > It works on the OSX we use now: > https://github.com/avar/git/runs/6092916612?check_suite_focus=true On my end-of-life'd 10.13 macOS, `sed` (which is the FreeBSD version) requires an argument after `-i`, thus it's taking `-e` as the extension name for the backup file, and then uses the raw 's/\(sha256.\).*/\1:no_check/' as the `sed` command. So, it's working, but only by accident since `-e` is optional when invoking `sed`. > I think it's fine to keep it, but we could also use "perl -pi -e" here, > or a rename dance... Although it's working (by accident), it's rather misleading since it looks like `-e` is introducing the `sed` command, even though it really isn't (the `-e` is being consumed by the `-i` option). Any of the following would likely be less confusing (in no particular order of preference): * sed -i .bak -e '...' "$path" * rename dance * perl -pi -e ...
