threads / patch / 37581

patch, 2 partsfixup! refs: make rev-parse --quiet actually quiet

Subject: [PATCH 1/2] fixup! refs: make rev-parse --quiet actually quiet

## tl;dr

5 messages between Sep 17, 2014 and Sep 19, 2014. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

David Aguilar· Sep 17, 2014, 04:35 UTC · lore

--- This is a fixup for da/rev-parse-verify-quiet in pu We now exit(128) and handle the "Log for XXX only has DDD entries" case.

 refs.c                      |  2 +-
 sha1_name.c                 |  3 +++
 t/t1503-rev-parse-verify.sh | 10 +++++++++-
 3 files changed, 13 insertions(+), 2 deletions(-)
Show changes to 3 files +13 −2

refs.c, sha1_name.c, t/t1503-rev-parse-verify.sh

diff --git a/refs.c b/refs.c
index 447e339..9e405f9 100644
--- a/refs.c
+++ b/refs.c
@@ -3128,7 +3128,7 @@ int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time,
 
 	if (!cb.reccnt) {
 		if (flags & GET_SHA1_QUIETLY)
-			exit(1);
+			exit(128);
 		else
 			die("Log for %s is empty.", refname);
 	}
diff --git a/sha1_name.c b/sha1_name.c
index d292e31..be9a9de 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -563,6 +563,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
 					"back to %s.", len, str,
 					show_date(co_time, co_tz, DATE_RFC2822));
 			else {
+				if (flags & GET_SHA1_QUIETLY) {
+					exit(128);
+				}
 				die("Log for '%.*s' only has %d entries.",
 				    len, str, co_cnt);
 			}
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index 4fe9f0e..7c3d830 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -86,12 +86,20 @@ test_expect_success 'fails silently when using -q' '
 test_expect_success 'fails silently when using -q with deleted reflogs' '
 	ref=$(git rev-parse HEAD) &&
 	: >.git/logs/refs/test &&
-	git update-ref -m "reflog message for refs/test" refs/test "$ref" &&
+	git update-ref -m "message for refs/test" refs/test "$ref" &&
 	git reflog delete --updateref --rewrite refs/test@{0} &&
 	test_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&
 	test_must_be_empty error
 '
 
+test_expect_success 'fails silently when using -q with not enough reflogs' '
+	ref=$(git rev-parse HEAD) &&
+	: >.git/logs/refs/test2 &&
+	git update-ref -m "message for refs/test2" refs/test2 "$ref" &&
+	test_must_fail git rev-parse -q --verify refs/test2@{999} >error 2>&1 &&
+	test_must_be_empty error
+'
+
 test_expect_success 'no stdout output on error' '
 	test -z "$(git rev-parse --verify)" &&
 	test -z "$(git rev-parse --verify foo)" &&

-- 
2.1.0.248.g6401287.dirty
David Aguilar· Sep 17, 2014, 04:35 UTC · re: David Aguilar · lore

[PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist

Make rev-parse --verify --quiet ref@{1.year.ago} when the reflog does not go back that far succeed silently with --quiet.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 sha1_name.c                 | 19 ++++++++++++-------
 t/t1503-rev-parse-verify.sh | 10 ++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)
Show changes to 2 files +22 −7

sha1_name.c, t/t1503-rev-parse-verify.sh

diff --git a/sha1_name.c b/sha1_name.c
index be9a9de..5836f94 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
 
 	if (warn_ambiguous_refs &&
 	    (refs_found > 1 ||
-	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
-		warning(warn_msg, len, str);
+	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {
+		if (!(flags & GET_SHA1_QUIETLY)) {
+			warning(warn_msg, len, str);
+		}
+	}
 
 	if (reflog_len) {
 		int nth, i;
@@ -558,11 +561,13 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
 					len = 4;
 				}
 			}
-			if (at_time)
-				warning("Log for '%.*s' only goes "
-					"back to %s.", len, str,
-					show_date(co_time, co_tz, DATE_RFC2822));
-			else {
+			if (at_time) {
+				if (!(flags & GET_SHA1_QUIETLY)) {
+					warning("Log for '%.*s' only goes "
+						"back to %s.", len, str,
+						show_date(co_time, co_tz, DATE_RFC2822));
+				}
+			} else {
 				if (flags & GET_SHA1_QUIETLY) {
 					exit(128);
 				}
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index 7c3d830..e7dece0 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -100,6 +100,16 @@ test_expect_success 'fails silently when using -q with not enough reflogs' '
 	test_must_be_empty error
 '
 
+test_expect_success 'succeeds silently when using -q with invalid dates' '
+	ref=$(git rev-parse HEAD) &&
+	: >.git/logs/refs/test3 &&
+	git update-ref -m "message for refs/test3" refs/test3 "$ref" &&
+	git rev-parse -q --verify refs/test3@{1.year.ago} >actual 2>error &&
+	test_must_be_empty error &&
+	echo "$ref" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'no stdout output on error' '
 	test -z "$(git rev-parse --verify)" &&
 	test -z "$(git rev-parse --verify foo)" &&

-- 
2.1.0.248.g6401287.dirty
Junio C Hamano· Sep 18, 2014, 16:07 UTC · re: David Aguilar · lore

Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist

David Aguilar <davvid@gmail.com> writes:
Show 6 quoted lines
> Subject: Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist
> ...
> Make rev-parse --verify --quiet ref@{1.year.ago} when the reflog
> does not go back that far succeed silently with --quiet.
> ...
> +test_expect_success 'succeeds silently when using -q with invalid dates' '

These phrases may invite confusion and misunderstanding. These dates on which you do not know where the tip of the ref was are not necessarily "do not exist", "does not go back that far", or "invalid".

You may have pruned old entries for a long running branch. It is just you no longer have records that go that far enough.

We would be able to tell two cases apart if we really wanted to by looking at the oldest reflog entry. If it records a creation of the ref, then the ref did not exist back then and the reflog does not go back that far. It may not be a bad idea to turn the current warning to an error in that case. And if the oldest entry knows where the ref used to point at, then we have expired older entries. We can continue giving the "oldest known" as before with a warning.

But that would be a separate issue.
Show 12 quoted lines
> +	ref=$(git rev-parse HEAD) &&
> +	: >.git/logs/refs/test3 &&
> +	git update-ref -m "message for refs/test3" refs/test3 "$ref" &&
> +	git rev-parse -q --verify refs/test3@{1.year.ago} >actual 2>error &&
> +	test_must_be_empty error &&
> +	echo "$ref" >expect &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'no stdout output on error' '
>  	test -z "$(git rev-parse --verify)" &&
>  	test -z "$(git rev-parse --verify foo)" &&
Junio C Hamano· Sep 18, 2014, 16:12 UTC · re: David Aguilar · lore

Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist

David Aguilar <davvid@gmail.com> writes:
Show 11 quoted lines
> @@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
>  
>  	if (warn_ambiguous_refs &&
>  	    (refs_found > 1 ||
> -	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
> -		warning(warn_msg, len, str);
> +	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {
> +		if (!(flags & GET_SHA1_QUIETLY)) {
> +			warning(warn_msg, len, str);
> +		}
> +	}

Hmph, wouldn't it be simpler to read and understand if it were done this way instead?

-	if (warn_ambiguous_refs &&
+	if (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&
 	    (refs_found > 1 || !get_short_sha1(...)))
		waqrning(...);
David Aguilar· Sep 19, 2014, 03:47 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist

On Thu, Sep 18, 2014 at 09:12:44AM -0700, Junio C Hamano wrote:
Show 22 quoted lines
> David Aguilar <davvid@gmail.com> writes:
> 
> > @@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
> >  
> >  	if (warn_ambiguous_refs &&
> >  	    (refs_found > 1 ||
> > -	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
> > -		warning(warn_msg, len, str);
> > +	     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {
> > +		if (!(flags & GET_SHA1_QUIETLY)) {
> > +			warning(warn_msg, len, str);
> > +		}
> > +	}
> 
> Hmph, wouldn't it be simpler to read and understand if it were done
> this way instead?
> 
> -	if (warn_ambiguous_refs &&
> +	if (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&
>  	    (refs_found > 1 || !get_short_sha1(...)))
> 		waqrning(...);
> 
Yes, it would.

I squashed this patch into the original "make rev-parse --quiet actually quiet" patch, along with your suggestions, and sent it as a replacement patch for the commit in pu.

-- 
David

← back to recent threads