Re: [RFC PATCH 0/4] git-gui: support SHA-256 repositories

[Carlo Arenas <carenas@gmail.com>]

On Mon, Oct 11, 2021 at 7:21 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Mon, Oct 11 2021, Carlo Marcelo Arenas Belón wrote:
>
> > [1] https://lore.kernel.org/git/20211011114723.204-1-carenas@gmail.com/
> >
> > Carlo Marcelo Arenas Belón (4):
> >   blame: prefer null_sha1 over nullid and retire later
> >   rename all *_sha1 variables and make null_oid hash aware
> >   expand regexp matching an oid to be hash agnostic
> >   track oid_size to allow for checks that are hash agnostic
> >
> >  git-gui.sh                   | 30 ++++++++++++++++--------------
> >  lib/blame.tcl                | 18 +++++++++---------
> >  lib/checkout_op.tcl          |  4 ++--
> >  lib/choose_repository.tcl    |  2 +-
> >  lib/commit.tcl               |  3 ++-
> >  lib/remote_branch_delete.tcl |  2 +-
> >  6 files changed, 31 insertions(+), 28 deletions(-)
>
> There was a similar series earlier this year which didn't make it that
> fixes some of the same issues:
> https://lore.kernel.org/git/pull.979.git.1623687519832.gitgitgadget@gmail.com/

This specific series is for git-gui, and the one posted before is for gitk,
but the code is still similar enough, and indeed the gitk part was
included in a reference.

> Just seems like a lot of needless work as opposed to just matching
> x{40,64} or whatever.  Yes that's not the same regex semantically, but I
> think the current code is just being overly strict, i.e. it's parsing
> some plumbing output, we can trust that the thing that looks like the
> OID in that position is the OID.
>
> If anything I'd think we could just match [0-9a-f]{4,} in most/all of
> these cases, would make things like this easier to read:

It makes me nervous though to see checks like the one I fixed on
commit[1] that use logic to check the correct size of the SHA as an
implication of it being a valid value.

considering the code is very old, maybe that was relevant long ago?,
but agree some checks seem to be unnecessarily strict.

I have relaxed some of the checks in the gitk patch and will be
posting it soon, so hopefully reviews from people that know the code
better could be collected.

Carlo

[1] https://lore.kernel.org/git/20211011121757.627-5-carenas@gmail.com/