config: let feature.experimental imply gc.cruftPacks=true

We are interested in exploring whether gc.cruftPacks=true should become
the default value.

To determine whether it is safe to do so, let's encourage more users to
try it out.

Users who have set feature.experimental=true have already volunteered to
try new and possibly-breaking config changes, so let's try this new
default with that set of users.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt
index cdecd04e5bb7ea05efee6124a669da72ad0c5384..95975e50912c6f423820a5440d6696ac5326dc51 100644 (file)
--- a/Documentation/config/feature.txt
+++ b/Documentation/config/feature.txt
@@ -14,6 +14,9 @@ feature.experimental::
 +
 * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
 skipping more commits at a time, reducing the number of round trips.
++
+* `gc.cruftPacks=true` reduces disk space used by unreachable objects during
+garbage collection, preventing loose object explosions.
 
 feature.manyFiles::
        Enable config options that optimize for repos with many files in the

diff --git a/builtin/gc.c b/builtin/gc.c
index eeff2b760e0cc58978618c791a195df0bca6627b..60a8f94c62f01acf85be11f2326939af479579cf 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -42,7 +42,7 @@ static const char * const builtin_gc_usage[] = {
 
 static int pack_refs = 1;
 static int prune_reflogs = 1;
-static int cruft_packs = 0;
+static int cruft_packs = -1;
 static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
@@ -600,6 +600,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
        if (prune_expire && parse_expiry_date(prune_expire, &dummy))
                die(_("failed to parse prune expiry value %s"), prune_expire);
 
+       prepare_repo_settings(the_repository);
+       if (cruft_packs < 0)
+               cruft_packs = the_repository->settings.gc_cruft_packs;
+
        if (aggressive) {
                strvec_push(&repack, "-f");
                if (aggressive_depth > 0)
@@ -711,7 +715,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
                clean_pack_garbage();
        }
 
-       prepare_repo_settings(the_repository);
        if (the_repository->settings.gc_write_commit_graph == 1)
                write_commit_graph_reachable(the_repository->objects->odb,
                                             !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,

diff --git a/repo-settings.c b/repo-settings.c
index 43bc881bfc90ca1db92cdbf917fc09182a04a798..a36b9411e145b157e8567a6332e088a9a21a2b05 100644 (file)
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -43,6 +43,7 @@ void prepare_repo_settings(struct repository *r)
        /* Defaults modified by feature.* */
        if (experimental) {
                r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
+               r->settings.gc_cruft_packs = 1;
        }
        if (manyfiles) {
                r->settings.index_version = 4;

diff --git a/repository.h b/repository.h
index 797f471cce9a5501ead146933d71f33bd45827e4..f235622a927fc4e471138fc60d682bf972480eb6 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -33,6 +33,7 @@ struct repo_settings {
        int commit_graph_generation_version;
        int commit_graph_read_changed_paths;
        int gc_write_commit_graph;
+       int gc_cruft_packs;
        int fetch_write_commit_graph;
        int command_requires_full_index;
        int sparse_index;

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 928a522194f602819ae12b823fdab7cd0ec25f80..d9acb639512ae6bdf30c8bdd4a0b4d498541dd84 100755 (executable)
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -221,6 +221,11 @@ assert_cruft_packs () {
        done <packs
 }
 
+assert_no_cruft_packs () {
+       find .git/objects/pack -name "*.mtimes" >mtimes &&
+       test_must_be_empty mtimes
+}
+
 test_expect_success 'gc --cruft generates a cruft pack' '
        test_when_finished "rm -fr crufts" &&
        git init crufts &&
@@ -245,6 +250,54 @@ test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
        )
 '
 
+test_expect_success 'feature.experimental=true generates a cruft pack' '
+       git init crufts &&
+       test_when_finished "rm -fr crufts" &&
+       (
+               cd crufts &&
+
+               prepare_cruft_history &&
+               git -c feature.experimental=true gc &&
+               assert_cruft_packs
+       )
+'
+
+test_expect_success 'feature.experimental=false allows explicit cruft packs' '
+       git init crufts &&
+       test_when_finished "rm -fr crufts" &&
+       (
+               cd crufts &&
+
+               prepare_cruft_history &&
+               git -c gc.cruftPacks=true -c feature.experimental=false gc &&
+               assert_cruft_packs
+       )
+'
+
+test_expect_success 'feature.experimental=true can be overridden' '
+       git init crufts &&
+       test_when_finished "rm -fr crufts" &&
+       (
+               cd crufts &&
+
+               prepare_cruft_history &&
+               git -c feature.expiremental=true -c gc.cruftPacks=false gc &&
+               assert_no_cruft_packs
+       )
+'
+
+test_expect_success 'feature.experimental=false avoids cruft packs by default' '
+       git init crufts &&
+       test_when_finished "rm -fr crufts" &&
+       (
+               cd crufts &&
+
+               prepare_cruft_history &&
+               git -c feature.experimental=false gc &&
+               assert_no_cruft_packs
+       )
+'
+
 run_and_wait_for_auto_gc () {
        # We read stdout from gc for the side effect of waiting until the
        # background gc process exits, closing its fd 9.  Furthermore, the