Re: [RFC] describe: add option --dirty
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Jul 23, 2007, 06:59 UTC
- Message-ID
- <20070723065912.GG32566@spearce.org>
- In-Reply-To
- <87odi3mxtl.wl@mail2.atmark-techno.com>
Yasushi SHOJI <yashi@atmark-techno.com> wrote:
Show 5 quoted lines
> when --dirty is given, git describe will check the working tree and > append "-dirty" to describe string if the tree is dirty. > --- > I'm not sure this is good idea or the current way (using diff-index in > shell script) is more prefered.
Yea, I'm actually torn on this. A lot of people like the output of git-describe for versions (where a lot is at least me!) and yet I also always tack in the -dirty if the directory is dirty according to diff-index. So having this built right into git-describe is actually quite handy. It simplifies a little bit of build rule logic.
Show 8 quoted lines
> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt > @@ -53,6 +53,9 @@ OPTIONS > being employed to standard error. The tag name will still > be printed to standard out. > > +--dirty:: > + Append "-dirty" to describe string if working tree is dirty. > +
It requires a working directory. Running this in a bare repository with --dirty won't work. You might want to discuss that in the documentation.
Show 15 quoted lines
> diff --git a/builtin-describe.c b/builtin-describe.c
> @@ -229,12 +231,23 @@ static void describe(const char *arg, int last_one)
> sha1_to_hex(gave_up_on->object.sha1));
> }
> }
> + if (check_dirty) {
> + const char **args = xmalloc(5 * sizeof(char*));
> + args[0] = "diff-index";
> + args[1] = "--quiet";
> + args[2] = "--name-only";
> + args[3] = "HEAD";
> + args[4] = NULL;
> + if (cmd_diff_index(4, args, prefix))
> + dirty_string = "-dirty";
> + }So if I describe two different commits at once in the same working tree you are going to run diff-index twice? That's not a great idea. The outcome of diff-index won't change between those two commits. Better to compute this up front before calling the describe() function, and instead of passing in prefix pass in dirty_string. Or just make it a global, like you did to the option flag.
Show 13 quoted lines
> if (abbrev == 0)
> - printf("%s\n", all_matches[0].name->path );
> + printf("%s%s\n", all_matches[0].name->path, dirty_string);
> else
> - printf("%s-%d-g%s\n", all_matches[0].name->path,
> + printf("%s-%d-g%s%s\n", all_matches[0].name->path,
> all_matches[0].depth,
> - find_unique_abbrev(cmit->object.sha1, abbrev));
> + find_unique_abbrev(cmit->object.sha1, abbrev),
> + dirty_string);
>
> if (!last_one)
> clear_commit_marks(cmit, -1);So if HEAD is exactly matching a tag you don't output the -dirty suffix, even if the working tree is dirty? That's counter to the documentation above. See l.150-154, we break out of the describe function very quickly if there is a tag on the input commit.
-- Shawn.