* [PATCH] apply: Recognize epoch timestamps with : in the timezone
@ 2010-09-29 20:49 Anders Kaseorg
2010-09-29 21:41 ` Jonathan Nieder
0 siblings, 1 reply; 7+ messages in thread
From: Anders Kaseorg @ 2010-09-29 20:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Some patches have a timezone formatted like ‘-08:00’ instead of
‘-0800’ (e.g. http://lwn.net/Articles/131729/), so git apply would
fail to recognize the epoch timestamp of deleted files and would
create empty files instead. Teach it to support both formats, and add
a test case.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
builtin/apply.c | 6 ++++--
t/t4132-apply-removal.sh | 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 23c18c5..0fa9a8d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -733,7 +733,7 @@ static int has_epoch_timestamp(const char *nameline)
" "
"[0-2][0-9]:[0-5][0-9]:00(\\.0+)?"
" "
- "([-+][0-2][0-9][0-5][0-9])\n";
+ "([-+][0-2][0-9]):?([0-5][0-9])\n";
const char *timestamp = NULL, *cp;
static regex_t *stamp;
regmatch_t m[10];
@@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)
}
zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
- zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
+ if (m[4].rm_so == m[3].rm_so + 3)
+ zoneoffset /= 100;
+ zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
if (timestamp[m[3].rm_so] == '-')
zoneoffset = -zoneoffset;
diff --git a/t/t4132-apply-removal.sh b/t/t4132-apply-removal.sh
index bb1ffe3..a2bc1cd 100755
--- a/t/t4132-apply-removal.sh
+++ b/t/t4132-apply-removal.sh
@@ -30,6 +30,7 @@ test_expect_success setup '
epocWest="1969-12-31 16:00:00.000000000 -0800" &&
epocGMT="1970-01-01 00:00:00.000000000 +0000" &&
epocEast="1970-01-01 09:00:00.000000000 +0900" &&
+ epocWest2="1969-12-31 16:00:00 -08:00" &&
sed -e "s/TS0/$epocWest/" -e "s/TS1/$timeWest/" <c >createWest.patch &&
sed -e "s/TS0/$epocEast/" -e "s/TS1/$timeEast/" <c >createEast.patch &&
@@ -46,6 +47,7 @@ test_expect_success setup '
sed -e "s/TS0/$timeWest/" -e "s/TS1/$epocWest/" <d >removeWest.patch &&
sed -e "s/TS0/$timeEast/" -e "s/TS1/$epocEast/" <d >removeEast.patch &&
sed -e "s/TS0/$timeGMT/" -e "s/TS1/$epocGMT/" <d >removeGMT.patch &&
+ sed -e "s/TS0/$timeWest/" -e "s/TS1/$epocWest2/" <d >removeWest2.patch &&
echo something >something &&
>empty
--
1.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-09-29 20:49 [PATCH] apply: Recognize epoch timestamps with : in the timezone Anders Kaseorg
@ 2010-09-29 21:41 ` Jonathan Nieder
2010-09-29 21:49 ` Jonathan Nieder
2010-10-13 18:59 ` Junio C Hamano
0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Nieder @ 2010-09-29 21:41 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: Junio C Hamano, git
Hi Anders,
Anders Kaseorg wrote:
> Some patches have a timezone formatted like '-08:00' instead of
> '-0800' (e.g. http://lwn.net/Articles/131729/)
Odd. Any idea what tool generates these patches?
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
[...]
> @@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)
> }
>
> zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
> - zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
> + if (m[4].rm_so == m[3].rm_so + 3)
> + zoneoffset /= 100;
> + zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
Might be clearer to write
if (timestamp[m[3].rm_so + 3] != ':')
With or without that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Maybe something like this on top?
-- 8< --
Subject: apply: handle patches with funny filename and colon in timezone
Some patches have a timezone formatted like '-08:00' instead of
'-0800' in their ---/+++ lines (e.g. http://lwn.net/Articles/131729/).
Take this into account when searching for the start of the timezone
(which is the end of the filename).
This does not actually affect the outcome of patching unless (1) a
file being patched has a non-' ' whitespace character (e.g., tab) in
its filename, or (2) the patch is whitespace-damaged, so the tab
between filename and timestamp has been replaced with spaces.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/apply.c | 24 ++++++++++++++++++++++--
t/t4135-apply-weird-filenames.sh | 16 ++++++++++++++++
t/t4135/damaged-tz.diff | 5 +++++
t/t4135/funny-tz.diff | 5 +++++
4 files changed, 48 insertions(+), 2 deletions(-)
create mode 100644 t/t4135/damaged-tz.diff
create mode 100644 t/t4135/funny-tz.diff
diff --git a/builtin/apply.c b/builtin/apply.c
index 0fa9a8d..7d91d8f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -449,7 +449,7 @@ static char *find_name_gnu(const char *line, char *def, int p_value)
return squash_slash(strbuf_detach(&name, NULL));
}
-static size_t tz_len(const char *line, size_t len)
+static size_t sane_tz_len(const char *line, size_t len)
{
const char *tz, *p;
@@ -467,6 +467,24 @@ static size_t tz_len(const char *line, size_t len)
return line + len - tz;
}
+static size_t tz_with_colon_len(const char *line, size_t len)
+{
+ const char *tz, *p;
+
+ if (len < strlen(" +08:00") || line[len - strlen(":00")] != ':')
+ return 0;
+ tz = line + len - strlen(" +08:00");
+
+ if (tz[0] != ' ' || (tz[1] != '+' && tz[1] != '-'))
+ return 0;
+ p = tz + 2;
+ if (!isdigit(*p++) || !isdigit(*p++) || *p++ != ':' ||
+ !isdigit(*p++) || !isdigit(*p++))
+ return 0;
+
+ return line + len - tz;
+}
+
static size_t date_len(const char *line, size_t len)
{
const char *date, *p;
@@ -561,7 +579,9 @@ static size_t diff_timestamp_len(const char *line, size_t len)
if (!isdigit(end[-1]))
return 0;
- n = tz_len(line, end - line);
+ n = sane_tz_len(line, end - line);
+ if (!n)
+ n = tz_with_colon_len(line, end - line);
end -= n;
n = short_time_len(line, end - line);
diff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh
index 1e5aad5..bf5dc57 100755
--- a/t/t4135-apply-weird-filenames.sh
+++ b/t/t4135-apply-weird-filenames.sh
@@ -72,4 +72,20 @@ test_expect_success 'whitespace-damaged traditional patch' '
test_cmp expected postimage.txt
'
+test_expect_success 'traditional patch with colon in timezone' '
+ echo postimage >expected &&
+ reset_preimage &&
+ rm -f "post image.txt" &&
+ git apply "$vector/funny-tz.diff" &&
+ test_cmp expected "post image.txt"
+'
+
+test_expect_success 'traditional, whitespace-damaged, colon in timezone' '
+ echo postimage >expected &&
+ reset_preimage &&
+ rm -f "post image.txt" &&
+ git apply "$vector/damaged-tz.diff" &&
+ test_cmp expected "post image.txt"
+'
+
test_done
diff --git a/t/t4135/damaged-tz.diff b/t/t4135/damaged-tz.diff
new file mode 100644
index 0000000..07aaf08
--- /dev/null
+++ b/t/t4135/damaged-tz.diff
@@ -0,0 +1,5 @@
+diff -urN -X /usr/people/jes/exclude-linux linux-2.6.12-rc2-mm3-vanilla/post image.txt linux-2.6.12-rc2-mm3/post image.txt
+--- linux-2.6.12-rc2-mm3-vanilla/post image.txt 1969-12-31 16:00:00 -08:00
++++ linux-2.6.12-rc2-mm3/post image.txt 2005-04-12 02:14:06 -07:00
+@@ -0,0 +1 @@
++postimage
diff --git a/t/t4135/funny-tz.diff b/t/t4135/funny-tz.diff
new file mode 100644
index 0000000..998e3a8
--- /dev/null
+++ b/t/t4135/funny-tz.diff
@@ -0,0 +1,5 @@
+diff -urN -X /usr/people/jes/exclude-linux linux-2.6.12-rc2-mm3-vanilla/post image.txt linux-2.6.12-rc2-mm3/post image.txt
+--- linux-2.6.12-rc2-mm3-vanilla/post image.txt 1969-12-31 16:00:00 -08:00
++++ linux-2.6.12-rc2-mm3/post image.txt 2005-04-12 02:14:06 -07:00
+@@ -0,0 +1 @@
++postimage
--
1.6.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-09-29 21:41 ` Jonathan Nieder
@ 2010-09-29 21:49 ` Jonathan Nieder
2010-10-13 18:59 ` Junio C Hamano
1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Nieder @ 2010-09-29 21:49 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: Junio C Hamano, git
Jonathan Nieder wrote:
> Some patches have a timezone formatted like '-08:00' instead of
> '-0800' in their ---/+++ lines (e.g. http://lwn.net/Articles/131729/).
> Take this into account when searching for the start of the timezone
s/timezone/timestamp/
> (which is the end of the filename).
Sorry for the noise.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-09-29 21:41 ` Jonathan Nieder
2010-09-29 21:49 ` Jonathan Nieder
@ 2010-10-13 18:59 ` Junio C Hamano
2010-10-13 19:31 ` Junio C Hamano
1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-10-13 18:59 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Anders Kaseorg, git
Jonathan Nieder <jrnieder@gmail.com> writes:
> Hi Anders,
>
> Anders Kaseorg wrote:
>> Some patches have a timezone formatted like '-08:00' instead of
>> '-0800' (e.g. http://lwn.net/Articles/131729/)
>
> Odd. Any idea what tool generates these patches?
>
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
> [...]
>> @@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)
>> }
>>
>> zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
>> - zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
>> + if (m[4].rm_so == m[3].rm_so + 3)
>> + zoneoffset /= 100;
>> + zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
>
> Might be clearer to write
>
> if (timestamp[m[3].rm_so + 3] != ':')
Neither the patch nor your suggestion makes much sense to me. With the
patch, the regexp is now
^(1969-12-31|1970-01-01) <time>(\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])
so $3 is always 3 letters long (i.e. hour with sign), no? IOW, zoneoffset
is never divided by 100 by the original patch.
What am I missing?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-10-13 18:59 ` Junio C Hamano
@ 2010-10-13 19:31 ` Junio C Hamano
2010-10-13 22:50 ` Jonathan Nieder
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-10-13 19:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Anders Kaseorg, git
Junio C Hamano <gitster@pobox.com> writes:
> Neither the patch nor your suggestion makes much sense to me. With the
> patch, the regexp is now
>
> ^(1969-12-31|1970-01-01) <time>(\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])
>
> so $3 is always 3 letters long (i.e. hour with sign), no? IOW, zoneoffset
> is never divided by 100 by the original patch.
>
> What am I missing?
Well, I was missing that without ':' strtol() goes through to parse $3$4
as a single integer, and the division was done to discard the minute part
and parse it again.
Calling strtol() twice only because some unusual input may have ':' there
feels ugly, but now I understand why the patch is written that way, at
least.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-10-13 19:31 ` Junio C Hamano
@ 2010-10-13 22:50 ` Jonathan Nieder
2010-10-13 23:37 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Nieder @ 2010-10-13 22:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Anders Kaseorg, git
Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>> Neither the patch nor your suggestion makes much sense to me. With the
>> patch, the regexp is now
>>
>> ^(1969-12-31|1970-01-01) <time>(\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])
[...]
> Well, I was missing that without ':' strtol() goes through to parse $3$4
> as a single integer
So maybe something like the following would make this easier to follow.
diff --git a/builtin/apply.c b/builtin/apply.c
index 0fa9a8d..000d3e5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -733,8 +733,8 @@ static int has_epoch_timestamp(const char *nameline)
" "
"[0-2][0-9]:[0-5][0-9]:00(\\.0+)?"
" "
- "([-+][0-2][0-9]):?([0-5][0-9])\n";
+ "([-+][0-2][0-9]:?[0-5][0-9])\n";
- const char *timestamp = NULL, *cp;
+ const char *timestamp = NULL, *cp, *colon;
static regex_t *stamp;
regmatch_t m[10];
int zoneoffset;
@@ -764,10 +764,11 @@ static int has_epoch_timestamp(const char *nameline)
return 0;
}
- zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
+ zoneoffset = strtol(timestamp + m[3].rm_so + 1, (char **) &colon, 10);
- if (m[4].rm_so == m[3].rm_so + 3)
- zoneoffset /= 100;
- zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
+ if (*colon == ':')
+ zoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);
+ else
+ zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
if (timestamp[m[3].rm_so] == '-')
zoneoffset = -zoneoffset;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
2010-10-13 22:50 ` Jonathan Nieder
@ 2010-10-13 23:37 ` Junio C Hamano
0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2010-10-13 23:37 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Anders Kaseorg, git
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Neither the patch nor your suggestion makes much sense to me. With the
>>> patch, the regexp is now
>>>
>>> ^(1969-12-31|1970-01-01) <time>(\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])
> [...]
>> Well, I was missing that without ':' strtol() goes through to parse $3$4
>> as a single integer
>
> So maybe something like the following would make this easier to follow.
Yeah, this feels much more natural.
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 0fa9a8d..000d3e5 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -733,8 +733,8 @@ static int has_epoch_timestamp(const char *nameline)
> " "
> "[0-2][0-9]:[0-5][0-9]:00(\\.0+)?"
> " "
> - "([-+][0-2][0-9]):?([0-5][0-9])\n";
> + "([-+][0-2][0-9]:?[0-5][0-9])\n";
> - const char *timestamp = NULL, *cp;
> + const char *timestamp = NULL, *cp, *colon;
> static regex_t *stamp;
> regmatch_t m[10];
> int zoneoffset;
> @@ -764,10 +764,11 @@ static int has_epoch_timestamp(const char *nameline)
> return 0;
> }
>
> - zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
> + zoneoffset = strtol(timestamp + m[3].rm_so + 1, (char **) &colon, 10);
> - if (m[4].rm_so == m[3].rm_so + 3)
> - zoneoffset /= 100;
> - zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
> + if (*colon == ':')
> + zoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);
> + else
> + zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
> if (timestamp[m[3].rm_so] == '-')
> zoneoffset = -zoneoffset;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-10-13 23:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-29 20:49 [PATCH] apply: Recognize epoch timestamps with : in the timezone Anders Kaseorg
2010-09-29 21:41 ` Jonathan Nieder
2010-09-29 21:49 ` Jonathan Nieder
2010-10-13 18:59 ` Junio C Hamano
2010-10-13 19:31 ` Junio C Hamano
2010-10-13 22:50 ` Jonathan Nieder
2010-10-13 23:37 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.