Re: [PATCH] Revert "osxkeychain: state to skip unnecessary store operations"

>
> "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Koji Nakamaru <koji.nakamaru@gree.net>
> >
> > This reverts commit e1ab45b2dab51f94db9548666dfd7af626d2aa7e.
>
> OK.  Let's make a mental note that e1ab45b2 (osxkeychain: state to
> skip unnecessary store operations, 2024-05-15) appeared in v2.46 or
> so.

> > That commit was trying to skip to store a credential returned by
> > "git-credential-osxkeychain get" by setting
> > "state[]=osxkeychain:seen=1". However, this state[] is kept even if a
> > credential returned by "git-credential-osxkeychain get" is invalid and
> > another subsequent helper's "get" returns a valid credential. Another
> > subsequent helper (such as [1]) may expect git-credential-osxkeychain to
> > store the valid credential so that "store" cannot be skipped by just
> > checking "state[]=osxkeychain:seen=1".
> >
> > In order to solve this issue, the state[] mechanism can be refined or
> > "osxkeychain:seen" can encode the whole information of the last
> > "get". For now, let's revert the change.
>
> Is anybody actively working on the proper solution?
>
> In a patch series that replaces the old commit with a more proper
> solution, it could be a reasonable layout of the series to make the
> first patch a revert like this patch to give the proper solution a
> clean slate to work from, but this looks different.
>
> If the problem you are trying to solve here were a regression that
> happened after Git 2.51 was released, a revert is totally warranted
> at this point in time, even during the pre-release freeze period.
>
> But it does not even look like a recent regression.  Wouldn't
> reverting this change at this point give existing users who are
> accustomed to the current behaviour another regression, essentially
> robbing Peter to pay Paul?  In such a case, I do not think "let's
> revert now and then hopefully a proper solution can come later" is a
> good approach.