Re: [PATCH v3 2/3] Setup working tree in describe
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jan 29, 2019, 20:53 UTC
- Message-ID
- <CAPig+cRUCmaiYqKG6ON==g1sFph-hz6nK5fTYc-9w2JaNFmdLA@mail.gmail.com>
- In-Reply-To
- <20190129131151.GB22211@sigill.intra.peff.net>
On Tue, Jan 29, 2019 at 8:12 AM Jeff King <peff@peff.net> wrote:
Show 17 quoted lines
> On Tue, Jan 29, 2019 at 06:18:58AM +0100, Sebastian Staudt wrote: > > This ensures the given working tree is used for --dirty. > > There's been a lot of digging and discussion on the list about what > happens if we don't do this. Could we summarize it here? > > Perhaps: > > We don't use NEED_WORK_TREE when running the git-describe builtin, > since you should be able to describe a commit even in a bare > repository. However, the --dirty flag does need a working tree. Since > we don't call setup_work_tree(), it uses whatever directory we happen > to be in. That's unlikely to match our index, meaning we'd say "dirty" > even when the real working tree is clean. > > We can fix that by calling setup_work_tree() once we know that the > user has asked for --dirty.
I have not particularly been following this thread, but this proposed commit message does an excellent job of summarizing and explaining the issue and making the fix obvious (so, now I don't have to go back and read the entire thread).
Show 10 quoted lines
> > The implementation of --broken uses diff-index which calls > > setup_work_tree() itself. > > If I hadn't just read the rest of the thread, I'd probably wonder why we > are talking about --broken at all. Maybe: > > The --broken option similarly needs a working tree. But because the > current implementation calls an external diff-index to do the work, > we don't have to bother setting up the working tree in the > git-describe process.
This rewrite left me slightly in the dark since I had to infer that git-diff-index calls setup_work_tree() itself. Perhaps:
...an external diff-index to do the work, which itself calls
setup_work_tree(), we don't have to bother...But that's minor.