threads / patch / 15099

patchMake reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Subject: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

## tl;dr

10 messages between Aug 19, 2008 and Aug 21, 2008. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Shawn O. Pearce· Aug 19, 2008, 23:44 UTC · lore

As we support seconds-since-epoch in $GIT_COMMITTER_TIME we should also support it in a reflog @{...} style notation. We can easily tell this apart from @{nth} style notation by looking to see if the value is unreasonably large for an @{nth} style notation.

The value 1112911993 was chosen for the limit as it is the commit timestamp for e83c516331 "Initial revision of "git" ...". Any reflogs in existance should contain timestamps dated later than the date Linus first stored Git into itself, as reflogs came about quite a bit after that.

Additionally a reflog with 1,112,911,993 record entries is also simply not valid. Such a reflog would require at least 87 TB to store just the old and new SHA-1 values. So our randomly chosen upper limit for @{nth} notation is "big enough" that users will not run into it by accident.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_name.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Show changes to sha1_name.c +4 −1
diff --git a/sha1_name.c b/sha1_name.c
index 4fb77f8..e25f56a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -349,7 +349,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 			else
 				nth = -1;
 		}
-		if (0 <= nth)
+		if (1112911993 <= nth) {
+			at_time = nth;
+			nth = -1;
+		} else if (0 <= nth)
 			at_time = 0;
 		else {
 			char *tmp = xstrndup(str + at + 2, reflog_len);
-- 
1.6.0.96.g2fad1.dirty

-- 
Shawn.
Junio C Hamano· Aug 19, 2008, 23:57 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 11 quoted lines
> The value 1112911993 was chosen for the limit as it is the commit
> timestamp for e83c516331 "Initial revision of "git" ...". Any
> reflogs in existance should contain timestamps dated later than
> the date Linus first stored Git into itself, as reflogs came about
> quite a bit after that.
>
> Additionally a reflog with 1,112,911,993 record entries is also
> simply not valid.  Such a reflog would require at least 87 TB to
> store just the old and new SHA-1 values.  So our randomly chosen
> upper limit for @{nth} notation is "big enough" that users will
> not run into it by accident.

Hmm, would we want to apply that logic to replace the magic "8-digit" rule in date.c::match_digit()?

Shawn O. Pearce· Aug 20, 2008, 00:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > The value 1112911993 was chosen for the limit as it is the commit
> > timestamp for e83c516331 "Initial revision of "git" ...". Any
> > reflogs in existance should contain timestamps dated later than
> > the date Linus first stored Git into itself, as reflogs came about
> > quite a bit after that.
> >
> > Additionally a reflog with 1,112,911,993 record entries is also
> > simply not valid.  Such a reflog would require at least 87 TB to
> > store just the old and new SHA-1 values.  So our randomly chosen
> > upper limit for @{nth} notation is "big enough" that users will
> > not run into it by accident.
> 
> Hmm, would we want to apply that logic to replace the magic "8-digit" rule
> in date.c::match_digit()?

No. Isn't match_digit() in the code path used by GIT_COMMITTER_DATE? We should handle dates as far back as Sat Mar 3 01:46:40 1973 as seconds-since-epoch to support conversion tools which bring in old history that predates Git's inception.

If anything the reflog code should change its test to be the same "8-digit" rule. A reflog of 100,000,000 entries requires at least 7.9G, a threshold which isn't utterly insane (as it fits on a DVD-R) but still is not really reasonable for current Git reflog.

-- 
Shawn.
Alex Riesen· Aug 20, 2008, 19:35 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Shawn O. Pearce, Wed, Aug 20, 2008 01:44:33 +0200:
Show 5 quoted lines
> The value 1112911993 was chosen for the limit as it is the commit
> timestamp for e83c516331 "Initial revision of "git" ...". Any
> reflogs in existance should contain timestamps dated later than
> the date Linus first stored Git into itself, as reflogs came about
> quite a bit after that.

Maybe I'm missing something, but aren't unsynchronized clocks a common thing in personal computing? Even maybe less common, but noticably frequent, the clocks with the date set way back in the past (by malice or accident)?

Shawn O. Pearce· Aug 20, 2008, 19:44 UTC · re: Alex Riesen · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Alex Riesen <raa.lkml@gmail.com> wrote:
Show 11 quoted lines
> Shawn O. Pearce, Wed, Aug 20, 2008 01:44:33 +0200:
> > The value 1112911993 was chosen for the limit as it is the commit
> > timestamp for e83c516331 "Initial revision of "git" ...". Any
> > reflogs in existance should contain timestamps dated later than
> > the date Linus first stored Git into itself, as reflogs came about
> > quite a bit after that.
> 
> Maybe I'm missing something, but aren't unsynchronized clocks a common
> thing in personal computing? Even maybe less common, but noticably
> frequent, the clocks with the date set way back in the past (by malice
> or accident)?

Oh, yea, clock skew is very common. Clock skew by years is not unexpected either.

We could pick any number for the limit, just so long as its so large that the size of the reflog for it to be a valid @{nth} request would be something like 1 TB, and thus be highly unlikely.

I was just trying to be cute by using the original commit timestamp of Git itself. Perhaps 12936648 (1TB / 83)?

-- 
Shawn.
Alex Riesen· Aug 20, 2008, 19:54 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Shawn O. Pearce, Wed, Aug 20, 2008 21:44:07 +0200:
Show 7 quoted lines
> 
> We could pick any number for the limit, just so long as its so
> large that the size of the reflog for it to be a valid @{nth}
> request would be something like 1 TB, and thus be highly unlikely.
> 
> I was just trying to be cute by using the original commit timestamp
> of Git itself.  Perhaps 12936648 (1TB / 83)?
How about the maximum value the platform's size_t can handle?

Not because it is "highly unlikely", but because you and me frankly have no idea exactly how unlikely for example a "12936648 terabytes" is?

Shawn O. Pearce· Aug 20, 2008, 20:00 UTC · re: Alex Riesen · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Alex Riesen <raa.lkml@gmail.com> wrote:
Show 10 quoted lines
> Shawn O. Pearce, Wed, Aug 20, 2008 21:44:07 +0200:
> > 
> > We could pick any number for the limit, just so long as its so
> > large that the size of the reflog for it to be a valid @{nth}
> > request would be something like 1 TB, and thus be highly unlikely.
> > 
> > I was just trying to be cute by using the original commit timestamp
> > of Git itself.  Perhaps 12936648 (1TB / 83)?
> 
> How about the maximum value the platform's size_t can handle?
So on 64 bit platforms we need to wait for another 2.92277266
x10^10 years before we will ever see a seconds-since-epoch which
can't possibly be mistaken for a position in the relfog file?
 
> Not because it is "highly unlikely", but because you and me frankly
> have no idea exactly how unlikely for example a "12936648 terabytes" is?

I have half a brain. Creating 12 million reflog entries would typically require 12 million git-update-ref forks. Anyone who is doing that many since reflog was introduced and has not yet truncated their reflog _really_ should reconsider what they are using it for.

Evaluating foo@{12936648} will be _horribly_ expensive. Anyone who is waiting for that result and _cares_ about it would have already started asking on the list for a reflog which is not based on a flat file. If they have already patched their Git to use something else (e.g. gdbm) I have no pity for them when this changes/breaks as they clearly have already patched their Git rather heavily.

-- 
Shawn.
Alex Riesen· Aug 20, 2008, 20:09 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Shawn O. Pearce, Wed, Aug 20, 2008 22:00:26 +0200:
Show 15 quoted lines
> Alex Riesen <raa.lkml@gmail.com> wrote:
> > Shawn O. Pearce, Wed, Aug 20, 2008 21:44:07 +0200:
> > > 
> > > We could pick any number for the limit, just so long as its so
> > > large that the size of the reflog for it to be a valid @{nth}
> > > request would be something like 1 TB, and thus be highly unlikely.
> > > 
> > > I was just trying to be cute by using the original commit timestamp
> > > of Git itself.  Perhaps 12936648 (1TB / 83)?
> > 
> > How about the maximum value the platform's size_t can handle?
> 
> So on 64 bit platforms we need to wait for another 2.92277266
> x10^10 years before we will ever see a seconds-since-epoch which
> can't possibly be mistaken for a position in the relfog file?
It is just a timestamp. Can be set to anything.
Show 7 quoted lines
> > Not because it is "highly unlikely", but because you and me frankly
> > have no idea exactly how unlikely for example a "12936648 terabytes" is?
> 
> I have half a brain.  Creating 12 million reflog entries would
> typically require 12 million git-update-ref forks. Anyone who is
> doing that many since reflog was introduced and has not yet truncated
> their reflog _really_ should reconsider what they are using it for.

Why? It may just as well work (unless there are some other, more technical restrictions).

> Evaluating foo@{12936648} will be _horribly_ expensive.  Anyone who

Depends what you evaluate it on. 640kb was also more than enough for anyone once.

Show 5 quoted lines
> is waiting for that result and _cares_ about it would have already
> started asking on the list for a reflog which is not based on a
> flat file.  If they have already patched their Git to use something
> else (e.g. gdbm) I have no pity for them when this changes/breaks
> as they clearly have already patched their Git rather heavily.
Why should you _care_?
Junio C Hamano· Aug 20, 2008, 22:20 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

"Shawn O. Pearce" <spearce@spearce.org> writes:
> I was just trying to be cute by using the original commit timestamp
> of Git itself.  Perhaps 12936648 (1TB / 83)?

Well, reverse psychology did not quite work, I guess, so I'd ask more directly. Why not re-send with an update to the same 8-digit rule we use elsewhere?

Shawn O. Pearce· Aug 21, 2008, 15:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make reflog query '@{1219188291}' act as '@{2008/08/19 16:24:51}'

Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > I was just trying to be cute by using the original commit timestamp
> > of Git itself.  Perhaps 12936648 (1TB / 83)?
> 
> Well, reverse psychology did not quite work, I guess, so I'd ask more
> directly.  Why not re-send with an update to the same 8-digit rule we use
> elsewhere?

--8<-- Make reflog query '@{1219188291}' act as '@{2008.8.19.16.24.51}'

As we support seconds-since-epoch in $GIT_COMMITTER_TIME we should also support it in a reflog @{...} style notation. We can easily tell this part from @{nth} style notation by looking to see if the value is unreasonably large for an @{nth} style notation.

The value 100000000 was chosen as it is already used by date.c to disambiguate yyyymmdd format from a seconds-since-epoch time value.

A reflog with 100,000,000 record entries is also simply not valid. Such a reflog would require at least 7.7 GB to store just the old and new SHA-1 values. So our randomly chosen upper limit for @{nth} notation is "big enough".

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 sha1_name.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Show changes to sha1_name.c +4 −1
diff --git a/sha1_name.c b/sha1_name.c
index 4fb77f8..41b6809 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -349,7 +349,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 			else
 				nth = -1;
 		}
-		if (0 <= nth)
+		if (100000000 <= nth) {
+			at_time = nth;
+			nth = -1;
+		} else if (0 <= nth)
 			at_time = 0;
 		else {
 			char *tmp = xstrndup(str + at + 2, reflog_len);
-- 
1.6.0.112.g9c75


-- 
Shawn.

← back to recent threads