{"thread":{"id":"53826","subject":"[PATCH 0/4] deterministic commit timestamps in tests","startedAt":"2020-07-09T20:33:40Z","lastAt":"2020-07-16T10:43:03Z","messageCount":13,"participants":["Jeff King","Junio C Hamano","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"401261","messageId":"20200709203336.GA2748777@coredump.intra.peff.net","threadId":"53826","inReplyTo":null,"subject":"[PATCH 0/4] deterministic commit timestamps in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-09T20:33:36Z","receivedAt":"2020-07-09T20:33:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most tests use test_tick or test_commit to get deterministic timestamps\nin commits. Some don't because they don't care about the timestamps. But\nthey can still be annoying to debug, since the output changes from run\nto run.\n\nThis series sets a default timestamp for those scripts, in a way that\ndoesn't interfere with anybody using test_tick. The change is in the\nfinal commit. The others are preparatory to deal with fallout in the\ntest suite. Normally fallout would give me pause about whether this is a\ngood idea, but a) there's not much of it and b) I think it helped shake\nout questionable assumptions in those tests.\n\nThis a replacement for the patches being discussed in:\n\n  https://lore.kernel.org/git/pull.816.git.git.1594149804313.gitgitgadget@gmail.com/\n\nIt looks like Junio already picked up one of the fixes as\njk/t6000-timestamp-fix. The first patch here is identical (so we can\neither drop that branch, or drop the first patch from this series and\napply on top).\n\n  [1/4]: t6000: use test_tick consistently\n  [2/4]: t9700: loosen ident timezone regex\n  [3/4]: t5539: make timestamp requirements more explicit\n  [4/4]: test-lib: set deterministic default author/committer date\n\n t/t5539-fetch-http-shallow.sh | 4 +++-\n t/t6000-rev-list-misc.sh      | 7 +++++--\n t/t9700/test.pl               | 6 +++---\n t/test-lib.sh                 | 3 +++\n 4 files changed, 14 insertions(+), 6 deletions(-)\n\n-Peff\n"},{"id":"401262","messageId":"20200709203402.GA661064@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200709203336.GA2748777@coredump.intra.peff.net","subject":"[PATCH 1/4] t6000: use test_tick consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-09T20:34:02Z","receivedAt":"2020-07-09T20:34:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The first two commits created in t6000 are done without test_tick,\nmeaning they use the current system clock. After that, we create one\nwith test_tick, which means it uses a deterministic time in the past.\n\nThe result of the \"symleft flag bit is propagated down from tag\" test\nrelies on the output order of commits from git-log, which in turn\ndepends on these timestamps. So this test is technically dependent on\nthe system clock time, though in practice it would only matter if your\nsystem clock was set before test_tick's default time (which is in 2005).\n\nHowever, let's use test_tick consistently for those early commits (and\nupdate the expected output to match). This makes the test deterministic,\nwhich is in turn easier to reason about and debug.\n\nNote that there's also a fourth commit here, and it does not use\ntest_tick. It does have a deterministic timestamp because of the prior\nuse of test_tick in the script, but it will always be the same time as\nthe third commit. Let's use test_tick here, too, for consistency.  The\nmatching timestamps between the third and fourth commit are not an\nimportant part of the test.\n\nWe could also use test_commit in all of these cases, as it runs\ntest_tick under the hood. But it would be awkward to do so, as these\ntests diverge from the usual test_commit patterns (e.g., by creating\nmultiple files in a single commit).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t6000-rev-list-misc.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 3dc1ad8f71..3bb0e4ff8f 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -8,6 +8,7 @@ test_expect_success setup '\n \techo content1 >wanted_file &&\n \techo content2 >unwanted_file &&\n \tgit add wanted_file unwanted_file &&\n+\ttest_tick &&\n \tgit commit -m one\n '\n \n@@ -21,6 +22,7 @@ test_expect_success 'rev-list --objects with pathspecs and deeper paths' '\n \tmkdir foo &&\n \t>foo/file &&\n \tgit add foo/file &&\n+\ttest_tick &&\n \tgit commit -m two &&\n \n \tgit rev-list --objects HEAD -- foo >output &&\n@@ -69,6 +71,7 @@ test_expect_success '--no-object-names and --object-names are last-one-wins' '\n '\n \n test_expect_success 'rev-list A..B and rev-list ^A B are the same' '\n+\ttest_tick &&\n \tgit commit --allow-empty -m another &&\n \tgit tag -a -m \"annotated\" v1.0 &&\n \tgit rev-list --objects ^v1.0^ v1.0 >expect &&\n@@ -84,10 +87,10 @@ test_expect_success 'propagate uninteresting flag down correctly' '\n test_expect_success 'symleft flag bit is propagated down from tag' '\n \tgit log --format=\"%m %s\" --left-right v1.0...master >actual &&\n \tcat >expect <<-\\EOF &&\n-\t> two\n-\t> one\n \t< another\n \t< that\n+\t> two\n+\t> one\n \tEOF\n \ttest_cmp expect actual\n '\n-- \n2.27.0.730.gcc195a083d\n\n"},{"id":"401263","messageId":"20200709203550.GB661064@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200709203336.GA2748777@coredump.intra.peff.net","subject":"[PATCH 2/4] t9700: loosen ident timezone regex","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-09T20:35:50Z","receivedAt":"2020-07-09T20:35:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"A few of the perl tests in t9700 ask for the author and committer ident,\nand then make sure we get something sensible. For the timestamp portion,\nwe just match [0-9]+, because the actual value will depend on when the\ntest is run. However, we do require that the timezone be \"+0000\". This\nworks reliably because we set $TZ in test-lib.sh. But in preparation for\nchanging the default timezone, let's be a bit more flexible. We don't\nactually care about the exact value here, just that we were able to get\na sensible output from the perl module's access methods.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nAlternatively, we could use test_tick here and then make sure we got the\nexpected timestamp. Or revisit it after patch 4 and use the default date\nfrom there. But I think the intent of the tests is fulfilled by\nremaining flexible.\n\n t/t9700/test.pl | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9700/test.pl b/t/t9700/test.pl\nindex 34cd01366f..c59a015f89 100755\n--- a/t/t9700/test.pl\n+++ b/t/t9700/test.pl\n@@ -59,15 +59,15 @@ sub adjust_dirsep {\n open STDERR, \">&\", $tmpstderr or die \"cannot restore STDERR\";\n \n # ident\n-like($r->ident(\"aUthor\"), qr/^A U Thor <author\\@example.com> [0-9]+ \\+0000$/,\n+like($r->ident(\"aUthor\"), qr/^A U Thor <author\\@example.com> [0-9]+ [+-]\\d{4}$/,\n      \"ident scalar: author (type)\");\n-like($r->ident(\"cOmmitter\"), qr/^C O Mitter <committer\\@example.com> [0-9]+ \\+0000$/,\n+like($r->ident(\"cOmmitter\"), qr/^C O Mitter <committer\\@example.com> [0-9]+ [+-]\\d{4}$/,\n      \"ident scalar: committer (type)\");\n is($r->ident(\"invalid\"), \"invalid\", \"ident scalar: invalid ident string (no parsing)\");\n my ($name, $email, $time_tz) = $r->ident('author');\n is_deeply([$name, $email], [\"A U Thor\", \"author\\@example.com\"],\n \t \"ident array: author\");\n-like($time_tz, qr/[0-9]+ \\+0000/, \"ident array: author\");\n+like($time_tz, qr/[0-9]+ [+-]\\d{4}/, \"ident array: author\");\n is_deeply([$r->ident(\"Name <email> 123 +0000\")], [\"Name\", \"email\", \"123 +0000\"],\n \t  \"ident array: ident string\");\n is_deeply([$r->ident(\"invalid\")], [], \"ident array: invalid ident string\");\n-- \n2.27.0.730.gcc195a083d\n\n"},{"id":"401264","messageId":"20200709203909.GC661064@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200709203336.GA2748777@coredump.intra.peff.net","subject":"[PATCH 3/4] t5539: make timestamp requirements more explicit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-09T20:39:09Z","receivedAt":"2020-07-09T20:39:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The test for \"no shallow lines after receiving ACK ready\" is very\nsensitive to the timestamps of the commits we create. It's looking for\nthe fetch negotiation to send a \"ready\", which in turn depends on the\norder in which we traverse commits during the negotiation.\n\nIt works reliably now because the base commit \"7\" is created without\ntest_commit, and thus gets a commit time matching the current system\nclock. Whereas the new commits created in this test do use test_commit,\nand get the usual test_tick time from 2005. So the fetch into the\n\"clone\" repository results in a commit graph like this (I omitted some\nof the \"unrelated\" commits for clarity; they're all just a sequence of\ntest_ticks):\n\n  $ git log --graph --format='%ct %s %d'\n  * 1112912953 new  (origin/master, origin/HEAD)\n  * 1594322236 7  (grafted, master)\n  * 1112912893 unrelated15  (origin/unrelated15, unrelated15)\n  [...]\n  * 1112912053 unrelated1  (origin/unrelated1, unrelated1)\n  * 1112911993 new-too  (HEAD -> newnew, tag: new-too)\n\nThe important things to see are:\n\n  - \"7\" is way in the future compared to the other commits\n\n  - \"new-too\" in the fetching repo is older than \"new\" (and its\n    \"unrelated\" ancestors) in the shallow repo\n\nIf we change our \"setup shallow clone\" step to use test_tick, too (and\nget rid of the dependency on the system clock), then the test will fail.\nThe resulting graph looks like this:\n\n  $ git log --graph --format='%ct %s %d'\n  * 1112913373 new  (origin/master, origin/HEAD)\n  * 1112912353 7  (grafted, master)\n  * 1112913313 unrelated15  (origin/unrelated15, unrelated15)\n  [...]\n  * 1112912473 unrelated1  (origin/unrelated1, unrelated1)\n  * 1112912413 new-too  (HEAD -> newnew, tag: new-too)\n\nOur \"new-too\" is still older than \"new\" and \"unrelated\", but now \"7\" is\nolder than all of them (because it advanced test_tick, which the other\ntests built on top of). In the original, we advertised \"7\" as the first\n\"have\" before anything else, but now \"new-too\" is more recent. You'd see\nthe same thing in the unlikely event that the system clock was set\nbefore our test_tick default in 2005.\n\nLet's make the timing requirements more explicit. The important thing is\nthat the client advertise all of its shared commits first, before\npresenting its unique \"new-too\" commit. We can do that and get rid of\nthe system clock dependency at the same time by creating all of the\nshared commits around time X (using test_tick), and then creating\n\"new-too\" with some time long before X. The resulting graph looks like\nthis:\n\n  $ git log --graph --format='%ct %s %d'\n  * 1500001380 new  (origin/master, origin/HEAD)\n  * 1500000420 7  (grafted, master)\n  * 1500001320 unrelated15  (origin/unrelated15, unrelated15)\n  [...]\n  * 1500000480 unrelated1  (origin/unrelated1, unrelated1)\n  * 1400000060 new-too  (HEAD -> newnew, tag: new-too)\n\nThat also lets us get rid of the hacky test_tick added by f0e802ca20\n(t5539: update a flaky test, 2014-07-14). That was clearly dancing\naround the same problem, but only addressed the relationship between\ncommits created in the two subshells (which did use test_tick, but\noverlapped because increments of test_tick in subshells are lost). Now\nthat we're using consistent and well-placed times for both lines of\nhistory, we don't have to care about a one-tick difference between the\ntwo sides.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nCuriously, the test still passes if \"7\" has the same timestamp as\n\"new-too\"! That's why I didn't notice the failure in the original\nthread, where I looked at setting the deterministic default time to the\ndefault test_tick time. I didn't dig in, but I believe it only worked\nbecause we currently use insertion order to break timestamp ties. And\nsince \"master\" sorts before \"newnew\", we'd queue \"7\" before \"new-too\".\n\n t/t5539-fetch-http-shallow.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh\nindex c0d02dee89..82aa99ae87 100755\n--- a/t/t5539-fetch-http-shallow.sh\n+++ b/t/t5539-fetch-http-shallow.sh\n@@ -9,10 +9,12 @@ start_httpd\n commit() {\n \techo \"$1\" >tracked &&\n \tgit add tracked &&\n+\ttest_tick &&\n \tgit commit -m \"$1\"\n }\n \n test_expect_success 'setup shallow clone' '\n+\ttest_tick=1500000000 &&\n \tcommit 1 &&\n \tcommit 2 &&\n \tcommit 3 &&\n@@ -48,7 +50,6 @@ EOF\n test_expect_success 'no shallow lines after receiving ACK ready' '\n \t(\n \t\tcd shallow &&\n-\t\ttest_tick &&\n \t\tfor i in $(test_seq 15)\n \t\tdo\n \t\t\tgit checkout --orphan unrelated$i &&\n@@ -66,6 +67,7 @@ test_expect_success 'no shallow lines after receiving ACK ready' '\n \t(\n \t\tcd clone &&\n \t\tgit checkout --orphan newnew &&\n+\t\ttest_tick=1400000000 &&\n \t\ttest_commit new-too &&\n \t\t# NEEDSWORK: If the overspecification of the expected result is reduced, we\n \t\t# might be able to run this test in all protocol versions.\n-- \n2.27.0.730.gcc195a083d\n\n"},{"id":"401265","messageId":"20200709204204.GD661064@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200709203336.GA2748777@coredump.intra.peff.net","subject":"[PATCH 4/4] test-lib: set deterministic default author/committer date","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-09T20:42:04Z","receivedAt":"2020-07-09T20:42:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We always set the name and email for committer and author idents to make\nthe test suite more deterministic, but not timestamps. Many scripts use\ntest_tick to get consistent and sensibly incrementing timestamps as they\ncreate commits. But other scripts don't particularly care about the\ntimestamp, and are happy to use whatever the current system time is.\n\nThis non-determinism can be annoying:\n\n  - when debugging a test, comparing results between two runs can be\n    difficult, because the commit ids change\n\n  - this can sometimes cause tests to be racy. E.g., traversal order\n    depends on timestamp order. Even in a well-ordered set of commands,\n    because our timestamp granularity is one second, two commits might\n    sometimes have the same timestamp and sometimes differ.\n\nLet's set a default timestamp for all scripts to use. Any that use\ntest_tick already will be unaffected (because their first test_tick call\nwill overwrite our default), but it will make things a bit more\ndeterministic for those that don't.\n\nWe should be able to choose any time we want here. I picked this one\nbecause:\n\n  - it differs from the initial test_tick default, which may make it\n    easier to distinguish when debugging tests. I picked \"April 1st\n    13:14:15\" in the hope that it might stand out.\n\n  - it's slightly before the test_tick default. Some tests create some\n    commits before the first call to test_tick, so using an older\n    timestamps for those makes sense chronologically. Note that this\n    isn't how things currently work (where system times are usually more\n    recent than test_tick), but that also allows us to flush out a few\n    hidden timestamp dependencies (like the one recently fixed in\n    t5539).\n\n  - we could likewise pick any timezone we want. Choosing +0000 would\n    have required fixing up fewer tests, but we're more likely to turn\n    up interesting cases by not matching $TZ exactly. And since\n    test_tick already checks \"-0700\", let's try something in the \"+\"\n    zone range for variety.\n\nIt's possible that the non-deterministic times could help flush out bugs\n(e.g., if something broke when the clock flipped over to 2021, our test\nsuite would let us know). But historically that hasn't been the case;\nall time-dependent outcomes we've seen turned out to be accidentally\nflaky tests (which we fixed by using test_tick). If we do want to cover\nhandling the current time, we should dedicate one script to doing so,\nand have it unset GIT_COMMITTER_DATE explicitly.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/test-lib.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 618a7c8d5b..ba224c86f5 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -441,15 +441,18 @@ TEST_AUTHOR_LOCALNAME=author\n TEST_AUTHOR_DOMAIN=example.com\n GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}\n GIT_AUTHOR_NAME='A U Thor'\n+GIT_AUTHOR_DATE='1112354055 +0200'\n TEST_COMMITTER_LOCALNAME=committer\n TEST_COMMITTER_DOMAIN=example.com\n GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}\n GIT_COMMITTER_NAME='C O Mitter'\n+GIT_COMMITTER_DATE='1112354055 +0200'\n GIT_MERGE_VERBOSITY=5\n GIT_MERGE_AUTOEDIT=no\n export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT\n export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\n export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n+export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n export EDITOR\n \n # Tests using GIT_TRACE typically don't want <timestamp> <file>:<line> output\n-- \n2.27.0.730.gcc195a083d\n"},{"id":"401388","messageId":"xmqqa707rpuu.fsf@gitster.c.googlers.com","threadId":"53826","inReplyTo":"20200709203336.GA2748777@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T22:02:01Z","receivedAt":"2020-07-10T22:02:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Most tests use test_tick or test_commit to get deterministic timestamps\n> in commits. Some don't because they don't care about the timestamps. But\n> they can still be annoying to debug, since the output changes from run\n> to run.\n>\n> This series sets a default timestamp for those scripts, in a way that\n> doesn't interfere with anybody using test_tick. The change is in the\n> final commit. The others are preparatory to deal with fallout in the\n> test suite. Normally fallout would give me pause about whether this is a\n> good idea, but a) there's not much of it and b) I think it helped shake\n> out questionable assumptions in those tests.\n>\n> This a replacement for the patches being discussed in:\n>\n>   https://lore.kernel.org/git/pull.816.git.git.1594149804313.gitgitgadget@gmail.com/\n>\n> It looks like Junio already picked up one of the fixes as\n> jk/t6000-timestamp-fix. The first patch here is identical (so we can\n> either drop that branch, or drop the first patch from this series and\n> apply on top).\n>\n>   [1/4]: t6000: use test_tick consistently\n>   [2/4]: t9700: loosen ident timezone regex\n>   [3/4]: t5539: make timestamp requirements more explicit\n>   [4/4]: test-lib: set deterministic default author/committer date\n>\n>  t/t5539-fetch-http-shallow.sh | 4 +++-\n>  t/t6000-rev-list-misc.sh      | 7 +++++--\n>  t/t9700/test.pl               | 6 +++---\n>  t/test-lib.sh                 | 3 +++\n>  4 files changed, 14 insertions(+), 6 deletions(-)\n>\n> -Peff\n\nI have this queued on top for today's integration run.  The last\nstep is something worth doing in the longer term, but certainly not\nfor the upcoming release ;-).\n\n-- >8 --\nSubject: [PATCH] BANDAID: t9100.17 & t9100.18\n\n---\n t/t9100-git-svn-basic.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 2c309a57d9..33ea813585 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -8,6 +8,8 @@ GIT_SVN_LC_ALL=${LC_ALL:-$LANG}\n \n . ./lib-git-svn.sh\n \n+unset GIT_AUTHOR_DATE GIT_COMMITTER_DATE\n+\n case \"$GIT_SVN_LC_ALL\" in\n *.UTF-8)\n \ttest_set_prereq UTF8\n-- \n2.28.0-rc0\n\n"},{"id":"401545","messageId":"20200714123142.GA2587001@coredump.intra.peff.net","threadId":"53826","inReplyTo":"xmqqa707rpuu.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-14T12:31:42Z","receivedAt":"2020-07-14T12:31:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 10, 2020 at 03:02:01PM -0700, Junio C Hamano wrote:\n\n> I have this queued on top for today's integration run.  The last\n> step is something worth doing in the longer term, but certainly not\n> for the upcoming release ;-).\n> \n> -- >8 --\n> Subject: [PATCH] BANDAID: t9100.17 & t9100.18\n\nOof, thanks for catching. I don't usually have subversion on my system\nat all. I have been trying to pay more attention to our CI, but I have\nanother topic I've been working on that has been causing failures in my\nintegration runs, so I didn't notice this one.\n\nIt took me a while to untangle just what's happening in the test, but I\nthink your \"unset\" workaround is actually what we want. The patch below\ncould go second-to-last in jk/tests-timestamp-fix, before the actual\nswitch to setting GIT_COMMITTER_DATE.\n\n-- >8 --\nSubject: [PATCH] t9100: explicitly unset GIT_COMMITTER_DATE\n\nThe early part of t9100 creates an unusual \"doubled\" history in the\n\"git-svn\" ref. When we get to t9100.17, it looks like this:\n\n  $ git log --oneline --graph git-svn\n  [...]\n  *   efd0303 detect node change from file to directory #2\n  |\\\n  * | 3e727c0 detect node change from file to directory #2\n  |/\n  *   3b00468 try a deep --rmdir with a commit\n  |\\\n  * | b4832d8 try a deep --rmdir with a commit\n  |/\n  * f0d7bd5 import for git svn\n\nEach commit we make with \"git commit\" is paired with one from \"git svn\nset-tree\", with the latter as a merge of the first and its grandparent.\n\nLater, t9100.17 wants to check that \"git svn fetch\" gets the same trees.\nAnd it does, but just one copy of each. So it uses rev-list to get the\ntree of each commit and pipes it to \"uniq\" to drop the duplicates. Our\ninput isn't sorted, but it will find adjacent duplicates. This works\nreliably because the order of commits from rev-list always shows the\nduplicates next to each other. For any one of those merges, we could\nchoose to show its duplicate or the grandparent first. But barring\nclocks running backwards, the duplicate will always have a time equal to\nor greater than the grandparent. Even if equal, we break ties by showing\nthe first-parent first, so the duplicates remain adjacent.\n\nBut this would break if the timestamps stopped moving in chronological\norder. Normally we would rely on test_tick for this, but we have _two_\nsources of time here:\n\n  - \"git commit\" creates one commit based on GIT_COMMITTER_DATE (which\n    respects test_tick)\n\n  - the \"svn set-tree\" one is based on subversion, which does not have\n    an easy way to specify a timestamp\n\nSo using test_tick actually breaks the test, because now the duplicates\nare far in the past, and we'll show the grandparent before the\nduplicate. And likewise, a proposed change to set GIT_COMMITTER_DATE in\nall scripts will break it.\n\nWe _could_ fix this by sorting before removing duplicates, but\npresumably it's a useful part of the test to make sure the trees appear\nin the same order in both spots. Likewise, we could use something like:\n\n  perl -ne 'print unless $seen{$_}++'\n\nto remove duplicates without impacting the order. But that doesn't work\neither, because there are actually multiple (non-duplicate) commits with\nthe same trees (we change a file mode and then change it back). So we'd\nactually have to de-duplicate the combination of subject and tree. Which\nthen further throws off t9100.18, which compares the tree hashes\nexactly; we'd have to strip the result back down.\n\nSince this test _isn't_ buggy, the simplest thing is to just work around\nthe proposed change by documenting our expectation that git-created\ncommits are correctly interleaved using the current time.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9100-git-svn-basic.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 9f2d19ecc4..b80952f0ac 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -8,6 +8,10 @@ GIT_SVN_LC_ALL=${LC_ALL:-$LANG}\n \n . ./lib-git-svn.sh\n \n+# Make sure timestamps of commits created by Git interleave\n+# with those created by \"svn set-tree\".\n+unset GIT_COMMITTER_DATE\n+\n case \"$GIT_SVN_LC_ALL\" in\n *.UTF-8)\n \ttest_set_prereq UTF8\n-- \n2.28.0.rc0.402.g7b15ae678a\n\n"},{"id":"401546","messageId":"20200714123334.GB2587001@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200714123142.GA2587001@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-14T12:33:34Z","receivedAt":"2020-07-14T12:33:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 14, 2020 at 08:31:42AM -0400, Jeff King wrote:\n\n> We _could_ fix this by sorting before removing duplicates, but\n> presumably it's a useful part of the test to make sure the trees appear\n> in the same order in both spots. Likewise, we could use something like:\n> \n>   perl -ne 'print unless $seen{$_}++'\n> \n> to remove duplicates without impacting the order. But that doesn't work\n> either, because there are actually multiple (non-duplicate) commits with\n> the same trees (we change a file mode and then change it back). So we'd\n> actually have to de-duplicate the combination of subject and tree. Which\n> then further throws off t9100.18, which compares the tree hashes\n> exactly; we'd have to strip the result back down.\n\nActually, that last one isn't _too_ bad. It looks something like this:\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex b80952f0ac..4502c5f97d 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -204,8 +204,10 @@ GIT_SVN_ID=alt\n export GIT_SVN_ID\n test_expect_success \"$name\" \\\n     'git svn init \"$svnrepo\" && git svn fetch &&\n-     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n-     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n+     git log --format=\"tree %T %s\" remotes/git-svn |\n+\tperl -lne \"print unless \\$seen{\\$_}++\" |\n+\tcut -d\" \" -f1-2 >a &&\n+     git log --format=\"tree %T\" remotes/alt >b &&\n      test_cmp a b'\n \n name='check imported tree checksums expected tree checksums'\n\nIt does lose a little bit of information, which is that in the original\nwe confirmed that the duplicates were always next to each other. But I'm\nnot sure that's important. We'd get confused if the same subject\nappeared twice, but all of the commits have distinct hard-coded\nsubjects in the earlier tests.\n\n-Peff\n"},{"id":"401568","messageId":"20200714214728.GA21660@dcvr","threadId":"53826","inReplyTo":"20200714123334.GB2587001@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-07-14T21:47:28Z","receivedAt":"2020-07-14T21:47:32Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> On Tue, Jul 14, 2020 at 08:31:42AM -0400, Jeff King wrote:\n> \n> > We _could_ fix this by sorting before removing duplicates, but\n> > presumably it's a useful part of the test to make sure the trees appear\n> > in the same order in both spots. Likewise, we could use something like:\n> > \n> >   perl -ne 'print unless $seen{$_}++'\n> > \n> > to remove duplicates without impacting the order. But that doesn't work\n> > either, because there are actually multiple (non-duplicate) commits with\n> > the same trees (we change a file mode and then change it back). So we'd\n> > actually have to de-duplicate the combination of subject and tree. Which\n> > then further throws off t9100.18, which compares the tree hashes\n> > exactly; we'd have to strip the result back down.\n\nRight, log order matters, so sorting isn't ideal.\n\n> Actually, that last one isn't _too_ bad. It looks something like this:\n> \n> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n> index b80952f0ac..4502c5f97d 100755\n> --- a/t/t9100-git-svn-basic.sh\n> +++ b/t/t9100-git-svn-basic.sh\n> @@ -204,8 +204,10 @@ GIT_SVN_ID=alt\n>  export GIT_SVN_ID\n>  test_expect_success \"$name\" \\\n>      'git svn init \"$svnrepo\" && git svn fetch &&\n> -     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n> -     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n> +     git log --format=\"tree %T %s\" remotes/git-svn |\n> +\tperl -lne \"print unless \\$seen{\\$_}++\" |\n> +\tcut -d\" \" -f1-2 >a &&\n> +     git log --format=\"tree %T\" remotes/alt >b &&\n>       test_cmp a b'\n\nThe future of non-strict one-liners with Perl7 on the horizon\nseems uncertain :<   cut is unnecessary either way, but\nI suggest awk, here:\n\n\tawk \"!seen[\\$0]++ { print \\$1, \\$2 }'\n\n>  name='check imported tree checksums expected tree checksums'\n> \n> It does lose a little bit of information, which is that in the original\n> we confirmed that the duplicates were always next to each other. But I'm\n> not sure that's important. We'd get confused if the same subject\n> appeared twice, but all of the commits have distinct hard-coded\n> subjects in the earlier tests.\n\nYeah, but I think it's fine.  It's been a while since I wrote\nthis\n"},{"id":"401576","messageId":"20200715074250.GB3249056@coredump.intra.peff.net","threadId":"53826","inReplyTo":"20200714214728.GA21660@dcvr","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-15T07:42:50Z","receivedAt":"2020-07-15T07:42:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 14, 2020 at 09:47:28PM +0000, Eric Wong wrote:\n\n> > -     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n> > -     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n> > +     git log --format=\"tree %T %s\" remotes/git-svn |\n> > +\tperl -lne \"print unless \\$seen{\\$_}++\" |\n> > +\tcut -d\" \" -f1-2 >a &&\n> > +     git log --format=\"tree %T\" remotes/alt >b &&\n> >       test_cmp a b'\n> \n> The future of non-strict one-liners with Perl7 on the horizon\n> seems uncertain :<   cut is unnecessary either way, but\n> I suggest awk, here:\n> \n> \tawk \"!seen[\\$0]++ { print \\$1, \\$2 }'\n\nThanks, that is nicer.\n\n> >  name='check imported tree checksums expected tree checksums'\n> > \n> > It does lose a little bit of information, which is that in the original\n> > we confirmed that the duplicates were always next to each other. But I'm\n> > not sure that's important. We'd get confused if the same subject\n> > appeared twice, but all of the commits have distinct hard-coded\n> > subjects in the earlier tests.\n> \n> Yeah, but I think it's fine.  It's been a while since I wrote\n> this\n\nOK. If you're on board, then I think doing it this way is slightly\nnicer, as it's less likely to be confusing or bite somebody in the\nfuture.\n\nHere's a revised patch (I see Junio already picked up the other fix; if\nthat ends up being merged instead, that's not the end of the world).\n\n(compared to the earlier version, you can skip everything in the commit\nmessage before \"One fix would be...\").\n\n-- >8 --\nSubject: [PATCH] t9100: stop depending on commit timestamps\n\nThe early part of t9100 creates an unusual \"doubled\" history in the\n\"git-svn\" ref. When we get to t9100.17, it looks like this:\n\n  $ git log --oneline --graph git-svn\n  [...]\n  *   efd0303 detect node change from file to directory #2\n  |\\\n  * | 3e727c0 detect node change from file to directory #2\n  |/\n  *   3b00468 try a deep --rmdir with a commit\n  |\\\n  * | b4832d8 try a deep --rmdir with a commit\n  |/\n  * f0d7bd5 import for git svn\n\nEach commit we make with \"git commit\" is paired with one from \"git svn\nset-tree\", with the latter as a merge of the first and its grandparent.\n\nLater, t9100.17 wants to check that \"git svn fetch\" gets the same trees.\nAnd it does, but just one copy of each. So it uses rev-list to get the\ntree of each commit and pipes it to \"uniq\" to drop the duplicates. Our\ninput isn't sorted, but it will find adjacent duplicates. This works\nreliably because the order of commits from rev-list always shows the\nduplicates next to each other. For any one of those merges, we could\nchoose to show its duplicate or the grandparent first. But barring\nclocks running backwards, the duplicate will always have a time equal to\nor greater than the grandparent. Even if equal, we break ties by showing\nthe first-parent first, so the duplicates remain adjacent.\n\nBut this would break if the timestamps stopped moving in chronological\norder. Normally we would rely on test_tick for this, but we have _two_\nsources of time here:\n\n  - \"git commit\" creates one commit based on GIT_COMMITTER_DATE (which\n    respects test_tick)\n\n  - the \"svn set-tree\" one is based on subversion, which does not have\n    an easy way to specify a timestamp\n\nSo using test_tick actually breaks the test, because now the duplicates\nare far in the past, and we'll show the grandparent before the\nduplicate. And likewise, a proposed change to set GIT_COMMITTER_DATE in\nall scripts will break it.\n\nOne fix would be to sort the list of trees before removing duplicates,\nbut that loses information:\n\n  - we do care that the fetched history is in the same order\n\n  - there's a tree which appears twice in the history, and we'd want to\n    make sure that it's there both times\n\nSo instead, let's de-duplicate using a hash (preserving the order), and\ndrop only lines with identical trees and subjects (preserving the tree\nwhich appears twice, since it has different subjects each time).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9100-git-svn-basic.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 9f2d19ecc4..3055943a22 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -200,8 +200,9 @@ GIT_SVN_ID=alt\n export GIT_SVN_ID\n test_expect_success \"$name\" \\\n     'git svn init \"$svnrepo\" && git svn fetch &&\n-     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n-     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n+     git log --format=\"tree %T %s\" remotes/git-svn |\n+\tawk \"!seen[\\$0]++ { print \\$1, \\$2 }\" >a &&\n+     git log --format=\"tree %T\" alt >b &&\n      test_cmp a b'\n \n name='check imported tree checksums expected tree checksums'\n-- \n2.28.0.rc0.394.ga62ae196ad\n\n"},{"id":"401587","messageId":"xmqqa700ltn2.fsf@gitster.c.googlers.com","threadId":"53826","inReplyTo":"20200715074250.GB3249056@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-15T14:50:25Z","receivedAt":"2020-07-15T14:50:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Here's a revised patch (I see Junio already picked up the other fix; if\n> that ends up being merged instead, that's not the end of the world).\n>\n> (compared to the earlier version, you can skip everything in the commit\n> message before \"One fix would be...\").\n\nHeh, it already is in 'next' and I do not think it is worth\nrewinding and rebuilding to cause downstream folks an additional\ntrouble.  I however think this updated solution is nicer and do not\nmind building on top i.e. \"while an earlier change did unbreak the\nsvn tests, relying on the current timestamps is not nice and here is\nan update\".\n\nHaving said that, I think it is more urgent to address the \"ouch, we\nmade it clera that repos with extensions.worktreeconfig set without\nmarking them as repoformat v1 are broken and without giving users\nenough hints to recover\" issue discussed elsewhere before -rc1 (and\nfor that reason I do not think I can tag -rc1 today), so I'd\nbackburner it.  This topic won't merge down from 'next' until final\nanyway.\n\nThanks.\n\n\n> -- >8 --\n> Subject: [PATCH] t9100: stop depending on commit timestamps\n>\n> The early part of t9100 creates an unusual \"doubled\" history in the\n> \"git-svn\" ref. When we get to t9100.17, it looks like this:\n>\n>   $ git log --oneline --graph git-svn\n>   [...]\n>   *   efd0303 detect node change from file to directory #2\n>   |\\\n>   * | 3e727c0 detect node change from file to directory #2\n>   |/\n>   *   3b00468 try a deep --rmdir with a commit\n>   |\\\n>   * | b4832d8 try a deep --rmdir with a commit\n>   |/\n>   * f0d7bd5 import for git svn\n>\n> Each commit we make with \"git commit\" is paired with one from \"git svn\n> set-tree\", with the latter as a merge of the first and its grandparent.\n>\n> Later, t9100.17 wants to check that \"git svn fetch\" gets the same trees.\n> And it does, but just one copy of each. So it uses rev-list to get the\n> tree of each commit and pipes it to \"uniq\" to drop the duplicates. Our\n> input isn't sorted, but it will find adjacent duplicates. This works\n> reliably because the order of commits from rev-list always shows the\n> duplicates next to each other. For any one of those merges, we could\n> choose to show its duplicate or the grandparent first. But barring\n> clocks running backwards, the duplicate will always have a time equal to\n> or greater than the grandparent. Even if equal, we break ties by showing\n> the first-parent first, so the duplicates remain adjacent.\n>\n> But this would break if the timestamps stopped moving in chronological\n> order. Normally we would rely on test_tick for this, but we have _two_\n> sources of time here:\n>\n>   - \"git commit\" creates one commit based on GIT_COMMITTER_DATE (which\n>     respects test_tick)\n>\n>   - the \"svn set-tree\" one is based on subversion, which does not have\n>     an easy way to specify a timestamp\n>\n> So using test_tick actually breaks the test, because now the duplicates\n> are far in the past, and we'll show the grandparent before the\n> duplicate. And likewise, a proposed change to set GIT_COMMITTER_DATE in\n> all scripts will break it.\n>\n> One fix would be to sort the list of trees before removing duplicates,\n> but that loses information:\n>\n>   - we do care that the fetched history is in the same order\n>\n>   - there's a tree which appears twice in the history, and we'd want to\n>     make sure that it's there both times\n>\n> So instead, let's de-duplicate using a hash (preserving the order), and\n> drop only lines with identical trees and subjects (preserving the tree\n> which appears twice, since it has different subjects each time).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  t/t9100-git-svn-basic.sh | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n> index 9f2d19ecc4..3055943a22 100755\n> --- a/t/t9100-git-svn-basic.sh\n> +++ b/t/t9100-git-svn-basic.sh\n> @@ -200,8 +200,9 @@ GIT_SVN_ID=alt\n>  export GIT_SVN_ID\n>  test_expect_success \"$name\" \\\n>      'git svn init \"$svnrepo\" && git svn fetch &&\n> -     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n> -     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n> +     git log --format=\"tree %T %s\" remotes/git-svn |\n> +\tawk \"!seen[\\$0]++ { print \\$1, \\$2 }\" >a &&\n> +     git log --format=\"tree %T\" alt >b &&\n>       test_cmp a b'\n>  \n>  name='check imported tree checksums expected tree checksums'\n"},{"id":"401591","messageId":"xmqqwo34keej.fsf@gitster.c.googlers.com","threadId":"53826","inReplyTo":"xmqqa700ltn2.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-15T15:04:52Z","receivedAt":"2020-07-15T15:04:58Z","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> Heh, it already is in 'next' and I do not think it is worth\n> rewinding and rebuilding to cause downstream folks an additional\n> trouble.  I however think this updated solution is nicer and do not\n> mind building on top i.e. \"while an earlier change did unbreak the\n> svn tests, relying on the current timestamps is not nice and here is\n> an update\".\n>\n> Having said that, I think it is more urgent to address the \"ouch, we\n> made it clera that repos with extensions.worktreeconfig set without\n> marking them as repoformat v1 are broken and without giving users\n> enough hints to recover\" issue discussed elsewhere before -rc1 (and\n> for that reason I do not think I can tag -rc1 today), so I'd\n> backburner it.  This topic won't merge down from 'next' until final\n> anyway.\n\nBefore I forget, here is what could be applied on top of the \"fix\nthe starting timestamp for the world\" step as a \"further polishing\non top of the completed series\".\n\n-- >8 --\nFrom: Jeff King <peff@peff.net>\nDate: Wed, 15 Jul 2020 03:42:50 -0400\nSubject: [PATCH] t9100: stop depending on commit timestamps\n\nAn earlier \"fix\" to this script gave up updating it not to rely on\nthe current time because we cannot control what timestamp subversion\ngives its commits.  We however could solve the issue in a different\nway and still use deterministic timestamps on Git commits.\n\nOne fix would be to sort the list of trees before removing duplicates,\nbut that loses information:\n\n  - we do care that the fetched history is in the same order\n\n  - there's a tree which appears twice in the history, and we'd want to\n    make sure that it's there both times\n\nSo instead, let's de-duplicate using a hash (preserving the order), and\ndrop only lines with identical trees and subjects (preserving the tree\nwhich appears twice, since it has different subjects each time).\n\nSigned-off-by: Jeff King <peff@peff.net>\nAcked-by: Eric Wong <e@80x24.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t9100-git-svn-basic.sh | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex a89d338aa7..8dd9645ce5 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -8,10 +8,6 @@ GIT_SVN_LC_ALL=${LC_ALL:-$LANG}\n \n . ./lib-git-svn.sh\n \n-# Make sure timestamps of commits created by Git interleave\n-# with those created by \"svn set-tree\".\n-unset GIT_COMMITTER_DATE\n-\n case \"$GIT_SVN_LC_ALL\" in\n *.UTF-8)\n \ttest_set_prereq UTF8\n@@ -204,8 +200,9 @@ GIT_SVN_ID=alt\n export GIT_SVN_ID\n test_expect_success \"$name\" \\\n     'git svn init \"$svnrepo\" && git svn fetch &&\n-     git rev-list --pretty=raw remotes/git-svn | grep ^tree | uniq > a &&\n-     git rev-list --pretty=raw remotes/alt | grep ^tree | uniq > b &&\n+     git log --format=\"tree %T %s\" remotes/git-svn |\n+\tawk \"!seen[\\$0]++ { print \\$1, \\$2 }\" >a &&\n+     git log --format=\"tree %T\" alt >b &&\n      test_cmp a b'\n \n name='check imported tree checksums expected tree checksums'\n-- \n2.28.0-rc0\n\n\n"},{"id":"401621","messageId":"20200716104300.GB376357@coredump.intra.peff.net","threadId":"53826","inReplyTo":"xmqqwo34keej.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] deterministic commit timestamps in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-07-16T10:43:00Z","receivedAt":"2020-07-16T10:43:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 15, 2020 at 08:04:52AM -0700, Junio C Hamano wrote:\n\n> Before I forget, here is what could be applied on top of the \"fix\n> the starting timestamp for the world\" step as a \"further polishing\n> on top of the completed series\".\n> \n> -- >8 --\n> From: Jeff King <peff@peff.net>\n> Date: Wed, 15 Jul 2020 03:42:50 -0400\n> Subject: [PATCH] t9100: stop depending on commit timestamps\n\nThanks, this looks good and sets up to merge it post-v2.28.\n\n-Peff\n"}]}