[PATCH] gc: call "prune --expire 2.weeks.ago"

[PATCH] gc: call "prune --expire 2.weeks.ago"

[Johannes Schindelin]

If "--prune" is passed to gc, it still just calls "git prune".
Otherwise, "prune --expire 2.weeks.ago" is called, where the grace
period is overrideable by the config variable gc.pruneExpire.

While adding a test to t5304-prune.sh (since it really tests the
implicit call to "prune"), the original test for "prune --expire"
is moved there from t1410-reflog.sh, where it did not belong.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I am really tempted to reduce the grace period further, but
	I'd like to hear opinions first.  Is 3.days.ago too short?

 Documentation/config.txt |    5 +++++
 Documentation/git-gc.txt |   16 +++++++++++-----
 builtin-gc.c             |   19 +++++++++++++++++--
 t/t1410-reflog.sh        |   18 ------------------
 t/t5304-prune.sh         |   36 ++++++++++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 14df635..adde89a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -590,6 +590,11 @@ gc.packrefs::
 	at some stage, and setting this to `false` will continue to
 	prevent `git pack-refs` from being run from `git gc`.
 
+gc.pruneexpire::
+	When `git gc` is run without `--prune`, it will still call
+	`prune`, but with `--expire 2.weeks.ago`.  Override the value
+	with this config variable.
+
 gc.reflogexpire::
 	`git reflog expire` removes reflog entries older than
 	this time; defaults to 90 days.
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2e7be91..2042d9f 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -28,13 +28,19 @@ OPTIONS
 --prune::
 	Usually `git-gc` packs refs, expires old reflog entries,
 	packs loose objects,
-	and removes old 'rerere' records.  Removal
+	and removes old 'rerere' records.  Unilateral removal
 	of unreferenced loose objects is an unsafe operation
 	while other git operations are in progress, so it is not
-	done by default.  Pass this option if you want it, and only
-	when you know nobody else is creating new objects in the
-	repository at the same time (e.g. never use this option
-	in a cron script).
+	done by default.
++
+Instead, `git-prune` is called with an option telling it to expire
+only unreferenced loose objects that are at least 2 weeks old.  Set
+the config variable `gc.pruneexpire` to override this grace period.
++
+Pass `--prune` to expire all unreferenced loose objects, but only
+when you know nobody else is creating new objects in the
+repository at the same time (e.g. never use this option
+in a cron script).
 
 --aggressive::
 	Usually 'git-gc' runs very quickly while providing good disk
diff --git a/builtin-gc.c b/builtin-gc.c
index 7cad366..8d07350 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,12 +26,13 @@ static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 20;
+static char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
 static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
-static const char *argv_prune[] = {"prune", NULL};
+static const char *argv_prune[] = {"prune", NULL, NULL, NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
 static int gc_config(const char *var, const char *value)
@@ -55,6 +56,14 @@ static int gc_config(const char *var, const char *value)
 		gc_auto_pack_limit = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "gc.pruneexpire")) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (!approxidate(value))
+			return error("Invalid gc.pruneExpire: '%s'", value);
+		prune_expire = xstrdup(value);
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 
@@ -235,7 +244,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_repack[0]);
 
-	if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
+	if (!prune) {
+		argv_prune[1] = "--expire";
+		argv_prune[2] = prune_expire;
+		argv_prune[3] = NULL;
+	}
+
+	if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_prune[0]);
 
 	if (run_command_v_opt(argv_rerere, RUN_GIT_CMD))
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 24476be..73f830d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -202,22 +202,4 @@ test_expect_success 'delete' '
 
 '
 
-test_expect_success 'prune --expire' '
-
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	git reset --hard &&
-	git prune --expire=1.hour.ago &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime -86500 $BLOB_FILE &&
-	git prune --expire 1.day &&
-	test $before = $(git count-objects | sed "s/ .*//") &&
-	! test -f $BLOB_FILE
-
-'
-
 test_done
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 6560af7..2a88b3f 100644
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -29,4 +29,40 @@ test_expect_success 'prune stale packs' '
 
 '
 
+test_expect_success 'prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	git prune --expire=1.hour.ago &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -86500 $BLOB_FILE &&
+	git prune --expire 1.day &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: implicit prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
+echo blob: $BLOB &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14-30)) $BLOB_FILE &&
+	git gc &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14+1)) $BLOB_FILE &&
+	git gc &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
 test_done
-- 
1.5.4.4.646.ge37ad

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> If "--prune" is passed to gc, it still just calls "git prune".
> Otherwise, "prune --expire 2.weeks.ago" is called, where the grace
> period is overrideable by the config variable gc.pruneExpire.

"What it does."

> While adding a test to t5304-prune.sh (since it really tests the
> implicit call to "prune"), the original test for "prune --expire"
> is moved there from t1410-reflog.sh, where it did not belong.

"What the fallouts from this change were."

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Can we also have "why this is a good idea", "what problem this solves"?

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Nicolas Pitre]

On Tue, 11 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > If "--prune" is passed to gc, it still just calls "git prune".
> > Otherwise, "prune --expire 2.weeks.ago" is called, where the grace
> > period is overrideable by the config variable gc.pruneExpire.
> 
> "What it does."
> 
> > While adding a test to t5304-prune.sh (since it really tests the
> > implicit call to "prune"), the original test for "prune --expire"
> > is moved there from t1410-reflog.sh, where it did not belong.
> 
> "What the fallouts from this change were."
> 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Can we also have "why this is a good idea", "what problem this solves"?

FWIW, my agreeing with the "why this is a good idea" can be translated 
into:

Acked-by: Nicolas Pitre <nico@cam.org>


Nicolas

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

>> Can we also have "why this is a good idea", "what problem this solves"?
>
> FWIW, my agreeing with the "why this is a good idea" can be translated 
> into:
>
> Acked-by: Nicolas Pitre <nico@cam.org>

Hmmm.  Is it _that_ obvious?

At least it would be easier to readers if we had something like this in
the documentation (and/or the commit message):

    "git gc" used to never prune unreachable objects without being
    explicitly told to, with its --prune option.  This left cruft to
    accumulate; the user eventually has to run "git prune" manually.

    It is safe to prune old objects that are unreachable from refs nor
    reflogs.  "git gc" is updated to run "git prune --expire 2.weeks.ago"
    so that users has to run "git prune" by hand much less often.

Is it too much to ask for regulars to set the example of justifying why
each of the change is a good idea?

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Johannes Schindelin]

Hi,

On Tue, 11 Mar 2008, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> >> Can we also have "why this is a good idea", "what problem this 
> >> solves"?
> >
> > FWIW, my agreeing with the "why this is a good idea" can be translated 
> > into:
> >
> > Acked-by: Nicolas Pitre <nico@cam.org>
> 
> Hmmm.  Is it _that_ obvious?
> 
> At least it would be easier to readers if we had something like this in 
> the documentation (and/or the commit message):
> 
>     "git gc" used to never prune unreachable objects without being
>     explicitly told to, with its --prune option.  This left cruft to
>     accumulate; the user eventually has to run "git prune" manually.
> 
>     It is safe to prune old objects that are unreachable from refs nor
>     reflogs.  "git gc" is updated to run "git prune --expire 2.weeks.ago"
>     so that users has to run "git prune" by hand much less often.
> 
> Is it too much to ask for regulars to set the example of justifying why 
> each of the change is a good idea?

I would have written something like

	Earlier, git-gc would not prune loose objects without being called 
	with --prune.  However, users were actively warned that it is not 
	a safe operation, so most users never called it.

	This makes the operation reasonably safe (unless you have critical 
	git operations running for over two weeks), by pruning only those 
	objects that are old (in the sense of git operations, which 
	typically take no more than a few seconds).

However, I think that it should have been obvious to those who know the 
internals of git-gc, and it is completely uninteresting to those that are 
just users.  All they will realise (or not) is that "git gc --auto" now 
less often complains about too many loose objects (hopefully).

The real question I asked was: is 2 weeks a sensible default?  As I said, 
I was almost tempted to reduce it to 3 days.

Hmm?

Ciao,
Dscho

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Nicolas Pitre]

On Wed, 12 Mar 2008, Johannes Schindelin wrote:

> The real question I asked was: is 2 weeks a sensible default?  As I said, 
> I was almost tempted to reduce it to 3 days.

3 days would make me nervous, even if it should be plenty safe.

2 weeks is OTOH maybe a bit too conservative.

What about one week instead?


Nicolas

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Pieter de Bie]

On Mar 12, 2008, at 4:45 PM, Nicolas Pitre wrote:

>
> 2 weeks is OTOH maybe a bit too conservative.
>
> What about one week instead?

I'd really like it to be at least 2 weeks

- Pieter

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Pieter de Bie wrote:

> On Mar 12, 2008, at 4:45 PM, Nicolas Pitre wrote:
> 
> > 2 weeks is OTOH maybe a bit too conservative.
> >
> > What about one week instead?
> 
> I'd really like it to be at least 2 weeks

Could you back that up with an explanation, as to why?

Thanks,
Dscho

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Jeff King]

On Wed, Mar 12, 2008 at 05:05:51PM +0100, Johannes Schindelin wrote:

> > I'd really like it to be at least 2 weeks
> 
> Could you back that up with an explanation, as to why?

I assume it's "because I wouldn't want to lose work I had done within
the last two weeks." Yes, I know that this expiration is actually after
the reflog has already expired, but there is a loophole there: branches
that have been deleted have their reflogs deleted (some have argued that
this doesn't matter, since the HEAD reflog will still mention the
commits. In most cases, this is true, though there are still a few
exceptions).

I think being conservative here is a good idea. The big reason for this
is to fix the spurious "gc --auto" runs caused by needing to prune. So
there is no downside to increasing the time limit unless you think
people will generate enough objects in that limit to cause the problem
again. In which case they will continue to complain to the list, and we
can drop the time.

-Peff

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Pieter de Bie]

On Mar 12, 2008, at 6:01 PM, Jeff King wrote:

> On Wed, Mar 12, 2008 at 05:05:51PM +0100, Johannes Schindelin wrote:
>
>>> I'd really like it to be at least 2 weeks
>>
>> Could you back that up with an explanation, as to why?
>
> I assume it's "because I wouldn't want to lose work I had done within
> the last two weeks." Yes, I know that this expiration is actually  
> after
> the reflog has already expired

Ah, I hadn't realised that. Then I don't really care, one week sounds  
fine too. 2 weeks just seemed a bit short, as the default reflog is 30  
days. But if it's 2 weeks after the 30 days, that should be more than  
enough

- Pieter

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Junio C Hamano]

Pieter de Bie <pdebie@ai.rug.nl> writes:

> On Mar 12, 2008, at 6:01 PM, Jeff King wrote:
>
>> On Wed, Mar 12, 2008 at 05:05:51PM +0100, Johannes Schindelin wrote:
>>
>>>> I'd really like it to be at least 2 weeks
>>>
>>> Could you back that up with an explanation, as to why?
>>
>> I assume it's "because I wouldn't want to lose work I had done within
>> the last two weeks." Yes, I know that this expiration is actually
>> after
>> the reflog has already expired
>
> Ah, I hadn't realised that. Then I don't really care, one week sounds
> fine too. 2 weeks just seemed a bit short, as the default reflog is 30
> days. But if it's 2 weeks after the 30 days, that should be more than
> enough

Wasn't the default 90 days?

In any case, after 90 days, my reading of the code is that these loose
ones become unreachable, and when we look at their timestamps, we notice
that they are already more than 2 weeks old, and they will immediately be
removed.

So it won't be "2 weeks after X", either.

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> In any case, after 90 days, my reading of the code is that these loose 
> ones become unreachable, and when we look at their timestamps, we notice 
> that they are already more than 2 weeks old, and they will immediately 
> be removed.

Yes.

In any case, most likely these objects would have been packed anyway 
during that period, so they would have been pruned by the repack called by 
git-gc.

My patch really, really is about _unreferenced_ loose objects, i.e. 
something you get by multiple "git add"s without commits.  And by merge 
conflicts.

IOW by objects that were never referenced by commit objects.

Ciao,
Dscho

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Yes.
>
> In any case, most likely these objects would have been packed anyway 
> during that period, so they would have been pruned by the repack called by 
> git-gc.

Yes, that is the most probable way for these unused cruft to get removed.

> My patch really, really is about _unreferenced_ loose objects, i.e. 
> something you get by multiple "git add"s without commits.  And by merge 
> conflicts.
>
> IOW by objects that were never referenced by commit objects.

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Geert Bosch]

On Mar 12, 2008, at 06:57, Johannes Schindelin wrote:
> The real question I asked was: is 2 weeks a sensible default?  As I  
> said,
> I was almost tempted to reduce it to 3 days.
>
> Hmm?

Two weeks is a sensible default. Many people don't use their SCM every
day (really!). Say you'd work on something friday, mess up and go home,
it would be quite bad on monday morning to find that gc kicks in and
removes some objects you're trying to recover.

The only point is to reduce build-up over long periods,
so two weeks seems a perfectly fine cut-off.

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Nicolas Pitre]

On Tue, 11 Mar 2008, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> >> Can we also have "why this is a good idea", "what problem this solves"?
> >
> > FWIW, my agreeing with the "why this is a good idea" can be translated 
> > into:
> >
> > Acked-by: Nicolas Pitre <nico@cam.org>
> 
> Hmmm.  Is it _that_ obvious?

To the average user, maybe not.  But My ack is orthogonal to that issue.


Nicolas

Re: [PATCH] gc: call "prune --expire 2.weeks.ago"

[Marko Kreen]

On 3/12/08, Nicolas Pitre <nico@cam.org> wrote:
> On Tue, 11 Mar 2008, Junio C Hamano wrote:
> > Nicolas Pitre <nico@cam.org> writes:
>  > >> Can we also have "why this is a good idea", "what problem this solves"?
>  > >
>  > > FWIW, my agreeing with the "why this is a good idea" can be translated
>  > > into:
>  > >
>  > > Acked-by: Nicolas Pitre <nico@cam.org>
>  >
>  > Hmmm.  Is it _that_ obvious?
>
> To the average user, maybe not.  But My ack is orthogonal to that issue.

Well, I'm a newbie user and now I'm trained to always do "gc --prune",
because "gc" itself does not make tree "really" clean.

But this will quite likely bit me in the long run.

So from my newbie perspective, anything that decreases
number of mandatory arguments to commands is good.
*cough* commit -a *cough*

But the difference from 'commit -a' is that its not a style
issue - "gc" without --prune will keep stuff around indefinitely,
which makes occasional --prune usage mandatory.  As its annoying
to memorize when it was last ran, its easier to use it always.

So from my newbie perpective, +1 for making plain "gc" work.

-- 
marko

[PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

The only reason we did not call "prune" in git-gc was that it is an
inherently dangerous operation: if there is a commit going on, you will
prune loose objects that were just created, and are, in fact, needed by the
commit object just about to be created.

Since it is dangerous, we told users so.  That led to many users not even
daring to run it when it was actually safe. Besides, they are users, and
should not have to remember such details as when to call git-gc with
--prune, or to call git-prune directly.

Of course, the consequence was that "git gc --auto" gets triggered much
more often than we would like, since unreferenced loose objects (such as
left-overs from a rebase or a reset --hard) were never pruned.

Alas, git-prune recently learnt the option --expire <minimum-age>, which
makes it a much safer operation.  This allows us to call prune from git-gc,
with a grace period of 2 weeks for the unreferenced loose objects (this
value was determined in a discussion on the git list as a safe one).

If you want to override this grace period, just set the config variable
gc.pruneExpire to a different value; an example would be

	[gc]
		pruneExpire = 6.months.ago

if you feel really paranoid.

Note that this new behaviour does not affect git-gc when you pass the
option --prune; in that case, prune will clean up the loose objects with no
grace period at all.

While adding a test to t5304-prune.sh (since it really tests the implicit
call to "prune"), also the original test for "prune --expire" was moved
there from t1410-reflog.sh, where it did not belong.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Nicolas Pitre <nico@cam.org>

---

	Since my original suggestion of 2 weeks was more or less agreed 
	upon, I only reworked the commit message, and added the Ack of 
	Nico.

	Junio, is this message good enough?

 Documentation/config.txt |    5 +++++
 Documentation/git-gc.txt |   16 +++++++++++-----
 builtin-gc.c             |   19 +++++++++++++++++--
 t/t1410-reflog.sh        |   18 ------------------
 t/t5304-prune.sh         |   36 ++++++++++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f64b269..db5b2dc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -590,6 +590,11 @@ gc.packrefs::
 	at some stage, and setting this to `false` will continue to
 	prevent `git pack-refs` from being run from `git gc`.
 
+gc.pruneexpire::
+	When `git gc` is run without `--prune`, it will still call
+	`prune`, but with `--expire 2.weeks.ago`.  Override the value
+	with this config variable.
+
 gc.reflogexpire::
 	`git reflog expire` removes reflog entries older than
 	this time; defaults to 90 days.
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2e7be91..2042d9f 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -28,13 +28,19 @@ OPTIONS
 --prune::
 	Usually `git-gc` packs refs, expires old reflog entries,
 	packs loose objects,
-	and removes old 'rerere' records.  Removal
+	and removes old 'rerere' records.  Unilateral removal
 	of unreferenced loose objects is an unsafe operation
 	while other git operations are in progress, so it is not
-	done by default.  Pass this option if you want it, and only
-	when you know nobody else is creating new objects in the
-	repository at the same time (e.g. never use this option
-	in a cron script).
+	done by default.
++
+Instead, `git-prune` is called with an option telling it to expire
+only unreferenced loose objects that are at least 2 weeks old.  Set
+the config variable `gc.pruneexpire` to override this grace period.
++
+Pass `--prune` to expire all unreferenced loose objects, but only
+when you know nobody else is creating new objects in the
+repository at the same time (e.g. never use this option
+in a cron script).
 
 --aggressive::
 	Usually 'git-gc' runs very quickly while providing good disk
diff --git a/builtin-gc.c b/builtin-gc.c
index 7cad366..8d07350 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,12 +26,13 @@ static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 20;
+static char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
 static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
-static const char *argv_prune[] = {"prune", NULL};
+static const char *argv_prune[] = {"prune", NULL, NULL, NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
 static int gc_config(const char *var, const char *value)
@@ -55,6 +56,14 @@ static int gc_config(const char *var, const char *value)
 		gc_auto_pack_limit = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "gc.pruneexpire")) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (!approxidate(value))
+			return error("Invalid gc.pruneExpire: '%s'", value);
+		prune_expire = xstrdup(value);
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 
@@ -235,7 +244,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_repack[0]);
 
-	if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
+	if (!prune) {
+		argv_prune[1] = "--expire";
+		argv_prune[2] = prune_expire;
+		argv_prune[3] = NULL;
+	}
+
+	if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_prune[0]);
 
 	if (run_command_v_opt(argv_rerere, RUN_GIT_CMD))
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 24476be..73f830d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -202,22 +202,4 @@ test_expect_success 'delete' '
 
 '
 
-test_expect_success 'prune --expire' '
-
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	git reset --hard &&
-	git prune --expire=1.hour.ago &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime -86500 $BLOB_FILE &&
-	git prune --expire 1.day &&
-	test $before = $(git count-objects | sed "s/ .*//") &&
-	! test -f $BLOB_FILE
-
-'
-
 test_done
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 6560af7..2a88b3f 100644
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -29,4 +29,40 @@ test_expect_success 'prune stale packs' '
 
 '
 
+test_expect_success 'prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	git prune --expire=1.hour.ago &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -86500 $BLOB_FILE &&
+	git prune --expire 1.day &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: implicit prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
+echo blob: $BLOB &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14-30)) $BLOB_FILE &&
+	git gc &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14+1)) $BLOB_FILE &&
+	git gc &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
 test_done
-- 
1.5.4.4.694.g43223

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Brandon Casey]

Johannes Schindelin wrote:
> If you want to override this grace period, just set the config variable
> gc.pruneExpire to a different value; an example would be
> 
> 	[gc]
> 		pruneExpire = 6.months.ago
> 
> if you feel really paranoid.
> 
> Note that this new behaviour does not affect git-gc when you pass the
> option --prune; in that case, prune will clean up the loose objects with no
> grace period at all.

Hmm. Perhaps 'git-gc' should always call 'prune' with the '--expire' argument
for simplicity of the 'git-gc' interface and --prune should become a noop?

Is 'git-gc --prune' still useful to end users when those in-the-know can use
git-prune when they really want all loose unreferenced objects to be removed?

Also, what about clones created with --shared or --reference? Should there be
a way to disable this functionality? gc.pruneExpire never

-brandon

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Jakub Narebski]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> Is 'git-gc --prune' still useful to end users when those in-the-know can use
> git-prune when they really want all loose unreferenced objects to be removed?

It is one command less to remember (just like with "git tag --verify"
and "git verify-tag"), so I'm all for "git gc --prune" to remain.
 
> Also, what about clones created with --shared or --reference? Should there be
> a way to disable this functionality? gc.pruneExpire never

That would be nice.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Jakub Narebski wrote:

> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
> > Is 'git-gc --prune' still useful to end users when those in-the-know 
> > can use git-prune when they really want all loose unreferenced objects 
> > to be removed?
> 
> It is one command less to remember (just like with "git tag --verify" 
> and "git verify-tag"), so I'm all for "git gc --prune" to remain.

I don't care one way or the other.

> > Also, what about clones created with --shared or --reference? Should 
> > there be a way to disable this functionality? gc.pruneExpire never
> 
> That would be nice.

Okay, so I just remove the !approxidate() check.  Then, "gc.pruneExpire = 
never" should work as you expect it to.

Ciao,
Dscho

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, so I just remove the !approxidate() check.  Then, "gc.pruneExpire = 
> never" should work as you expect it to.

Huh?  date.c::special[] has "never" defined for this exact reason.

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Okay, so I just remove the !approxidate() check.  Then, "gc.pruneExpire = 
> > never" should work as you expect it to.
> 
> Huh?  date.c::special[] has "never" defined for this exact reason.

Oops.  I thought that approxidate() returns 0 on error, but apparently 
this is not so.  Instead, it returns "now"!

So first of all, my patch is incorrect, and second: invalid dates cannot 
be caught reliably.

Darn.

Ciao,
Dscho "who'll think about a way around that"

[PATCH v3] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

The only reason we did not call "prune" in git-gc was that it is an
inherently dangerous operation: if there is a commit going on, you will
prune loose objects that were just created, and are, in fact, needed by the
commit object just about to be created.

Since it is dangerous, we told users so.  That led to many users not even
daring to run it when it was actually safe. Besides, they are users, and
should not have to remember such details as when to call git-gc with
--prune, or to call git-prune directly.

Of course, the consequence was that "git gc --auto" gets triggered much
more often than we would like, since unreferenced loose objects (such as
left-overs from a rebase or a reset --hard) were never pruned.

Alas, git-prune recently learnt the option --expire <minimum-age>, which
makes it a much safer operation.  This allows us to call prune from git-gc,
with a grace period of 2 weeks for the unreferenced loose objects (this
value was determined in a discussion on the git list as a safe one).

If you want to override this grace period, just set the config variable
gc.pruneExpire to a different value; an example would be

	[gc]
		pruneExpire = 6.months.ago

if you feel really paranoid.

Note that this new behaviour does not affect git-gc when you pass the
option --prune; in that case, prune will clean up the loose objects with no
grace period at all.

While adding a test to t5304-prune.sh (since it really tests the implicit
call to "prune"), also the original test for "prune --expire" was moved
there from t1410-reflog.sh, where it did not belong.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This checks for an invalid gc.pruneExpire by assuming that an 
	invalid date string is not "now", but parses to the same value
	(or actually newer, since between the two calls to approxidate(), 
	there is a slight chance of a wrapover to the next second).

	So yes, gc.pruneExpire = never should work now.

 Documentation/config.txt |    5 +++++
 Documentation/git-gc.txt |   16 +++++++++++-----
 builtin-gc.c             |   20 ++++++++++++++++++--
 t/t1410-reflog.sh        |   18 ------------------
 t/t5304-prune.sh         |   44 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 78 insertions(+), 25 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f64b269..db5b2dc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -590,6 +590,11 @@ gc.packrefs::
 	at some stage, and setting this to `false` will continue to
 	prevent `git pack-refs` from being run from `git gc`.
 
+gc.pruneexpire::
+	When `git gc` is run without `--prune`, it will still call
+	`prune`, but with `--expire 2.weeks.ago`.  Override the value
+	with this config variable.
+
 gc.reflogexpire::
 	`git reflog expire` removes reflog entries older than
 	this time; defaults to 90 days.
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2e7be91..2042d9f 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -28,13 +28,19 @@ OPTIONS
 --prune::
 	Usually `git-gc` packs refs, expires old reflog entries,
 	packs loose objects,
-	and removes old 'rerere' records.  Removal
+	and removes old 'rerere' records.  Unilateral removal
 	of unreferenced loose objects is an unsafe operation
 	while other git operations are in progress, so it is not
-	done by default.  Pass this option if you want it, and only
-	when you know nobody else is creating new objects in the
-	repository at the same time (e.g. never use this option
-	in a cron script).
+	done by default.
++
+Instead, `git-prune` is called with an option telling it to expire
+only unreferenced loose objects that are at least 2 weeks old.  Set
+the config variable `gc.pruneexpire` to override this grace period.
++
+Pass `--prune` to expire all unreferenced loose objects, but only
+when you know nobody else is creating new objects in the
+repository at the same time (e.g. never use this option
+in a cron script).
 
 --aggressive::
 	Usually 'git-gc' runs very quickly while providing good disk
diff --git a/builtin-gc.c b/builtin-gc.c
index 7cad366..9663fae 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,12 +26,13 @@ static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 20;
+static char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
 static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
-static const char *argv_prune[] = {"prune", NULL};
+static const char *argv_prune[] = {"prune", NULL, NULL, NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
 static int gc_config(const char *var, const char *value)
@@ -55,6 +56,15 @@ static int gc_config(const char *var, const char *value)
 		gc_auto_pack_limit = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "gc.pruneexpire")) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (strcmp(value, "now") &&
+				approxidate(value) - approxidate("now") >= 0)
+			return error("Invalid gc.pruneExpire: '%s'", value);
+		prune_expire = xstrdup(value);
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 
@@ -235,7 +245,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_repack[0]);
 
-	if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
+	if (!prune) {
+		argv_prune[1] = "--expire";
+		argv_prune[2] = prune_expire;
+		argv_prune[3] = NULL;
+	}
+
+	if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_prune[0]);
 
 	if (run_command_v_opt(argv_rerere, RUN_GIT_CMD))
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 24476be..73f830d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -202,22 +202,4 @@ test_expect_success 'delete' '
 
 '
 
-test_expect_success 'prune --expire' '
-
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	git reset --hard &&
-	git prune --expire=1.hour.ago &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime -86500 $BLOB_FILE &&
-	git prune --expire 1.day &&
-	test $before = $(git count-objects | sed "s/ .*//") &&
-	! test -f $BLOB_FILE
-
-'
-
 test_done
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 6560af7..3b6b01d 100644
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -29,4 +29,48 @@ test_expect_success 'prune stale packs' '
 
 '
 
+test_expect_success 'prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	git prune --expire=1.hour.ago &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -86500 $BLOB_FILE &&
+	git prune --expire 1.day &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: implicit prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14-30)) $BLOB_FILE &&
+	git gc &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14+1)) $BLOB_FILE &&
+	git gc &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
+
+	git config gc.pruneExpire invalid &&
+	test_must_fail git gc &&
+	git config gc.pruneExpire now &&
+	git gc
+
+'
+
 test_done
-- 
1.5.4.4.694.g43223

Re: [PATCH v3] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

... and here is the interdiff:

 builtin-gc.c     |    3 ++-
 t/t5304-prune.sh |   10 +++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 8d07350..9663fae 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -59,7 +59,8 @@ static int gc_config(const char *var, const char *value)
 	if (!strcmp(var, "gc.pruneexpire")) {
 		if (!value)
 			return config_error_nonbool(var);
-		if (!approxidate(value))
+		if (strcmp(value, "now") &&
+				approxidate(value) - approxidate("now") >= 0)
 			return error("Invalid gc.pruneExpire: '%s'", value);
 		prune_expire = xstrdup(value);
 		return 0;
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 2a88b3f..3b6b01d 100644
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -50,7 +50,6 @@ test_expect_success 'gc: implicit prune --expire' '
 
 	before=$(git count-objects | sed "s/ .*//") &&
 	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
-echo blob: $BLOB &&
 	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
 	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
 	test -f $BLOB_FILE &&
@@ -65,4 +64,13 @@ echo blob: $BLOB &&
 
 '
 
+test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
+
+	git config gc.pruneExpire invalid &&
+	test_must_fail git gc &&
+	git config gc.pruneExpire now &&
+	git gc
+
+'
+
 test_done

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Brandon Casey]

Jakub Narebski wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> Is 'git-gc --prune' still useful to end users when those in-the-know can use
>> git-prune when they really want all loose unreferenced objects to be removed?
> 
> It is one command less to remember (just like with "git tag --verify"
> and "git verify-tag"), so I'm all for "git gc --prune" to remain.

I think the issue here is that the prune behavior of git-gc is more complex when
both 'git-gc' does prune and 'git-gc --prune' does prune, but only one of them
is controlled by gc.pruneExpire. From a high level perspective, this is
non-obvious and so has to be explicitly outlined in the help text _and_ users
have to remember it. The git-gc command and the documentation and suggested work
flows can all be simplified by just always doing a safe prune.

I think git-prune is seldomly used by normal users for the reasons Dscho
described, and I think once the behavior implemented by his patch becomes
standard it will never be used by normal users (except the ones who always use
--prune for the reasons Geert Bosch described, and they'll probably want the
new behavior). So I think git-prune will sink a little lower into plumbing and
common users won't need to know anything about pruning, and only sophisticated
users will need to know git-prune.

-brandon

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Brandon Casey wrote:

> I think git-prune is seldomly used by normal users for the reasons Dscho 
> described, and I think once the behavior implemented by his patch 
> becomes standard it will never be used by normal users (except the ones 
> who always use --prune for the reasons Geert Bosch described, and 
> they'll probably want the new behavior). So I think git-prune will sink 
> a little lower into plumbing and common users won't need to know 
> anything about pruning, and only sophisticated users will need to know 
> git-prune.

But because we are nice people, we will deprecate --prune before we remove 
it, should we go that route at all.

Ciao,
Dscho

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Brandon Casey]

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 12 Mar 2008, Brandon Casey wrote:
> 
>> I think git-prune is seldomly used by normal users for the reasons Dscho 
>> described, and I think once the behavior implemented by his patch 
>> becomes standard it will never be used by normal users (except the ones 
>> who always use --prune for the reasons Geert Bosch described, and 
>> they'll probably want the new behavior). So I think git-prune will sink 
>> a little lower into plumbing and common users won't need to know 
>> anything about pruning, and only sophisticated users will need to know 
>> git-prune.
> 
> But because we are nice people, we will deprecate --prune before we remove 
> it, should we go that route at all.

I am not suggesting that git-gc stop parsing --prune and instead start erroring
out on it. If it is ok to change the behavior of git-gc, it seems like it
is ok to change the behavior of 'git-gc --prune' in the same way, especially if
the change makes it less destructive.

I would suggest not having the somewhat ambiguous state of 'git-gc' prunes one
way and 'git-gc --prune' prunes in another more dangerous way. And only one is
controlled by a config option named gc.pruneExpire (_and_ it's not the obvious
usage of git-gc which actually has the word 'prune' on the command line).

So I hope making --prune a noop fits your definition of nice deprecation.

-brandon

Re: [PATCH v2] gc: call "prune --expire 2.weeks.ago" by default

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> I am not suggesting that git-gc stop parsing --prune and instead start
> erroring out on it. If it is ok to change the behavior of git-gc, it
> seems like it is ok to change the behavior of 'git-gc --prune' in the
> same way, especially if the change makes it less destructive.

Yeah, gc.pruneexpire does sound like it applies to both implicit ones and
explicit ones, doesn't it?  I think your suggestion makes sense.

[PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

The only reason we did not call "prune" in git-gc was that it is an
inherently dangerous operation: if there is a commit going on, you will
prune loose objects that were just created, and are, in fact, needed by the
commit object just about to be created.

Since it is dangerous, we told users so.  That led to many users not even
daring to run it when it was actually safe. Besides, they are users, and
should not have to remember such details as when to call git-gc with
--prune, or to call git-prune directly.

Of course, the consequence was that "git gc --auto" gets triggered much
more often than we would like, since unreferenced loose objects (such as
left-overs from a rebase or a reset --hard) were never pruned.

Alas, git-prune recently learnt the option --expire <minimum-age>, which
makes it a much safer operation.  This allows us to call prune from git-gc,
with a grace period of 2 weeks for the unreferenced loose objects (this
value was determined in a discussion on the git list as a safe one).

If you want to override this grace period, just set the config variable
gc.pruneExpire to a different value; an example would be

	[gc]
		pruneExpire = 6.months.ago

or even "never", if you feel really paranoid.

Note that this new behaviour makes "--prune" be a no-op.

While adding a test to t5304-prune.sh (since it really tests the implicit
call to "prune"), also the original test for "prune --expire" was moved
there from t1410-reflog.sh, where it did not belong.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Wed, 12 Mar 2008, Junio C Hamano wrote:

	> Brandon Casey <casey@nrlssc.navy.mil> writes:
	> 
	> > I am not suggesting that git-gc stop parsing --prune and 
	> > instead start erroring out on it. If it is ok to change the behavior 
	> > of git-gc, it seems like it is ok to change the behavior of 'git-gc 
	> > --prune' in the same way, especially if the change makes it less 
	> > destructive.
	> 
	> Yeah, gc.pruneexpire does sound like it applies to both implicit 
	> ones and explicit ones, doesn't it?  I think your suggestion makes 
	> sense.

	Okay, I give up my resistance.

	Changes since v3: --prune is not even described in the man page 
	anymore, since it now is a no-op.

	Interdiff will follow in its own mail...

 Documentation/config.txt |    4 ++++
 Documentation/git-gc.txt |   17 +++++------------
 builtin-gc.c             |   15 +++++++++++++--
 t/t1410-reflog.sh        |   18 ------------------
 t/t5304-prune.sh         |   44 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 32 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f64b269..a2f1df2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -590,6 +590,10 @@ gc.packrefs::
 	at some stage, and setting this to `false` will continue to
 	prevent `git pack-refs` from being run from `git gc`.
 
+gc.pruneexpire::
+	When `git gc` is run, it will call `prune --expire 2.weeks.ago`.
+	Override the grace period with this config variable.
+
 gc.reflogexpire::
 	`git reflog expire` removes reflog entries older than
 	this time; defaults to 90 days.
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2e7be91..229a7c9 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -8,7 +8,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository
 
 SYNOPSIS
 --------
-'git-gc' [--prune] [--aggressive] [--auto] [--quiet]
+'git-gc' [--aggressive] [--auto] [--quiet]
 
 DESCRIPTION
 -----------
@@ -25,17 +25,6 @@ operating performance. Some git commands may automatically run
 OPTIONS
 -------
 
---prune::
-	Usually `git-gc` packs refs, expires old reflog entries,
-	packs loose objects,
-	and removes old 'rerere' records.  Removal
-	of unreferenced loose objects is an unsafe operation
-	while other git operations are in progress, so it is not
-	done by default.  Pass this option if you want it, and only
-	when you know nobody else is creating new objects in the
-	repository at the same time (e.g. never use this option
-	in a cron script).
-
 --aggressive::
 	Usually 'git-gc' runs very quickly while providing good disk
 	space utilization and performance.  This option will cause
@@ -104,6 +93,10 @@ the value, the more time is spent optimizing the delta compression.  See
 the documentation for the --window' option in linkgit:git-repack[1] for
 more details.  This defaults to 10.
 
+The optional configuration variable 'gc.pruneExpire' controls how old
+the unreferenced loose objects have to be before they are pruned.  The
+default is "2 weeks ago".
+
 See Also
 --------
 linkgit:git-prune[1]
diff --git a/builtin-gc.c b/builtin-gc.c
index 7cad366..8bd54d8 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,12 +26,13 @@ static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 20;
+static char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
 static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
-static const char *argv_prune[] = {"prune", NULL};
+static const char *argv_prune[] = {"prune", "--expire", NULL, NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
 static int gc_config(const char *var, const char *value)
@@ -55,6 +56,15 @@ static int gc_config(const char *var, const char *value)
 		gc_auto_pack_limit = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "gc.pruneexpire")) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (strcmp(value, "now") &&
+				approxidate(value) - approxidate("now") >= 0)
+			return error("Invalid gc.pruneExpire: '%s'", value);
+		prune_expire = xstrdup(value);
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 
@@ -235,7 +245,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_repack[0]);
 
-	if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
+	argv_prune[2] = prune_expire;
+	if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_prune[0]);
 
 	if (run_command_v_opt(argv_rerere, RUN_GIT_CMD))
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 24476be..73f830d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -202,22 +202,4 @@ test_expect_success 'delete' '
 
 '
 
-test_expect_success 'prune --expire' '
-
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	git reset --hard &&
-	git prune --expire=1.hour.ago &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime -86500 $BLOB_FILE &&
-	git prune --expire 1.day &&
-	test $before = $(git count-objects | sed "s/ .*//") &&
-	! test -f $BLOB_FILE
-
-'
-
 test_done
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 6560af7..3b6b01d 100644
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -29,4 +29,48 @@ test_expect_success 'prune stale packs' '
 
 '
 
+test_expect_success 'prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	git prune --expire=1.hour.ago &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -86500 $BLOB_FILE &&
+	git prune --expire 1.day &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: implicit prune --expire' '
+
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14-30)) $BLOB_FILE &&
+	git gc &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	test-chmtime -$((86400*14+1)) $BLOB_FILE &&
+	git gc &&
+	test $before = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
+test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
+
+	git config gc.pruneExpire invalid &&
+	test_must_fail git gc &&
+	git config gc.pruneExpire now &&
+	git gc
+
+'
+
 test_done
-- 
1.5.4.4.694.g43223

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

And here is the interdiff:

 Documentation/config.txt |    5 ++---
 Documentation/git-gc.txt |   23 +++++------------------
 builtin-gc.c             |    9 ++-------
 3 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index db5b2dc..a2f1df2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -591,9 +591,8 @@ gc.packrefs::
 	prevent `git pack-refs` from being run from `git gc`.
 
 gc.pruneexpire::
-	When `git gc` is run without `--prune`, it will still call
-	`prune`, but with `--expire 2.weeks.ago`.  Override the value
-	with this config variable.
+	When `git gc` is run, it will call `prune --expire 2.weeks.ago`.
+	Override the grace period with this config variable.
 
 gc.reflogexpire::
 	`git reflog expire` removes reflog entries older than
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2042d9f..229a7c9 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -8,7 +8,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository
 
 SYNOPSIS
 --------
-'git-gc' [--prune] [--aggressive] [--auto] [--quiet]
+'git-gc' [--aggressive] [--auto] [--quiet]
 
 DESCRIPTION
 -----------
@@ -25,23 +25,6 @@ operating performance. Some git commands may automatically run
 OPTIONS
 -------
 
---prune::
-	Usually `git-gc` packs refs, expires old reflog entries,
-	packs loose objects,
-	and removes old 'rerere' records.  Unilateral removal
-	of unreferenced loose objects is an unsafe operation
-	while other git operations are in progress, so it is not
-	done by default.
-+
-Instead, `git-prune` is called with an option telling it to expire
-only unreferenced loose objects that are at least 2 weeks old.  Set
-the config variable `gc.pruneexpire` to override this grace period.
-+
-Pass `--prune` to expire all unreferenced loose objects, but only
-when you know nobody else is creating new objects in the
-repository at the same time (e.g. never use this option
-in a cron script).
-
 --aggressive::
 	Usually 'git-gc' runs very quickly while providing good disk
 	space utilization and performance.  This option will cause
@@ -110,6 +93,10 @@ the value, the more time is spent optimizing the delta compression.  See
 the documentation for the --window' option in linkgit:git-repack[1] for
 more details.  This defaults to 10.
 
+The optional configuration variable 'gc.pruneExpire' controls how old
+the unreferenced loose objects have to be before they are pruned.  The
+default is "2 weeks ago".
+
 See Also
 --------
 linkgit:git-prune[1]
diff --git a/builtin-gc.c b/builtin-gc.c
index 9663fae..8bd54d8 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -32,7 +32,7 @@ static char *prune_expire = "2.weeks.ago";
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
 static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
-static const char *argv_prune[] = {"prune", NULL, NULL, NULL};
+static const char *argv_prune[] = {"prune", "--expire", NULL, NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
 static int gc_config(const char *var, const char *value)
@@ -245,12 +245,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 	if (run_command_v_opt(argv_repack, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_repack[0]);
 
-	if (!prune) {
-		argv_prune[1] = "--expire";
-		argv_prune[2] = prune_expire;
-		argv_prune[3] = NULL;
-	}
-
+	argv_prune[2] = prune_expire;
 	if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_prune[0]);
 

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> ---prune::

I am fairly paranoid about end users wondering about what is described in
ancient documentation and complaining that we do not talk about it
anymore.  I am tempted to suggest:

	This is a no-op but you may see it mentioned in older docs and
	scripts.  Older git-gc never ran 'prune' without being told, and
	this option was a way to tell it to.

but this would lead to littering the documentation with too much
historical information in the long run.  I dunno.  I am inclined to favor
the removal as your patch did, but somebody else may have clever ideas.

> +	if (!strcmp(var, "gc.pruneexpire")) {
> +		if (!value)
> +			return config_error_nonbool(var);
> +		if (strcmp(value, "now") &&
> +				approxidate(value) - approxidate("now") >= 0)
> +			return error("Invalid gc.pruneExpire: '%s'", value);

Yuck; approxidate() returns ulong.  Can subtracting a ulong from another
ever go negative?

Besides, because there is no guarantee of the order of evaluation between
these two approxidate() calls, you may get +1 or -1 on the second boundary.

I think the reason why you did not catch it in your test is because your
tests are half complete; they test only what you wanted to catch
(misconfigured case) and do not test the other half (properly working
case).

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Nicolas Pitre]

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > ---prune::
> 
> I am fairly paranoid about end users wondering about what is described in
> ancient documentation and complaining that we do not talk about it
> anymore.  I am tempted to suggest:
> 
> 	This is a no-op but you may see it mentioned in older docs and
> 	scripts.  Older git-gc never ran 'prune' without being told, and
> 	this option was a way to tell it to.
> 
> but this would lead to littering the documentation with too much
> historical information in the long run.  I dunno.  I am inclined to favor
> the removal as your patch did, but somebody else may have clever ideas.

Historical notes of that sort belong in RelNotes.


Nicolas

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > ---prune::
> 
> I am fairly paranoid about end users wondering about what is described in
> ancient documentation and complaining that we do not talk about it
> anymore.  I am tempted to suggest:
> 
> 	This is a no-op but you may see it mentioned in older docs and
> 	scripts.  Older git-gc never ran 'prune' without being told, and
> 	this option was a way to tell it to.
> 
> but this would lead to littering the documentation with too much 
> historical information in the long run.  I dunno.  I am inclined to 
> favor the removal as your patch did, but somebody else may have clever 
> ideas.

Well, if you want to, I am quite willing to adapt the patch.  (I care 
enough about the real issue, namely that "prune" should be called 
automatically.)

> > +	if (!strcmp(var, "gc.pruneexpire")) {
> > +		if (!value)
> > +			return config_error_nonbool(var);
> > +		if (strcmp(value, "now") &&
> > +				approxidate(value) - approxidate("now") >= 0)
> > +			return error("Invalid gc.pruneExpire: '%s'", value);
> 
> Yuck; approxidate() returns ulong.  Can subtracting a ulong from another
> ever go negative?
> 
> Besides, because there is no guarantee of the order of evaluation between
> these two approxidate() calls, you may get +1 or -1 on the second boundary.
> 
> I think the reason why you did not catch it in your test is because your
> tests are half complete; they test only what you wanted to catch
> (misconfigured case) and do not test the other half (properly working
> case).

Yes, probably.  Of course, comparing a difference to 0 is absolutely 
moronic.

I should have written

				approxidate(value) >= approxidate("now"))

in the first place.

So, could you tell me, please, if I should resend the patch with your 
--prune documentation, or without?

Ciao,
Dscho

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Yuck; approxidate() returns ulong.  Can subtracting a ulong from another
>> ever go negative?
>> 
>> Besides, because there is no guarantee of the order of evaluation between
>> these two approxidate() calls, you may get +1 or -1 on the second boundary.
>> 
>> I think the reason why you did not catch it in your test is because your
>> tests are half complete; they test only what you wanted to catch
>> (misconfigured case) and do not test the other half (properly working
>> case).
>
> Yes, probably.  Of course, comparing a difference to 0 is absolutely 
> moronic.
>
> I should have written
>
> 				approxidate(value) >= approxidate("now"))
>
> in the first place.

Eh, sorry, but why?

> So, could you tell me, please, if I should resend the patch with your 
> --prune documentation, or without?

I like Nico's suggestion to put that "historical notes" in RelNotes, so
the documentation part is fine as is.

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Yuck; approxidate() returns ulong.  Can subtracting a ulong from 
> >> another ever go negative?
> >> 
> >> Besides, because there is no guarantee of the order of evaluation 
> >> between these two approxidate() calls, you may get +1 or -1 on the 
> >> second boundary.
> >> 
> >> I think the reason why you did not catch it in your test is because 
> >> your tests are half complete; they test only what you wanted to catch 
> >> (misconfigured case) and do not test the other half (properly working 
> >> case).
> >
> > Yes, probably.  Of course, comparing a difference to 0 is absolutely 
> > moronic.
> >
> > I should have written
> >
> > 				approxidate(value) >= approxidate("now"))
> >
> > in the first place.
> 
> Eh, sorry, but why?

The thing is: I want to prevent invalid dates in gc.pruneExpire from going 
unnoticed, _especially_ since they would default to "now".  IOW if you 
said something like "one.weak.ago", it would actually have the same effect 
as "now" and offer _no_ grace period.

But like you said, comparing the difference of two unsigned longs to >= 0 
might be quite stupid.  Instead, I compare them _directly_.

Since I compare the value to "now" first, and only if it is not, compare 
the approxidate() of the value to the current time stamp, I can verify 
that no invalid date was specified.

Unfortunately, this check includes future dates.  Fortunately, they do not 
make sense at all.

To make my reasoning clear, how about this comment above that if() clause?

		/*
		 * In case of an invalid date, approxidate() returns the
		 * same as approxidate("now").  Since the millisecond
		 * boundary could have been crossed between the two calls
		 * to approxidate(), we compare not only for equality,
		 * but also if the former is greater than the latter.
		 *
		 * Note: this assumes that future dates are invalid, which
		 * makes sense, really.
		 */

Hmm?

Ciao,
Dscho

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Eh, sorry, but why?
>
> The thing is: I want to prevent invalid dates in gc.pruneExpire from going 
> unnoticed, _especially_ since they would default to "now".  IOW if you 
> said something like "one.weak.ago", it would actually have the same effect 
> as "now" and offer _no_ grace period.
>
> But like you said, comparing the difference of two unsigned longs to >= 0 
> might be quite stupid.  Instead, I compare them _directly_.
>
> Since I compare the value to "now" first, and only if it is not, compare 
> the approxidate() of the value to the current time stamp, I can verify 
> that no invalid date was specified.
>
> Unfortunately, this check includes future dates.  Fortunately, they do not 
> make sense at all.
>
> To make my reasoning clear, how about this comment above that if() clause?
>
> 		/*
> 		 * In case of an invalid date, approxidate() returns the
> 		 * same as approxidate("now").  Since the millisecond
> 		 * boundary could have been crossed between the two calls
> 		 * to approxidate(), we compare not only for equality,
> 		 * but also if the former is greater than the latter.
> 		 *
> 		 * Note: this assumes that future dates are invalid, which
> 		 * makes sense, really.
> 		 */
>
> Hmm?

Ah,...

But C language rules haven't changed in such a way that it guarantees B to
be evaluated before A when you write "A >= B", have it?

So at least I think you would need something like this if you go that
route:

  		if (strcmp(value, "now")) {
                	unsigned long now = approxidate("now");
                	if (approxidate(value) >= now)
				return error("Invalid %s: '%s'", var, value);
			...
		}

Also the resolution of approxidate() is in seconds so millisecond boundary
does not matter, but that issue is, eh, secondary ;-).

I have to wonder if approxidate_with_error() function that takes a pointer
to receive an error condition may be a better way to solve this cleanly.

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:

> I have to wonder if approxidate_with_error() function that takes a 
> pointer to receive an error condition may be a better way to solve this 
> cleanly.

Right.  But that will have to wait for at least tomorrow, if it waits for 
me.

Ciao,
Dscho

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Wincent Colaiuta]

El 13/3/2008, a las 0:39, Junio C Hamano escribió:

> Ah,...
>
> But C language rules haven't changed in such a way that it  
> guarantees B to
> be evaluated before A when you write "A >= B", have it?
>
> So at least I think you would need something like this if you go that
> route:
>
>  		if (strcmp(value, "now")) {
>                	unsigned long now = approxidate("now");
>                	if (approxidate(value) >= now)
> 				return error("Invalid %s: '%s'", var, value);
> 			...
> 		}


Are you sure that that alternative provides any guarantees about  
evaluation order either? (I'm not a compiler expert, nor am I  
consulting a copy of the standard; but I don't think it does.)

In order to enforce evaluation in the required order I think it might  
have to be something like this:

	if (strcmp(value, "now")) {
		unsigned long now;
		if ((now = approxidate("now")) &&
		    approxidate(value) >= now)
			return error("Invalid %s: '%s'", var, value);
		...
	}

ie. the && operator guarantees left to right evaluation order, and  
since approxidate always returns a non-zero ulong the second  
expression (after the &&) will always be evaluated.

Cheers,
Wincent

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Sixt]

Wincent Colaiuta schrieb:
> El 13/3/2008, a las 0:39, Junio C Hamano escribió:
>>          if (strcmp(value, "now")) {
>>                    unsigned long now = approxidate("now");
>>                    if (approxidate(value) >= now)
>>                 return error("Invalid %s: '%s'", var, value);
>>             ...
>>         }
> 
> 
> Are you sure that that alternative provides any guarantees about
> evaluation order either? (I'm not a compiler expert, nor am I consulting
> a copy of the standard; but I don't think it does.)

There is a sequence point at the semicolon. This means that all observable
side effects of approxidate("now") are visible when approxidate(value) is
evaluated (and no observable side effect of the latter is visible when the
former is evaluated), which doesn't leave much choice for the compiler.
So, yes, this does guarantee the intended evaluation order.

-- Hannes

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Wincent Colaiuta]

El 12/3/2008, a las 21:55, Johannes Schindelin escribió:

> Alas, git-prune recently learnt the option --expire <minimum-age>,  
> which
> makes it a much safer operation.  This allows us to call prune from  
> git-gc,
> with a grace period of 2 weeks for the unreferenced loose objects  
> (this
> value was determined in a discussion on the git list as a safe one).

Just a really minor quibble: I don't think you mean "alas" here;  
"alas" basically means "unfortunately" or "regrettably".

Cheers,
Wincent

Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 752 bytes --]

Hi,

On Thu, 13 Mar 2008, Wincent Colaiuta wrote:

> El 12/3/2008, a las 21:55, Johannes Schindelin escribió:
> 
> >Alas, git-prune recently learnt the option --expire <minimum-age>, 
> >which makes it a much safer operation.  This allows us to call prune 
> >from git-gc, with a grace period of 2 weeks for the unreferenced loose 
> >objects (this value was determined in a discussion on the git list as a 
> >safe one).
> 
> Just a really minor quibble: I don't think you mean "alas" here; "alas" 
> basically means "unfortunately" or "regrettably".

No, I really meant "alas" here, since it is more of a sigh.  The sigh is 
because I had expected somebody else to care more than me about it, and I 
had done the --expire patch already.

Ciao,
Dscho