{"thread":{"id":"55134","subject":"[PATCH] t1500: ensure current --since= behavior remains","startedAt":"2021-02-10T21:56:26Z","lastAt":"2021-02-11T17:09:35Z","messageCount":3,"participants":["Eric Wong","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"416667","messageId":"20210210215543.18960-1-e@80x24.org","threadId":"55134","inReplyTo":null,"subject":"[PATCH] t1500: ensure current --since= behavior remains","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2021-02-10T21:55:43Z","receivedAt":"2021-02-10T21:56:26Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"This behavior of git-rev-parse is observed since git 1.8.3.1\nat least(*), and likely earlier versions.\n\nAt least one git-reliant project in-the-wild relies on this\ncurrent behavior of git-rev-parse being able to handle multiple\n--since= arguments without squeezing identical results together.\nSo add a test to prevent the potential for regression in\ndownstream projects.\n\n(*) 1.8.3.1 the version packaged for CentOS 7.x\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n Said git-reliant project does use allow /optional/ use of libgit2\n via Perl Inline::C; but libgit2's git__date_parse() isn't part of\n the public API; and it's unlikely anybody using CentOS 7.x would\n run the latest libgit2 even if it were added today.\n\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex abdda360d5..deae916707 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -243,4 +243,19 @@ test_expect_success 'showing the superproject correctly' '\n \ttest_cmp expect out\n '\n \n+# at least one external project depends on this behavior:\n+test_expect_success 'rev-parse --since= unsqueezed ordering' '\n+\tx1=--since=1970-01-01T00:00:01Z &&\n+\tx2=--since=1970-01-01T00:00:02Z &&\n+\tx3=--since=1970-01-01T00:00:03Z &&\n+\tgit rev-parse $x1 $x1 $x3 $x2 >actual &&\n+\tcat >expect <<-EOF &&\n+\t--max-age=1\n+\t--max-age=1\n+\t--max-age=3\n+\t--max-age=2\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n"},{"id":"416742","messageId":"YCUSaXCg8Abg+vGs@coredump.intra.peff.net","threadId":"55134","inReplyTo":"20210210215543.18960-1-e@80x24.org","subject":"Re: [PATCH] t1500: ensure current --since= behavior remains","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-11T11:18:01Z","receivedAt":"2021-02-11T11:22:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2021 at 09:55:43PM +0000, Eric Wong wrote:\n\n> This behavior of git-rev-parse is observed since git 1.8.3.1\n> at least(*), and likely earlier versions.\n> \n> At least one git-reliant project in-the-wild relies on this\n> current behavior of git-rev-parse being able to handle multiple\n> --since= arguments without squeezing identical results together.\n> So add a test to prevent the potential for regression in\n> downstream projects.\n\nI had to read this a few times to understand what \"this behavior\" meant.\nIt is just: when given multiple --since options, output a --max-age for\neach of them, even though internally, Git's revision traversal will only\nuse one (in the usual last-one-wins fashion).\n\nI'm not sure if I was just being dense, or if this could be spelled out\nmore clearly. :)\n\nOut of curiosity, why does the other project want that? From your\nmention of libgit2's git__date_parse(), I assume it's something that\nwants to parse approxidates into timestamps in a script. Maybe we ought\nto provide a more direct and robust way of doing that. We have a similar\nneed in t0006, but we use a test-helper program for it.\n\n(I have no problem in the meantime with this patch, though; any new\nmethod for accomplishing this would want to give other projects time to\nadapt to its use).\n\n-Peff\n"},{"id":"416748","messageId":"20210211170625.GA8280@dcvr","threadId":"55134","inReplyTo":"YCUSaXCg8Abg+vGs@coredump.intra.peff.net","subject":"Re: [PATCH] t1500: ensure current --since= behavior remains","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2021-02-11T17:06:25Z","receivedAt":"2021-02-11T17:09:35Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> On Wed, Feb 10, 2021 at 09:55:43PM +0000, Eric Wong wrote:\n> \n> > This behavior of git-rev-parse is observed since git 1.8.3.1\n> > at least(*), and likely earlier versions.\n> > \n> > At least one git-reliant project in-the-wild relies on this\n> > current behavior of git-rev-parse being able to handle multiple\n> > --since= arguments without squeezing identical results together.\n> > So add a test to prevent the potential for regression in\n> > downstream projects.\n> \n> I had to read this a few times to understand what \"this behavior\" meant.\n> It is just: when given multiple --since options, output a --max-age for\n> each of them, even though internally, Git's revision traversal will only\n> use one (in the usual last-one-wins fashion).\n> \n> I'm not sure if I was just being dense, or if this could be spelled out\n> more clearly. :)\n\n*shrug* :>  My brain struggles with coherent thought so I'm\nsurprised anybody is able to understand me at all :x\n\n> Out of curiosity, why does the other project want that? From your\n> mention of libgit2's git__date_parse(), I assume it's something that\n> wants to parse approxidates into timestamps in a script. Maybe we ought\n> to provide a more direct and robust way of doing that. We have a similar\n> need in t0006, but we use a test-helper program for it.\n\nIt takes about 5ms for my system to run git-rev-parse once.  I\nmay be getting multiple approxidates at once, 1-2 in a typical\ninput (start..end); but a strange or malicious input could have\nhundreds/thousands of approxidates to parse.\n\nThus, I'm batching up all the approxidates into one rev-parse\ninvocation (up to system argv limits right now).  With the\noutput lines split into an array, walking the output/input\narrays in parallel will match them up.  Hypothetically, if\nrev-parse were to get clever and deduplicate or reject identical\ninputs; then the parallel walk would be broken.\n\n> (I have no problem in the meantime with this patch, though; any new\n> method for accomplishing this would want to give other projects time to\n> adapt to its use).\n\nYes.  I think I've mentioned some years/decade ago having\ngeneral functionality along the lines of \"git cat-file --batch\"\nor fast-import would be nice (even for some existing scripts and\ntests shipped with git).  (v)fork+execve is painful even on\nLinux.\n"}]}