From: Koji Nakamaru Date: Thu, 13 Nov 2025 08:17:06 GMT Subject: Re: [PATCH] Revert "osxkeychain: state to skip unnecessary store operations" Message-ID: In-Reply-To: On Thu, Nov 13, 2025 at 1:47 AM Junio C Hamano wrote: > > "Koji Nakamaru via GitGitGadget" writes: > > > From: Koji Nakamaru > > > > 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. I see. > > 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. I see. I'll work on the following approach and submit another patch. > > "osxkeychain:seen" can encode the whole information of the last > > "get".