[PATCH 1/2] Document gc.<pattern>.reflogexpire variables

[PATCH 1/2] Document gc.<pattern>.reflogexpire variables

[Junio C Hamano]

3cb22b8 (Per-ref reflog expiry configuration, 2008-06-15) added support
for setting the expiry parameters differently for different reflog, but
it was never documented.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This comes on top of b4ca1db (reflog --expire-unreachable: avoid
   merge-base computation, 2010-04-07).

 Documentation/config.txt |   10 ++++++++--
 Documentation/git-gc.txt |   10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a1e36d7..4e7dab6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -885,13 +885,19 @@ gc.pruneexpire::
 	unreachable objects immediately.
 
 gc.reflogexpire::
+gc.<pattern>.reflogexpire::
 	'git-reflog expire' removes reflog entries older than
-	this time; defaults to 90 days.
+	this time; defaults to 90 days.  With "<pattern>" (e.g.
+	"refs/stash") in the middle the setting applies only to
+	the refs that match the <pattern>.
 
 gc.reflogexpireunreachable::
+gc.<ref>.reflogexpireunreachable::
 	'git-reflog expire' removes reflog entries older than
 	this time and are not reachable from the current tip;
-	defaults to 30 days.
+	defaults to 30 days.  With "<pattern>" (e.g. "refs/stash")
+	in the middle, the setting applies only to the refs that
+	match the <pattern>.
 
 gc.rerereresolved::
 	Records of conflicted merge you resolved earlier are
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 4cd9cdf..85d7111 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -88,6 +88,16 @@ commits prior to the amend or rebase occurring.  Since these changes
 are not part of the current project most users will want to expire
 them sooner.  This option defaults to '30 days'.
 
+The above two configuration variables can be given to a pattern.  For
+example, this sets non-default expiry values only to remote tracking
+branches:
+
+------------
+[gc "refs/remotes/*"]
+	reflogExpire = never
+	reflogexpireUnreachable = 3 days
+------------
+
 The optional configuration variable 'gc.rerereresolved' indicates
 how long records of conflicted merge you resolved earlier are
 kept.  This defaults to 60 days.
-- 
1.7.1.rc1.252.gce30c

[PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

"git reflog expire" (and "git gc") examines the reflog entries and
discards old/stale ones using two criteria.  The entries that are older
than "reflogexpire" (defaults to 90 days) are unconditionally removed, and
the entries that are older than "reflogexpireunreachable" (defaults to 30
days) are removed if the entry point at commits that are not reachable
from the value of the ref.

This is reasonable for local branches, remote tracking branches and tags.
You (or other people) may have failed experiments that have been made and
then later discarded by resetting the tip of the branch back, and setting
the value of "reflogexpireunreachable" shorter than that of "reflogexpire"
will prune the entries that describe these failed experiments earlier than
the entries that describe the steps that led to the current history.

It however doesn't make much sense for "HEAD" reflog.  When you switch
between branches, it is normal that the tip of the branch you were on is
not an ancestor of the branch you have switched to.  Expiring the entries
that describe what you did while on the previous branch earlier than usual
does not help.

The test t7701 relied on this incorrect behaviour of entries being pruned
from the "HEAD" reflog.  Work it around by setting the "HEAD" reflog
entries to be expired immediately.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-reflog.c                     |   35 ++++++++++++++++++++++++---------
 t/t7701-repack-unpack-unreachable.sh |    1 +
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/builtin-reflog.c b/builtin-reflog.c
index 9792090..c4bc5be 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -34,8 +34,11 @@ struct cmd_reflog_expire_cb {
 
 struct expire_reflog_cb {
 	FILE *newlog;
-	const char *ref;
-	struct commit *ref_commit;
+	enum {
+		UE_NORMAL,
+		UE_ALWAYS,
+		UE_NEVER
+	} unreachable_expire_kind;
 	struct commit_list *mark_list;
 	unsigned long mark_limit;
 	struct cmd_reflog_expire_cb *cmd;
@@ -304,8 +307,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	    (!keep_entry(&old, osha1) || !keep_entry(&new, nsha1)))
 		goto prune;
 
-	if (timestamp < cb->cmd->expire_unreachable) {
-		if (!cb->ref_commit)
+	if ((cb->unreachable_expire_kind != UE_NEVER) &&
+	    (timestamp < cb->cmd->expire_unreachable)) {
+		if (cb->unreachable_expire_kind == UE_ALWAYS)
 			goto prune;
 		if (unreachable(cb, old, osha1) || unreachable(cb, new, nsha1))
 			goto prune;
@@ -338,6 +342,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 	struct expire_reflog_cb cb;
 	struct ref_lock *lock;
 	char *log_file, *newlog_path = NULL;
+	struct commit *tip_commit;
 	int status = 0;
 
 	memset(&cb, 0, sizeof(cb));
@@ -357,18 +362,28 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 		cb.newlog = fopen(newlog_path, "w");
 	}
 
-	cb.ref_commit = lookup_commit_reference_gently(sha1, 1);
-	cb.ref = ref;
 	cb.cmd = cmd;
-	if (cb.ref_commit) {
+
+	if (!cmd->expire_unreachable || !strcmp(ref, "HEAD")) {
+		tip_commit = NULL;
+		cb.unreachable_expire_kind = UE_NEVER;
+	} else {
+		tip_commit = lookup_commit_reference_gently(sha1, 1);
+		if (!tip_commit)
+			cb.unreachable_expire_kind = UE_ALWAYS;
+		else
+			cb.unreachable_expire_kind = UE_NORMAL;
+	}
+
+	if (cb.unreachable_expire_kind == UE_NORMAL) {
 		cb.mark_list = NULL;
-		commit_list_insert(cb.ref_commit, &cb.mark_list);
+		commit_list_insert(tip_commit, &cb.mark_list);
 		cb.mark_limit = cmd->expire_total;
 		mark_reachable(&cb);
 	}
 	for_each_reflog_ent(ref, expire_reflog_ent, &cb);
-	if (cb.ref_commit)
-		clear_commit_marks(cb.ref_commit, REACHABLE);
+	if (cb.unreachable_expire_kind == UE_NORMAL)
+		clear_commit_marks(tip_commit, REACHABLE);
  finish:
 	if (cb.newlog) {
 		if (fclose(cb.newlog)) {
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 5babdf2..e14b110 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -9,6 +9,7 @@ csha1=
 tsha1=
 
 test_expect_success '-A with -d option leaves unreachable objects unpacked' '
+	git config gc.HEAD.reflogexpire now &&
 	echo content > file1 &&
 	git add . &&
 	git commit -m initial_commit &&
-- 
1.7.1.rc1.252.gce30c

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Johannes Sixt]

Am 4/14/2010 22:35, schrieb Junio C Hamano:
> "git reflog expire" (and "git gc") examines the reflog entries and
> discards old/stale ones using two criteria.  The entries that are older
> than "reflogexpire" (defaults to 90 days) are unconditionally removed, and
> the entries that are older than "reflogexpireunreachable" (defaults to 30
> days) are removed if the entry point at commits that are not reachable
> from the value of the ref.
> 
> This is reasonable for local branches, remote tracking branches and tags.
> You (or other people) may have failed experiments that have been made and
> then later discarded by resetting the tip of the branch back, and setting
> the value of "reflogexpireunreachable" shorter than that of "reflogexpire"
> will prune the entries that describe these failed experiments earlier than
> the entries that describe the steps that led to the current history.
> 
> It however doesn't make much sense for "HEAD" reflog.  When you switch
> between branches, it is normal that the tip of the branch you were on is
> not an ancestor of the branch you have switched to.  Expiring the entries
> that describe what you did while on the previous branch earlier than usual
> does not help.

Does not help what? What is the problem?

-- Hannes

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Does not help what? What is the problem?

You will lose the record from HEAD reflog that records the fact that you
were at the tip of "next" less than "reflogexpire" but more than
"reflogexpireunreachable" time ago, if you run "gc" while on "master".

Such a pruning does not have much to do with the real reason why
expireunreachable would be a useful thing (namely, to prune failed
histories that have been rewound away faster than the history that
survived from reflog of individual branches).

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Johannes Sixt]

Am 4/15/2010 9:40, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> Does not help what? What is the problem?
> 
> You will lose the record from HEAD reflog that records the fact that you
> were at the tip of "next" less than "reflogexpire" but more than
> "reflogexpireunreachable" time ago, if you run "gc" while on "master".
> 
> Such a pruning does not have much to do with the real reason why
> expireunreachable would be a useful thing (namely, to prune failed
> histories that have been rewound away faster than the history that
> survived from reflog of individual branches).

But what is the benefit?

We expire reflogs so that we can garbage-collect objects. If
expire-unreachable is ignored for HEAD, then the objects of failed
experiments survive due to HEAD's reflog until reflog.expire is reached.
If this is what you want, why do we have expire-unreachable at all?

-- Hannes

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 4/15/2010 9:40, schrieb Junio C Hamano:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>> 
>>> Does not help what? What is the problem?
>> 
>> You will lose the record from HEAD reflog that records the fact that you
>> were at the tip of "next" less than "reflogexpire" but more than
>> "reflogexpireunreachable" time ago, if you run "gc" while on "master".
>> 
>> Such a pruning does not have much to do with the real reason why
>> expireunreachable would be a useful thing (namely, to prune failed
>> histories that have been rewound away faster than the history that
>> survived from reflog of individual branches).
>
> But what is the benefit?

That is a funny thing to ask.  Since when not losing information is a
lower priority?

> We expire reflogs so that we can garbage-collect objects.

I am tempted to say "Wrong!", but would stop at saying "We seem to have
differing opinions" for now.

A reflog consists of entries, each of which records how you got to the
current history by pointing to different commit objects.  Some entries
matter more than others do.  Dead-end experiments stop mattering faster
than others.  It is these _entries_ that we expire, because keeping them
indefinitely is a wasteful clutter.

Removing stale objects is not a goal; objects are removed merely as an
effect of them becoming stale after no refs and reflog entries that matter
point at them.

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Johannes Sixt]

Am 4/15/2010 14:30, schrieb Junio C Hamano:
> A reflog consists of entries, each of which records how you got to the
> current history by pointing to different commit objects.  Some entries
> matter more than others do.  Dead-end experiments stop mattering faster
> than others.  It is these _entries_ that we expire, because keeping them
> indefinitely is a wasteful clutter.

I have no problem with this point of view, i.e., that expiration is merely
about the entries, and that garbage-collecting objects is just a
side-effect of removed entries.

But I don't see yet, why an entry pointing to a dead-end experiment is
less important when it is an entry in a branch reflog than when it is an
entry in the HEAD reflog. Care to explain?

-- Hannes

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Jeff King]

On Thu, Apr 15, 2010 at 02:58:29PM +0200, Johannes Sixt wrote:

> Am 4/15/2010 14:30, schrieb Junio C Hamano:
> > A reflog consists of entries, each of which records how you got to the
> > current history by pointing to different commit objects.  Some entries
> > matter more than others do.  Dead-end experiments stop mattering faster
> > than others.  It is these _entries_ that we expire, because keeping them
> > indefinitely is a wasteful clutter.
> 
> I have no problem with this point of view, i.e., that expiration is merely
> about the entries, and that garbage-collecting objects is just a
> side-effect of removed entries.
> 
> But I don't see yet, why an entry pointing to a dead-end experiment is
> less important when it is an entry in a branch reflog than when it is an
> entry in the HEAD reflog. Care to explain?

I think it is because the reachability test is defined as "from the
current ref". So if I am doing a reachability test on next@{20}, I will
check whether it is in the current version of next. Unreachable things
are therefore dead-end experiments. But on HEAD, if I do:

  $ git checkout topic1
  $ git checkout topic2

Then the entry for HEAD@{1} will be unreachable from the current HEAD,
even though it is _not_ a failed experiment. It is just that we are no
longer pointing to that branch.

I think another way of addressing the same problem would be to redefine
"reachable" in this context as "reachable from any current ref".

-Peff

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think another way of addressing the same problem would be to redefine
> "reachable" in this context as "reachable from any current ref".

I tend to agree, but I haven't thought through the case for entries that
point at commits on detached HEAD.  My gut feeling is that unless they are
connected to a real ref by now after all these expireunreachable days,
they are "failed experiments" and deserve to get pruned, so "from any ref"

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Nicolas Pitre]

On Thu, 15 Apr 2010, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think another way of addressing the same problem would be to redefine
> > "reachable" in this context as "reachable from any current ref".
> 
> I tend to agree, but I haven't thought through the case for entries that
> point at commits on detached HEAD.  My gut feeling is that unless they are
> connected to a real ref by now after all these expireunreachable days,
> they are "failed experiments" and deserve to get pruned, so "from any ref"
> would probably work well for them.

I'm a bit worried about this discussion.

What's the point of having a reflog for unreachable stuff if it is to be 
pruned faster than stuff that is already reachable without any reflog?


Nicolas

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Nicolas Pitre <nico@fluxnic.net> writes:

> I'm a bit worried about this discussion.
>
> What's the point of having a reflog for unreachable stuff if it is to be 
> pruned faster than stuff that is already reachable without any reflog?

To keep recently failed experiments alive for some time (30 days), but not
overly long (90 days)?

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Nicolas Pitre]

On Thu, 15 Apr 2010, Junio C Hamano wrote:

> Nicolas Pitre <nico@fluxnic.net> writes:
> 
> > I'm a bit worried about this discussion.
> >
> > What's the point of having a reflog for unreachable stuff if it is to be 
> > pruned faster than stuff that is already reachable without any reflog?
> 
> To keep recently failed experiments alive for some time (30 days), but not
> overly long (90 days)?

What is a "failed" experiment is still subjective.  It might be possible 
to realize that part of it was not that bad after all and some pieces 
could be worth cherry-picking.

Again, keeping reflogs 90 days for stuff that is _already_ reachable 
through existing refs is much less useful than keeping otherwise 
unreachable stuff 90 days.  So I still don't see the point of this 
eagerness to prune deleted stuff faster.

If you explicitly want to get rid of failed experiments then it should 
be done through an explicit prune command.  Otherwise I'd argue that 
reflogs should take care not to lose track of unreachable stuff, even 
more so than stuff already reachable.

Some people even tried to convince me that reflogs should never expire 
by default, and that the 3 month grace period was already too short.


Nicolas

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Nicolas Pitre <nico@fluxnic.net> writes:

> Again, keeping reflogs 90 days for stuff that is _already_ reachable 
> through existing refs is much less useful than keeping otherwise 
> unreachable stuff 90 days.  So I still don't see the point of this 
> eagerness to prune deleted stuff faster.

Hmph, I honestly cannot care about this very much without knowing where
this is going, because the distinction between the two has been with us
practically forever, since the day one of "reflog expire".

Is there any constructive conclusion you are aiming for?  For example, is
a proposal to update the default value of both to 90 or 120 days coming?

FWIW, I have this in my primary repository.

    [gc]
            packrefs = no
            reflogexpire = '2005-01-01 00:00:00 +0000'
            reflogexpireunreachable = '2005-01-01 00:00:00 +0000'

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Nicolas Pitre]

On Thu, 15 Apr 2010, Junio C Hamano wrote:

> Nicolas Pitre <nico@fluxnic.net> writes:
> 
> > Again, keeping reflogs 90 days for stuff that is _already_ reachable 
> > through existing refs is much less useful than keeping otherwise 
> > unreachable stuff 90 days.  So I still don't see the point of this 
> > eagerness to prune deleted stuff faster.
> 
> Hmph, I honestly cannot care about this very much without knowing where
> this is going, because the distinction between the two has been with us
> practically forever, since the day one of "reflog expire".

Argh.  OK you can paint me confused. I was mixing up 
--expire-unreachable with --stale-fix.

Having my head screwed back on now, I do agree with Jeff when he says:

|I think another way of addressing the same problem would be to redefine
|"reachable" in this context as "reachable from any current ref".

Otherwise having a special exception for HEAD would effectively make 
those unreachable objects non prunable until the HEAD reflog entries are 
themselves expired.

> Is there any constructive conclusion you are aiming for?  For example, is
> a proposal to update the default value of both to 90 or 120 days coming?

I think that the default value for reflogexpireunreachable should be the 
value of reflogexpire, and not 30 days.  In my opinion, having a shorter 
expiration period for non reachable objects by default makes little 
sense.  Again, it is for the non reachable objects that the reflog is 
most useful.


Nicolas

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Johannes Sixt]

Am 4/15/2010 18:36, schrieb Jeff King:
> I think it is because the reachability test is defined as "from the
> current ref".

Thanks, this clears my confusion. I see the problem that Junio wants to
fix, but IMHO this:

> I think another way of addressing the same problem would be to redefine
> "reachable" in this context as "reachable from any current ref".

would be a better approach for a fix rather than an exception for HEAD.

-- Hannes

Re: [PATCH 2/2] reflog: ignore expire-unreachable for "HEAD" reflog

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> But I don't see yet, why an entry pointing to a dead-end experiment is
> less important when it is an entry in a branch reflog than when it is an
> entry in the HEAD reflog. Care to explain?

You can set both expiry criteria to the same timestamp, unless you feel
"dead-end experiment is _more_ important", so I frankly don't see a strong
need to explain.  The two expiry setting came from the discussion in:

  http://thread.gmane.org/gmane.comp.version-control.git/34653/focus=34734

I can understand the "Oh, I made a commit that adds a huge garbage blob by
mistake---I want to get rid of it from my odb" use case.  It may go like
this:

    # starting from master
    git checkout maint ; did something
    git checkout next ; did something
    git checkout master
    git add huge-gunk ; git commit ; oops
    git reset --hard HEAD^

    git gc --expire-unreachable=now

In the current behaviour, HEAD reflog loses everything that is not
reachable from the current tip, just like the reflog for ordinary
branches.  You would lose the last pointer to the commit made by mistake
and the huge blob will immediately be kicked out from the odb.

But then I have to lose the record that I was working on 'next' before
coming to master, while I still have the record that says I was on
'maint'.  That is what "git checkout -" (and git log -g HEAD) will give us
after the above sequence.  I cannot explain how that could possibly be a
sane behaviour to the users.

It would be nicer to lose entries that talk about commits that do not
appear in history of any ref, and we can have the best of both worlds.
The patch does not do that, but it should be easy to implement.  Instead
of marking from the tip of the ref that the reflog we are expiring is for,
when expiring HEAD reflog, you would mark from the tips of _all_ the refs
(and you would need to clean up afterwards the same way).