git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git-log --parents broken post v1.3.0
@ 2006-05-03 11:56 Martin Langhoff
  2006-05-03 12:10 ` Martin Langhoff
  0 siblings, 1 reply; 5+ messages in thread
From: Martin Langhoff @ 2006-05-03 11:56 UTC (permalink / raw)
  To: git, Junio C Hamano, Linus Torvalds

Soon after v1.3.0 git-log --parents got broken. When using --parents,
the 'commit <sha1>' opening line would also list the SHA1 of the
parent commits. This seems to have broken git-cvsserver.

Just by testing git-cvsserver, git-bisect has said that

  9153983310a169a340bd1023dccafd80b70b05bc is first bad commit
  Author: Linus Torvalds <torvalds@osdl.org>
  Date:   Mon Apr 17 11:59:32 2006 -0700

    Log message printout cleanups

But I am not too confident that it's this particular commit -- but it
is definitely one in this series. I had suspected of changes in
git-diff-tree, but the output of git-diff-tree remains unchanged as
far as I could test it.

cheers,


martin

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

* Re: git-log --parents broken post v1.3.0
  2006-05-03 11:56 git-log --parents broken post v1.3.0 Martin Langhoff
@ 2006-05-03 12:10 ` Martin Langhoff
  2006-05-03 14:59   ` Linus Torvalds
  0 siblings, 1 reply; 5+ messages in thread
From: Martin Langhoff @ 2006-05-03 12:10 UTC (permalink / raw)
  To: git, Junio C Hamano, Linus Torvalds

On 5/3/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> Soon after v1.3.0 git-log --parents got broken. When using --parents,

Ok -- perhaps that was a bit of a rushed statement. Reading back on
the archives, it seems like it may have been intentional. The new
header format is more succint, but while technically I can fish the
data at the porcelain layer, it is quite convenient to get it directly
from the commit header. Specially given that git-log has the info at
that time...

I have to confess, I don't quite follow the changes happening in that
series of commits. If --parents is really not coming back I'll change
the log entry parsing in cvsserver. However, I suspect git-log should
error out on it ("fatal: deprecated option") so porcelains break
explicitly, rather than silently.

cheers,


martin

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

* Re: git-log --parents broken post v1.3.0
  2006-05-03 12:10 ` Martin Langhoff
@ 2006-05-03 14:59   ` Linus Torvalds
  2006-05-03 22:14     ` Martin Langhoff
  2006-05-03 22:53     ` [PATCH] cvsserver: use git-rev-list instead of git-log Martin Langhoff
  0 siblings, 2 replies; 5+ messages in thread
From: Linus Torvalds @ 2006-05-03 14:59 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, Junio C Hamano



On Thu, 4 May 2006, Martin Langhoff wrote:

> On 5/3/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> > Soon after v1.3.0 git-log --parents got broken. When using --parents,
> 
> Ok -- perhaps that was a bit of a rushed statement. Reading back on
> the archives, it seems like it may have been intentional.

No it wasn't. "git log --parents" was definitely supposed to still work. 

That said, I suspect a git-cvsserver kind of usage is better off using 
"git-rev-list --parents HEAD" instead, which didn't break in the first 
place.

> I have to confess, I don't quite follow the changes happening in that
> series of commits. If --parents is really not coming back I'll change
> the log entry parsing in cvsserver. However, I suspect git-log should
> error out on it ("fatal: deprecated option") so porcelains break
> explicitly, rather than silently.

No, the option really isn't deprecated, it was just missed because nothing 
seemed to use it.. How about this diff?

Signed-off-by: Yadda yadda

		Linus

---
diff --git a/log-tree.c b/log-tree.c
index 9634c46..f9c6f7c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -3,6 +3,15 @@ #include "diff.h"
 #include "commit.h"
 #include "log-tree.h"
 
+static void show_parents(struct commit *commit, int abbrev)
+{
+	struct commit_list *p;
+	for (p = commit->parents; p ; p = p->next) {
+		struct commit *parent = p->item;
+		printf(" %s", diff_unique_abbrev(parent->object.sha1, abbrev));
+	}
+}
+
 void show_log(struct rev_info *opt, struct log_info *log, const char *sep)
 {
 	static char this_header[16384];
@@ -14,7 +23,10 @@ void show_log(struct rev_info *opt, stru
 
 	opt->loginfo = NULL;
 	if (!opt->verbose_header) {
-		puts(sha1_to_hex(commit->object.sha1));
+		fputs(diff_unique_abbrev(commit->object.sha1, abbrev_commit), stdout);
+		if (opt->parents)
+			show_parents(commit, abbrev_commit);
+		putchar('\n');
 		return;
 	}
 
@@ -40,6 +52,8 @@ void show_log(struct rev_info *opt, stru
 	printf("%s%s",
 		opt->commit_format == CMIT_FMT_ONELINE ? "" : "commit ",
 		diff_unique_abbrev(commit->object.sha1, abbrev_commit));
+	if (opt->parents)
+		show_parents(commit, abbrev_commit);
 	if (parent) 
 		printf(" (from %s)", diff_unique_abbrev(parent->object.sha1, abbrev_commit));
 	putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n');

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

* Re: git-log --parents broken post v1.3.0
  2006-05-03 14:59   ` Linus Torvalds
@ 2006-05-03 22:14     ` Martin Langhoff
  2006-05-03 22:53     ` [PATCH] cvsserver: use git-rev-list instead of git-log Martin Langhoff
  1 sibling, 0 replies; 5+ messages in thread
From: Martin Langhoff @ 2006-05-03 22:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Junio C Hamano

On 5/4/06, Linus Torvalds <torvalds@osdl.org> wrote:
> That said, I suspect a git-cvsserver kind of usage is better off using
> "git-rev-list --parents HEAD" instead, which didn't break in the first
> place.

After a good sleep, I woke up thinking exactly the same. Patch coming soon.


m

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

* [PATCH] cvsserver: use git-rev-list instead of git-log
  2006-05-03 14:59   ` Linus Torvalds
  2006-05-03 22:14     ` Martin Langhoff
@ 2006-05-03 22:53     ` Martin Langhoff
  1 sibling, 0 replies; 5+ messages in thread
From: Martin Langhoff @ 2006-05-03 22:53 UTC (permalink / raw)
  To: git, junkio; +Cc: Martin Langhoff

On 5/4/06, Linus Torvalds <torvalds@osdl.org> wrote:
> No it wasn't. "git log --parents" was definitely supposed to still work.
>
> That said, I suspect a git-cvsserver kind of usage is better off using
> "git-rev-list --parents HEAD" instead, which didn't break in the first
> place.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>


---

 git-cvsserver.perl |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

a248c9614fdd130229fb5f9565abbd77bd1d0cc9
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 11d153c..ffd9c66 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -2076,14 +2076,15 @@ sub update
     # TODO: log processing is memory bound
     # if we can parse into a 2nd file that is in reverse order
     # we can probably do something really efficient
-    my @git_log_params = ('--parents', '--topo-order');
+    my @git_log_params = ('--pretty', '--parents', '--topo-order');
 
     if (defined $lastcommit) {
         push @git_log_params, "$lastcommit..$self->{module}";
     } else {
         push @git_log_params, $self->{module};
     }
-    open(GITLOG, '-|', 'git-log', @git_log_params) or die "Cannot call git-log: $!";
+    # git-rev-list is the backend / plumbing version of git-log
+    open(GITLOG, '-|', 'git-rev-list', @git_log_params) or die "Cannot call git-rev-list: $!";
 
     my @commits;
 
-- 
1.3.1.g24e1-dirty

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

end of thread, other threads:[~2006-05-03 22:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-03 11:56 git-log --parents broken post v1.3.0 Martin Langhoff
2006-05-03 12:10 ` Martin Langhoff
2006-05-03 14:59   ` Linus Torvalds
2006-05-03 22:14     ` Martin Langhoff
2006-05-03 22:53     ` [PATCH] cvsserver: use git-rev-list instead of git-log Martin Langhoff

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).