On Sun, Aug 24, 2014 at 08:22:41PM +0700, Gábor Szeder wrote: > On Aug 23, 2014 12:26 PM, Jeff King <peff@xxxxxxxx> 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 <peff@xxxxxxxx> > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html