{"thread":{"id":"49360","subject":"[PATCH] t5551: compare sorted cookies files","startedAt":"2018-09-17T19:18:14Z","lastAt":"2018-09-17T21:46:56Z","messageCount":7,"participants":["Thomas Gummerer","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"358320","messageId":"20180917191806.19958-1-t.gummerer@gmail.com","threadId":"49360","inReplyTo":null,"subject":"[PATCH] t5551: compare sorted cookies files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-09-17T19:18:06Z","receivedAt":"2018-09-17T19:18:14Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"In t5551 we check that we save cookies correctly to a file when\nhttp.cookiefile and http.savecookies are set.  To do so we create an\nexpect file that expects the cookies in a certain order.\n\nHowever after e2ef8d6fa (\"cookies: support creation-time attribute for\ncookies\", 2018-08-28) in curl.git (released in curl 7.61.1) that order\nchanged.\n\nWe document the file format as \"Netscape/Mozilla cookie file\nformat (see curl(1))\", so any format produced by libcurl should be\nfine here.  Sort the files, to be agnostic to the order of the\ncookies, and make the test pass with both curl versions > 7.61.1 and\nearlier curl versions.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n t/t5551-http-fetch-smart.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 771f36f9ff..d13b993201 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -206,7 +206,7 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n cat >cookies.txt <<EOF\n 127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n EOF\n-cat >expect_cookies.txt <<EOF\n+cat <<EOF | sort >expect_cookies.txt\n \n 127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n 127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n@@ -215,7 +215,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n \tgit config http.cookiefile cookies.txt &&\n \tgit config http.savecookies true &&\n \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n-\ttail -3 cookies.txt >cookies_tail.txt &&\n+\ttail -3 cookies.txt | sort >cookies_tail.txt &&\n \ttest_cmp expect_cookies.txt cookies_tail.txt\n '\n \n-- \n2.19.0.444.g18242da7ef\n\n"},{"id":"358327","messageId":"xmqqtvmn6gra.fsf@gitster-ct.c.googlers.com","threadId":"49360","inReplyTo":"20180917191806.19958-1-t.gummerer@gmail.com","subject":"Re: [PATCH] t5551: compare sorted cookies files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-17T19:55:21Z","receivedAt":"2018-09-17T19:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> In t5551 we check that we save cookies correctly to a file when\n> http.cookiefile and http.savecookies are set.  To do so we create an\n> expect file that expects the cookies in a certain order.\n>\n> However after e2ef8d6fa (\"cookies: support creation-time attribute for\n> cookies\", 2018-08-28) in curl.git (released in curl 7.61.1) that order\n> changed.\n>\n> We document the file format as \"Netscape/Mozilla cookie file\n> format (see curl(1))\", so any format produced by libcurl should be\n> fine here.  Sort the files, to be agnostic to the order of the\n> cookies, and make the test pass with both curl versions > 7.61.1 and\n> earlier curl versions.\n>\n> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n> ---\n\nThanks.  f5b2c9c9 (\"t5551-http-fetch-smart.sh: sort cookies before\ncomparing\", 2018-09-07) that came from\n\n    https://public-inbox.org/git/20180907232205.31328-1-tmz@pobox.com\n\nhas almost the identical patch text, and this (presumably an\nindependent effort) confirms that the patch is needed.  The other\neffort implicitly depends on the expected output is kept sorted, but\nthis one is more explicit---I tend to prefer this approach as tools\nand automation is easier to maintain than having to remember that\nthe source must be sorted.\n\nThanks.\n\n>  t/t5551-http-fetch-smart.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index 771f36f9ff..d13b993201 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -206,7 +206,7 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n>  cat >cookies.txt <<EOF\n>  127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n>  EOF\n> -cat >expect_cookies.txt <<EOF\n> +cat <<EOF | sort >expect_cookies.txt\n>  \n>  127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n>  127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n> @@ -215,7 +215,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n>  \tgit config http.cookiefile cookies.txt &&\n>  \tgit config http.savecookies true &&\n>  \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n> -\ttail -3 cookies.txt >cookies_tail.txt &&\n> +\ttail -3 cookies.txt | sort >cookies_tail.txt &&\n>  \ttest_cmp expect_cookies.txt cookies_tail.txt\n>  '\n"},{"id":"358335","messageId":"20180917211026.GA4095@hank.intra.tgummerer.com","threadId":"49360","inReplyTo":"xmqqtvmn6gra.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] t5551: compare sorted cookies files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-09-17T21:10:26Z","receivedAt":"2018-09-17T21:10:31Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 09/17, Junio C Hamano wrote:\n> Thomas Gummerer <t.gummerer@gmail.com> writes:\n> \n> > In t5551 we check that we save cookies correctly to a file when\n> > http.cookiefile and http.savecookies are set.  To do so we create an\n> > expect file that expects the cookies in a certain order.\n> >\n> > However after e2ef8d6fa (\"cookies: support creation-time attribute for\n> > cookies\", 2018-08-28) in curl.git (released in curl 7.61.1) that order\n> > changed.\n> >\n> > We document the file format as \"Netscape/Mozilla cookie file\n> > format (see curl(1))\", so any format produced by libcurl should be\n> > fine here.  Sort the files, to be agnostic to the order of the\n> > cookies, and make the test pass with both curl versions > 7.61.1 and\n> > earlier curl versions.\n> >\n> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n> > ---\n> \n> Thanks.  f5b2c9c9 (\"t5551-http-fetch-smart.sh: sort cookies before\n> comparing\", 2018-09-07) that came from\n> \n>     https://public-inbox.org/git/20180907232205.31328-1-tmz@pobox.com\n> \n> has almost the identical patch text, and this (presumably an\n> independent effort) confirms that the patch is needed.\n\nWhoops awkward, I should have checked 'pu' before starting to work on\nthis.  This was an independent effort, but I really should\nhave checked 'pu' before starting on this.\n\n>                                                         The other\n> effort implicitly depends on the expected output is kept sorted, but\n> this one is more explicit---I tend to prefer this approach as tools\n> and automation is easier to maintain than having to remember that\n> the source must be sorted.\n\nI'm happy going with either patch, but if we want to go with mine, I'd\nlike to make sure Todd is credited appropriately, as he sent a very\nsimilar patch first.  Not sure what the appropriate way here is\nthough?\n\n> Thanks.\n> \n> >  t/t5551-http-fetch-smart.sh | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > index 771f36f9ff..d13b993201 100755\n> > --- a/t/t5551-http-fetch-smart.sh\n> > +++ b/t/t5551-http-fetch-smart.sh\n> > @@ -206,7 +206,7 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n> >  cat >cookies.txt <<EOF\n> >  127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n> >  EOF\n> > -cat >expect_cookies.txt <<EOF\n> > +cat <<EOF | sort >expect_cookies.txt\n> >  \n> >  127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n> >  127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n> > @@ -215,7 +215,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n> >  \tgit config http.cookiefile cookies.txt &&\n> >  \tgit config http.savecookies true &&\n> >  \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n> > -\ttail -3 cookies.txt >cookies_tail.txt &&\n> > +\ttail -3 cookies.txt | sort >cookies_tail.txt &&\n> >  \ttest_cmp expect_cookies.txt cookies_tail.txt\n> >  '\n"},{"id":"358336","messageId":"20180917211926.GI140909@aiede.svl.corp.google.com","threadId":"49360","inReplyTo":"20180917211026.GA4095@hank.intra.tgummerer.com","subject":"Re: [PATCH] t5551: compare sorted cookies files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-09-17T21:19:26Z","receivedAt":"2018-09-17T21:19:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thomas Gummerer wrote:\n> On 09/17, Junio C Hamano wrote:\n\n>>                                                         The other\n>> effort implicitly depends on the expected output is kept sorted, but\n>> this one is more explicit---I tend to prefer this approach as tools\n>> and automation is easier to maintain than having to remember that\n>> the source must be sorted.\n>\n> I'm happy going with either patch, but if we want to go with mine, I'd\n> like to make sure Todd is credited appropriately, as he sent a very\n> similar patch first.  Not sure what the appropriate way here is\n> though?\n\nThanks for asking.  Credit is a subject that is dear to my heart.\n\nYou can for example use\nReported-by: Todd Zullinger <tmz@pobox.com>\n\nto credit him for the patch and analysis that appears to have helped\nwith reviews (and to signal that this fixes the bug he reported).\n\n[...]\n>>> --- a/t/t5551-http-fetch-smart.sh\n>>> +++ b/t/t5551-http-fetch-smart.sh\n>>> @@ -206,7 +206,7 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n>>>  cat >cookies.txt <<EOF\n>>>  127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n>>>  EOF\n>>> -cat >expect_cookies.txt <<EOF\n>>> +cat <<EOF | sort >expect_cookies.txt\n\nShould this be\n\n\tsort >expect_cookies.txt <<\\EOF\n\n?  That is simpler since it avoids a pipe and means the reader doesn't\nhave to look out for shell metacharacters like $ inside the text.\n\nBonus points if this kind of setup moves to inside the test (using\n<<-\\EOF), which can make the test script easier to read.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"358350","messageId":"20180917214628.28325-1-t.gummerer@gmail.com","threadId":"49360","inReplyTo":"20180917191806.19958-1-t.gummerer@gmail.com","subject":"[PATCH v2 0/2] t5551: compare sorted cookies files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-09-17T21:46:26Z","receivedAt":"2018-09-17T21:46:52Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks Jonathan and Junio for the comments on the first round.\n\nChanges since the first round:\n- add a preparatory patch to modernize the test script\n- add Reported-by to credit Todd\n- just use 'sort' instead of 'cat | sort'\n\nThomas Gummerer (2):\n  t5551: move setup code inside test_expect blocks\n  t5551: compare sorted cookies files\n\n t/t5551-http-fetch-smart.sh | 68 ++++++++++++++++++-------------------\n 1 file changed, 34 insertions(+), 34 deletions(-)\n\n-- \n2.19.0.444.g18242da7ef\n\n"},{"id":"358351","messageId":"20180917214628.28325-2-t.gummerer@gmail.com","threadId":"49360","inReplyTo":"20180917214628.28325-1-t.gummerer@gmail.com","subject":"[PATCH v2 1/2] t5551: move setup code inside test_expect blocks","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-09-17T21:46:27Z","receivedAt":"2018-09-17T21:46:54Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Move setup code inside test_expect blocks, to catch unexpected\nfailures in the setup steps, and bring the test scripts in line with\nour modern test style.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n t/t5551-http-fetch-smart.sh | 66 ++++++++++++++++++-------------------\n 1 file changed, 33 insertions(+), 33 deletions(-)\n\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 771f36f9ff..71535631d3 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -23,26 +23,26 @@ test_expect_success 'create http-accessible bare repository' '\n \n setup_askpass_helper\n \n-cat >exp <<EOF\n-> GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1\n-> Accept: */*\n-> Accept-Encoding: ENCODINGS\n-> Pragma: no-cache\n-< HTTP/1.1 200 OK\n-< Pragma: no-cache\n-< Cache-Control: no-cache, max-age=0, must-revalidate\n-< Content-Type: application/x-git-upload-pack-advertisement\n-> POST /smart/repo.git/git-upload-pack HTTP/1.1\n-> Accept-Encoding: ENCODINGS\n-> Content-Type: application/x-git-upload-pack-request\n-> Accept: application/x-git-upload-pack-result\n-> Content-Length: xxx\n-< HTTP/1.1 200 OK\n-< Pragma: no-cache\n-< Cache-Control: no-cache, max-age=0, must-revalidate\n-< Content-Type: application/x-git-upload-pack-result\n-EOF\n test_expect_success 'clone http repository' '\n+\tcat >exp <<-\\EOF &&\n+\t> GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1\n+\t> Accept: */*\n+\t> Accept-Encoding: ENCODINGS\n+\t> Pragma: no-cache\n+\t< HTTP/1.1 200 OK\n+\t< Pragma: no-cache\n+\t< Cache-Control: no-cache, max-age=0, must-revalidate\n+\t< Content-Type: application/x-git-upload-pack-advertisement\n+\t> POST /smart/repo.git/git-upload-pack HTTP/1.1\n+\t> Accept-Encoding: ENCODINGS\n+\t> Content-Type: application/x-git-upload-pack-request\n+\t> Accept: application/x-git-upload-pack-result\n+\t> Content-Length: xxx\n+\t< HTTP/1.1 200 OK\n+\t< Pragma: no-cache\n+\t< Cache-Control: no-cache, max-age=0, must-revalidate\n+\t< Content-Type: application/x-git-upload-pack-result\n+\tEOF\n \tGIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&\n \ttest_cmp file clone/file &&\n \ttr '\\''\\015'\\'' Q <err |\n@@ -96,13 +96,13 @@ test_expect_success 'fetch changes via http' '\n \ttest_cmp file clone/file\n '\n \n-cat >exp <<EOF\n-GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200\n-POST /smart/repo.git/git-upload-pack HTTP/1.1 200\n-GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200\n-POST /smart/repo.git/git-upload-pack HTTP/1.1 200\n-EOF\n test_expect_success 'used upload-pack service' '\n+\tcat >exp <<-\\EOF &&\n+\tGET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200\n+\tPOST /smart/repo.git/git-upload-pack HTTP/1.1 200\n+\tGET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200\n+\tPOST /smart/repo.git/git-upload-pack HTTP/1.1 200\n+\tEOF\n \tcheck_access_log exp\n '\n \n@@ -203,15 +203,15 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n \ttest_cmp expect actual\n '\n \n-cat >cookies.txt <<EOF\n-127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n-EOF\n-cat >expect_cookies.txt <<EOF\n-\n-127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n-127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n-EOF\n test_expect_success 'cookies stored in http.cookiefile when http.savecookies set' '\n+\tcat >cookies.txt <<-\\EOF &&\n+\t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n+\tEOF\n+\tcat >expect_cookies.txt <<-\\EOF &&\n+\n+\t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n+\t127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n+\tEOF\n \tgit config http.cookiefile cookies.txt &&\n \tgit config http.savecookies true &&\n \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n-- \n2.19.0.444.g18242da7ef\n\n"},{"id":"358352","messageId":"20180917214628.28325-3-t.gummerer@gmail.com","threadId":"49360","inReplyTo":"20180917214628.28325-1-t.gummerer@gmail.com","subject":"[PATCH v2 2/2] t5551: compare sorted cookies files","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-09-17T21:46:28Z","receivedAt":"2018-09-17T21:46:56Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"In t5551 we check that we save cookies correctly to a file when\nhttp.cookiefile and http.savecookies are set.  To do so we create an\nexpect file that expects the cookies in a certain order.\n\nHowever after e2ef8d6fa (\"cookies: support creation-time attribute for\ncookies\", 2018-08-28) in curl.git (released in curl 7.61.1) that order\nchanged.\n\nWe document the file format as \"Netscape/Mozilla cookie file\nformat (see curl(1))\", so any format produced by libcurl should be\nfine here.  Sort the files, to be agnostic to the order of the\ncookies, and make the test pass with both curl versions > 7.61.1 and\nearlier curl versions.\n\nReported-by: Todd Zullinger <tmz@pobox.com>\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n t/t5551-http-fetch-smart.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 71535631d3..3dc8f8ecec 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -207,7 +207,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n \tcat >cookies.txt <<-\\EOF &&\n \t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n \tEOF\n-\tcat >expect_cookies.txt <<-\\EOF &&\n+\tsort >expect_cookies.txt <<-\\EOF &&\n \n \t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n \t127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n@@ -215,7 +215,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n \tgit config http.cookiefile cookies.txt &&\n \tgit config http.savecookies true &&\n \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n-\ttail -3 cookies.txt >cookies_tail.txt &&\n+\ttail -3 cookies.txt | sort >cookies_tail.txt &&\n \ttest_cmp expect_cookies.txt cookies_tail.txt\n '\n \n-- \n2.19.0.444.g18242da7ef\n\n"}]}