{"thread":{"id":"65806","subject":"[PATCH] commit-graph: use timestamp_t for max parent generation accumulator","startedAt":"2026-06-14T06:57:54Z","lastAt":"2026-06-19T14:05:43Z","messageCount":4,"participants":["Elijah Newren via GitGitGadget","Patrick Steinhardt","Derrick Stolee","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"545485","messageId":"pull.2148.git.1781420271100.gitgitgadget@gmail.com","threadId":"65806","inReplyTo":null,"subject":"[PATCH] commit-graph: use timestamp_t for max parent generation accumulator","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-06-14T06:57:50Z","receivedAt":"2026-06-14T06:57:54Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\ncompute_reachable_generation_numbers() computes each commit's\ngeneration as\n\n    max(c->date, max(parent.generation)) + 1\n\nby walking its parents and accumulating their generations into a\nlocal\n\n    uint32_t max_gen = 0;\n\nwhile info->get_generation() returns timestamp_t and\ncompute_generation_from_max() already takes its max_gen parameter\nas timestamp_t.  For v1 (topological levels) the narrowing is\nharmless because GENERATION_NUMBER_V1_MAX is less than 2^30, but\nfor v2 (corrected committer dates) it silently truncates any\nparent generation that does not fit in 32 bits, i.e. any parent\nwhose committer timestamp is at or beyond 2106-02-07 UTC\n(>= 2^32).\n\nThe truncated max then causes child commits to end up with a\ncorrected committer date that matches the parent's instead of being\nat least 1 higher.  The bad value gets written into the commit-graph\nand causes problems later, and can be noticed by running `git\ncommit-graph verify`.\n\nWiden the accumulator to timestamp_t.\n\nThis is solely an in-memory arithmetic fix with no on-disk format\nchange: the on-disk format already encodes timestamp_t values and\nexisting readers handle them unchanged.  This merely allows the code to\ncompute the correct value to write to disk.\n\nThe narrowing was introduced in 80c928d947c2 (commit-graph:\nsimplify compute_generation_numbers(), 2023-03-20), which rewired\nv2 to use the shared compute_reachable_generation_numbers()\nhelper; the helper's local accumulator had been declared uint32_t\nin the immediately preceding 368d19b0b7fa (commit-graph: refactor\ncompute_topological_levels(), 2023-03-20) when only v1 was using\nit, where it was harmless.\n\nAdd a new test with a future-dated parent and a present-day child;\nwithout the above fix, `git commit-graph verify` reports the\ndescendant's stored generation as below parent + 1.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n    commit-graph: use timestamp_t for max parent generation accumulator\n    \n    We found a few repositories in the wild with commits whose authors were\n    apparently on a computer in the year 2120 when they recorded their\n    commits. Apparently, in a century from now, some folks are going to have\n    a really weird timezone as well (-13068837), though the timezone doesn't\n    factor into this patch at all.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2148%2Fnewren%2Fcommit-graph-fix-ccd-uint32-truncation-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2148/newren/commit-graph-fix-ccd-uint32-truncation-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2148\n\n commit-graph.c                     | 2 +-\n t/t5328-commit-graph-64bit-time.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 9abe62bd5a..4b7156fd76 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1669,7 +1669,7 @@ static void compute_reachable_generation_numbers(\n \t\t\tstruct commit *current = list->item;\n \t\t\tstruct commit_list *parent;\n \t\t\tint all_parents_computed = 1;\n-\t\t\tuint32_t max_gen = 0;\n+\t\t\ttimestamp_t max_gen = 0;\n \n \t\t\tfor (parent = current->parents; parent; parent = parent->next) {\n \t\t\t\trepo_parse_commit(info->r, parent->item);\ndiff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh\nindex d8891e6a92..bc651b69de 100755\n--- a/t/t5328-commit-graph-64bit-time.sh\n+++ b/t/t5328-commit-graph-64bit-time.sh\n@@ -74,6 +74,15 @@ test_expect_success 'single commit with generation data exceeding UINT32_MAX' '\n \tgit -C repo-uint32-max commit-graph verify\n '\n \n+test_expect_success 'descendant of commit with date exceeding UINT32_MAX' '\n+\tgit init repo-uint32-max-descendant &&\n+\ttest_commit -C repo-uint32-max-descendant \\\n+\t\t--date \"@4294967300 +0000\" future-parent &&\n+\ttest_commit -C repo-uint32-max-descendant present-day-child &&\n+\tgit -C repo-uint32-max-descendant commit-graph write --reachable &&\n+\tgit -C repo-uint32-max-descendant commit-graph verify\n+'\n+\n test_expect_success PERL_TEST_HELPERS 'reader notices out-of-bounds generation overflow' '\n \tgraph=.git/objects/info/commit-graph &&\n \ttest_when_finished \"rm -rf $graph\" &&\n\nbase-commit: 600fe743028cbfb640855f659e9851522214bc0b\n-- \ngitgitgadget\n"},{"id":"545520","messageId":"ai-zzWn9Ls6-j9h8@pks.im","threadId":"65806","inReplyTo":"pull.2148.git.1781420271100.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit-graph: use timestamp_t for max parent generation accumulator","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-06-15T08:11:57Z","receivedAt":"2026-06-15T08:12:07Z","isPatch":true,"body":"On Sun, Jun 14, 2026 at 06:57:50AM +0000, Elijah Newren via GitGitGadget wrote:\n>     commit-graph: use timestamp_t for max parent generation accumulator\n>     \n>     We found a few repositories in the wild with commits whose authors were\n>     apparently on a computer in the year 2120 when they recorded their\n>     commits. Apparently, in a century from now, some folks are going to have\n>     a really weird timezone as well (-13068837), though the timezone doesn't\n>     factor into this patch at all.\n\nI'd really be curious which other parts of Git will start to break once\nwe cross that threshold. Would it make sense if we maybe expanded our\nlinux-TEST-VARS job to create commits with a date beyond UINT32_MAX?\nSomething like the patch at the end of this mail. And yes, many tests\nbreak with the patch applied. From all I've seen though many of those\nfailures are benign, even though I'd bet that there might even be some\n\"proper\" failures in there.\n\nAnyway, this is of course outside the scope of this patch series.\n\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 9abe62bd5a..4b7156fd76 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -1669,7 +1669,7 @@ static void compute_reachable_generation_numbers(\n>  \t\t\tstruct commit *current = list->item;\n>  \t\t\tstruct commit_list *parent;\n>  \t\t\tint all_parents_computed = 1;\n> -\t\t\tuint32_t max_gen = 0;\n> +\t\t\ttimestamp_t max_gen = 0;\n>  \n>  \t\t\tfor (parent = current->parents; parent; parent = parent->next) {\n>  \t\t\t\trepo_parse_commit(info->r, parent->item);\n\nThis looks obviously correct.\n\n> diff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh\n> index d8891e6a92..bc651b69de 100755\n> --- a/t/t5328-commit-graph-64bit-time.sh\n> +++ b/t/t5328-commit-graph-64bit-time.sh\n> @@ -74,6 +74,15 @@ test_expect_success 'single commit with generation data exceeding UINT32_MAX' '\n>  \tgit -C repo-uint32-max commit-graph verify\n>  '\n>  \n> +test_expect_success 'descendant of commit with date exceeding UINT32_MAX' '\n> +\tgit init repo-uint32-max-descendant &&\n> +\ttest_commit -C repo-uint32-max-descendant \\\n> +\t\t--date \"@4294967300 +0000\" future-parent &&\n> +\ttest_commit -C repo-uint32-max-descendant present-day-child &&\n> +\tgit -C repo-uint32-max-descendant commit-graph write --reachable &&\n> +\tgit -C repo-uint32-max-descendant commit-graph verify\n> +'\n\nMakes sense. Thanks!\n\nPatrick\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 809c662124..e78902b671 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -136,12 +136,19 @@ sane_unset () {\n test_tick () {\n \tif test -z \"${test_tick+set}\"\n \tthen\n-\t\ttest_tick=1112911993\n+\t\tif test_bool_env GIT_TEST_FUTURE false\n+\t\tthen\n+\t\t\ttest_tick=4294697600\n+\t\t\ttest_tick_prefix=@\n+\t\telse\n+\t\t\ttest_tick=1112911993\n+\t\t\ttest_tick_prefix=\n+\t\tfi\n \telse\n \t\ttest_tick=$(($test_tick + 60))\n \tfi\n-\tGIT_COMMITTER_DATE=\"$test_tick -0700\"\n-\tGIT_AUTHOR_DATE=\"$test_tick -0700\"\n+\tGIT_COMMITTER_DATE=\"$test_tick_prefix$test_tick -0700\"\n+\tGIT_AUTHOR_DATE=\"$test_tick_prefix$test_tick -0700\"\n \texport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n }\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 4a7357b547..54798fb3f1 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -558,12 +558,26 @@ 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+\n+case \"${GIT_TEST_FUTURE:-false}\" in\n+1|on|true|yes)\n+\tGIT_AUTHOR_DATE=\"${GIT_TEST_DATE:-@4294697300 +0200}\"\n+\tGIT_COMMITTER_DATE=\"${GIT_TEST_DATE:-@4294697300 +0200}\"\n+\t;;\n+0|off|false|no)\n+\tGIT_AUTHOR_DATE=\"${GIT_TEST_DATE:-1112354055 +0200}\"\n+\tGIT_COMMITTER_DATE=\"${GIT_TEST_DATE:-1112354055 +0200}\"\n+\t;;\n+*)\n+\techo \"GIT_TEST_FUTURE requires a boolean\" >&2\n+\texit 1\n+\t;;\n+esac\n+\n GIT_MERGE_VERBOSITY=5\n GIT_MERGE_AUTOEDIT=no\n export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT\n"},{"id":"545540","messageId":"09e50180-e165-48d8-a9d0-485283342f5c@gmail.com","threadId":"65806","inReplyTo":"ai-zzWn9Ls6-j9h8@pks.im","subject":"Re: [PATCH] commit-graph: use timestamp_t for max parent generation accumulator","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-06-15T11:44:19Z","receivedAt":"2026-06-15T11:44:21Z","isPatch":true,"body":"On 6/15/26 4:11 AM, Patrick Steinhardt wrote:\n> On Sun, Jun 14, 2026 at 06:57:50AM +0000, Elijah Newren via GitGitGadget wrote:\n>>      commit-graph: use timestamp_t for max parent generation accumulator\n>>      \n>>      We found a few repositories in the wild with commits whose authors were\n>>      apparently on a computer in the year 2120 when they recorded their\n>>      commits. Apparently, in a century from now, some folks are going to have\n>>      a really weird timezone as well (-13068837), though the timezone doesn't\n>>      factor into this patch at all.\n\n>> @@ -1669,7 +1669,7 @@ static void compute_reachable_generation_numbers(\n>>   \t\t\tstruct commit *current = list->item;\n>>   \t\t\tstruct commit_list *parent;\n>>   \t\t\tint all_parents_computed = 1;\n>> -\t\t\tuint32_t max_gen = 0;\n>> +\t\t\ttimestamp_t max_gen = 0;\n>>   \n>>   \t\t\tfor (parent = current->parents; parent; parent = parent->next) {\n>>   \t\t\t\trepo_parse_commit(info->r, parent->item);\n> \n> This looks obviously correct.\n\nI agree. I was surprised this was the only necessary change, but\nyour message clearly describes how the timing of the patch that\ndelivered this change contributed to the mismatch.\n\nThanks,\n-Stolee\n\n"},{"id":"545954","messageId":"ajVMtJ73EdBI52sz@nand.local","threadId":"65806","inReplyTo":"09e50180-e165-48d8-a9d0-485283342f5c@gmail.com","subject":"Re: [PATCH] commit-graph: use timestamp_t for max parent generation accumulator","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-06-19T14:05:40Z","receivedAt":"2026-06-19T14:05:43Z","isPatch":true,"body":"On Mon, Jun 15, 2026 at 07:44:19AM -0400, Derrick Stolee wrote:\n> On 6/15/26 4:11 AM, Patrick Steinhardt wrote:\n> > On Sun, Jun 14, 2026 at 06:57:50AM +0000, Elijah Newren via GitGitGadget wrote:\n> > >      commit-graph: use timestamp_t for max parent generation accumulator\n> > >      We found a few repositories in the wild with commits whose authors were\n> > >      apparently on a computer in the year 2120 when they recorded their\n> > >      commits. Apparently, in a century from now, some folks are going to have\n> > >      a really weird timezone as well (-13068837), though the timezone doesn't\n> > >      factor into this patch at all.\n>\n> > > @@ -1669,7 +1669,7 @@ static void compute_reachable_generation_numbers(\n> > >   \t\t\tstruct commit *current = list->item;\n> > >   \t\t\tstruct commit_list *parent;\n> > >   \t\t\tint all_parents_computed = 1;\n> > > -\t\t\tuint32_t max_gen = 0;\n> > > +\t\t\ttimestamp_t max_gen = 0;\n> > >   \t\t\tfor (parent = current->parents; parent; parent = parent->next) {\n> > >   \t\t\t\trepo_parse_commit(info->r, parent->item);\n> >\n> > This looks obviously correct.\n>\n> I agree. I was surprised this was the only necessary change, but\n> your message clearly describes how the timing of the patch that\n> delivered this change contributed to the mismatch.\n\nDitto. I reviewed a version of this patch before Elijah sent it to the\nlist, but this LGTM and is\n\n    Acked-by: Taylor Blau <me@ttaylorr.com>\n\nThanks,\nTaylor\n"}]}