* [PATCH v4 1/3] t1503: use test_must_be_empty @ 2014-09-16 3:24 David Aguilar 2014-09-16 3:24 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet David Aguilar 0 siblings, 1 reply; 4+ messages in thread From: David Aguilar @ 2014-09-16 3:24 UTC (permalink / raw) To: Junio C Hamano Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason, Jon Seymour, Ronnie Sahlberg, Michael Haggerty, Fabian Ruch, Johannes Sixt Use `test_must_be_be_empty <file>` instead of `test -z "$(cat <file>)"`. Suggested-by: Fabian Ruch <bafain@gmail.com> Signed-off-by: David Aguilar <davvid@gmail.com> --- This patch should probably be applied on top of the da/rev-parse-verify-quiet which is currently in pu and contains the rev-parse documentation patch. This patch has been rebased to be 1/3. t/t1503-rev-parse-verify.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh index 813cc1b..d1f93b3 100755 --- a/t/t1503-rev-parse-verify.sh +++ b/t/t1503-rev-parse-verify.sh @@ -72,15 +72,15 @@ test_expect_success 'fails with any bad rev or many good revs' ' test_expect_success 'fails silently when using -q' ' test_must_fail git rev-parse --verify --quiet 2>error && - test -z "$(cat error)" && + test_must_be_empty error && test_must_fail git rev-parse -q --verify foo 2>error && - test -z "$(cat error)" && + test_must_be_empty error && test_must_fail git rev-parse --verify -q HEAD bar 2>error && - test -z "$(cat error)" && + test_must_be_empty error && test_must_fail git rev-parse --quiet --verify baz HEAD 2>error && - test -z "$(cat error)" && + test_must_be_empty error && test_must_fail git rev-parse -q --verify $HASH2 HEAD 2>error && - test -z "$(cat error)" + test_must_be_empty error ' test_expect_success 'no stdout output on error' ' -- 2.1.0.30.g05c535b.dirty ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet 2014-09-16 3:24 [PATCH v4 1/3] t1503: use test_must_be_empty David Aguilar @ 2014-09-16 3:24 ` David Aguilar 2014-09-16 3:24 ` [PATCH v4 3/3] stash: prefer --quiet over shell redirection of the standard error stream David Aguilar 2014-09-16 18:26 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet Junio C Hamano 0 siblings, 2 replies; 4+ messages in thread From: David Aguilar @ 2014-09-16 3:24 UTC (permalink / raw) To: Junio C Hamano Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason, Jon Seymour, Ronnie Sahlberg, Michael Haggerty, Fabian Ruch, Johannes Sixt When a reflog is deleted, e.g. when "git stash" clears its stashes, "git rev-parse --verify --quiet" dies: fatal: Log for refs/stash is empty. The reason is that the get_sha1() code path does not allow us to suppress this message. Pass the flags bitfield through get_sha1_with_context() so that read_ref_at() can suppress the message. Use get_sha1_with_context1() instead of get_sha1() in rev-parse so that the --quiet flag is honored. Signed-off-by: David Aguilar <davvid@gmail.com> --- This patch now has the t1503 test case squashed into it. It was previously a separate patch, but it makes more sense for them to go together. builtin/rev-parse.c | 5 ++++- builtin/show-branch.c | 5 +++-- refs.c | 10 +++++++--- refs.h | 3 ++- sha1_name.c | 7 ++++--- t/t1503-rev-parse-verify.sh | 9 +++++++++ 6 files changed, 29 insertions(+), 10 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index c911b45..35d3c43 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -508,7 +508,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) int has_dashdash = 0; int output_prefix = 0; unsigned char sha1[20]; + unsigned int flags = 0; const char *name = NULL; + struct object_context unused; if (argc > 1 && !strcmp("--parseopt", argv[1])) return cmd_parseopt(argc - 1, argv + 1, prefix); @@ -596,6 +598,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) } if (!strcmp(arg, "--quiet") || !strcmp(arg, "-q")) { quiet = 1; + flags |= GET_SHA1_QUIETLY; continue; } if (!strcmp(arg, "--short") || @@ -818,7 +821,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) name++; type = REVERSED; } - if (!get_sha1(name, sha1)) { + if (!get_sha1_with_context(name, flags, sha1, &unused)) { if (verify) revs_count++; else diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 298c95e..46498e1 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -723,6 +723,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) char nth_desc[256]; char *ref; int base = 0; + unsigned int flags = 0; if (ac == 0) { static const char *fake_av[2]; @@ -749,7 +750,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) /* Ah, that is a date spec... */ unsigned long at; at = approxidate(reflog_base); - read_ref_at(ref, at, -1, sha1, NULL, + read_ref_at(ref, flags, at, -1, sha1, NULL, NULL, NULL, &base); } } @@ -760,7 +761,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) unsigned long timestamp; int tz; - if (read_ref_at(ref, 0, base+i, sha1, &logmsg, + if (read_ref_at(ref, flags, 0, base+i, sha1, &logmsg, ×tamp, &tz, NULL)) { reflog = i; break; diff --git a/refs.c b/refs.c index 2ce5d69..447e339 100644 --- a/refs.c +++ b/refs.c @@ -3108,7 +3108,7 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1, return 1; } -int read_ref_at(const char *refname, unsigned long at_time, int cnt, +int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt) { @@ -3126,8 +3126,12 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt, for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb); - if (!cb.reccnt) - die("Log for %s is empty.", refname); + if (!cb.reccnt) { + if (flags & GET_SHA1_QUIETLY) + exit(1); + else + die("Log for %s is empty.", refname); + } if (cb.found_it) return 0; diff --git a/refs.h b/refs.h index 68c5770..0ca6059 100644 --- a/refs.h +++ b/refs.h @@ -206,7 +206,8 @@ extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, cons int log_ref_setup(const char *refname, char *logfile, int bufsize); /** Reads log for the value of ref during at_time. **/ -extern int read_ref_at(const char *refname, unsigned long at_time, int cnt, +extern int read_ref_at(const char *refname, unsigned int flags, + unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt); diff --git a/sha1_name.c b/sha1_name.c index 7098b10..d292e31 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -432,7 +432,8 @@ static inline int upstream_mark(const char *string, int len) static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags); static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf); -static int get_sha1_basic(const char *str, int len, unsigned char *sha1) +static int get_sha1_basic(const char *str, int len, unsigned char *sha1, + unsigned int flags) { static const char *warn_msg = "refname '%.*s' is ambiguous."; static const char *object_name_msg = N_( @@ -545,7 +546,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) return -1; } } - if (read_ref_at(real_ref, at_time, nth, sha1, NULL, + if (read_ref_at(real_ref, flags, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { if (!len) { if (starts_with(real_ref, "refs/heads/")) { @@ -801,7 +802,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l if (!ret) return 0; - ret = get_sha1_basic(name, len, sha1); + ret = get_sha1_basic(name, len, sha1, lookup_flags); if (!ret) return 0; diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh index d1f93b3..4fe9f0e 100755 --- a/t/t1503-rev-parse-verify.sh +++ b/t/t1503-rev-parse-verify.sh @@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' ' test_must_be_empty error ' +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 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 'no stdout output on error' ' test -z "$(git rev-parse --verify)" && test -z "$(git rev-parse --verify foo)" && -- 2.1.0.30.g05c535b.dirty ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v4 3/3] stash: prefer --quiet over shell redirection of the standard error stream 2014-09-16 3:24 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet David Aguilar @ 2014-09-16 3:24 ` David Aguilar 2014-09-16 18:26 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet Junio C Hamano 1 sibling, 0 replies; 4+ messages in thread From: David Aguilar @ 2014-09-16 3:24 UTC (permalink / raw) To: Junio C Hamano Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason, Jon Seymour, Ronnie Sahlberg, Michael Haggerty, Fabian Ruch, Johannes Sixt Use `git rev-parse --verify --quiet` instead of redirecting stderr to /dev/null. Signed-off-by: David Aguilar <davvid@gmail.com> --- This patch is unchanged from when it was first written, but is now correct thanks to the preceding patch. git-stash.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 9c1ba8e..7ece0f1 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -50,7 +50,7 @@ clear_stash () { then die "$(gettext "git stash clear with parameters is unimplemented")" fi - if current=$(git rev-parse --verify $ref_stash 2>/dev/null) + if current=$(git rev-parse --verify --quiet $ref_stash) then git update-ref -d $ref_stash $current fi @@ -292,7 +292,7 @@ save_stash () { } have_stash () { - git rev-parse --verify $ref_stash >/dev/null 2>&1 + git rev-parse --verify --quiet $ref_stash >/dev/null } list_stash () { @@ -392,12 +392,12 @@ parse_flags_and_rev() ;; esac - REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || { + REV=$(git rev-parse --symbolic --verify --quiet "$1") || { reference="$1" die "$(eval_gettext "\$reference is not valid reference")" } - i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) && + i_commit=$(git rev-parse --verify --quiet "$REV^2") && set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) && s=$1 && w_commit=$1 && @@ -409,7 +409,7 @@ parse_flags_and_rev() test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" && IS_STASH_REF=t - u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) && + u_commit=$(git rev-parse --verify --quiet "$REV^3") && u_tree=$(git rev-parse "$REV^3:" 2>/dev/null) } @@ -531,7 +531,8 @@ drop_stash () { die "$(eval_gettext "\${REV}: Could not drop stash entry")" # clear_stash if we just dropped the last stash entry - git rev-parse --verify "$ref_stash@{0}" >/dev/null 2>&1 || clear_stash + git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null || + clear_stash } apply_to_branch () { -- 2.1.0.30.g05c535b.dirty ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet 2014-09-16 3:24 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet David Aguilar 2014-09-16 3:24 ` [PATCH v4 3/3] stash: prefer --quiet over shell redirection of the standard error stream David Aguilar @ 2014-09-16 18:26 ` Junio C Hamano 1 sibling, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2014-09-16 18:26 UTC (permalink / raw) To: David Aguilar Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason, Jon Seymour, Ronnie Sahlberg, Michael Haggerty, Fabian Ruch, Johannes Sixt David Aguilar <davvid@gmail.com> writes: > This patch now has the t1503 test case squashed into it. > It was previously a separate patch, but it makes more sense > for them to go together. Yes. > diff --git a/refs.c b/refs.c > index 2ce5d69..447e339 100644 > --- a/refs.c > +++ b/refs.c > @@ -3108,7 +3108,7 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1, > return 1; > } > > -int read_ref_at(const char *refname, unsigned long at_time, int cnt, > +int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time, int cnt, > unsigned char *sha1, char **msg, > unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt) > { > @@ -3126,8 +3126,12 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt, > > for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb); > > - if (!cb.reccnt) > - die("Log for %s is empty.", refname); > + if (!cb.reccnt) { > + if (flags & GET_SHA1_QUIETLY) > + exit(1); Do we want 1 or 128 just like die()? > + else > + die("Log for %s is empty.", refname); > + } Given that I see this behaviour: $ git rev-parse da/rev-parse-verify-quiet@{1} 2892dfeec3f98f7e65a2746d271471d2c3c4af57 $ git rev-parse da/rev-parse-verify-quiet@{10} fatal: Log for 'da/rev-parse-verify-quiet' only has 4 entries and I do not see a change in this patch to touch "only has %d entries" (found in sha1_name.c), I have to suspect that this change is not sufficient to say "rev-parse --quiet will now be quiet". $ git rev-parse --quiet --verify da/rev-parse-verify-quiet@{10.years.ago} may want to return the oldest-known value as it has always done without giving a warning. There probably are other cases that relate to reflogs, e.g. $ git rev-parse --quiet --verify @{-99999} to ask the tip of the branch you were on 99999 "git checkout"s ago. I think the last one does not have to be in the same commit as this fix for "empty" and "you do not have that many" cases, though. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-16 18:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-16 3:24 [PATCH v4 1/3] t1503: use test_must_be_empty David Aguilar 2014-09-16 3:24 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet David Aguilar 2014-09-16 3:24 ` [PATCH v4 3/3] stash: prefer --quiet over shell redirection of the standard error stream David Aguilar 2014-09-16 18:26 ` [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).