commit-graph: fix buggy --expire-time option

This is embarassing. I should have noticed this when writing it the first time, or when integrating the feature into Scalar and VFS for Git. Sorry!

Update in V2:

* Updated the helptext. Thanks, Taylor!
* Added a patch for the "test-tool chmtime" in t5319
* Used an absolute time with "test-tool chmtime" in t5324 instead of "touch -m"
* Added a file that should be kept on the other side of the expire time to protect against off-by-one errors and future date errors.

Thanks, -Stolee

Derrick Stolee (2):
t5319: replace 'touch -m' with 'test-tool chmtime'
commit-graph: fix buggy --expire-time option

 builtin/commit-graph.c        | 2 +-
 commit-graph.c                | 2 +-
 t/t5319-multi-pack-index.sh   | 8 ++++----
 t/t5324-split-commit-graph.sh | 8 +++++++-
 4 files changed, 13 insertions(+), 7 deletions(-)

base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-596%2Fderrickstolee%2Fcommit-graph-expire-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-596/derrickstolee/commit-graph-expire-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/596

Range-diff vs v1:

 -:  ----------- > 1:  24e26ecda63 t5319: replace 'touch -m' with 'test-tool chmtime'
 1:  32c55c0fc21 ! 2:  56a312695fe commit-graph: fix buggy --expire-time option

@@ Commit message
Update t5323-split-commit-graph.sh to demonstrate the correct value of the --expire-time option by actually creating a crud .graph file with mtime earlier than the expire time. Instead of using a super-
- early time (1980) we need to use a recent time or else the old
- logic actually passes by accident. This test will start passing
- again on the old logic in 40 years or so.
+ early time (1980) we use an explicit, and recent, time. Using
+ test-tool chmtime to create two files on either end of an exact
+ second, we create a test that catches this failure no matter the
+ current time. Using a fixed date is more portable than trying to
+ format a relative date string into the --expiry-date input.

I noticed this when inspecting some Scalar repos that had an excess number of commit-graph files. In Scalar, we were using this second
@@ builtin/commit-graph.c: static int graph_write(int argc, const char **argv)
N_("maximum ratio between two levels of a split commit-graph")),
OPT_EXPIRY_DATE(0, "expire-time", &split_opts.expire_time,
- N_("maximum number of commits in a non-base split commit-graph")),
-+ N_("do not expire files newer than a number of seconds before now")),
++ N_("only expire files older than a given date-time")),
OPT_END(),
};

@@ t/t5324-split-commit-graph.sh: test_expect_success 'test merge stragety constant
test_line_count = 2 $graphdir/commit-graph-chain && test_commit 15 &&
- git commit-graph write --reachable --split --size-multiple=10 --expire-time=1980-01-01 &&
-+ touch -m -t 201801010000.00 $graphdir/extra.graph &&
-+ git commit-graph write --reachable --split --size-multiple=10 --expire-time=2019-01-01 &&
++ touch $graphdir/to-delete.graph $graphdir/to-keep.graph &&
++ test-tool chmtime =1546362000 $graphdir/to-delete.graph &&
++ test-tool chmtime =1546362001 $graphdir/to-keep.graph &&
++ git commit-graph write --reachable --split --size-multiple=10 \
++ --expire-time="2019-01-01 12:00 -05:00" &&
test_line_count = 1 $graphdir/commit-graph-chain &&
-+ test_path_is_missing $graphdir/extra.graph &&
++ test_path_is_missing $graphdir/to-delete.graph &&
++ test_path_is_file $graphdir/to-keep.graph &&
ls $graphdir/graph-*.graph >graph-files &&
test_line_count = 3 graph-files
) &&

gitgitgadget