Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream "perforce" package

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 21, 2022, 21:29 UTC
Message-ID: <CAPig+cTWj2Xf4sfMJBkzLhG5oUBS+Pb5-D-K50xMt9rP0doBRA@mail.gmail.com>
In-Reply-To: <220421.86pmla5eja.gmgdl@evledraar.gmail.com>

On Thu, Apr 21, 2022 at 5:10 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 16 quoted lines

> On Thu, Apr 21 2022, Junio C Hamano wrote:
> > Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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 ...