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

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:22

Johannes Schindelin [off-list ref] 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

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:44:22

On Wed, 12 Mar 2008, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
---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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:44:22

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
---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.)
quoted
+	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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help