* RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? @ 2010-08-18 11:36 Jon Seymour 2010-08-18 11:41 ` Jon Seymour 2010-08-18 20:50 ` Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-18 11:36 UTC (permalink / raw) To: Git Mailing List While reworking my detached-stash topic, I noticed that git rev-parse ref@{n} will exit with a warning message on stderr, the SHA1 of ref and a non-zero exit code if ref exists, but ref@{n} does not. I understand why ref@{time-spec} might behave this way, but I reckon that if you ask for ref@{n} for n > N-1, where N is the length of the reflog, then you should either get empty output and a non-zero status (preferred) or a ref@{N-1} on the output with a status code of zero and a warning message (less optimal, IMHO). What say the list? jon. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? 2010-08-18 11:36 RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? Jon Seymour @ 2010-08-18 11:41 ` Jon Seymour 2010-08-18 20:50 ` Junio C Hamano 1 sibling, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-18 11:41 UTC (permalink / raw) To: Git Mailing List | Argh! Sorry, that last message made no sense. Corrections marked with _. On Wed, Aug 18, 2010 at 9:36 PM, Jon Seymour <jon.seymour@gmail.com> wrote: > While reworking my detached-stash topic, I noticed that git rev-parse > ref@{n} will exit with a warning message on stderr, the SHA1 of ref > and a _zero_ exit code if ref exists, but ref@{n} does not. > > I understand why ref@{time-spec} might behave this way, but I reckon > that if you ask for ref@{n} for n > N-1, where N is the length of the > reflog, then you should either get empty output and a non-zero status > (preferred) or a ref@{N-1} on the output with a status code of zero > and a warning message (less optimal, IMHO). > > What say the list? > > jon. > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? 2010-08-18 11:36 RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? Jon Seymour 2010-08-18 11:41 ` Jon Seymour @ 2010-08-18 20:50 ` Junio C Hamano 2010-08-18 23:02 ` Jon Seymour 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2010-08-18 20:50 UTC (permalink / raw) To: Jon Seymour; +Cc: Git Mailing List Jon Seymour <jon.seymour@gmail.com> writes: > I understand why ref@{time-spec} might behave this way, but I reckon > that if you ask for ref@{n} for n > N-1, where N is the length of the > reflog, then you should either get empty output and a non-zero status > (preferred) or a ref@{N-1} on the output with a status code of zero > and a warning message (less optimal, IMHO). Yeah, the behaviour of ref@{...} syntax parser is way suboptimal: $ git rev-parse --verify jch@{99999}; echo $? warning: Log for 'jch' only has 1368 entries. cfb88e9a8d4926b0011ae2dd67e1f57a98f4b768 0 It even knows that it is running off the cut-off point; it should just cause the caller to notice that fact. I don't think changing it to error out should cause any harm to existing callers. It may just be the matter of doing something like this (totally untested)... sha1_name.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4af94fa..c1e51c9 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); static int get_sha1_basic(const char *str, int len, unsigned char *sha1) { - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; + static const char *warn_msg = "warning: refname '%.*s' is ambiguous.\n"; char *real_ref = NULL; int refs_found = 0; int at, reflog_len; @@ -390,7 +390,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) return -1; if (warn_ambiguous_refs && refs_found > 1) - fprintf(stderr, warning, len, str); + fprintf(stderr, warn_msg, len, str); if (reflog_len) { int nth, i; @@ -426,14 +426,12 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (read_ref_at(real_ref, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { if (at_time) - fprintf(stderr, - "warning: Log for '%.*s' only goes " + warning("Log for '%.*s' only goes " "back to %s.\n", len, str, show_date(co_time, co_tz, DATE_RFC2822)); else - fprintf(stderr, - "warning: Log for '%.*s' only has " - "%d entries.\n", len, str, co_cnt); + return error("Log for '%.*s' only has " + "%d entries.\n", len, str, co_cnt); } } ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? 2010-08-18 20:50 ` Junio C Hamano @ 2010-08-18 23:02 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 0/4] rev-parse: improve reporting of invalid log references Jon Seymour ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-18 23:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List Thanks for that. Not quite sure why yet, but the error message is duplicated. I wonder also if the free() call at the end of the function needs to be duplicated before the early return? Anyway, I am happy to look at this. Would you prefer this to be delivered as a separate patch or can I re-roll it at the base of detached-stash? (I realise I have to re-roll v5 anyway, because some of the later tests don't quite reflect the intent (e.g. ! git stash stash@{0} should be ! git stash drop stash@{0}). jon. On Thu, Aug 19, 2010 at 6:50 AM, Junio C Hamano <gitster@pobox.com> wrote: > Jon Seymour <jon.seymour@gmail.com> writes: > >> I understand why ref@{time-spec} might behave this way, but I reckon >> that if you ask for ref@{n} for n > N-1, where N is the length of the >> reflog, then you should either get empty output and a non-zero status >> (preferred) or a ref@{N-1} on the output with a status code of zero >> and a warning message (less optimal, IMHO). > > Yeah, the behaviour of ref@{...} syntax parser is way suboptimal: > > $ git rev-parse --verify jch@{99999}; echo $? > warning: Log for 'jch' only has 1368 entries. > cfb88e9a8d4926b0011ae2dd67e1f57a98f4b768 > 0 > > It even knows that it is running off the cut-off point; it should just > cause the caller to notice that fact. I don't think changing it to error > out should cause any harm to existing callers. > > It may just be the matter of doing something like this (totally > untested)... > > sha1_name.c | 12 +++++------- > 1 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/sha1_name.c b/sha1_name.c > index 4af94fa..c1e51c9 100644 > --- a/sha1_name.c > +++ b/sha1_name.c > @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); > > static int get_sha1_basic(const char *str, int len, unsigned char *sha1) > { > - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; > + static const char *warn_msg = "warning: refname '%.*s' is ambiguous.\n"; > char *real_ref = NULL; > int refs_found = 0; > int at, reflog_len; > @@ -390,7 +390,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) > return -1; > > if (warn_ambiguous_refs && refs_found > 1) > - fprintf(stderr, warning, len, str); > + fprintf(stderr, warn_msg, len, str); > > if (reflog_len) { > int nth, i; > @@ -426,14 +426,12 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) > if (read_ref_at(real_ref, at_time, nth, sha1, NULL, > &co_time, &co_tz, &co_cnt)) { > if (at_time) > - fprintf(stderr, > - "warning: Log for '%.*s' only goes " > + warning("Log for '%.*s' only goes " > "back to %s.\n", len, str, > show_date(co_time, co_tz, DATE_RFC2822)); > else > - fprintf(stderr, > - "warning: Log for '%.*s' only has " > - "%d entries.\n", len, str, co_cnt); > + return error("Log for '%.*s' only has " > + "%d entries.\n", len, str, co_cnt); > } > } > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/4] rev-parse: improve reporting of invalid log references 2010-08-18 23:02 ` Jon Seymour @ 2010-08-21 1:43 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 1/4] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour ` (3 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-21 1:43 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour This series fixes git rev-parse so that references of the form ref@{n} cause rev-parse to fail with non-zero status codes when n >= N, where N is the size of the reflog for ref. The first commit causes git rev-parse to exit with a non-zero status code but has the flaw that the error message is duplicated if --verify is not specified. The second commit fixes the duplicate error messages produced by the first commit, but prevents the diagnostic being displayed if the --verify option is specified. The third commit restores the diagnostic if the --verify option is specified. The fourth commit adds tests for the changed behaviour. Jon Seymour (4): rev-parse: exit with non-zero status if ref@{n} is not valid. rev-parse: suppress duplicate log limit exceeded message. rev-parse: introduce get_sha1_with_gently rev-parse: tests that git rev-parse --verify master@{n} builtin/rev-parse.c | 2 +- cache.h | 1 + sha1_name.c | 52 ++++++++++++++++++++++++--------------- t/t1503-rev-parse-verify.sh | 11 ++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++ 5 files changed, 54 insertions(+), 21 deletions(-) -- 1.7.2.1.152.g499e.dirty ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] rev-parse: exit with non-zero status if ref@{n} is not valid. 2010-08-18 23:02 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 0/4] rev-parse: improve reporting of invalid log references Jon Seymour @ 2010-08-21 1:43 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message Jon Seymour ` (2 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-21 1:43 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour >From Junio's e-mail: "The current behaviour of ref@{...} syntax parser is suboptimal: $ git rev-parse --verify jch@{99999}; echo $? warning: Log for 'jch' only has 1368 entries. cfb88e9a8d4926b0011ae2dd67e1f57a98f4b768 0 It even knows that it is running off the cut-off point; it should just cause the caller to notice that fact. I don't think changing it to error out should cause any harm to existing callers." This commit is based on a patch suggested by Junio. Note that the error message reporting the bad log reference is printed twice if --verify is not specified because the function is called from two different paths. This is fixed in subsequent commits. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- sha1_name.c | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4af94fa..82ad0f9 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); static int get_sha1_basic(const char *str, int len, unsigned char *sha1) { - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; + static const char *warn_msg = "warning: refname '%.*s' is ambiguous."; char *real_ref = NULL; int refs_found = 0; int at, reflog_len; @@ -390,7 +390,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) return -1; if (warn_ambiguous_refs && refs_found > 1) - fprintf(stderr, warning, len, str); + fprintf(stderr, warn_msg, len, str); if (reflog_len) { int nth, i; @@ -426,14 +426,17 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (read_ref_at(real_ref, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { if (at_time) - fprintf(stderr, - "warning: Log for '%.*s' only goes " - "back to %s.\n", len, str, + warning("Log for '%.*s' only goes " + "back to %s.", len, str, show_date(co_time, co_tz, DATE_RFC2822)); - else - fprintf(stderr, - "warning: Log for '%.*s' only has " - "%d entries.\n", len, str, co_cnt); + else { + error("Log for '%.*s' only has %d entries.", + len, + str, + co_cnt); + free(real_ref); + return -1; + } } } -- 1.7.2.1.156.gf148c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message. 2010-08-18 23:02 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 0/4] rev-parse: improve reporting of invalid log references Jon Seymour 2010-08-21 1:43 ` [PATCH 1/4] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour @ 2010-08-21 1:43 ` Jon Seymour 2010-08-22 21:20 ` Junio C Hamano 2010-08-21 1:43 ` [PATCH 3/4] rev-parse: introduce get_sha1_gently Jon Seymour 2010-08-21 1:43 ` [PATCH 4/4] rev-parse: tests that git rev-parse --verify master@{n} Jon Seymour 4 siblings, 1 reply; 20+ messages in thread From: Jon Seymour @ 2010-08-21 1:43 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour Modifies get_sha1_basic to accept a gently parameter to indicate whether error should be used to report the log limit exceeded condition. Prior to this change, git rev-parse master@{99999} reported: error: Log for 'master' only has 166 entries. master@{99999} error: Log for 'master' only has 166 entries. fatal: ambiguous argument 'master@{99999}': unknown revision or path not in the working tree. Use '--' to separate paths from revisions With this change, git rev-parse master@{99999} reports: master@{99999} error: Log for 'master' only has 166 entries. fatal: ambiguous argument 'master@{99999}': unknown revision or path not in the working tree. Use '--' to separate paths from revisions Note: git rev-parse --verify master@{99999} now does not report the error message although it does still exit with a non-zero status code. This is fixed by the next commit. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- sha1_name.c | 31 ++++++++++++++++--------------- 1 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 82ad0f9..6e706eb 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -338,9 +338,9 @@ static inline int upstream_mark(const char *string, int len) return 0; } -static int get_sha1_1(const char *name, int len, unsigned char *sha1); +static int get_sha1_1(const char *name, int len, unsigned char *sha1, int gently); -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, int gently) { static const char *warn_msg = "warning: refname '%.*s' is ambiguous."; char *real_ref = NULL; @@ -375,7 +375,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) ret = interpret_branch_name(str+at, &buf); if (ret > 0) { /* substitute this branch name and restart */ - return get_sha1_1(buf.buf, buf.len, sha1); + return get_sha1_1(buf.buf, buf.len, sha1, 1); } else if (ret == 0) { return -1; } @@ -430,10 +430,11 @@ 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 { - error("Log for '%.*s' only has %d entries.", - len, - str, - co_cnt); + if (!gently) + error("Log for '%.*s' only has %d entries.", + len, + str, + co_cnt); free(real_ref); return -1; } @@ -448,7 +449,7 @@ static int get_parent(const char *name, int len, unsigned char *result, int idx) { unsigned char sha1[20]; - int ret = get_sha1_1(name, len, sha1); + int ret = get_sha1_1(name, len, sha1, 1); struct commit *commit; struct commit_list *p; @@ -481,7 +482,7 @@ static int get_nth_ancestor(const char *name, int len, struct commit *commit; int ret; - ret = get_sha1_1(name, len, sha1); + ret = get_sha1_1(name, len, sha1, 1); if (ret) return ret; commit = lookup_commit_reference(sha1); @@ -504,7 +505,7 @@ struct object *peel_to_type(const char *name, int namelen, namelen = strlen(name); if (!o) { unsigned char sha1[20]; - if (get_sha1_1(name, namelen, sha1)) + if (get_sha1_1(name, namelen, sha1, 1)) return NULL; o = parse_object(sha1); } @@ -566,7 +567,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) else return -1; - if (get_sha1_1(name, sp - name - 2, outer)) + if (get_sha1_1(name, sp - name - 2, outer, 1)) return -1; o = parse_object(outer); @@ -614,7 +615,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1) return -1; } -static int get_sha1_1(const char *name, int len, unsigned char *sha1) +static int get_sha1_1(const char *name, int len, unsigned char *sha1, int gently) { int ret, has_suffix; const char *cp; @@ -650,7 +651,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1) if (!ret) return 0; - ret = get_sha1_basic(name, len, sha1); + ret = get_sha1_basic(name, len, sha1, gently); if (!ret) return 0; @@ -1059,7 +1060,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1, memset(oc, 0, sizeof(*oc)); oc->mode = S_IFINVALID; - ret = get_sha1_1(name, namelen, sha1); + ret = get_sha1_1(name, namelen, sha1, gently); if (!ret) return ret; /* sha1:path --> object name of path in ent sha1 @@ -1122,7 +1123,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1, strncpy(object_name, name, cp-name); object_name[cp-name] = '\0'; } - if (!get_sha1_1(name, cp-name, tree_sha1)) { + if (!get_sha1_1(name, cp-name, tree_sha1, 1)) { const char *filename = cp+1; ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode); if (!gently) { -- 1.7.2.1.156.gf148c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message. 2010-08-21 1:43 ` [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message Jon Seymour @ 2010-08-22 21:20 ` Junio C Hamano 2010-08-23 14:59 ` Jon Seymour 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2010-08-22 21:20 UTC (permalink / raw) To: Jon Seymour; +Cc: git Jon Seymour <jon.seymour@gmail.com> writes: > With this change, git rev-parse master@{99999} reports: > > master@{99999} > error: Log for 'master' only has 166 entries. > fatal: ambiguous argument 'master@{99999}': unknown revision or path not in the working tree. > Use '--' to separate paths from revisions If we are going to say "fatal:" and die at the end, I think we probably do not want to say a separate "error" message. Instead of adding a boolean parameter "fail silently or warn?", it may be better to make it a pointer to a strbuf and have it filled with error details (or a machine readable struct and make it responsibility of the caller to generate a message). Then die_verify_filename can become, e.g.: static void NORETURN die_verify_filename(const char *prefix, const char *arg) { unsigned char sha1[20]; unsigned mode; struct strbuf e; /* learn in what way is it bad? */ get_sha1_with_mode_1(arg, sha1, &mode, &e, prefix); /* ... or fall back the most general message. */ die("ambiguous argument '%s': unknown revision or path not in the working tree.\n" "%s" "Use '--' to separate paths from revisions", arg, e.buf); } and we can later reuse the same mechanism to cover other kind of errors, not just "the log does not have that many entries" error. With your "gently" approach it may not be easy to do that without adding more parameters to all the functions in the codepath, no? I also wonder if this can simply become part of "struct object_context", which is to pass extra information in addition to the SHA-1 (which the original API returned) back to the caller. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message. 2010-08-22 21:20 ` Junio C Hamano @ 2010-08-23 14:59 ` Jon Seymour 2010-08-23 16:33 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Jon Seymour @ 2010-08-23 14:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Mon, Aug 23, 2010 at 7:20 AM, Junio C Hamano <gitster@pobox.com> wrote: > Jon Seymour <jon.seymour@gmail.com> writes: > >> With this change, git rev-parse master@{99999} reports: >> >> master@{99999} >> error: Log for 'master' only has 166 entries. >> fatal: ambiguous argument 'master@{99999}': unknown revision or path not in the working tree. >> Use '--' to separate paths from revisions > > If we are going to say "fatal:" and die at the end, I think we probably do > not want to say a separate "error" message. Instead of adding a boolean > parameter "fail silently or warn?", it may be better to make it a pointer > to a strbuf and have it filled with error details (or a machine readable > struct and make it responsibility of the caller to generate a message). > I agree that it is not ideal that we have an error message and a fatal message in this case. Ideally, I think we should simply be reporting the log limit exceeded condition as a fatal condition on its own, and not reporting the ambiguous ref catch all at all in this case. We have already decided that the argument is a reference at this point and as such it is not really ambiguous, it just happens to be out of bounds. > Then die_verify_filename can become, e.g.: > > static void NORETURN die_verify_filename(const char *prefix, const char *arg) > { > unsigned char sha1[20]; > unsigned mode; > struct strbuf e; > > /* learn in what way is it bad? */ > get_sha1_with_mode_1(arg, sha1, &mode, &e, prefix); > > /* ... or fall back the most general message. */ > die("ambiguous argument '%s': unknown revision or path not in the working tree.\n" > "%s" > "Use '--' to separate paths from revisions", arg, e.buf); > } > > and we can later reuse the same mechanism to cover other kind of errors, > not just "the log does not have that many entries" error. With your > "gently" approach it may not be easy to do that without adding more > parameters to all the functions in the codepath, no? > I agree that the gently approach used here doesn't generalise to other cases. It seemed like the smallest change I could make that was reasonably consistent with existing code. I am happy to look at a more general solution. > I also wonder if this can simply become part of "struct object_context", > which is to pass extra information in addition to the SHA-1 (which the > original API returned) back to the caller. > I'll have a look at this. object_context seems like a reasonably coherent structure at the moment - do we risk diluting that coherence by attaching error reporting state to it? Should we consider passing an error callback function (+ struct) around so that the calling context can decide whether to directly report errors at the point of detection or defer them until later? Calling contexts that don't care about reporting immediately can do so, calling contexts that do care can choose how to record errors into the struct, as required. jon. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message. 2010-08-23 14:59 ` Jon Seymour @ 2010-08-23 16:33 ` Junio C Hamano 2010-08-23 23:11 ` [PATCH v2 0/2] rev-parse: strengthen validation of ref@{n} references Jon Seymour ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Junio C Hamano @ 2010-08-23 16:33 UTC (permalink / raw) To: Jon Seymour; +Cc: git Jon Seymour <jon.seymour@gmail.com> writes: > Ideally, I think we should simply be reporting the log limit exceeded > condition as a fatal condition on its own, and not reporting the > ambiguous ref catch all at all in this case. We have already decided > that the argument is a reference at this point and as such it is not > really ambiguous, it just happens to be out of bounds. A very good point. Would simply replacing that error-then-return-nonzero with a die work? If so we do not even need to worry about new mechanisms to cause different soft error conditions reported back to the caller to be printed. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/2] rev-parse: strengthen validation of ref@{n} references 2010-08-23 16:33 ` Junio C Hamano @ 2010-08-23 23:11 ` Jon Seymour 2010-08-23 23:11 ` [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour 2010-08-23 23:11 ` [PATCH v2 2/2] " Jon Seymour 2 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-23 23:11 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour This series changes git rev-parse so that references of the form: ref@{n} cause rev-parse to fail with a non-zero status code if n >= size of the reference log for ref. The first commit implements the changed behaviour, the second commit updates the tests to include tests for the changed behaviour. This is the second revision of this series. It differs from the first in that it uses die rather than error to report the condition and thus exits early, without generating any output on stdout for the failing argument. It also reports just a single fatal message instead of both an error message and a fatal message. The original patch and this revision are both based on suggestions from Junio. Jon Seymour (2): rev-parse: exit with non-zero status if ref@{n} is not valid. rev-parse: tests git rev-parse --verify master@{n}, for various n sha1_name.c | 20 +++++++++++--------- t/t1503-rev-parse-verify.sh | 11 +++++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) -- 1.7.2.1.99.geab11 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid. 2010-08-23 16:33 ` Junio C Hamano 2010-08-23 23:11 ` [PATCH v2 0/2] rev-parse: strengthen validation of ref@{n} references Jon Seymour @ 2010-08-23 23:11 ` Jon Seymour 2010-08-24 0:14 ` Jonathan Nieder 2010-08-23 23:11 ` [PATCH v2 2/2] " Jon Seymour 2 siblings, 1 reply; 20+ messages in thread From: Jon Seymour @ 2010-08-23 23:11 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour >From Junio's e-mail: "The current behaviour of ref@{...} syntax parser is suboptimal: $ git rev-parse --verify jch@{99999} && echo true warning: Log for 'jch' only has 1368 entries. cfb88e9a8d4926b0011ae2dd67e1f57a98f4b768 true It even knows that it is running off the cut-off point; it should just cause the caller to notice that fact. I don't think changing it to error out should cause any harm to existing callers." With this change: $ git rev-parse --verify jch@{99999} || echo false fatal: Log for 'jch' only has 1368 entries. false $ git rev-parse jch@{99999} || echo false fatal: Log for 'jch' only has 1368 entries. false Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- sha1_name.c | 20 +++++++++++--------- 1 files changed, 11 insertions(+), 9 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4af94fa..d40ae48 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); static int get_sha1_basic(const char *str, int len, unsigned char *sha1) { - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; + static const char *warn_msg = "warning: refname '%.*s' is ambiguous."; char *real_ref = NULL; int refs_found = 0; int at, reflog_len; @@ -390,7 +390,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) return -1; if (warn_ambiguous_refs && refs_found > 1) - fprintf(stderr, warning, len, str); + fprintf(stderr, warn_msg, len, str); if (reflog_len) { int nth, i; @@ -426,14 +426,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (read_ref_at(real_ref, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { if (at_time) - fprintf(stderr, - "warning: Log for '%.*s' only goes " - "back to %s.\n", len, str, + warning("Log for '%.*s' only goes " + "back to %s.", len, str, show_date(co_time, co_tz, DATE_RFC2822)); - else - fprintf(stderr, - "warning: Log for '%.*s' only has " - "%d entries.\n", len, str, co_cnt); + else { + free(real_ref); + die("Log for '%.*s' only has %d entries.", + len, + str, + co_cnt); + } } } -- 1.7.2.1.99.geab11 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid. 2010-08-23 23:11 ` [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour @ 2010-08-24 0:14 ` Jonathan Nieder 2010-08-24 4:52 ` [PATCH v3 0/3] rev-parse: strengthen validation of ref@{n} references Jon Seymour ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Jonathan Nieder @ 2010-08-24 0:14 UTC (permalink / raw) To: Jon Seymour; +Cc: git, gitster Jon Seymour wrote: > +++ b/sha1_name.c > @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); > > static int get_sha1_basic(const char *str, int len, unsigned char *sha1) > { > - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; > + static const char *warn_msg = "warning: refname '%.*s' is ambiguous."; $ git tag master $ bin-wrappers/git rev-parse master warning: refname 'master' is ambiguous.cfee1ae8f0c56eed7d8ffa821f650789f8b11de2 Here's a fixup. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- diff --git a/sha1_name.c b/sha1_name.c index d40ae48..823e582 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); static int get_sha1_basic(const char *str, int len, unsigned char *sha1) { - static const char *warn_msg = "warning: refname '%.*s' is ambiguous."; + static const char *warn_msg = "warning: refname '%.*s' is ambiguous.\n"; char *real_ref = NULL; int refs_found = 0; int at, reflog_len; -- ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 0/3] rev-parse: strengthen validation of ref@{n} references 2010-08-24 0:14 ` Jonathan Nieder @ 2010-08-24 4:52 ` Jon Seymour 2010-08-24 4:52 ` [PATCH v3 1/3] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-24 4:52 UTC (permalink / raw) To: git; +Cc: gitster, jrnieder, Jon Seymour This series changes git rev-parse so that references of the form: ref@{n} cause rev-parse to fail with a non-zero status code if n >= size of the reference log for ref. The first commit implements the changed behaviour; the second commit replaces uses of fprintf (within sha1_name.c) with warning; the third commit updates the tests to include tests for the changed behaviour. This is the third revision of this series. It differs from the first in that it uses die rather than error to report the condition and thus exits early, without generating any output on stdout for the failing argument. It also reports just a single fatal message instead of both an error message and a fatal message. The original patch and this revision are both based on suggestions from Junio. Corrections to diagnostic output inspired by Jonathan Nieder's review. Jon Seymour (3): rev-parse: exit with non-zero status if ref@{n} is not valid. sha1_name.c: use warning in preference to fprintf(stderr rev-parse: tests git rev-parse --verify master@{n}, for various n sha1_name.c | 20 +++++++++++--------- t/t1503-rev-parse-verify.sh | 11 +++++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) -- 1.7.2.1.100.gceecd ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 1/3] rev-parse: exit with non-zero status if ref@{n} is not valid. 2010-08-24 0:14 ` Jonathan Nieder 2010-08-24 4:52 ` [PATCH v3 0/3] rev-parse: strengthen validation of ref@{n} references Jon Seymour @ 2010-08-24 4:52 ` Jon Seymour 2010-08-24 4:52 ` [PATCH v3 2/3] sha1_name.c: use warning in preference to fprintf(stderr Jon Seymour 2010-08-24 4:52 ` [PATCH v3 3/3] rev-parse: tests git rev-parse --verify master@{n}, for various n Jon Seymour 3 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-24 4:52 UTC (permalink / raw) To: git; +Cc: gitster, jrnieder, Jon Seymour >From Junio's e-mail: "The current behaviour of ref@{...} syntax parser is suboptimal: $ git rev-parse --verify jch@{99999} && echo true warning: Log for 'jch' only has 1368 entries. cfb88e9a8d4926b0011ae2dd67e1f57a98f4b768 true It even knows that it is running off the cut-off point; it should just cause the caller to notice that fact. I don't think changing it to error out should cause any harm to existing callers." With this change: $ git rev-parse --verify jch@{99999} || echo false fatal: Log for 'jch' only has 1368 entries. false $ git rev-parse jch@{99999} || echo false fatal: Log for 'jch' only has 1368 entries. false Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- sha1_name.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4af94fa..10a4efe 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -430,10 +430,13 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) "warning: Log for '%.*s' only goes " "back to %s.\n", len, str, show_date(co_time, co_tz, DATE_RFC2822)); - else - fprintf(stderr, - "warning: Log for '%.*s' only has " - "%d entries.\n", len, str, co_cnt); + else { + free(real_ref); + die("Log for '%.*s' only has %d entries.", + len, + str, + co_cnt); + } } } -- 1.7.2.1.100.gceecd ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 2/3] sha1_name.c: use warning in preference to fprintf(stderr 2010-08-24 0:14 ` Jonathan Nieder 2010-08-24 4:52 ` [PATCH v3 0/3] rev-parse: strengthen validation of ref@{n} references Jon Seymour 2010-08-24 4:52 ` [PATCH v3 1/3] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour @ 2010-08-24 4:52 ` Jon Seymour 2010-08-24 4:52 ` [PATCH v3 3/3] rev-parse: tests git rev-parse --verify master@{n}, for various n Jon Seymour 3 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-24 4:52 UTC (permalink / raw) To: git; +Cc: gitster, jrnieder, Jon Seymour This commit changes sha1_name.c to use warning instead of fprintf(stderr). Trailing newlines from message formats have been removed since warning adds one itself. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- sha1_name.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 10a4efe..3b8aea2 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -342,7 +342,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1); static int get_sha1_basic(const char *str, int len, unsigned char *sha1) { - static const char *warning = "warning: refname '%.*s' is ambiguous.\n"; + static const char *warn_msg = "refname '%.*s' is ambiguous."; char *real_ref = NULL; int refs_found = 0; int at, reflog_len; @@ -390,7 +390,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) return -1; if (warn_ambiguous_refs && refs_found > 1) - fprintf(stderr, warning, len, str); + warning(warn_msg, len, str); if (reflog_len) { int nth, i; @@ -426,9 +426,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (read_ref_at(real_ref, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { if (at_time) - fprintf(stderr, - "warning: Log for '%.*s' only goes " - "back to %s.\n", len, str, + warning("Log for '%.*s' only goes " + "back to %s.", len, str, show_date(co_time, co_tz, DATE_RFC2822)); else { free(real_ref); -- 1.7.2.1.100.gceecd ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 3/3] rev-parse: tests git rev-parse --verify master@{n}, for various n 2010-08-24 0:14 ` Jonathan Nieder ` (2 preceding siblings ...) 2010-08-24 4:52 ` [PATCH v3 2/3] sha1_name.c: use warning in preference to fprintf(stderr Jon Seymour @ 2010-08-24 4:52 ` Jon Seymour 3 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-24 4:52 UTC (permalink / raw) To: git; +Cc: gitster, jrnieder, Jon Seymour This commit introduces tests that verify that rev-parse parses master@{n} correctly for various values of n less than, equal to and greater than the number of revisions in the reference log. In particular, these tests check that rev-parse exits with a non-zero status code and prints a message of the following form to stderr. fatal: Log for [^ ]* only has [0-9][0-9]* entries. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- t/t1503-rev-parse-verify.sh | 11 +++++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++++ 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh index cc65394..61092f7 100755 --- a/t/t1503-rev-parse-verify.sh +++ b/t/t1503-rev-parse-verify.sh @@ -104,4 +104,15 @@ test_expect_success 'use --default' ' test_must_fail git rev-parse --verify --default bar ' +test_expect_success 'master@{n} for various n' ' + N=$(git reflog | wc -l) && + Nm1=$((N-1)) && + Np1=$((N+1)) && + git rev-parse --verify master@{0} && + git rev-parse --verify master@{1} && + git rev-parse --verify master@{$Nm1} && + test_must_fail "git rev-parse --verify master@{$N}" && + test_must_fail "git rev-parse --verify master@{$Np1}" +' + test_done diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index af721f9..0eeeb0e 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -66,4 +66,13 @@ test_expect_success 'incorrect file in :path and :N:path' ' grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error ' +test_expect_success 'invalid @{n} reference' ' + test_must_fail git rev-parse master@{99999} >output 2>error && + test -z "$(cat output)" && + grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error && + test_must_fail git rev-parse --verify master@{99999} >output 2>error && + test -z "$(cat output)" && + grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error +' + test_done -- 1.7.2.1.100.gceecd ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] rev-parse: tests git rev-parse --verify master@{n}, for various n 2010-08-23 16:33 ` Junio C Hamano 2010-08-23 23:11 ` [PATCH v2 0/2] rev-parse: strengthen validation of ref@{n} references Jon Seymour 2010-08-23 23:11 ` [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour @ 2010-08-23 23:11 ` Jon Seymour 2 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-23 23:11 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour This commit introduces tests that verify that rev-parse parses master@{n} correctly for various values of n less than, equal to and greater than the number of revisions in the reference log. In particular, these tests check that rev-parse exits with a non-zero status code and prints a message of the following form to stderr. fatal: Log for [^ ]* only has [0-9][0-9]* entries. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- t/t1503-rev-parse-verify.sh | 11 +++++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++++ 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh index cc65394..61092f7 100755 --- a/t/t1503-rev-parse-verify.sh +++ b/t/t1503-rev-parse-verify.sh @@ -104,4 +104,15 @@ test_expect_success 'use --default' ' test_must_fail git rev-parse --verify --default bar ' +test_expect_success 'master@{n} for various n' ' + N=$(git reflog | wc -l) && + Nm1=$((N-1)) && + Np1=$((N+1)) && + git rev-parse --verify master@{0} && + git rev-parse --verify master@{1} && + git rev-parse --verify master@{$Nm1} && + test_must_fail "git rev-parse --verify master@{$N}" && + test_must_fail "git rev-parse --verify master@{$Np1}" +' + test_done diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index af721f9..0eeeb0e 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -66,4 +66,13 @@ test_expect_success 'incorrect file in :path and :N:path' ' grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error ' +test_expect_success 'invalid @{n} reference' ' + test_must_fail git rev-parse master@{99999} >output 2>error && + test -z "$(cat output)" && + grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error && + test_must_fail git rev-parse --verify master@{99999} >output 2>error && + test -z "$(cat output)" && + grep "fatal: Log for [^ ]* only has [0-9][0-9]* entries." error +' + test_done -- 1.7.2.1.99.geab11 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] rev-parse: introduce get_sha1_gently 2010-08-18 23:02 ` Jon Seymour ` (2 preceding siblings ...) 2010-08-21 1:43 ` [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message Jon Seymour @ 2010-08-21 1:43 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 4/4] rev-parse: tests that git rev-parse --verify master@{n} Jon Seymour 4 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-21 1:43 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour Use of this function allows git rev-parse --verify ref@{n} to report an error message if n >= the size of the reference log for ref. However, if the user has not specified --verify, get_sha1_gently will be called again via verify_filename and the message will be reported then, if required. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- builtin/rev-parse.c | 2 +- cache.h | 1 + sha1_name.c | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index a5a1c86..dbc6a8c 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -707,7 +707,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) name++; type = REVERSED; } - if (!get_sha1(name, sha1)) { + if (!get_sha1_gently(name, sha1, !verify)) { if (verify) revs_count++; else diff --git a/cache.h b/cache.h index 37ef9d8..625728c 100644 --- a/cache.h +++ b/cache.h @@ -767,6 +767,7 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st { return get_sha1_with_context_1(str, sha1, orc, 1, NULL); } +extern inline int get_sha1_gently(const char *str, unsigned char *sha1, int gently); extern int get_sha1_hex(const char *hex, unsigned char *sha1); extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */ extern int read_ref(const char *filename, unsigned char *sha1); diff --git a/sha1_name.c b/sha1_name.c index 6e706eb..48e7aa0 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1050,6 +1050,14 @@ int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, return ret; } +int get_sha1_gently(const char *name, unsigned char *sha1, int gently) +{ + struct object_context unused; + int ret; + ret = get_sha1_with_context_1(name, sha1, &unused, gently, NULL); + return ret; +} + int get_sha1_with_context_1(const char *name, unsigned char *sha1, struct object_context *oc, int gently, const char *prefix) -- 1.7.2.1.156.gf148c ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] rev-parse: tests that git rev-parse --verify master@{n} 2010-08-18 23:02 ` Jon Seymour ` (3 preceding siblings ...) 2010-08-21 1:43 ` [PATCH 3/4] rev-parse: introduce get_sha1_gently Jon Seymour @ 2010-08-21 1:43 ` Jon Seymour 4 siblings, 0 replies; 20+ messages in thread From: Jon Seymour @ 2010-08-21 1:43 UTC (permalink / raw) To: git; +Cc: gitster, Jon Seymour This commit introduces tests that verify that rev-parse parses master@{n} correctly for various values of n less than, equal to and greater than the number of revisions in the reference log. In particular, these tests check that rev-parse sets a non-zero status code and prints a message of the following form to stderr. error: Log for [^ ]* only has [0-9][0-9]* entries. Also tests that output for failing cases is generated if, and only if, --verify is not specified. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- t/t1503-rev-parse-verify.sh | 11 +++++++++++ t/t1506-rev-parse-diagnosis.sh | 9 +++++++++ 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh index cc65394..61092f7 100755 --- a/t/t1503-rev-parse-verify.sh +++ b/t/t1503-rev-parse-verify.sh @@ -104,4 +104,15 @@ test_expect_success 'use --default' ' test_must_fail git rev-parse --verify --default bar ' +test_expect_success 'master@{n} for various n' ' + N=$(git reflog | wc -l) && + Nm1=$((N-1)) && + Np1=$((N+1)) && + git rev-parse --verify master@{0} && + git rev-parse --verify master@{1} && + git rev-parse --verify master@{$Nm1} && + test_must_fail "git rev-parse --verify master@{$N}" && + test_must_fail "git rev-parse --verify master@{$Np1}" +' + test_done diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index af721f9..427c67e 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -66,4 +66,13 @@ test_expect_success 'incorrect file in :path and :N:path' ' grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error ' +test_expect_success 'invalid @{n} reference' ' + test_must_fail git rev-parse master@{99999} >output 2>error && + test "$(cat output)" = "master@{99999}" && + grep "error: Log for [^ ]* only has [0-9][0-9]* entries." error && + test_must_fail git rev-parse --verify master@{99999} >output 2>error && + test -z "$(cat output)" && + grep "error: Log for [^ ]* only has [0-9][0-9]* entries." error +' + test_done -- 1.7.2.1.156.gf148c ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-08-24 4:53 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-08-18 11:36 RFD: should git rev-parse exit with non-zero status if ref@{n} is not valid? Jon Seymour 2010-08-18 11:41 ` Jon Seymour 2010-08-18 20:50 ` Junio C Hamano 2010-08-18 23:02 ` Jon Seymour 2010-08-21 1:43 ` [PATCH 0/4] rev-parse: improve reporting of invalid log references Jon Seymour 2010-08-21 1:43 ` [PATCH 1/4] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour 2010-08-21 1:43 ` [PATCH 2/4] rev-parse: suppress duplicate log limit exceeded message Jon Seymour 2010-08-22 21:20 ` Junio C Hamano 2010-08-23 14:59 ` Jon Seymour 2010-08-23 16:33 ` Junio C Hamano 2010-08-23 23:11 ` [PATCH v2 0/2] rev-parse: strengthen validation of ref@{n} references Jon Seymour 2010-08-23 23:11 ` [PATCH v2 1/2] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour 2010-08-24 0:14 ` Jonathan Nieder 2010-08-24 4:52 ` [PATCH v3 0/3] rev-parse: strengthen validation of ref@{n} references Jon Seymour 2010-08-24 4:52 ` [PATCH v3 1/3] rev-parse: exit with non-zero status if ref@{n} is not valid Jon Seymour 2010-08-24 4:52 ` [PATCH v3 2/3] sha1_name.c: use warning in preference to fprintf(stderr Jon Seymour 2010-08-24 4:52 ` [PATCH v3 3/3] rev-parse: tests git rev-parse --verify master@{n}, for various n Jon Seymour 2010-08-23 23:11 ` [PATCH v2 2/2] " Jon Seymour 2010-08-21 1:43 ` [PATCH 3/4] rev-parse: introduce get_sha1_gently Jon Seymour 2010-08-21 1:43 ` [PATCH 4/4] rev-parse: tests that git rev-parse --verify master@{n} Jon Seymour
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).