Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 27, 2010, 18:50 UTC
- Message-ID
- <7viqanqt5s.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20100127102417.GD6262@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> A minor nit, but wouldn't: > > int approxidate_careful(const char *str, unsigned long *out); > > returning an error code be the more usual pattern for a function with > error-plus-output (your approxidate wrapper would have to be a function then, > not a macro)?
I don't have strong preference either way; the one in the patch was modelled after setup_git_directory_gently(&nongit_ok), and slightly easier to work with for existing callers that don't care enough.
Show 22 quoted lines
>> @@ -413,8 +413,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>> } else if (0 <= nth)
>> at_time = 0;
>> else {
>> + int errors = 0;
>> char *tmp = xstrndup(str + at + 2, reflog_len);
>> - at_time = approxidate(tmp);
>> + at_time = approxidate_careful(tmp, &errors);
>> + if (errors)
>> + die("Bogus timestamp '%s'", tmp);
>> free(tmp);
>
> I was just going to "return -1" here, which yields:
>
> $ git show @{bogosity}
> fatal: ambiguous argument '@{bogosity}': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions
>
> instead of
>
> $ git show @{bogosity}
> fatal: Bogus timestamp 'bogosity'Good point. Let's change it to silently return -1 and let the caller take care of it. Perhaps there are some callers that say "does this name an object? If not, let's try pathname".
Show 6 quoted lines
>> +test_expect_success '@{30.years.ago} shows old' '
>> + check_at @{30.years.ago} one
>
> Side note: I chose this because we needed to go back from the current
> time beyond where test_tick would place the commit. Which means this
> test has a 2035 bug. :)Can't we use an absolute date, given that test_tick gives fixed timestamp sequence to pretend as if we were still in 2005 when we are running these tests?
sha1_name.c | 4 ++-- t/t0101-at-syntax.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c index f4a74fe..04fb3b8 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -398,9 +398,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) int errors = 0; char *tmp = xstrndup(str + at + 2, reflog_len); at_time = approxidate_careful(tmp, &errors); - if (errors) - die("Bogus timestamp '%s'", tmp); free(tmp); + if (errors) + return -1; } if (read_ref_at(real_ref, at_time, nth, sha1, NULL, &co_time, &co_tz, &co_cnt)) { diff --git a/t/t0101-at-syntax.sh b/t/t0101-at-syntax.sh index ccabc37..5e298c5 100755 --- a/t/t0101-at-syntax.sh +++ b/t/t0101-at-syntax.sh @@ -26,8 +26,8 @@ test_expect_success '@{now} shows current' ' check_at @{now} two ' -test_expect_success '@{30.years.ago} shows old' ' - check_at @{30.years.ago} one +test_expect_success '@{2001-09-17} (before the first commit) shows old' ' + check_at @{2001-09-17} one ' test_expect_success 'silly approxidates work' '