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

2 messages, 2 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:
quoted
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

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
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help