# Re: [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR 2 messages from 2014-08-24 to 2014-08-25. Participants: Gábor Szeder, Jeff King. Thread: https://gitlist.dev/t/37409 ## Gábor Szeder, 2014-08-24 13:22 Subject: Re: [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR Message-ID: URL: https://gitlist.dev/e/E1XLXkh-0002sL-IJ%40iramx2.ira.uni-karlsruhe.de ``` Hi, On Aug 23, 2014 12:26 PM, Jeff King wrote: > Since dd0b72c (bash prompt: use bash builtins to check stash > state, 2011-04-01), git-prompt checks whether we have a > stash by looking for $GIT_DIR/refs/stash. Generally external > programs should never do this, because they would miss > packed-refs. Not sure whether the prompt script is external program or not, but doesn't matter, this is the right thing to do. > That commit claims that packed-refs does not pack > refs/stash, but that is not quite true. It does pack the > ref, but due to a bug, fails to prune the ref. When we fix > that bug, we would want to be doing the right thing here. > > Signed-off-by: Jeff King > --- > I know we are pretty sensitive to forks in the prompt code (after all, > that was the point of dd0b72c). This patch is essentially a reversion of > this hunk of dd0b72c, and is definitely safe. I'm not sure, but if I remember correctly (don't have the means to check it at the moment, sorry) in that commit I also added a 'git pack-ref' invocation to the relevant test(s?) to guard us against breakages due to changes in 'git pack-refs'.  If that is so, then I think those invocations should be removed as well, as they'll become useless. Best, Gábor ``` ## Jeff King, 2014-08-25 12:46 Subject: Re: [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR Message-ID: <20140825124631.GC17288@peff.net> URL: https://gitlist.dev/e/20140825124631.GC17288%40peff.net In-Reply-To: ``` On Sun, Aug 24, 2014 at 08:22:41PM +0700, Gábor Szeder wrote: > On Aug 23, 2014 12:26 PM, Jeff King wrote: > > Since dd0b72c (bash prompt: use bash builtins to check stash > > state, 2011-04-01), git-prompt checks whether we have a > > stash by looking for $GIT_DIR/refs/stash. Generally external > > programs should never do this, because they would miss > > packed-refs. > > Not sure whether the prompt script is external program or not, but > doesn't matter, this is the right thing to do. Yeah, by external I just meant "nothing outside of refs.c should make this assumption". > > That commit claims that packed-refs does not pack > > refs/stash, but that is not quite true. It does pack the > > ref, but due to a bug, fails to prune the ref. When we fix > > that bug, we would want to be doing the right thing here. > > > > Signed-off-by: Jeff King > > --- > > I know we are pretty sensitive to forks in the prompt code (after all, > > that was the point of dd0b72c). This patch is essentially a reversion of > > this hunk of dd0b72c, and is definitely safe. > > I'm not sure, but if I remember correctly (don't have the means to > check it at the moment, sorry) in that commit I also added a 'git > pack-ref' invocation to the relevant test(s?) to guard us against > breakages due to changes in 'git pack-refs'.  If that is so, then I > think those invocations should be removed as well, as they'll become > useless. It did add that change (that's actually how I noticed the problem! Thank you for being thorough in dd0b72c). My inclination is to leave the pack-refs invocations, as they protect against a certain class of errors (we are not doing the risky behavior now, but the purpose of the test suite is to detect regressions; the next person to touch that code may not be so careful as you were). I don't feel too strongly, though, so if we want them gone, I'm OK with that. -Peff ```