Skip to contentSkip to conversation

describe: add option --dirty

RFC In review Yasushi SHOJI 3 participants 7 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

RFC 1/1 describe: add option --dirty

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.

one thing I found out is that we s/-/./g the out put of describe before we append "-dirty". this patch doesn't take care that. so with this patch what we get is either

	v1.5.3-rc2-840-g1c0e2-dirty, or
	v1.5.3.rc2.840.g1c0e2.dirty

if we don't put more complecated sed command in GIT-VERSION-GEN.

anyway, comments welcome.

ps. another thing I don't like about this patch is that it changed to pass prefix down to describe() as the last argument because I was lazy to call cmd_diff_index() in describe(). if there is better way to do it, please let me know.

 Documentation/git-describe.txt |    5 ++++-
 builtin-describe.c             |   29 ++++++++++++++++++++++-------
 2 files changed, 26 insertions(+), 8 deletions(-)
Documentation/git-describe.txt +4 -1
index f0bcb61..627c4d5 100644
@@ -9,7 +9,7 @@ git-describe - Show the most recent tag that is reachable from a commit
 SYNOPSIS
 --------
 'git-describe' [--all] [--tags] [--contains] [--abbrev=<n>]
-               [--candidates=<n>] [--debug]
+               [--candidates=<n>] [--debug] [--dirty]
                <committish>...
 
 DESCRIPTION
@@ -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.
+
 EXAMPLES
 --------
 
builtin-describe.c +22 -7
index e94f867..bebc16b 100644
@@ -9,11 +9,12 @@
 #define MAX_TAGS	(FLAG_BITS - 1)
 
 static const char describe_usage[] =
-"git-describe [--all] [--tags] [--contains] [--abbrev=<n>] [--candidates] [--debug] <committish>*";
+"git-describe [--all] [--tags] [--contains] [--abbrev=<n>] [--candidates] [--debug] [--dirty] <committish>*";
 
 static int debug;	/* Display lots of verbose info */
 static int all;	/* Default to annotated tags only */
 static int tags;	/* But allow any tags if --tags is specified */
+static int check_dirty;	/* Append "-dirty" to describe string if working tree is dirty */
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
 
@@ -125,7 +126,7 @@ static unsigned long finish_depth_computation(
 	return seen_commits;
 }
 
-static void describe(const char *arg, int last_one)
+static void describe(const char *arg, int last_one, const char *prefix)
 {
 	unsigned char sha1[20];
 	struct commit *cmit, *gave_up_on = NULL;
@@ -135,6 +136,7 @@ static void describe(const char *arg, int last_one)
 	struct possible_tag all_matches[MAX_TAGS];
 	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
 	unsigned long seen_commits = 0;
+	char *dirty_string = "";
 
 	if (get_sha1(arg, sha1))
 		die("Not a valid object name %s", arg);
@@ -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";
+	}
 	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);
@@ -250,6 +263,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 
 		if (*arg != '-')
 			break;
+		else if (!strcmp(arg, "--dirty"))
+			check_dirty = 1;
 		else if (!strcmp(arg, "--contains"))
 			contains = 1;
 		else if (!strcmp(arg, "--debug"))
@@ -290,10 +305,10 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	}
 
 	if (argc <= i)
-		describe("HEAD", 1);
+		describe("HEAD", 1, prefix);
 	else
 		while (i < argc) {
-			describe(argv[i], (i == argc - 1));
+			describe(argv[i], (i == argc - 1), prefix);
 			i++;
 		}
 
Signature · 1 line
1.5.3.rc2.4.g726f9
#1 lore raw
Quoted 5 lines · Yasushi SHOJI

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.

Documentation/git-describe.txt OPTIONS · line 58

Quoted 8 lines
Documentation/git-describe.txt +3 -0
@@ -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.

Quoted 5 lines
builtin-describe.c +0 -0
@@ -229,12 +231,23 @@ static void describe(const char *arg, int last_one)
 				sha1_to_hex(gave_up_on->object.sha1));
 		}
 	}

builtin-describe.c static void describe(const char *arg, int last_one) · line 243

Quoted 10 lines
builtin-describe.c +10 -0
+	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.

builtin-describe.c static void describe(const char *arg, int last_one) · line 253

Quoted 13 lines
builtin-describe.c +4 -3
 	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.

Signature · 1 line
Shawn.
#2 lore raw
In reply to Yasushi SHOJI
Quoted 5 lines · Yasushi SHOJI

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.

Hmph, this makes sense _ONLY_ for HEAD, doesn't it?

IOW, what should this output?

	$ git checkout v1.5.0 ;# detached HEAD
        $ git reset --hard  ;# clean slate
        $ echo >>Makefile ;# not anymore
        $ git describe --dirty v1.4.0^1

Should it say "v1.4.0-rc2-156-g0a8f4f0-dirty"? The dirtiness does not have anything to do with commit v1.4.0^1, so...

#3 lore raw
Quoted 12 lines · Junio C Hamano
Quoted 5 lines · Yasushi SHOJI

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.

Hmph, this makes sense _ONLY_ for HEAD, doesn't it?

IOW, what should this output?

Quoted 7 lines
	$ git checkout v1.5.0 ;# detached HEAD
        $ git reset --hard  ;# clean slate
        $ echo >>Makefile ;# not anymore
        $ git describe --dirty v1.4.0^1

Should it say "v1.4.0-rc2-156-g0a8f4f0-dirty"? The dirtiness does not have anything to do with commit v1.4.0^1, so...

Good catch. I had that in my mind when I was reading the patch, but failed to mention it. I blame metze on #git, he interrupted my train of thought. ;-)

I think the answer is the user passes either --dirty OR one or more commit-ish. But not --dirty and a commit-ish. In other words you can either describe the working directory state, or a commit, but not both at once. Which also neatly solves my issue with diff-index running more than once.

Signature · 1 line
Shawn.
#4 lore raw

At Mon, 23 Jul 2007 03:08:18 -0400,

Quoted 19 lines · Shawn O. Pearce
Quoted 17 lines · Junio C Hamano
Quoted 5 lines · Yasushi SHOJI

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.

Hmph, this makes sense _ONLY_ for HEAD, doesn't it?

IOW, what should this output?

	$ git checkout v1.5.0 ;# detached HEAD
        $ git reset --hard  ;# clean slate
        $ echo >>Makefile ;# not anymore
        $ git describe --dirty v1.4.0^1
Quoted 6 lines
Quoted 2 lines

Should it say "v1.4.0-rc2-156-g0a8f4f0-dirty"? The dirtiness does not have anything to do with commit v1.4.0^1, so...

Good catch. I had that in my mind when I was reading the patch, but failed to mention it. I blame metze on #git, he interrupted my train of thought. ;-)

I knew the issue be failed to note about it. Thanks.

Quoted 5 lines

I think the answer is the user passes either --dirty OR one or more commit-ish. But not --dirty and a commit-ish. In other words you can either describe the working directory state, or a commit, but not both at once. Which also neatly solves my issue with diff-index running more than once.

So the point is would it be worth implementing in usable form?

Quoted 1 line

From the comments I'd add an option "--workinig-tree" instead of

--dirty to describe the working tree. because that, the special case, is what we want after all,

synopsis would be:

	SYNOPSIS
	--------
	'git-describe' [--all] [--tags] [--contains] [--abbrev=<n>]
	               [--candidates=<n>] [--debug]
	               --working-tree | <committish>...
	    :
	    :
	--working-tree::
		Describe the working tree instead of committishes.  if the
		working tree is dirty, the describe string will have "-dirty"
		appended.
	
		As you can assume from the name, this option requires working
		tree; running it on a bare repository will fail.

what do you think?

Signature · 1 line
         yashi
#5 lore raw
Quoted 21 lines · Yasushi SHOJI

From the comments I'd add an option "--workinig-tree" instead of --dirty to describe the working tree. because that, the special case, is what we want after all,

synopsis would be:

	SYNOPSIS
	--------
	'git-describe' [--all] [--tags] [--contains] [--abbrev=<n>]
	               [--candidates=<n>] [--debug]
	               --working-tree | <committish>...
	    :
	    :
	--working-tree::
		Describe the working tree instead of committishes.  if the
		working tree is dirty, the describe string will have "-dirty"
		appended.
	
		As you can assume from the name, this option requires working
		tree; running it on a bare repository will fail.
Quoted 1 line

what do you think?

That's reasonable. It seems like a lot of work in core Git just to avoid a small chunk of shell, but I think almost everyone has that same small chunk of shell in their build scripts...

Signature · 1 line
Shawn.
#6 lore raw

At Mon, 23 Jul 2007 03:58:34 -0400,

Quoted 20 lines · Shawn O. Pearce
Quoted 18 lines · Yasushi SHOJI

From the comments I'd add an option "--workinig-tree" instead of --dirty to describe the working tree. because that, the special case, is what we want after all,

synopsis would be:

	SYNOPSIS
	--------
	'git-describe' [--all] [--tags] [--contains] [--abbrev=<n>]
	               [--candidates=<n>] [--debug]
	               --working-tree | <committish>...
	    :
	    :
	--working-tree::
		Describe the working tree instead of committishes.  if the
		working tree is dirty, the describe string will have "-dirty"
		appended.
Quoted 8 lines
Quoted 4 lines

As you can assume from the name, this option requires working tree; running it on a bare repository will fail.

what do you think?

That's reasonable. It seems like a lot of work in core Git just to avoid a small chunk of shell, but I think almost everyone has that same small chunk of shell in their build scripts...

it's not that much. here it is. hope you like it.

Quoted 1 line

From 25acf0fad4866b87998b51f1e66f540f2bcc5f0d Mon Sep 17 00:00:00 2001

From: Yasushi SHOJI <[email protected]>
Date: Mon, 23 Jul 2007 17:49:11 +0900
Subject: [PATCH] describe: add a new option --working-tree

Many people like to use git-describe for version number, and yet people always tack in the -dirty if the directory is dirty according to diff-index. So this patch tries to scratch the itchy.

With --working-tree, the command will describe the working tree instead of committishes. If the working tree is dirty, the describe string will have "-dirty" appended.

As you can assume from the name, this option requires working tree. running it on a bare repository will fail.

---
 Documentation/git-describe.txt |   10 +++++++++-
 builtin-describe.c             |   38 +++++++++++++++++++++++++++++++-------
 2 files changed, 40 insertions(+), 8 deletions(-)
Documentation/git-describe.txt +9 -1
index f0bcb61..d9b1550 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 'git-describe' [--all] [--tags] [--contains] [--abbrev=<n>]
                [--candidates=<n>] [--debug]
-               <committish>...
+               --working-tree | <committish>...
 
 DESCRIPTION
 -----------
@@ -53,6 +53,14 @@ OPTIONS
 	being employed to standard error.  The tag name will still
 	be printed to standard out.
 
+--working-tree::
+	Describe the working tree instead of committishes.  if the
+	working tree is dirty, the describe string will have "-dirty"
+	appended.
+
+	As you can assume from the name, this option requires a
+	working tree. Running it on a bare repository will fail.
+
 EXAMPLES
 --------
 
builtin-describe.c +31 -7
index e94f867..1c58480 100644
@@ -9,11 +9,12 @@
 #define MAX_TAGS	(FLAG_BITS - 1)
 
 static const char describe_usage[] =
-"git-describe [--all] [--tags] [--contains] [--abbrev=<n>] [--candidates] [--debug] <committish>*";
+"git-describe [--all] [--tags] [--contains] [--abbrev=<n>] [--candidates] [--debug] --working-tree | <committish>*";
 
 static int debug;	/* Display lots of verbose info */
 static int all;	/* Default to annotated tags only */
 static int tags;	/* But allow any tags if --tags is specified */
+static int working_tree;/* describe the working tree instead of committish */
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
 
@@ -125,7 +126,7 @@ static unsigned long finish_depth_computation(
 	return seen_commits;
 }
 
-static void describe(const char *arg, int last_one)
+static void describe(const char *arg, int last_one, int dirty)
 {
 	unsigned char sha1[20];
 	struct commit *cmit, *gave_up_on = NULL;
@@ -135,6 +136,7 @@ static void describe(const char *arg, int last_one)
 	struct possible_tag all_matches[MAX_TAGS];
 	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
 	unsigned long seen_commits = 0;
+	char *dirty_string = dirty ? "-dirty" : "";
 
 	if (get_sha1(arg, sha1))
 		die("Not a valid object name %s", arg);
@@ -230,11 +232,12 @@ static void describe(const char *arg, int last_one)
 		}
 	}
 	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);
@@ -244,12 +247,15 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	int contains = 0;
+	int dirty = 0;
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
 		if (*arg != '-')
 			break;
+		else if (!strcmp(arg, "--working-tree"))
+			working_tree = 1;
 		else if (!strcmp(arg, "--contains"))
 			contains = 1;
 		else if (!strcmp(arg, "--debug"))
@@ -276,6 +282,24 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 
 	save_commit_buffer = 0;
 
+	if (working_tree) {
+		const char **args;
+
+		if (!is_inside_work_tree() || is_inside_git_dir())
+			die("%s with --working-tree must be run in a working tree", argv[0]);
+		if (argc > i)
+			die("--working-tree doesn't take any committish\n");
+
+		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 = 1;
+	}
+
 	if (contains) {
 		const char **args = xmalloc((4 + argc - i) * sizeof(char*));
 		args[0] = "name-rev";
@@ -290,10 +314,10 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	}
 
 	if (argc <= i)
-		describe("HEAD", 1);
+		describe("HEAD", 1, dirty);
 	else
 		while (i < argc) {
-			describe(argv[i], (i == argc - 1));
+			describe(argv[i], (i == argc - 1), dirty);
 			i++;
 		}
 
Signature · 1 line
1.5.3.rc2.4.g726f9