{"thread":{"id":"25290","subject":"[PATCH] apply: Recognize epoch timestamps with : in the timezone","startedAt":"2010-09-29T20:49:49Z","lastAt":"2010-10-13T23:37:15Z","messageCount":7,"participants":["Anders Kaseorg","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"152091","messageId":"alpine.DEB.2.00.1009291644440.15192@dr-wily.mit.edu","threadId":"25290","inReplyTo":null,"subject":"[PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Anders Kaseorg","fromEmail":"andersk@ksplice.com","sentAt":"2010-09-29T20:49:49Z","receivedAt":"2010-09-29T20:49:49Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Some patches have a timezone formatted like ‘-08:00’ instead of\n‘-0800’ (e.g. http://lwn.net/Articles/131729/), so git apply would\nfail to recognize the epoch timestamp of deleted files and would\ncreate empty files instead.  Teach it to support both formats, and add\na test case.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/apply.c          |    6 ++++--\n t/t4132-apply-removal.sh |    2 ++\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..0fa9a8d 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -733,7 +733,7 @@ static int has_epoch_timestamp(const char *nameline)\n \t\t\" \"\n \t\t\"[0-2][0-9]:[0-5][0-9]:00(\\\\.0+)?\"\n \t\t\" \"\n-\t\t\"([-+][0-2][0-9][0-5][0-9])\\n\";\n+\t\t\"([-+][0-2][0-9]):?([0-5][0-9])\\n\";\n \tconst char *timestamp = NULL, *cp;\n \tstatic regex_t *stamp;\n \tregmatch_t m[10];\n@@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)\n \t}\n \n \tzoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);\n-\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n+\tif (m[4].rm_so == m[3].rm_so + 3)\n+\t\tzoneoffset /= 100;\n+\tzoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);\n \tif (timestamp[m[3].rm_so] == '-')\n \t\tzoneoffset = -zoneoffset;\n \ndiff --git a/t/t4132-apply-removal.sh b/t/t4132-apply-removal.sh\nindex bb1ffe3..a2bc1cd 100755\n--- a/t/t4132-apply-removal.sh\n+++ b/t/t4132-apply-removal.sh\n@@ -30,6 +30,7 @@ test_expect_success setup '\n \tepocWest=\"1969-12-31 16:00:00.000000000 -0800\" &&\n \t epocGMT=\"1970-01-01 00:00:00.000000000 +0000\" &&\n \tepocEast=\"1970-01-01 09:00:00.000000000 +0900\" &&\n+\tepocWest2=\"1969-12-31 16:00:00 -08:00\" &&\n \n \tsed -e \"s/TS0/$epocWest/\" -e \"s/TS1/$timeWest/\" <c >createWest.patch &&\n \tsed -e \"s/TS0/$epocEast/\" -e \"s/TS1/$timeEast/\" <c >createEast.patch &&\n@@ -46,6 +47,7 @@ test_expect_success setup '\n \tsed -e \"s/TS0/$timeWest/\" -e \"s/TS1/$epocWest/\" <d >removeWest.patch &&\n \tsed -e \"s/TS0/$timeEast/\" -e \"s/TS1/$epocEast/\" <d >removeEast.patch &&\n \tsed -e \"s/TS0/$timeGMT/\" -e \"s/TS1/$epocGMT/\" <d >removeGMT.patch &&\n+\tsed -e \"s/TS0/$timeWest/\" -e \"s/TS1/$epocWest2/\" <d >removeWest2.patch &&\n \n \techo something >something &&\n \t>empty\n-- \n1.7.3\n"},{"id":"152093","messageId":"20100929214107.GA4485@capella.cs.uchicago.edu","threadId":"25290","inReplyTo":"alpine.DEB.2.00.1009291644440.15192@dr-wily.mit.edu","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-29T21:41:08Z","receivedAt":"2010-09-29T21:41:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Anders,\n\nAnders Kaseorg wrote:\n> Some patches have a timezone formatted like '-08:00' instead of\n> '-0800' (e.g. http://lwn.net/Articles/131729/)\n\nOdd.  Any idea what tool generates these patches?\n\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n[...]\n> @@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)\n>  \t}\n>  \n>  \tzoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);\n> -\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n> +\tif (m[4].rm_so == m[3].rm_so + 3)\n> +\t\tzoneoffset /= 100;\n> +\tzoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);\n\nMight be clearer to write\n\n\tif (timestamp[m[3].rm_so + 3] != ':')\n\nWith or without that change,\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nMaybe something like this on top?\n\n-- 8< --\nSubject: apply: handle patches with funny filename and colon in timezone\n\nSome patches have a timezone formatted like '-08:00' instead of\n'-0800' in their ---/+++ lines (e.g. http://lwn.net/Articles/131729/).\nTake this into account when searching for the start of the timezone\n(which is the end of the filename).\n\nThis does not actually affect the outcome of patching unless (1) a\nfile being patched has a non-' ' whitespace character (e.g., tab) in\nits filename, or (2) the patch is whitespace-damaged, so the tab\nbetween filename and timestamp has been replaced with spaces.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/apply.c                  |   24 ++++++++++++++++++++++--\n t/t4135-apply-weird-filenames.sh |   16 ++++++++++++++++\n t/t4135/damaged-tz.diff          |    5 +++++\n t/t4135/funny-tz.diff            |    5 +++++\n 4 files changed, 48 insertions(+), 2 deletions(-)\n create mode 100644 t/t4135/damaged-tz.diff\n create mode 100644 t/t4135/funny-tz.diff\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 0fa9a8d..7d91d8f 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -449,7 +449,7 @@ static char *find_name_gnu(const char *line, char *def, int p_value)\n \treturn squash_slash(strbuf_detach(&name, NULL));\n }\n \n-static size_t tz_len(const char *line, size_t len)\n+static size_t sane_tz_len(const char *line, size_t len)\n {\n \tconst char *tz, *p;\n \n@@ -467,6 +467,24 @@ static size_t tz_len(const char *line, size_t len)\n \treturn line + len - tz;\n }\n \n+static size_t tz_with_colon_len(const char *line, size_t len)\n+{\n+\tconst char *tz, *p;\n+\n+\tif (len < strlen(\" +08:00\") || line[len - strlen(\":00\")] != ':')\n+\t\treturn 0;\n+\ttz = line + len - strlen(\" +08:00\");\n+\n+\tif (tz[0] != ' ' || (tz[1] != '+' && tz[1] != '-'))\n+\t\treturn 0;\n+\tp = tz + 2;\n+\tif (!isdigit(*p++) || !isdigit(*p++) || *p++ != ':' ||\n+\t    !isdigit(*p++) || !isdigit(*p++))\n+\t\treturn 0;\n+\n+\treturn line + len - tz;\n+}\n+\n static size_t date_len(const char *line, size_t len)\n {\n \tconst char *date, *p;\n@@ -561,7 +579,9 @@ static size_t diff_timestamp_len(const char *line, size_t len)\n \tif (!isdigit(end[-1]))\n \t\treturn 0;\n \n-\tn = tz_len(line, end - line);\n+\tn = sane_tz_len(line, end - line);\n+\tif (!n)\n+\t\tn = tz_with_colon_len(line, end - line);\n \tend -= n;\n \n \tn = short_time_len(line, end - line);\ndiff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh\nindex 1e5aad5..bf5dc57 100755\n--- a/t/t4135-apply-weird-filenames.sh\n+++ b/t/t4135-apply-weird-filenames.sh\n@@ -72,4 +72,20 @@ test_expect_success 'whitespace-damaged traditional patch' '\n \ttest_cmp expected postimage.txt\n '\n \n+test_expect_success 'traditional patch with colon in timezone' '\n+\techo postimage >expected &&\n+\treset_preimage &&\n+\trm -f \"post image.txt\" &&\n+\tgit apply \"$vector/funny-tz.diff\" &&\n+\ttest_cmp expected \"post image.txt\"\n+'\n+\n+test_expect_success 'traditional, whitespace-damaged, colon in timezone' '\n+\techo postimage >expected &&\n+\treset_preimage &&\n+\trm -f \"post image.txt\" &&\n+\tgit apply \"$vector/damaged-tz.diff\" &&\n+\ttest_cmp expected \"post image.txt\"\n+'\n+\n test_done\ndiff --git a/t/t4135/damaged-tz.diff b/t/t4135/damaged-tz.diff\nnew file mode 100644\nindex 0000000..07aaf08\n--- /dev/null\n+++ b/t/t4135/damaged-tz.diff\n@@ -0,0 +1,5 @@\n+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\n+--- linux-2.6.12-rc2-mm3-vanilla/post image.txt 1969-12-31 16:00:00 -08:00\n++++ linux-2.6.12-rc2-mm3/post image.txt 2005-04-12 02:14:06 -07:00\n+@@ -0,0 +1 @@\n++postimage\ndiff --git a/t/t4135/funny-tz.diff b/t/t4135/funny-tz.diff\nnew file mode 100644\nindex 0000000..998e3a8\n--- /dev/null\n+++ b/t/t4135/funny-tz.diff\n@@ -0,0 +1,5 @@\n+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\n+--- linux-2.6.12-rc2-mm3-vanilla/post image.txt\t1969-12-31 16:00:00 -08:00\n++++ linux-2.6.12-rc2-mm3/post image.txt\t2005-04-12 02:14:06 -07:00\n+@@ -0,0 +1 @@\n++postimage\n-- \n1.6.5\n"},{"id":"152095","messageId":"AANLkTikUoyiWmSkp5sdi1oA2k2fBweqeUj_oBZBEEP3T@mail.gmail.com","threadId":"25290","inReplyTo":"20100929214107.GA4485@capella.cs.uchicago.edu","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-29T21:49:34Z","receivedAt":"2010-09-29T21:49:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Some patches have a timezone formatted like '-08:00' instead of\n> '-0800' in their ---/+++ lines (e.g. http://lwn.net/Articles/131729/).\n> Take this into account when searching for the start of the timezone\n\ns/timezone/timestamp/\n\n> (which is the end of the filename).\n\nSorry for the noise.\n"},{"id":"153427","messageId":"7v4ocpncli.fsf@alter.siamese.dyndns.org","threadId":"25290","inReplyTo":"20100929214107.GA4485@capella.cs.uchicago.edu","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T18:59:05Z","receivedAt":"2010-10-13T18:59:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hi Anders,\n>\n> Anders Kaseorg wrote:\n>> Some patches have a timezone formatted like '-08:00' instead of\n>> '-0800' (e.g. http://lwn.net/Articles/131729/)\n>\n> Odd.  Any idea what tool generates these patches?\n>\n>> --- a/builtin/apply.c\n>> +++ b/builtin/apply.c\n> [...]\n>> @@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)\n>>  \t}\n>>  \n>>  \tzoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);\n>> -\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n>> +\tif (m[4].rm_so == m[3].rm_so + 3)\n>> +\t\tzoneoffset /= 100;\n>> +\tzoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);\n>\n> Might be clearer to write\n>\n> \tif (timestamp[m[3].rm_so + 3] != ':')\n\nNeither the patch nor your suggestion makes much sense to me.  With the\npatch, the regexp is now\n\n    ^(1969-12-31|1970-01-01) <time>(\\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])\n\nso $3 is always 3 letters long (i.e. hour with sign), no?  IOW, zoneoffset\nis never divided by 100 by the original patch.\n\nWhat am I missing?\n"},{"id":"153431","messageId":"7vvd55lwjl.fsf@alter.siamese.dyndns.org","threadId":"25290","inReplyTo":"7v4ocpncli.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T19:31:10Z","receivedAt":"2010-10-13T19:31:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Neither the patch nor your suggestion makes much sense to me.  With the\n> patch, the regexp is now\n>\n>     ^(1969-12-31|1970-01-01) <time>(\\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])\n>\n> so $3 is always 3 letters long (i.e. hour with sign), no?  IOW, zoneoffset\n> is never divided by 100 by the original patch.\n>\n> What am I missing?\n\nWell, I was missing that without ':' strtol() goes through to parse $3$4\nas a single integer, and the division was done to discard the minute part\nand parse it again.\n\nCalling strtol() twice only because some unusual input may have ':' there\nfeels ugly, but now I understand why the patch is written that way, at\nleast.\n"},{"id":"153467","messageId":"20101013225012.GB9509@burratino","threadId":"25290","inReplyTo":"7vvd55lwjl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-13T22:50:12Z","receivedAt":"2010-10-13T22:50:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n\n>> Neither the patch nor your suggestion makes much sense to me.  With the\n>> patch, the regexp is now\n>>\n>>     ^(1969-12-31|1970-01-01) <time>(\\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])\n[...]\n> Well, I was missing that without ':' strtol() goes through to parse $3$4\n> as a single integer\n\nSo maybe something like the following would make this easier to follow.\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 0fa9a8d..000d3e5 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -733,8 +733,8 @@ static int has_epoch_timestamp(const char *nameline)\n \t\t\" \"\n \t\t\"[0-2][0-9]:[0-5][0-9]:00(\\\\.0+)?\"\n \t\t\" \"\n-\t\t\"([-+][0-2][0-9]):?([0-5][0-9])\\n\";\n+\t\t\"([-+][0-2][0-9]:?[0-5][0-9])\\n\";\n-\tconst char *timestamp = NULL, *cp;\n+\tconst char *timestamp = NULL, *cp, *colon;\n \tstatic regex_t *stamp;\n \tregmatch_t m[10];\n \tint zoneoffset;\n@@ -764,10 +764,11 @@ static int has_epoch_timestamp(const char *nameline)\n \t\treturn 0;\n \t}\n \n-\tzoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);\n+\tzoneoffset = strtol(timestamp + m[3].rm_so + 1, (char **) &colon, 10);\n-\tif (m[4].rm_so == m[3].rm_so + 3)\n-\t\tzoneoffset /= 100;\n-\tzoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);\n+\tif (*colon == ':')\n+\t\tzoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);\n+\telse\n+\t\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n \tif (timestamp[m[3].rm_so] == '-')\n \t\tzoneoffset = -zoneoffset;\n \n"},{"id":"153474","messageId":"7vvd55fyvo.fsf@alter.siamese.dyndns.org","threadId":"25290","inReplyTo":"20101013225012.GB9509@burratino","subject":"Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-13T23:37:15Z","receivedAt":"2010-10-13T23:37:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> Neither the patch nor your suggestion makes much sense to me.  With the\n>>> patch, the regexp is now\n>>>\n>>>     ^(1969-12-31|1970-01-01) <time>(\\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])\n> [...]\n>> Well, I was missing that without ':' strtol() goes through to parse $3$4\n>> as a single integer\n>\n> So maybe something like the following would make this easier to follow.\n\nYeah, this feels much more natural.\n\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index 0fa9a8d..000d3e5 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -733,8 +733,8 @@ static int has_epoch_timestamp(const char *nameline)\n>  \t\t\" \"\n>  \t\t\"[0-2][0-9]:[0-5][0-9]:00(\\\\.0+)?\"\n>  \t\t\" \"\n> -\t\t\"([-+][0-2][0-9]):?([0-5][0-9])\\n\";\n> +\t\t\"([-+][0-2][0-9]:?[0-5][0-9])\\n\";\n> -\tconst char *timestamp = NULL, *cp;\n> +\tconst char *timestamp = NULL, *cp, *colon;\n>  \tstatic regex_t *stamp;\n>  \tregmatch_t m[10];\n>  \tint zoneoffset;\n> @@ -764,10 +764,11 @@ static int has_epoch_timestamp(const char *nameline)\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tzoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);\n> +\tzoneoffset = strtol(timestamp + m[3].rm_so + 1, (char **) &colon, 10);\n> -\tif (m[4].rm_so == m[3].rm_so + 3)\n> -\t\tzoneoffset /= 100;\n> -\tzoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);\n> +\tif (*colon == ':')\n> +\t\tzoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);\n> +\telse\n> +\t\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n>  \tif (timestamp[m[3].rm_so] == '-')\n>  \t\tzoneoffset = -zoneoffset;\n>  \n"}]}