From 780fbeba63e792199a0974826a5ef0267af83c1a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 22 Aug 2017 13:49:02 -0700 Subject: [PATCH 1/6] t4200: give us a clean slate after "rerere gc" tests The "multiple identical conflicts" test counts the number of entries in the rerere database after trying a handful of mergy operations and recording their resolutions, but without initializing the rerere database to a known state, allowing the state left by previous tests to trigger a false failure. Make it robust by cleaning the database before it starts. Signed-off-by: Junio C Hamano --- t/t4200-rerere.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 1a080e7823..8f5f268baf 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -446,6 +446,8 @@ merge_conflict_resolve () { } test_expect_success 'multiple identical conflicts' ' + rm -fr .git/rr-cache && + mkdir .git/rr-cache && git reset --hard && test_seq 1 6 >early && From c277344182b6ff423b8395ea0bf7a75ee0db78e2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 22 Aug 2017 14:14:03 -0700 Subject: [PATCH 2/6] t4200: make "rerere gc" test more robust The test blindly trusted that there may be _some_ entries left in the rerere database, and used them by updating their timestamps to see if the gc threshold variables are honoured correctly. This won't work if there is no entry in the database when the test begins. Instead, clear the rerere database, and populate it with a few known entries (which are bogus, but for the purpose of testing "garbage collection", it does not matter---we want to make sure we collect old cruft, even if the files are corrupt rerere database entries), and use them for the expiry test. Signed-off-by: Junio C Hamano --- t/t4200-rerere.sh | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 8f5f268baf..1e23031cdb 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -420,19 +420,28 @@ count_pre_post () { } test_expect_success 'rerere gc' ' - find .git/rr-cache -type f >original && - xargs test-chmtime -172800 "$rr/preimage" && + >"$rr/postimage" && + + two_days_ago=$((-2*86400)) && + test-chmtime =$two_days_ago "$rr/preimage" && + test-chmtime =$two_days_ago "$rr/postimage" && + + find .git/rr-cache -type f | sort >original && git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc && - find .git/rr-cache -type f >actual && + find .git/rr-cache -type f | sort >actual && test_cmp original actual && git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f >actual && + find .git/rr-cache -type f | sort >actual && test_cmp original actual && git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f >actual && + find .git/rr-cache -type f | sort >actual && >expect && test_cmp expect actual ' From 1ad8b47354979515cfd1a50035208b1185f99f9c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 22 Aug 2017 14:20:24 -0700 Subject: [PATCH 3/6] t4200: gather "rerere gc" together Move the "rerere gc with custom expiry" test up, so that it is close to the existing basic "rerere gc" tests. Signed-off-by: Junio C Hamano --- t/t4200-rerere.sh | 54 +++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 1e23031cdb..b007b67e9a 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -239,6 +239,33 @@ test_expect_success 'old records rest in peace' ' ! test -f $rr2/preimage ' +test_expect_success 'rerere gc with custom expiry' ' + rm -fr .git/rr-cache && + rr=.git/rr-cache/$_z40 && + mkdir -p "$rr" && + >"$rr/preimage" && + >"$rr/postimage" && + + two_days_ago=$((-2*86400)) && + test-chmtime =$two_days_ago "$rr/preimage" && + test-chmtime =$two_days_ago "$rr/postimage" && + + find .git/rr-cache -type f | sort >original && + + git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc && + find .git/rr-cache -type f | sort >actual && + test_cmp original actual && + + git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc && + find .git/rr-cache -type f | sort >actual && + test_cmp original actual && + + git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc && + find .git/rr-cache -type f | sort >actual && + >expect && + test_cmp expect actual +' + test_expect_success 'setup: file2 added differently in two branches' ' git reset --hard && @@ -419,33 +446,6 @@ count_pre_post () { test_line_count = "$2" actual } -test_expect_success 'rerere gc' ' - rm -fr .git/rr-cache && - rr=.git/rr-cache/$_z40 && - mkdir -p "$rr" && - >"$rr/preimage" && - >"$rr/postimage" && - - two_days_ago=$((-2*86400)) && - test-chmtime =$two_days_ago "$rr/preimage" && - test-chmtime =$two_days_ago "$rr/postimage" && - - find .git/rr-cache -type f | sort >original && - - git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc && - find .git/rr-cache -type f | sort >actual && - test_cmp original actual && - - git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f | sort >actual && - test_cmp original actual && - - git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f | sort >actual && - >expect && - test_cmp expect actual -' - merge_conflict_resolve () { git reset --hard && test_must_fail git merge six.1 && From e579aaa64d8a24013de6cdd3fb3028e25d9a0917 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 22 Aug 2017 14:25:09 -0700 Subject: [PATCH 4/6] t4200: parameterize "rerere gc" custom expiry test The test creates a rerere database entry that is two days old, and tries to expire with three different custom expiry configuration (keep ones less than 5 days old, keep ones used less than 5 days ago, and expire everything right now). We'll be introducing a different way to spell the same "5 days" and "right now" parameter in a later step; parameterize the test to make it easier to test the new spelling when it happens. Signed-off-by: Junio C Hamano --- t/t4200-rerere.sh | 50 +++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index b007b67e9a..8d437534f2 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -239,32 +239,40 @@ test_expect_success 'old records rest in peace' ' ! test -f $rr2/preimage ' -test_expect_success 'rerere gc with custom expiry' ' - rm -fr .git/rr-cache && - rr=.git/rr-cache/$_z40 && - mkdir -p "$rr" && - >"$rr/preimage" && - >"$rr/postimage" && +rerere_gc_custom_expiry_test () { + five_days="$1" right_now="$2" + test_expect_success "rerere gc with custom expiry ($five_days, $right_now)" ' + rm -fr .git/rr-cache && + rr=.git/rr-cache/$_z40 && + mkdir -p "$rr" && + >"$rr/preimage" && + >"$rr/postimage" && - two_days_ago=$((-2*86400)) && - test-chmtime =$two_days_ago "$rr/preimage" && - test-chmtime =$two_days_ago "$rr/postimage" && + two_days_ago=$((-2*86400)) && + test-chmtime =$two_days_ago "$rr/preimage" && + test-chmtime =$two_days_ago "$rr/postimage" && - find .git/rr-cache -type f | sort >original && + find .git/rr-cache -type f | sort >original && - git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc && - find .git/rr-cache -type f | sort >actual && - test_cmp original actual && + git -c "gc.rerereresolved=$five_days" \ + -c "gc.rerereunresolved=$five_days" rerere gc && + find .git/rr-cache -type f | sort >actual && + test_cmp original actual && - git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f | sort >actual && - test_cmp original actual && + git -c "gc.rerereresolved=$five_days" \ + -c "gc.rerereunresolved=$right_now" rerere gc && + find .git/rr-cache -type f | sort >actual && + test_cmp original actual && - git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc && - find .git/rr-cache -type f | sort >actual && - >expect && - test_cmp expect actual -' + git -c "gc.rerereresolved=$right_now" \ + -c "gc.rerereunresolved=$right_now" rerere gc && + find .git/rr-cache -type f | sort >actual && + >expect && + test_cmp expect actual + ' +} + +rerere_gc_custom_expiry_test 5 0 test_expect_success 'setup: file2 added differently in two branches' ' git reset --hard && From 5ea82279c066f51d446b344e82492a3554409d7d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 19 Aug 2017 11:16:01 -0700 Subject: [PATCH 5/6] rerere: represent time duration in timestamp_t internally The two configuration variables, gc.rerereResolved and gc.rerereUnresolved, are measured in days and are passed as such into the prune_one() helper function, which worked in time_t to see if an entry in the rerere database is past its expiry. Instead, have the caller turn the number of days into the expiry timestamp. Further, use timestamp_t instead of time_t. This will make it possible to extend the way the configuration variable is spelled by using date.c::parse_expiry_date() that gives the expiry timestamp in timestamp_t. Signed-off-by: Junio C Hamano --- rerere.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/rerere.c b/rerere.c index 70634d456c..f0b4bce881 100644 --- a/rerere.c +++ b/rerere.c @@ -1133,14 +1133,14 @@ int rerere_forget(struct pathspec *pathspec) * Garbage collection support */ -static time_t rerere_created_at(struct rerere_id *id) +static timestamp_t rerere_created_at(struct rerere_id *id) { struct stat st; return stat(rerere_path(id, "preimage"), &st) ? (time_t) 0 : st.st_mtime; } -static time_t rerere_last_used_at(struct rerere_id *id) +static timestamp_t rerere_last_used_at(struct rerere_id *id) { struct stat st; @@ -1157,11 +1157,11 @@ static void unlink_rr_item(struct rerere_id *id) id->collection->status[id->variant] = 0; } -static void prune_one(struct rerere_id *id, time_t now, - int cutoff_resolve, int cutoff_noresolve) +static void prune_one(struct rerere_id *id, + timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve) { - time_t then; - int cutoff; + timestamp_t then; + timestamp_t cutoff; then = rerere_last_used_at(id); if (then) @@ -1172,25 +1172,35 @@ static void prune_one(struct rerere_id *id, time_t now, return; cutoff = cutoff_noresolve; } - if (then < now - cutoff * 86400) + if (then < cutoff) unlink_rr_item(id); } +static void config_get_expiry(const char *key, timestamp_t *cutoff, timestamp_t now) +{ + int days; + + if (!git_config_get_int(key, &days)) { + const int scale = 86400; + *cutoff = now - days * scale; + } +} + void rerere_gc(struct string_list *rr) { struct string_list to_remove = STRING_LIST_INIT_DUP; DIR *dir; struct dirent *e; int i; - time_t now = time(NULL); - int cutoff_noresolve = 15; - int cutoff_resolve = 60; + timestamp_t now = time(NULL); + timestamp_t cutoff_noresolve = now - 15 * 86400; + timestamp_t cutoff_resolve = now - 60 * 86400; if (setup_rerere(rr, 0) < 0) return; - git_config_get_int("gc.rerereresolved", &cutoff_resolve); - git_config_get_int("gc.rerereunresolved", &cutoff_noresolve); + config_get_expiry("gc.rerereresolved", &cutoff_resolve, now); + config_get_expiry("gc.rerereunresolved", &cutoff_noresolve, now); git_config(git_default_config, NULL); dir = opendir(git_path("rr-cache")); if (!dir) @@ -1211,7 +1221,7 @@ void rerere_gc(struct string_list *rr) for (id.variant = 0, id.collection = rr_dir; id.variant < id.collection->status_nr; id.variant++) { - prune_one(&id, now, cutoff_resolve, cutoff_noresolve); + prune_one(&id, cutoff_resolve, cutoff_noresolve); if (id.collection->status[id.variant]) now_empty = 0; } From 6e96cb5286105bbcf19d5c47e45334ef9a75d09d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 19 Aug 2017 11:43:39 -0700 Subject: [PATCH 6/6] rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved These two configuration variables are described in the documentation to take an expiry period expressed in the number of days: gc.rerereResolved:: Records of conflicted merge you resolved earlier are kept for this many days when 'git rerere gc' is run. The default is 60 days. gc.rerereUnresolved:: Records of conflicted merge you have not resolved are kept for this many days when 'git rerere gc' is run. The default is 15 days. There is no strong reason not to allow a more general "approxidate" expiry specification, e.g. "5.days.ago", or "never". Rename the config_get_expiry() helper introduced in the previous step to git_config_get_expiry_in_days() and move it to a more generic place, config.c, and use date.c::parse_expiry_date() to do so. Give it an ability to allow the caller to tell among three cases (i.e. there is no "gc.rerereResolved" config, there is and it is correctly parsed into the *expiry variable, and there was an error in parsing the given value). The current caller can work correctly without using the return value, though. In the future, we may find other variables that only allow an integer that specifies "this many days" or other unit of time, and when it happens we may need to drop "_days" suffix from the name of the function and instead pass the "scale" value as another parameter. But this will do for now. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 2 ++ config.c | 22 ++++++++++++++++++++++ config.h | 3 +++ rerere.c | 14 ++------------ t/t4200-rerere.sh | 2 ++ 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d5c9c4cab6..ac95f5f954 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1553,11 +1553,13 @@ gc..reflogExpireUnreachable:: gc.rerereResolved:: Records of conflicted merge you resolved earlier are kept for this many days when 'git rerere gc' is run. + You can also use more human-readable "1.month.ago", etc. The default is 60 days. See linkgit:git-rerere[1]. gc.rerereUnresolved:: Records of conflicted merge you have not resolved are kept for this many days when 'git rerere gc' is run. + You can also use more human-readable "1.month.ago", etc. The default is 15 days. See linkgit:git-rerere[1]. gitcvs.commitMsgAnnotation:: diff --git a/config.c b/config.c index 231f9a750b..ffca15f594 100644 --- a/config.c +++ b/config.c @@ -2066,6 +2066,28 @@ int git_config_get_expiry(const char *key, const char **output) return ret; } +int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, timestamp_t now) +{ + char *expiry_string; + intmax_t days; + timestamp_t when; + + if (git_config_get_string(key, &expiry_string)) + return 1; /* no such thing */ + + if (git_parse_signed(expiry_string, &days, maximum_signed_value_of_type(int))) { + const int scale = 86400; + *expiry = now - days * scale; + return 0; + } + + if (!parse_expiry_date(expiry_string, &when)) { + *expiry = when; + return 0; + } + return -1; /* thing exists but cannot be parsed */ +} + int git_config_get_untracked_cache(void) { int val = -1; diff --git a/config.h b/config.h index 0352da117b..34ddd3eb8d 100644 --- a/config.h +++ b/config.h @@ -205,6 +205,9 @@ extern int git_config_get_max_percent_split_change(void); /* This dies if the configured or default date is in the future */ extern int git_config_get_expiry(const char *key, const char **output); +/* parse either "this many days" integer, or "5.days.ago" approxidate */ +extern int git_config_get_expiry_in_days(const char *key, timestamp_t *, timestamp_t now); + struct key_value_info { const char *filename; int linenr; diff --git a/rerere.c b/rerere.c index f0b4bce881..d77235645e 100644 --- a/rerere.c +++ b/rerere.c @@ -1176,16 +1176,6 @@ static void prune_one(struct rerere_id *id, unlink_rr_item(id); } -static void config_get_expiry(const char *key, timestamp_t *cutoff, timestamp_t now) -{ - int days; - - if (!git_config_get_int(key, &days)) { - const int scale = 86400; - *cutoff = now - days * scale; - } -} - void rerere_gc(struct string_list *rr) { struct string_list to_remove = STRING_LIST_INIT_DUP; @@ -1199,8 +1189,8 @@ void rerere_gc(struct string_list *rr) if (setup_rerere(rr, 0) < 0) return; - config_get_expiry("gc.rerereresolved", &cutoff_resolve, now); - config_get_expiry("gc.rerereunresolved", &cutoff_noresolve, now); + git_config_get_expiry_in_days("gc.rerereresolved", &cutoff_resolve, now); + git_config_get_expiry_in_days("gc.rerereunresolved", &cutoff_noresolve, now); git_config(git_default_config, NULL); dir = opendir(git_path("rr-cache")); if (!dir) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 8d437534f2..d97d2bebc9 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -274,6 +274,8 @@ rerere_gc_custom_expiry_test () { rerere_gc_custom_expiry_test 5 0 +rerere_gc_custom_expiry_test 5.days.ago now + test_expect_success 'setup: file2 added differently in two branches' ' git reset --hard &&