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

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

[David Aguilar]

---
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(-)

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

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

[David Aguilar]

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(-)

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

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

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> 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.

> +	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)" &&

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

[Junio C Hamano]

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(...);

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

[David Aguilar]

On Thu, Sep 18, 2014 at 09:12:44AM -0700, Junio C Hamano wrote:
> 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