git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Joining cg-*-id
@ 2005-09-19 21:40 Pavel Roskin
  2005-09-19 21:56 ` Petr Baudis
  0 siblings, 1 reply; 22+ messages in thread
From: Pavel Roskin @ 2005-09-19 21:40 UTC (permalink / raw)
  To: Petr Baudis, git

Hello!

I believe Cogito should keep the command list short.  When the *-id
utilities got the cg-prefix, the list became longer by 3 commands.

What if we join cg-commit-id, cg-parent-id and cg-tree-id into one cg-id
(or cg-admin-id) utility?  It would work like cg-commit-id in absence of
switches, -p would make it work like cg-parent-id, and -t would make it
cg-tree-id.

Alternatively, this functionality should go to git.  It's low-level
enough to be there.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 21:40 Joining cg-*-id Pavel Roskin
@ 2005-09-19 21:56 ` Petr Baudis
  2005-09-19 22:16   ` Linus Torvalds
  2005-09-19 22:30   ` Pavel Roskin
  0 siblings, 2 replies; 22+ messages in thread
From: Petr Baudis @ 2005-09-19 21:56 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git

Dear diary, on Mon, Sep 19, 2005 at 11:40:49PM CEST, I got a letter
where Pavel Roskin <proski@gnu.org> told me that...
> Hello!

Hello,

> I believe Cogito should keep the command list short.  When the *-id
> utilities got the cg-prefix, the list became longer by 3 commands.

well, those commands are semi-public, actually. E.g. cg-help does not
list them.

> What if we join cg-commit-id, cg-parent-id and cg-tree-id into one cg-id
> (or cg-admin-id) utility?  It would work like cg-commit-id in absence of
> switches, -p would make it work like cg-parent-id, and -t would make it
> cg-tree-id.

Perhaps cg-object-id? I'm not principially opposed to that, but I don't
see much value in it either. Perhaps if this would come along with
the usage of git-rev-parse (see below)...

> Alternatively, this functionality should go to git.  It's low-level
> enough to be there.

There is git-rev-parse, but it is not as powerful (cannot take dates and
sha1 completion does not work when listing parents) - OTOH it can
complete sha1 ids even inside packs, which is really useful. Still, IMHO
its usage is awful, so it'd take place for cg-Xnormid, but we would
still keep the *-id frontends (which would also handle the dates).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 21:56 ` Petr Baudis
@ 2005-09-19 22:16   ` Linus Torvalds
  2005-09-19 22:54     ` Petr Baudis
  2005-09-20  5:52     ` Junio C Hamano
  2005-09-19 22:30   ` Pavel Roskin
  1 sibling, 2 replies; 22+ messages in thread
From: Linus Torvalds @ 2005-09-19 22:16 UTC (permalink / raw)
  To: Petr Baudis, Junio C Hamano; +Cc: Pavel Roskin, git



On Mon, 19 Sep 2005, Petr Baudis wrote:
> 
> There is git-rev-parse, but it is not as powerful (cannot take dates and
> sha1 completion does not work when listing parents)

Hmm.

That lack of sha1 completion is a bug. 

How does the date thing work? It sounds like something horribly ambiguous, 
but hey, maybe some useful semantics could be found.

The short sha1 problem is fixed thus,

		Linus

----
Fix extended short SHA1 name completion

get_sha1() would not do sha1 completion of short SHA1's when they were 
part of a more complex expression. So doing

	git-rev-parse 727132834e6be48a93c1bd6458a29d474ce7d5d5^

would work, and return 87c6aeb4efdd4355918d127a91bd0adc5a02f8ff. But using 
the shorthand version 

	git-rev-list 72713^

wouldn't work.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/sha1_name.c b/sha1_name.c
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -84,19 +84,19 @@ static int find_short_packed_object(int 
 	return 0;
 }
 
-static int get_short_sha1(const char *name, unsigned char *sha1)
+static int get_short_sha1(const char *name, int len, unsigned char *sha1)
 {
 	int i;
 	char canonical[40];
 	unsigned char res[20];
 
+	if (len < 4)
+		return -1;
 	memset(res, 0, 20);
 	memset(canonical, 'x', 40);
-	for (i = 0;;i++) {
+	for (i = 0; i < len ;i++) {
 		unsigned char c = name[i];
 		unsigned char val;
-		if (!c || i > 40)
-			break;
 		if (c >= '0' && c <= '9')
 			val = c - '0';
 		else if (c >= 'a' && c <= 'f')
@@ -112,8 +112,6 @@ static int get_short_sha1(const char *na
 			val <<= 4;
 		res[i >> 1] |= val;
 	}
-	if (i < 4)
-		return -1;
 	if (find_short_object_filename(i, canonical, sha1))
 		return 0;
 	if (find_short_packed_object(i, res, sha1))
@@ -254,7 +252,7 @@ static int get_sha1_1(const char *name, 
 	ret = get_sha1_basic(name, len, sha1);
 	if (!ret)
 		return 0;
-	return get_short_sha1(name, sha1);
+	return get_short_sha1(name, len, sha1);
 }
 
 /*

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 21:56 ` Petr Baudis
  2005-09-19 22:16   ` Linus Torvalds
@ 2005-09-19 22:30   ` Pavel Roskin
  2005-09-19 22:58     ` Petr Baudis
  1 sibling, 1 reply; 22+ messages in thread
From: Pavel Roskin @ 2005-09-19 22:30 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Hello, Petr!

Thanks for quick replies.  I'm replying to both your messages.

On Mon, 2005-09-19 at 23:56 +0200, Petr Baudis wrote:
> Dear diary, on Mon, Sep 19, 2005 at 11:40:49PM CEST, I got a letter
> where Pavel Roskin <proski@gnu.org> told me that...
> > Hello!
> 
> Hello,
> 
> > I believe Cogito should keep the command list short.  When the *-id
> > utilities got the cg-prefix, the list became longer by 3 commands.
> 
> well, those commands are semi-public, actually. E.g. cg-help does not
> list them.

But cg-<TAB><TAB> does.

> > What if we join cg-commit-id, cg-parent-id and cg-tree-id into one cg-id
> > (or cg-admin-id) utility?  It would work like cg-commit-id in absence of
> > switches, -p would make it work like cg-parent-id, and -t would make it
> > cg-tree-id.
> 
> Perhaps cg-object-id? I'm not principially opposed to that, but I don't
> see much value in it either. Perhaps if this would come along with
> the usage of git-rev-parse (see below)...

The value is having one command, one manpage and one place to fix little
bugs and add more functionality.  Potentially, cg-Xnormid could be
incorporated into that script, so bash wouldn't have to open one more
file.

> > Alternatively, this functionality should go to git.  It's low-level
> > enough to be there.
> 
> There is git-rev-parse, but it is not as powerful (cannot take dates and
> sha1 completion does not work when listing parents) - OTOH it can
> complete sha1 ids even inside packs, which is really useful. Still, IMHO
> its usage is awful, so it'd take place for cg-Xnormid, but we would
> still keep the *-id frontends (which would also handle the dates).

I see.  I guess time for git is just as abstract as phases of the moon -
nobody would goes crazy when they repeat.

Please don't apply my patch yet - I forgot to document the optional
argument.  I'll try to make cg-object-id now.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 22:16   ` Linus Torvalds
@ 2005-09-19 22:54     ` Petr Baudis
  2005-09-20  0:50       ` Linus Torvalds
  2005-09-20  5:52     ` Junio C Hamano
  1 sibling, 1 reply; 22+ messages in thread
From: Petr Baudis @ 2005-09-19 22:54 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git

Dear diary, on Tue, Sep 20, 2005 at 12:16:03AM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> How does the date thing work? It sounds like something horribly ambiguous, 
> but hey, maybe some useful semantics could be found.

Currently, in Cogito it is basically:

	git-rev-list --min-age="$(date --date=$id +%s)" --max-count=1 HEAD

The point is that you can then say

	cg-log -r yesterday

to see all the changes which happenned during the last day, and such
(so it is really useful to have the syntactic power of date(1) available).

But that semantics sucks, and I wouldn't mind changing it at all. I
think taking the previous commit works for everything but the start of
cg-log range, and it is how CVS behaves. And with cg-log, you'll just
have one commit more, not so big a deal I think.

Jonas was who introduced this, IIRC, so cc'ing him too.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 22:30   ` Pavel Roskin
@ 2005-09-19 22:58     ` Petr Baudis
  2005-09-20  0:19       ` Pavel Roskin
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Baudis @ 2005-09-19 22:58 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git

Dear diary, on Tue, Sep 20, 2005 at 12:30:21AM CEST, I got a letter
where Pavel Roskin <proski@gnu.org> told me that...
> On Mon, 2005-09-19 at 23:56 +0200, Petr Baudis wrote:
> > Dear diary, on Mon, Sep 19, 2005 at 11:40:49PM CEST, I got a letter
> > where Pavel Roskin <proski@gnu.org> told me that...
> > > I believe Cogito should keep the command list short.  When the *-id
> > > utilities got the cg-prefix, the list became longer by 3 commands.
> > 
> > well, those commands are semi-public, actually. E.g. cg-help does not
> > list them.
> 
> But cg-<TAB><TAB> does.

Good point.

> > > What if we join cg-commit-id, cg-parent-id and cg-tree-id into one cg-id
> > > (or cg-admin-id) utility?  It would work like cg-commit-id in absence of
> > > switches, -p would make it work like cg-parent-id, and -t would make it
> > > cg-tree-id.
> > 
> > Perhaps cg-object-id? I'm not principially opposed to that, but I don't
> > see much value in it either. Perhaps if this would come along with
> > the usage of git-rev-parse (see below)...
> 
> The value is having one command, one manpage and one place to fix little
> bugs and add more functionality.  Potentially, cg-Xnormid could be
> incorporated into that script, so bash wouldn't have to open one more
> file.

Well, cg-Xnormid is basically pointless if it's not going to be a common
backend for multiple other commands. So I'd say either supersede it with
git-rev-parse or just merge it with cg-object-id.

> Please don't apply my patch yet - I forgot to document the optional
> argument.  I'll try to make cg-object-id now.

Which patch?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 22:58     ` Petr Baudis
@ 2005-09-20  0:19       ` Pavel Roskin
  2005-09-20  0:26         ` Petr Baudis
  0 siblings, 1 reply; 22+ messages in thread
From: Pavel Roskin @ 2005-09-20  0:19 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

On Tue, 2005-09-20 at 00:58 +0200, Petr Baudis wrote:
> Dear diary, on Tue, Sep 20, 2005 at 12:30:21AM CEST, I got a letter
> where Pavel Roskin <proski@gnu.org> told me that...
> > The value is having one command, one manpage and one place to fix little
> > bugs and add more functionality.  Potentially, cg-Xnormid could be
> > incorporated into that script, so bash wouldn't have to open one more
> > file.
> 
> Well, cg-Xnormid is basically pointless if it's not going to be a common
> backend for multiple other commands. So I'd say either supersede it with
> git-rev-parse or just merge it with cg-object-id.

Agreed.

> > Please don't apply my patch yet - I forgot to document the optional
> > argument.  I'll try to make cg-object-id now.
> 
> Which patch?

The one that adds USAGE.  Never mind - I see it's fixed already.

As for merging, I see that the existing *-id scripts are a mess.  For
instance, cg-parent returns parents separated by spaces, but cg-diff
used "head -1" on its output to get the first parent, i.e it assumes
newline separated output.

Another thing that is broken is "cg-commit-id origin^" - apparently the
"^" parsing is only designed for cg-parent-id.

I think I'll start a local branch for fixing this, and then I'll send
separate patches.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20  0:19       ` Pavel Roskin
@ 2005-09-20  0:26         ` Petr Baudis
  0 siblings, 0 replies; 22+ messages in thread
From: Petr Baudis @ 2005-09-20  0:26 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git

Dear diary, on Tue, Sep 20, 2005 at 02:19:50AM CEST, I got a letter
where Pavel Roskin <proski@gnu.org> told me that...
> As for merging, I see that the existing *-id scripts are a mess.  For
> instance, cg-parent returns parents separated by spaces, but cg-diff
> used "head -1" on its output to get the first parent, i.e it assumes
> newline separated output.

Actually, cg-parent-id's documentation seems to be wrong. We always
return only the first parent. I've just updated the documentation for
now, even though in the future we might want to change this to reflect
the original documentation instead.

> Another thing that is broken is "cg-commit-id origin^" - apparently the
> "^" parsing is only designed for cg-parent-id.

That's a bug, IIRC it was originally meant to be general.

> I think I'll start a local branch for fixing this, and then I'll send
> separate patches.

Good idea.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 22:54     ` Petr Baudis
@ 2005-09-20  0:50       ` Linus Torvalds
  2005-09-20 13:57         ` Petr Baudis
  0 siblings, 1 reply; 22+ messages in thread
From: Linus Torvalds @ 2005-09-20  0:50 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git



On Tue, 20 Sep 2005, Petr Baudis wrote:
> 
> Currently, in Cogito it is basically:
> 
> 	git-rev-list --min-age="$(date --date=$id +%s)" --max-count=1 HEAD

Yes, for git-rev-list it makes sense, because that already more or less
"serializes" things by date.

> But that semantics sucks, and I wouldn't mind changing it at all. I
> think taking the previous commit works for everything but the start of
> cg-log range, and it is how CVS behaves.

The thing is, it may have sensible behaviour for CVS, but only because CVS 
doesn't have any notion of concurrent development (even a branch isn't 
concurrent - it's totally separate, and when you merge it, it becomes just 
one big diff at the time of the merge).

So I'd love to have

	git diff yesterday..

but the fact is, there's no sensible semantics for it. _which_ yesterday? 
There might be five different points that are "close to 24 hours ago", 
along five different paths backwards in the history.

I suspect that the date-based thing really doesn't make sense as a
revision, only as a git log cut-off (ie the "--min-age=" flag is as close
to sensibility as you can get).

		Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-19 22:16   ` Linus Torvalds
  2005-09-19 22:54     ` Petr Baudis
@ 2005-09-20  5:52     ` Junio C Hamano
  2005-09-20 10:11       ` Matthias Urlichs
  1 sibling, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2005-09-20  5:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Pavel Roskin, git

Linus Torvalds <torvalds@osdl.org> writes:

> That lack of sha1 completion is a bug. 
>
> How does the date thing work? It sounds like something horribly ambiguous, 
> but hey, maybe some useful semantics could be found.

I am not even sure what I would want to see, let alone what I
could show, when I say "show logs of what happened last week",
after I just pulled from a remote or two.

I would imagine a gitk graph with nodes colored red for commits
with last week's timestamps, would be what I would want, but
probably that is useful only because I can see in the same gitk
window how those red nodes connect with each other and with
commits outside the specified time window.  Without that context
information I would probably be lost.

> The short sha1 problem is fixed thus,

Thanks.  Applied.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20  5:52     ` Junio C Hamano
@ 2005-09-20 10:11       ` Matthias Urlichs
  2005-09-20 12:32         ` Paul Mackerras
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Urlichs @ 2005-09-20 10:11 UTC (permalink / raw)
  To: git

Hi, Junio C Hamano wrote:

> I would imagine a gitk graph with nodes colored red for commits
> with last week's timestamps,

One feature I'd like to see in this context is a gitk tree view that lists
nodes in date order, instead of the "walk down one branch as far as
possible before doing the next one" process it currently uses.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
How often I found where I should be going only by setting out for somewhere
else.
		-- R. Buckminster Fuller

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 10:11       ` Matthias Urlichs
@ 2005-09-20 12:32         ` Paul Mackerras
  0 siblings, 0 replies; 22+ messages in thread
From: Paul Mackerras @ 2005-09-20 12:32 UTC (permalink / raw)
  To: Matthias Urlichs; +Cc: git

Matthias Urlichs writes:

> One feature I'd like to see in this context is a gitk tree view that lists
> nodes in date order, instead of the "walk down one branch as far as
> possible before doing the next one" process it currently uses.

It's there already: the -d flag to gitk does that.

That shows them in date order subject to the constraint that children
are shown before parents.  Yes, it is possible to have a commit that
is older than its parent(s), if it or its parent(s) were created on a
machine whose date is set wrong.

Paul.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20  0:50       ` Linus Torvalds
@ 2005-09-20 13:57         ` Petr Baudis
  2005-09-20 14:53           ` Linus Torvalds
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Baudis @ 2005-09-20 13:57 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git

Dear diary, on Tue, Sep 20, 2005 at 02:50:21AM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> On Tue, 20 Sep 2005, Petr Baudis wrote:
> > But that semantics sucks, and I wouldn't mind changing it at all. I
> > think taking the previous commit works for everything but the start of
> > cg-log range, and it is how CVS behaves.
> 
> The thing is, it may have sensible behaviour for CVS, but only because CVS 
> doesn't have any notion of concurrent development (even a branch isn't 
> concurrent - it's totally separate, and when you merge it, it becomes just 
> one big diff at the time of the merge).
> 
> So I'd love to have
> 
> 	git diff yesterday..
> 
> but the fact is, there's no sensible semantics for it. _which_ yesterday? 
> There might be five different points that are "close to 24 hours ago", 
> along five different paths backwards in the history.

A well-defined meaning for this from Cogito standpoint would be "the
last commit on our HEAD before the date and all commits committed and
merged to the HEAD". In Cogito, you don't merge two branches _together_,
you merge one branch _into_ another (represented by the parents order),
so this would be sensible.

But we would have to sort the log in merge order for this to be possible
(and probably teach git-rev-list about it anyway).  I would absolutely
*love* to have cg-log in merge order instead of date order - longer I
see it the more I believe that it's evil and you should really want
merge order instead. The trouble is that cg-log would be no longer
incremental and would have to load the whole history first, a no-go for
any measurably long history.

By the way, did I understand it right that the only difference between
the results of merge ordering and topo ordering is the order commits
with regard to the order of parents?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 13:57         ` Petr Baudis
@ 2005-09-20 14:53           ` Linus Torvalds
  2005-09-20 15:07             ` Petr Baudis
  2005-09-20 15:13             ` Matthias Urlichs
  0 siblings, 2 replies; 22+ messages in thread
From: Linus Torvalds @ 2005-09-20 14:53 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git



On Tue, 20 Sep 2005, Petr Baudis wrote:
> > 
> > So I'd love to have
> > 
> > 	git diff yesterday..
> > 
> > but the fact is, there's no sensible semantics for it. _which_ yesterday? 
> > There might be five different points that are "close to 24 hours ago", 
> > along five different paths backwards in the history.
> 
> A well-defined meaning for this from Cogito standpoint would be "the
> last commit on our HEAD before the date and all commits committed and
> merged to the HEAD". In Cogito, you don't merge two branches _together_,
> you merge one branch _into_ another (represented by the parents order),
> so this would be sensible.

No, it's _not_ sensible or well-defined.

The order of the parents does not matter. It fundamentally _cannot_ 
matter. I realize that both git and cogito put the "primary parent" first, 
but that doesn't help one iota - because git is distributed, if the other 
side merged and we just did a fast-forward, the "primary parent" will be 
the _other_ side.

In other words, anybody who thinks that the order of parents is meaningful 
is in for some nasty shocks. It really _fundamentally_ isn't true. I 
realize that's hard to accept, but it's a truism in a distributed system. 
You really cannot have parent ordering and distribution at the same time.

> But we would have to sort the log in merge order for this to be possible
> (and probably teach git-rev-list about it anyway).

It's not sensible regardless of how you order things. There _is_ no 
ordering. 

So if you give a date, and there is a merge after that date, you will
always have at least two ways to reach the next older commit (now, if
you're lucky, it will be a common one and unambiguous, but more commonly
you'll simply have _two_ commits that are "most recent commit older than
the date".

And if you select one of them at random (or by deciding which parent is 
the "primary" one), then 

	git diff yesterday..

will show not the changes since yesterday, but _all_ the changes that came 
in through the merge.

> By the way, did I understand it right that the only difference between
> the results of merge ordering and topo ordering is the order commits
> with regard to the order of parents?

The only guarantees topo-ordering gives is that a parent will be shown 
before its children. So it may still mix up commits that happened 
between two parallell workflows, unlike merge-order that will always show 
one parallell chain of development as a contiguous set of commits.

Only merge-order gives any meaning to the order of the parents, but as 
mentioned, that is irrelevant - a merge on the other side will "flip" the 
whole world-view.

And they happen.

		Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 14:53           ` Linus Torvalds
@ 2005-09-20 15:07             ` Petr Baudis
  2005-09-20 16:54               ` Linus Torvalds
  2005-09-20 15:13             ` Matthias Urlichs
  1 sibling, 1 reply; 22+ messages in thread
From: Petr Baudis @ 2005-09-20 15:07 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git

Dear diary, on Tue, Sep 20, 2005 at 04:53:11PM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> On Tue, 20 Sep 2005, Petr Baudis wrote:
> > > 
> > > So I'd love to have
> > > 
> > > 	git diff yesterday..
> > > 
> > > but the fact is, there's no sensible semantics for it. _which_ yesterday? 
> > > There might be five different points that are "close to 24 hours ago", 
> > > along five different paths backwards in the history.
> > 
> > A well-defined meaning for this from Cogito standpoint would be "the
> > last commit on our HEAD before the date and all commits committed and
> > merged to the HEAD". In Cogito, you don't merge two branches _together_,
> > you merge one branch _into_ another (represented by the parents order),
> > so this would be sensible.
> 
> No, it's _not_ sensible or well-defined.
> 
> The order of the parents does not matter. It fundamentally _cannot_ 
> matter. I realize that both git and cogito put the "primary parent" first, 
> but that doesn't help one iota - because git is distributed, if the other 
> side merged and we just did a fast-forward, the "primary parent" will be 
> the _other_ side.
> 
> In other words, anybody who thinks that the order of parents is meaningful 
> is in for some nasty shocks. It really _fundamentally_ isn't true. I 
> realize that's hard to accept, but it's a truism in a distributed system. 
> You really cannot have parent ordering and distribution at the same time.

Ok ok. I can now remember already learning about this once, but I forgot
again. I should write it down into some README for myself or so, I
guess. ;-) Sorry about the noise.

I'll just drop the date revision specifier support from Cogito. I don't
know if any measurable number of people actually use it in the real
world anyway.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 14:53           ` Linus Torvalds
  2005-09-20 15:07             ` Petr Baudis
@ 2005-09-20 15:13             ` Matthias Urlichs
  1 sibling, 0 replies; 22+ messages in thread
From: Matthias Urlichs @ 2005-09-20 15:13 UTC (permalink / raw)
  To: git

Hi, Linus Torvalds wrote:

> And if you select one of them at random (or by deciding which parent is
> the "primary" one), then
> 
> 	git diff yesterday..
> 
> will show not the changes since yesterday, but _all_ the changes that came
> in through the merge.

IMHO the way out is to realize that you did the same thing yesterday,
so presumably, if we teach git to remember what yesterday's head was,
there's no problem.

So let's do a "git diff-since yesterday" which simply forks a
"git diff since/yesterday..HEAD", and then copies .git/HEAD to
.git/refs/since/yesterday.

Just don't call it "yesterday" in the documentation ;-)  and print a nice
error message instead of an ugly one if the user hasn't run the program
before. (But still do the copy.)

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
What if everything is an illusion and nothing exists?  In that case, I
definitely overpaid for my carpet.
		-- Woody Allen, "Without Feathers"

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 15:07             ` Petr Baudis
@ 2005-09-20 16:54               ` Linus Torvalds
  2005-09-20 19:34                 ` Linus Torvalds
  2005-09-21  8:50                 ` Petr Baudis
  0 siblings, 2 replies; 22+ messages in thread
From: Linus Torvalds @ 2005-09-20 16:54 UTC (permalink / raw)
  To: Petr Baudis, Paul Mackerras; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git



On Tue, 20 Sep 2005, Petr Baudis wrote:
> 
> I'll just drop the date revision specifier support from Cogito. I don't
> know if any measurable number of people actually use it in the real
> world anyway.

I think the date specifier makes sense for cg-log, though. 

Ie it doesn't make sense as a generic cg-*-id thing, but it _does_ make 
sense as a totally log-specific case.

Maybe it could use "-d" instead of "-r", since it's really a totally 
separate control. For example, there's nothing wrong with

	cg-log -d yesterday v2.6.12..

which would limit the revisions _both_ to a date _and_ to a version.

So it's just "diff against time" that makes no sense. But both "git log" 
and "git whatchanged" are sensible in time: there's no problem with saying 
"what commits happened in the last 24 hours", but there _is_ a problem 
with saying "show me the difference from 24 hours ago".

See? The "list of commits" is sensible, it's just the "one revision" thing 
that isn't.

Here's a stupid example of how to do this in git. 

With this silly patch, this like

	git whatchanged --since="1 month ago"
	git log --since="5 days ago"
	gitk --since=yesterday

all work. I don't know how _useful_ it is, but it's kind of cool.

(Side note: the "gitk" thing works really badly. gitk doesn't quote its 
arguments to "git-rev-list", so something like

	gitk --since="1 month ago"

does NOT work, while "--since=yesterday" does, because it has no spaces. 
Gaah. "Obi-Paul Mackerras, you're our only hope")

		Linus

---
diff --git a/rev-parse.c b/rev-parse.c
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -6,6 +6,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "refs.h"
+#include "quote.h"
 
 #define DO_REVS		1
 #define DO_NOREV	2
@@ -125,6 +126,27 @@ static int show_reference(const char *re
 	return 0;
 }
 
+static void show_datestring(const char *datestr)
+{
+	FILE *date;
+	static char buffer[100] = "--max-age=";
+	static char cmd[1000];
+	int len;
+
+	/* date handling requires both flags and revs */
+	if ((filter & (DO_FLAGS | DO_REVS)) != (DO_FLAGS | DO_REVS))
+		return;
+	snprintf(cmd, sizeof(cmd), "date --date=%s +%%s", sq_quote(datestr));
+	date = popen(cmd, "r");
+	if (!date || !fgets(buffer + 10, sizeof(buffer) - 10, date))
+		die("git-rev-list: bad date string");
+	pclose(date);
+	len = strlen(buffer);
+	if (buffer[len-1] == '\n')
+		buffer[--len] = 0;
+	show(buffer);
+}
+
 int main(int argc, char **argv)
 {
 	int i, as_is = 0, verify = 0;
@@ -207,6 +229,10 @@ int main(int argc, char **argv)
 				printf("%s/.git\n", cwd);
 				continue;
 			}
+			if (!strncmp(arg, "--since=", 8)) {
+				show_datestring(arg+8);
+				continue;
+			}
 			if (verify)
 				die("Needed a single revision");
 			show_flag(arg);

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 16:54               ` Linus Torvalds
@ 2005-09-20 19:34                 ` Linus Torvalds
  2005-09-21 10:35                   ` Paul Mackerras
  2005-09-21  8:50                 ` Petr Baudis
  1 sibling, 1 reply; 22+ messages in thread
From: Linus Torvalds @ 2005-09-20 19:34 UTC (permalink / raw)
  To: Petr Baudis, Paul Mackerras; +Cc: Junio C Hamano, Pavel Roskin, fonseca, git



On Tue, 20 Sep 2005, Linus Torvalds wrote:
> 
> (Side note: the "gitk" thing works really badly. gitk doesn't quote its 
> arguments to "git-rev-list", so something like
> 
> 	gitk --since="1 month ago"
> 
> does NOT work, while "--since=yesterday" does, because it has no spaces. 
> Gaah. "Obi-Paul Mackerras, you're our only hope")

Fixed thusly, I think.

There is 'eval' magic with pure lists. I'm getting convinced that tcl is 
evil after all - that's just _confusing_. A real language shouldn't have 
semantic differences from this change, but it clearly does.

However, getting gitk working also shows a problem with --topo-order
(which gitk uses) and the time limits. See

	[torvalds@g5 git]$ git-rev-list $(git-rev-parse --since="8 days ago" HEAD) | wc
	     80      80    3280
	[torvalds@g5 git]$ git-rev-list --topo-order $(git-rev-parse --since="8 days ago" HEAD) | wc
	     10      10     410

which is because when we sort things topologically, they won't be in date 
order any more (surprise surprise), so the whole date-cut-off logic 
doesn't actually work.

So I'm sad to say that

	gitk --since="5 days ago"

still doesn't do what you'd actually _want_ it to do with gitk. But at 
least gitk can now parse the arguments right.

		Linus

---
diff --git a/gitk b/gitk
--- a/gitk
+++ b/gitk
@@ -33,8 +33,8 @@ proc getcommits {rargs} {
     set nextupdate [expr $startmsecs + 100]
     set ncmupdate 1
     if [catch {
-	set parse_args [concat --default HEAD $rargs]
-	set parsed_args [split [eval exec git-rev-parse $parse_args] "\n"]
+	set parse_cmd [concat git-rev-parse --default HEAD $rargs]
+	set parsed_args [split [eval exec $parse_cmd] "\n"]
     }] {
 	# if git-rev-parse failed for some reason...
 	if {$rargs == {}} {

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 16:54               ` Linus Torvalds
  2005-09-20 19:34                 ` Linus Torvalds
@ 2005-09-21  8:50                 ` Petr Baudis
  1 sibling, 0 replies; 22+ messages in thread
From: Petr Baudis @ 2005-09-21  8:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Paul Mackerras, Junio C Hamano, Pavel Roskin, fonseca, git

Dear diary, on Tue, Sep 20, 2005 at 06:54:57PM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> On Tue, 20 Sep 2005, Petr Baudis wrote:
> > 
> > I'll just drop the date revision specifier support from Cogito. I don't
> > know if any measurable number of people actually use it in the real
> > world anyway.
> 
> I think the date specifier makes sense for cg-log, though. 
> 
> Ie it doesn't make sense as a generic cg-*-id thing, but it _does_ make 
> sense as a totally log-specific case.
> 
> Maybe it could use "-d" instead of "-r", since it's really a totally 
> separate control. For example, there's nothing wrong with
> 
> 	cg-log -d yesterday v2.6.12..

Yes, I agree on that. I added the -d option now and will remove the -r
date functionality soon.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-20 19:34                 ` Linus Torvalds
@ 2005-09-21 10:35                   ` Paul Mackerras
  2005-09-21 14:51                     ` Linus Torvalds
  0 siblings, 1 reply; 22+ messages in thread
From: Paul Mackerras @ 2005-09-21 10:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Junio C Hamano, Pavel Roskin, fonseca, git

Linus Torvalds writes:

> On Tue, 20 Sep 2005, Linus Torvalds wrote:
> > 
> > (Side note: the "gitk" thing works really badly. gitk doesn't quote its 
> > arguments to "git-rev-list", so something like
> > 
> > 	gitk --since="1 month ago"
> > 
> > does NOT work, while "--since=yesterday" does, because it has no spaces. 
> > Gaah. "Obi-Paul Mackerras, you're our only hope")

<scratches head> It works for me - with your rev-list.c and
rev-parse.c patches, an unmodified gitk (as in current git.git) works
just fine; both gitk --since=yesterday and gitk --since="1 day ago"
give the same set of commits (two merges and an NTFS change, plus
"Linux v2.6.14-rc2" and a SCSI zfcp patch shown with open circles).

> There is 'eval' magic with pure lists. I'm getting convinced that tcl is 
> evil after all - that's just _confusing_. A real language shouldn't have 
> semantic differences from this change, but it clearly does.

I can't see any semantic difference...

Can you still replicate the problem with an unmodified gitk?

Paul.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-21 10:35                   ` Paul Mackerras
@ 2005-09-21 14:51                     ` Linus Torvalds
  2005-09-21 21:19                       ` Paul Mackerras
  0 siblings, 1 reply; 22+ messages in thread
From: Linus Torvalds @ 2005-09-21 14:51 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Petr Baudis, Junio C Hamano, Pavel Roskin, fonseca, git



On Wed, 21 Sep 2005, Paul Mackerras wrote:
> 
> Can you still replicate the problem with an unmodified gitk?

Oooh. I can't. Very strange. I definitely could yesterday:  
"--since=yesterday" worked, but "--since='1 week ago'" didn't.

Magic. Maybe I mistyped something.

		Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Joining cg-*-id
  2005-09-21 14:51                     ` Linus Torvalds
@ 2005-09-21 21:19                       ` Paul Mackerras
  0 siblings, 0 replies; 22+ messages in thread
From: Paul Mackerras @ 2005-09-21 21:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Junio C Hamano, Pavel Roskin, fonseca, git

Linus Torvalds writes:

> Oooh. I can't. Very strange. I definitely could yesterday:  
> "--since=yesterday" worked, but "--since='1 week ago'" didn't.

Ahhh... "--since='1 week ago'" doesn't work because of the extra set
of single quotes: we end up doing:

	git-rev-parse --default HEAD "--since='1 week ago'"

which gives this error:

	date: invalid date `'1 week ago''
	fatal: git-rev-list: bad date string

Gitk's error message is admittedly less than informative, but that's
the worst it's guilty of (this time :).

Paul.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2005-09-21 21:19 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-19 21:40 Joining cg-*-id Pavel Roskin
2005-09-19 21:56 ` Petr Baudis
2005-09-19 22:16   ` Linus Torvalds
2005-09-19 22:54     ` Petr Baudis
2005-09-20  0:50       ` Linus Torvalds
2005-09-20 13:57         ` Petr Baudis
2005-09-20 14:53           ` Linus Torvalds
2005-09-20 15:07             ` Petr Baudis
2005-09-20 16:54               ` Linus Torvalds
2005-09-20 19:34                 ` Linus Torvalds
2005-09-21 10:35                   ` Paul Mackerras
2005-09-21 14:51                     ` Linus Torvalds
2005-09-21 21:19                       ` Paul Mackerras
2005-09-21  8:50                 ` Petr Baudis
2005-09-20 15:13             ` Matthias Urlichs
2005-09-20  5:52     ` Junio C Hamano
2005-09-20 10:11       ` Matthias Urlichs
2005-09-20 12:32         ` Paul Mackerras
2005-09-19 22:30   ` Pavel Roskin
2005-09-19 22:58     ` Petr Baudis
2005-09-20  0:19       ` Pavel Roskin
2005-09-20  0:26         ` Petr Baudis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).