On 3/7/2022 5:34 AM, Patrick Steinhardt wrote: > On Fri, Mar 04, 2022 at 09:03:15AM -0500, Derrick Stolee wrote: >> On 3/3/2022 11:00 AM, Derrick Stolee wrote: ... >>> I will continue investigating and try to reproduce with this >>> additional constraint of working across an alternate. >> >> My attempts to reproduce this across an alternate have failed. I >> tried running the following test against Git without these patches, >> then verify with the newer version of Git. (I also have generated >> a few new layers on top with these patches, and they correctly drop >> the GDA2 and GDO2 chunks when the lower layers "don't have gen v2".) >> >> >> test_description='commit-graph with offsets across alternates' >> . ./test-lib.sh >> >> if ! test_have_prereq TIME_IS_64BIT || ! test_have_prereq TIME_T_IS_64BIT >> then >> skip_all='skipping 64-bit timestamp tests' >> test_done >> fi >> >> >> UNIX_EPOCH_ZERO="@0 +0000" >> FUTURE_DATE="@4147483646 +0000" >> >> GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0 >> >> test_expect_success 'generate alternate split commit-graph' ' >> git init alternate && >> ( >> cd alternate && >> test_commit --date "$UNIX_EPOCH_ZERO" 1 && >> test_commit --date "$FUTURE_DATE" 2 && >> git commit-graph write --reachable && >> test_commit --date "$UNIX_EPOCH_ZERO" 3 && >> test_commit --date "$FUTURE_DATE" 4 && >> git commit-graph write --reachable --split=no-merge >> ) && >> git clone --shared alternate fork && >> ( >> cd fork && >> test_commit --date "$UNIX_EPOCH_ZERO" 5 && >> test_commit --date "$FUTURE_DATE" 6 && >> git commit-graph write --reachable --split=no-merge && >> test_commit --date "$UNIX_EPOCH_ZERO" 7 && >> test_commit --date "$FUTURE_DATE" 8 && >> git commit-graph write --reachable --split=no-merge >> ) >> ' >> >> test_done >> >> >> My testing after running this with -d allows me to reliably see these >> layers being created with GDAT and GDOV chunks. Running the 'git >> commit-graph verify' command with the new code does not show those >> errors, even after adding commits and another layer to the split >> commit-graph. >> >> I look forward to any additional insights you might have here. > > I don't really know why, but now I've become unable to reproduce it > again. I think we should just go with your patch 5/4 on top -- it does > fix the most important issue, which is the `die()` I saw on almost all > commands. The second part about the warnings I'm just not sure about, > but I don't think it should stop this patch series given my own > uncertainty. Thanks for following up. I agree that with 5/4 we should be safe. I'll remain available to quickly respond if anything else surprising comes up in this area. Thanks! -Stolee
