Git development
 help / color / mirror / Atom feed
* Re: [PATCH] cvsimport move over to using git for each ref to read refs
From: Junio C Hamano @ 2006-09-20 16:45 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <45116888.4050806@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

>> About the quoting and parsing, language specific quoting mode is
>> meant for git-for-each-ref to produce a string that can be eval'ed
>> in the host language.  Think of the command as a tool to write a
>> short program for you.
>
> Thanks for the education.  Very simple, and very powerful.  I knew there
> was a reason for it out there.  Will respin a V3 patch in a bit.

This probably showed that my initial description and example for
the feature found in Documentation/git-for-each-ref was lacking.

I would appreciate a separate patch to enhance it if you are so
inclined.

^ permalink raw reply

* Re: [OT] What do you use to find message on git mailing list with given Message-Id?
From: Junio C Hamano @ 2006-09-20 16:48 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <eerb10$7k5$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Petr Baudis wrote:
>
>>   Unfortunately I haven't found any either. I use /~i <messageid> in
>> mutt.
>
> I have found two places for git mailing list. I was hoping for something 
> more universal, but alas, no such luck (and most probably not possible)
>
> Example Message-ID:
>
>  Message-ID: <200609200739.48732.alan@chandlerfamily.org.uk>
>
> Links to git mailing list archive: MARC and GMane (out of MARC, GMane,
> Gelato, Spinics, mail-archive)
>
>  http://marc.theaimsgroup.com/?i=200609200739.48732.alan%20()%20chandlerfamily%20!%20org%20!%20uk
>  http://news.gmane.org/find-root.php?message_id=<200609200739.48732.alan@chandlerfamily.org.uk> 

Yeah, the latter is what I always end up doing, after finding
out the message-id using Pasky's local mailstore method, but I
always have hard time remembering if the keyword is message-id
or message_id X-<.

Thanks for documenting them.
 

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: Shawn Pearce @ 2006-09-20 16:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <7v1wq6jy3q.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> I realize I am going around in circles, but Pasky's "remotes/"
> argument made me realize that this mirroring is not much more
> than "fetch --force --all".  I initially had an impression that
> this was for only strict mirroring where you do not even want
> your own "origin", but if you arrange the .git/remotes/origin
> file the right way, "fetch --force --all" (if you remembered to
> put '+' in front of the refspecs, even without --force) would
> what --mirror-all would do wouldn't it?

I started this change with '--all' and realized that ideally you
want '--all' to copy all available refs/heads/* from the remote to
refs/remotes/<name>/* here.  You want to create any new branches
which the remote has introduced since your last fetch.

You probably don't want to force a non-fast forward unless there's a
'+' in the corresponding Pull line of remotes/<name> or if --force is
used.  However you probably also want to delete any removed branches.

Which I think is quite different from a mirror.  A mirror wants to
replace the entire ref namespace with what's on the remote as it
has no need for a local namespace of its own.


Originally I gave Pasky a one-liner on #git:

  git fetch --force origin $(git ls-remote origin \
      | awk '{if(!/\^{}$/){print $2":"$2}}')

but he expressed interest in it being a native feature of the
core-Git fetch Porcelain.  To be honest I disagreed with him but
submitted the patch anyway.

I think --all copying into .git/refs/remotes/<name>/* makes perfect
sense.

And I think this mirror thing may make more sense as a small wrapper
around git-fetch.  A wrapper that checks for:

	- its running in a bare repository;
	- it has a single remote named origin;
	- HEAD isn't a symlink or a symref (its a normal ref in its
	  own right);
	- git-mirror.permitted is true in the config file.
 
-- 
Shawn.

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Linus Torvalds @ 2006-09-20 16:49 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Petr Baudis, Johannes Schindelin, git
In-Reply-To: <20060920163437.GC23260@spearce.org>



On Wed, 20 Sep 2006, Shawn Pearce wrote:
> > 
> > This is not just a random thing. It is in fact one of the very core issues 
> > of having multiple people work together on the same remote repo. We don't 
> > do it very much (because it's often easier for everybody to have their 
> > own), but the "CVS workflow" with one common repository is another example 
> > why WE MUST NOT JUST RESET THE HEADS!
> 
> BTW `git push --force` works just great to reset the remote head.

Yes. That's why "--force" exists - it's a way of saying "the other end is 
wrong, and I really do want to force this update".

> I worked on a project not to long ago in which a user tried `git
> push`, received a "not a fast-forward" error, didn't know what it
> meant, tried `git push --force`, found that worked, and proceeded
> to force every push he did from then on.  To much gnashing of teeth
> from everyone else.

Ouch. That implies that we made it a bit too easy to force things, or that 
we have an insufficiently clear error message.

I think the current error message is fairly good: it says

	"remote '%s' is not a strict subset of local ref '%s'. maybe you 
	 are not up-to-date and need to pull first?"

which should be clear enough, but I'm hoping this was a long time ago when 
we weren't as clear (we added the "maybe you're not up-to-date .." 
language later)

> Of course an update hook finally took care of the problem, but having
> non fast-forward pushs be permitted on a shared, bare repository
> by default is interesting to say the least.  :-)

Yeah, well, it's not permitted "by default", but obviously "--force" ends 
up being a client-side decision, so with clueless clients, the default 
behaviour may not be enough to save you.

			Linus

^ permalink raw reply

* Re: [PATCH] cvsimport move over to using git for each ref to read refs V3
From: Junio C Hamano @ 2006-09-20 16:55 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <20060920163704.GA27407@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

> +	my $fmt = '($ref, $author) = (%(refname), %(author));';
> +	open(H, "git-for-each-ref --perl --format='$fmt'|") or
> +		die "Cannot run git-for-each-ref: $!\n";
> +	while(defined(my $entry = <H>)) {
> +		my ($ref, $author);
> +		eval($entry) || die "cannot eval refs list: $@";
> +
> +		next if ($ref !~ m@^refs/heads/(.*)$@);
> +		my ($head) = ($1);
> +		$author =~ /^.*\s(\d+)\s[-+]\d{4}$/;
> +		$branch_date{$head} = $1;
>  	}
> -	closedir(D);
> +	close(H);
>  }

for-each-ref let's you limit the refs by leading path, so I do
not think "next if" inside the loop is needed if you say

	for-each-ref --perl --format=$fmt refs/heads

Any reason you did not like my version that allows you to get
rid of the while() loop altogether?  (hint: replace eval in my
example with "print" and see what you are evaling)

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Shawn Pearce @ 2006-09-20 16:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Stephen Hemminger, Jeff Garzik, git
In-Reply-To: <Pine.LNX.4.64.0609200902190.4388@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:
> The thing is, if you don't understand how rebasing etc destroys history, 
> you may do things like do a "git pull" or a "git merge" of a branch that 
> the other side WILL THROW AWAY! That will later result in major pain, 
> because when you then try to merge it later, you will get all kinds of 
> nasty behaviour, because the history you merged earlier no longer matches 
> the history you're now trying to merge again, and the work you merged 
> earlier is simply not there any more.

Yet people (typically those new to Git) will still pull or merge
the wrong branch in, work on top of that merge, publish it, others
will build on that... and wham; that topic branch head which you
wanted to rebase prior to merging is now wedged 50 commits deep in
your history.

Just yesterday I found such a case in a shared repository.  Now I
have a branch wedged in our shared mainline that I can't get out
and shouldn't have been there in the first place.


If only the shared repository had a way of advising clients that
commits stored in ref 'BAAAD' may not survive and thus shouldn't
be merged.  So that git-merge wouldn't let you merge them in.
Unfortunately there isn't a way to do this that's sane so I'm not
even going to try.


Probably what I should have done (now that I think about it) was to
put a check into our update hook on the shared repository to look for
a rebaseable branch (which are listed in some info file) being pushed
into a non-rebaseable one.  If that happens then abort the update.

Unfortunately our current Git client (1.4.2)/Git server version(1.3.1)
combinations means we get no output from our update hook when it
fails, so I can't tell the newbie what they did wrong.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] cvsimport move over to using git for each ref to read refs V3
From: Andy Whitcroft @ 2006-09-20 17:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodtaiijt.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> +	my $fmt = '($ref, $author) = (%(refname), %(author));';
>> +	open(H, "git-for-each-ref --perl --format='$fmt'|") or
>> +		die "Cannot run git-for-each-ref: $!\n";
>> +	while(defined(my $entry = <H>)) {
>> +		my ($ref, $author);
>> +		eval($entry) || die "cannot eval refs list: $@";
>> +
>> +		next if ($ref !~ m@^refs/heads/(.*)$@);
>> +		my ($head) = ($1);
>> +		$author =~ /^.*\s(\d+)\s[-+]\d{4}$/;
>> +		$branch_date{$head} = $1;
>>  	}
>> -	closedir(D);
>> +	close(H);
>>  }
> 
> for-each-ref let's you limit the refs by leading path, so I do
> not think "next if" inside the loop is needed if you say
> 
> 	for-each-ref --perl --format=$fmt refs/heads

Stupid boy :)

> 
> Any reason you did not like my version that allows you to get
> rid of the while() loop altogether?  (hint: replace eval in my
> example with "print" and see what you are evaling)

To my mind we avoid the 'mozilla repo' issue of 1000's of heads
busting some string length limit, or requiring some _vast_ string to
hold it (as perl is likely to cope).  Given we are sifting a small
percentage of the data out of it.

Let me know which way you want it and we'll go that way :).

-apw

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Shawn Pearce @ 2006-09-20 17:10 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Johannes Schindelin, git
In-Reply-To: <Pine.LNX.4.64.0609200942550.4388@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:
> On Wed, 20 Sep 2006, Shawn Pearce wrote:
> > I worked on a project not to long ago in which a user tried `git
> > push`, received a "not a fast-forward" error, didn't know what it
> > meant, tried `git push --force`, found that worked, and proceeded
> > to force every push he did from then on.  To much gnashing of teeth
> > from everyone else.
> 
> Ouch. That implies that we made it a bit too easy to force things, or that 
> we have an insufficiently clear error message.

I've been lucky that I've only run into two people in my life that
when faced with an error message they don't understand immediately
try adding "-f" and "--force" to the command line until something
happens.  Its entertaining to read their terminal scrollback and
see what they did in response to errors; its less so when they've
done mildy destructive things that you now must cleanup.

Sometimes I wonder if they've managed to reformat their root
filesystem while they had it mounted.  Never asked.  Not sure I
want to hear the answer.
 
> I think the current error message is fairly good: it says
> 
> 	"remote '%s' is not a strict subset of local ref '%s'. maybe you 
> 	 are not up-to-date and need to pull first?"
> 
> which should be clear enough, but I'm hoping this was a long time ago when 
> we weren't as clear (we added the "maybe you're not up-to-date .." 
> language later)

Yes; this problem was back with Git 1.2 so the newer language is
much better and should help new users better.
 
> > Of course an update hook finally took care of the problem, but having
> > non fast-forward pushs be permitted on a shared, bare repository
> > by default is interesting to say the least.  :-)
> 
> Yeah, well, it's not permitted "by default", but obviously "--force" ends 
> up being a client-side decision, so with clueless clients, the default 
> behaviour may not be enough to save you.

I'm wondering if maybe git-receive-pack should deny forcing an
update in a shared repository unless there's either an update hook
that its going to run (which would get to vote yea or neigh) or
there's a configuration setting enabled which isn't set by default.

I'd think most users of a shared repository wouldn't want to allow
forcing an update except in some very special cases.  For those
they could install an update hook or just push a new temporary
branch name and then use git-update-ref or git-branch directly on
the remote repository.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: Junio C Hamano @ 2006-09-20 17:13 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git, Petr Baudis
In-Reply-To: <20060920164912.GD23260@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> I started this change with '--all' and realized that ideally you
> want '--all' to copy all available refs/heads/* from the remote to
> refs/remotes/<name>/* here.  You want to create any new branches
> which the remote has introduced since your last fetch.
>
> You probably don't want to force a non-fast forward unless there's a
> '+' in the corresponding Pull line of remotes/<name> or if --force is
> used.  However you probably also want to delete any removed branches.

I think we are in agreement here.

> Which I think is quite different from a mirror.  A mirror wants to
> replace the entire ref namespace with what's on the remote as it
> has no need for a local namespace of its own.

If you have remotes/origin that does not use the refs/remotes
hierarchy, does not copy master->origin (iow, the assumption
that "origin" branch is to be merged into local "master") but
copies remote master to local master, and forces all refs by
default, then your local ref namespace is effectively a mirror
of the remote, so it is very tempting to think --mirror is a
logical extension of "fetch --all".

Except that new refs are not created and old refs are not
deleted.  And these two are rather big exceptions.

If we are to have another command (git-mirror which is a thin
wrapper around git-fetch as you propose later), these two
exceptions would be cleanly handled there.  Otherwise, we would
need to give the command a hint (git-mirror.permitted) to allow
them to be done in addition to the usual "fetch --all", but then
we would end up doing the equivalent of your one-liner anyway,
so there is not much point having it in git-fetch.  So I am in
favor of your thin-wrapper idea.

> Originally I gave Pasky a one-liner on #git:
>
>   git fetch --force origin $(git ls-remote origin \
>       | awk '{if(!/\^{}$/){print $2":"$2}}')

Which sounds a very sane thing to do, especially given git is
designed to be scripted this way.

> And I think this mirror thing may make more sense as a small wrapper
> around git-fetch.  A wrapper that checks for:
>
> 	- its running in a bare repository;

Ok.

> 	- it has a single remote named origin;

Do you mean ".git/remotes must have only one file 'origin' and
nothing else", or do you mean "There must be .git/remotes/foobla
for 'git-mirror foobla' to work"?

> 	- HEAD isn't a symlink or a symref (its a normal ref in its
> 	  own right);

Why?  Usually HEAD is a pointer to the active branch.  I
consider it is a bug in the upload-pack protocol not to convey
that information (but that is a separate topic).

> 	- git-mirror.permitted is true in the config file.

Ok.

^ permalink raw reply

* Re: [PATCH] cvsimport move over to using git for each ref to read refs V3
From: Junio C Hamano @ 2006-09-20 17:17 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <451173B1.4060305@shadowen.org>

Andy Whitcroft <apw@shadowen.org> writes:

> Junio C Hamano wrote:
>> 
>> for-each-ref let's you limit the refs by leading path, so I do
>> not think "next if" inside the loop is needed if you say
>> 
>> 	for-each-ref --perl --format=$fmt refs/heads
>
> Stupid boy :)
>> 
>> Any reason you did not like my version that allows you to get
>> rid of the while() loop altogether?  (hint: replace eval in my
>> example with "print" and see what you are evaling)
>
> To my mind we avoid the 'mozilla repo' issue of 1000's of heads
> busting some string length limit, or requiring some _vast_ string to
> hold it (as perl is likely to cope).  Given we are sifting a small
> percentage of the data out of it.

Quite valid reasoning.

Will apply V3 with refs/heads filtering added; no need to
resend.

^ permalink raw reply

* Re: Subversion-style incrementing revision numbers
From: Robin Rosenberg @ 2006-09-20 17:28 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Linus Torvalds, Joel Dice, git
In-Reply-To: <20060919220604.GE8259@pasky.or.cz>

onsdag 20 september 2006 00:06 skrev Petr Baudis:
[...]
> But having the commits
> numbered inside a repository would indeed make for simple usage if you
> need to type in commit ids frequently, and could make Git a bit
> friendlier to newcomers.

Typing those numbers is really insane, so the idea never occurred to me. Most 
environments have simple methods to copy and paste strings using a clipboard,
even in a console. So typically you can left-doubleclick on a string and paste 
with the middle button in *nix like systems (right button in cygwin/windows 
command line).

-- robin

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: Shawn Pearce @ 2006-09-20 17:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <7vac4uihr3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> > Originally I gave Pasky a one-liner on #git:
> >
> >   git fetch --force origin $(git ls-remote origin \
> >       | awk '{if(!/\^{}$/){print $2":"$2}}')
> 
> Which sounds a very sane thing to do, especially given git is
> designed to be scripted this way.

Exactly.  So I'm not sure why he was against it.  After all he must
have some script that's looping over all mirrored repositories and
invoking git fetch in them.  :-)
 
> > 	- it has a single remote named origin;
> 
> Do you mean ".git/remotes must have only one file 'origin' and
> nothing else", or do you mean "There must be .git/remotes/foobla
> for 'git-mirror foobla' to work"?

I meant ".git/remotes must have only one file 'origin' and
nothing else".  Because what does it mean to mirror two different
repositories at the same time into the same ref namespace?  The most
recently fetched would always win.

But now that I'm writing this I can see someone wanting multiple
remotes so they could store different URLs for the same repository
and call fetch on another if the first isn't responding.  So I
retract this entirely.  So there doesn't even need to be remotes
file; the URL could just be passed on the command line.  A remotes
file (like the default "origin" one) is just a handy place to store
the URL.
 
> > 	- HEAD isn't a symlink or a symref (its a normal ref in its
> > 	  own right);
> 
> Why?  Usually HEAD is a pointer to the active branch.  I
> consider it is a bug in the upload-pack protocol not to convey
> that information (but that is a separate topic).

Because we don't want to update HEAD with the SHA1 that's on the
remote if that's actually going to update some other ref which
doesn't have the same value.  :-)

I think its simple enough to verify HEAD isn't a symlink or symref
(or just doesn't exist) early in the mirror script.  If the user
really wants to do a mirror and they just did a `git init-db --bare`
they could also do a `rm HEAD` before starting `git-mirror`.

But I agree with you entirely about HEAD being a SHA1 and not its
actual link target in the network protocol.  An annoying bug.

-- 
Shawn.

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Linus Torvalds @ 2006-09-20 17:34 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Petr Baudis, Stephen Hemminger, Jeff Garzik, git
In-Reply-To: <20060920165931.GE23260@spearce.org>



On Wed, 20 Sep 2006, Shawn Pearce wrote:
> 
> Yet people (typically those new to Git) will still pull or merge
> the wrong branch in, work on top of that merge, publish it, others
> will build on that... and wham; that topic branch head which you
> wanted to rebase prior to merging is now wedged 50 commits deep in
> your history.

Yes. It might well be a good idea to mark temporary branches some way on 
the sending side, and have "git pull" honor that marking by default.

The only really good marking we'd have (unless we extended the protocol a 
lot) is name-based, ie we could have a separate directory for "temporary 
branches". 

Of course, nothing will ever really avoid outright mistakes, which is 
probably the bulk of things. I think a lot of those go away when you get 
used to the flow, but especially in the beginning, people _will_ make 
mistakes.

So maybe trying to avoid them too much is just futile.

		Linus

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: Junio C Hamano @ 2006-09-20 17:50 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git, Petr Baudis
In-Reply-To: <20060920173126.GG23260@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> I meant ".git/remotes must have only one file 'origin' and
> nothing else".  Because what does it mean to mirror two different
> repositories at the same time into the same ref namespace?  The most
> recently fetched would always win.
>
> But now that I'm writing this I can see someone wanting multiple
> remotes so they could store different URLs for the same repository
> and call fetch on another if the first isn't responding.  So I
> retract this entirely.  So there doesn't even need to be remotes
> file; the URL could just be passed on the command line.  A remotes
> file (like the default "origin" one) is just a handy place to store
> the URL.

Another possibility.  The mirror might want to do something
like:

        .git/refs/remotes/davem/sparc-2.6.git/...
	.git/refs/remotes/torvalds/linux-2.6.git/...
	.git/refs/remotes/jgarzik/netdev-2.6.git/...

to be a super-mirror of related projects.  This would give a
convenient way for the visitors to browse them all at one place,
and would save object store.  There are minor details such as
gitweb may not allow browsing refs/remotes and git-fetch may not
let you fetch from refs/remotes (I haven't checked), but they
are fixable as needed.

> Because we don't want to update HEAD with the SHA1 that's on the
> remote if that's actually going to update some other ref which
> doesn't have the same value.  :-)

What I thought you were going to do with HEAD would be to update
it not by the object name but modifying where it points at.
IOW, if HEAD is a symlink pointing at refs/heads/master and the
remote HEAD now points at refs/heads/next, you do not:

	echo $the_commit_object_name_of_remote_next >.git/HEAD 

which obviously is insane, but you would do

	rm -f .git/HEAD && ln -s refs/heads/next .git/HEAD

> But I agree with you entirely about HEAD being a SHA1 and not its
> actual link target in the network protocol.  An annoying bug.

Which fortunately is fixable ;-).

^ permalink raw reply

* Re: [PATCH 2/2] gitweb: Make git_get_refs_list do work of git_get_references
From: Junio C Hamano @ 2006-09-20 18:12 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200609191433.22878.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Make git_get_refs_list do also work of git_get_references, to avoid
> calling git-peek-remote twice.  Change meaning of git_get_refs_list
> meaning: it is now type, and not a full path, e.g. we now use
> git_get_refs_list("heads") instead of former
> git_get_refs_list("refs/heads").
>
> Modify git_summary to use only one call to git_get_refs_list instead
> of one call to git_get_references and two to git_get_refs_list.

Will apply as-is; the following comments might be useful for
further improvements.

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 034a88c..01fae94 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1127,14 +1128,21 @@ sub git_get_refs_list {
> +		if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?([^\^]+))(\^\{\})?$/) {
> +			if (defined $refs{$1}) {
> +				push @{$refs{$1}}, $2;
> +			} else {
> +				$refs{$1} = [ $2 ];
> +			}

Logically this should be "exists $refs{$1}" but defined would
work fine in this particular case, because the elements of the
hash are always an arrayref (this is just an advise to have the
right habit).

> +			if (! $4) { # unpeeled, direct reference
> +				push @refs, { hash => $1, name => $3 }; # without type
> +			} elsif ($3 eq $refs[-1]{'name'}) {
> +				# most likely a tag is followed by its peeled
> +				# (deref) one, and when that happens we know the
> +				# previous one was of type 'tag'.
> +				$refs[-1]{'type'} = "tag";
> +			}
>...
> -	return \@reflist;
> +	return (\@reflist, \%refs);

You are maintaining reflist (an array of hashrefs each element
of which describes name, type, hash and other parse_ref()
information for the ref) and refs (a hash that maps from name to
hash) separatly.  I wonder if this really has the performance
advantage over just compute and return reflist from here and
have the callers who need the mapping to derive it from the list
(i.e.

	my ($reflist) = git_get_refs_list();
	my %refs = map { $_->{name} => $_->{hash} } @$reflist;
).

> @@ -2114,14 +2122,14 @@ sub git_tags_body {
>  
>  sub git_heads_body {
>  	# uses global variable $project
> -	my ($taglist, $head, $from, $to, $extra) = @_;
> +	my ($headlist, $head, $from, $to, $extra) = @_;
>  	$from = 0 unless defined $from;
> -	$to = $#{$taglist} if (!defined $to || $#{$taglist} < $to);
> +	$to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);
>  
>  	print "<table class=\"heads\" cellspacing=\"0\">\n";
>  	my $alternate = 0;
>  	for (my $i = $from; $i <= $to; $i++) {
> -		my $entry = $taglist->[$i];
> +		my $entry = $headlist->[$i];
>  		my %tag = %$entry;
>  		my $curr = $tag{'id'} eq $head;
>  		if ($alternate) {

I guess either would do but I think it is more conventional to
say:

	my $limit = @list;
        for (my $i = $from; $i < $limit; $i++) { ... }

than 

	my $limit = $#list;
        for (my $i = $from; $i <= $limit; $i++) { ... }

At least the former would be easier to read for C types among us.

The remaining hunks are very nice improvements.

> @@ -2291,7 +2299,19 @@ sub git_summary {
> @@ -2321,17 +2341,15 @@ sub git_summary {
> @@ -2542,7 +2560,7 @@ sub git_tags {
> @@ -2555,9 +2573,9 @@ sub git_heads {

^ permalink raw reply

* Re: [PATCH 1/2] gitweb: Always use git-peek-remote in git_get_references
From: Jakub Narebski @ 2006-09-20 18:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64fijz90.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Instead of trying to read info/refs file, which might not be present
> > (we did fallback to git-ls-remote), always use git-peek-remote in
> > git_get_references.
> >
> > It is preparation for git_get_refs_info to also return references
> > info. We cannot use info/refs for git_get_refs_info as the information
> > contained therein is usually stale.
> 
> What the patch does is sane, but I think the last sentence of
> the proposed log message is not.  If info/refs is "usually
> stale", it is a bug in the repository to have such a stale file.
> 
> The real reason for this patch is that a repository served by
> gitweb is not necessarily meant to be fetched over HTTP and
> info/refs does not have to be there.

If the repository served by gitweb is updated using push, then
info/refs updated using post-update hook is up-to-date. If repository
is "live" repository, updated also using commit, rebase and such,
info/refs is usually stale.

If there were post-commit hook, and it's contents was the default 
post-update hook, info/refs would be never stale. And we could read
from into/refs, and fallback to git-peek-remote if it doesn't exist...
but we don't know if info/refs has current info.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Subversion-style incrementing revision numbers
From: Petr Baudis @ 2006-09-20 18:22 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Linus Torvalds, Joel Dice, git
In-Reply-To: <200609201928.46024.robin.rosenberg.lists@dewire.com>

Dear diary, on Wed, Sep 20, 2006 at 07:28:45PM CEST, I got a letter
where Robin Rosenberg <robin.rosenberg.lists@dewire.com> said that...
> Typing those numbers is really insane, so the idea never occurred to me. Most 
> environments have simple methods to copy and paste strings using a clipboard,
> even in a console. So typically you can left-doubleclick on a string and paste 
> with the middle button in *nix like systems (right button in cygwin/windows 
> command line).

  ...which is certainly appealing for the Plan9 fans, but I'm not so
excited from having to grab my mouse all the time. :-) That said, typing
just the few leading digits is not really that painful.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: Petr Baudis @ 2006-09-20 18:29 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20060920162145.GA23260@spearce.org>

Dear diary, on Wed, Sep 20, 2006 at 06:21:45PM CEST, I got a letter
where Shawn Pearce <spearce@spearce.org> said that...
> > >      combined with set of packs with staggered spans to help
> > >      commit walkers Pasky talked about quite a while ago).
> > 
> > Yes, this is certainly one of things I would like to implement at
> > repo.or.cz.
> 
> Borrowing your line:
> Hmm, I think I've missed this, I must look that in the archive.
> :-)

I've proposed something long time ago in
<20051112135947.GC30496@pasky.or.cz>.

Dear diary, on Wed, Sep 20, 2006 at 07:31:26PM CEST, I got a letter
where Shawn Pearce <spearce@spearce.org> said that...
> Junio C Hamano <junkio@cox.net> wrote:
> > Shawn Pearce <spearce@spearce.org> writes:
> > > Originally I gave Pasky a one-liner on #git:
> > >
> > >   git fetch --force origin $(git ls-remote origin \
> > >       | awk '{if(!/\^{}$/){print $2":"$2}}')
> > 
> > Which sounds a very sane thing to do, especially given git is
> > designed to be scripted this way.
> 
> Exactly.  So I'm not sure why he was against it.  After all he must
> have some script that's looping over all mirrored repositories and
> invoking git fetch in them.  :-)

Basically, I imagined that what I wanted would be very close or
identical to git fetch --all (or whatever you call it, just doing it
with .git/refs/remotes) which I think should certainly be supported
without funny awk pipes. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply

* Re: [PATCH] Added --mirror-all to git-fetch.
From: A Large Angry SCM @ 2006-09-20 18:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn Pearce, git, Petr Baudis
In-Reply-To: <7vu032h1g1.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Junio C Hamano wrote:
[...]
> 
> Another possibility.  The mirror might want to do something
> like:
> 
>         .git/refs/remotes/davem/sparc-2.6.git/...
> 	.git/refs/remotes/torvalds/linux-2.6.git/...
> 	.git/refs/remotes/jgarzik/netdev-2.6.git/...
> 
> to be a super-mirror of related projects.  This would give a
> convenient way for the visitors to browse them all at one place,
> and would save object store.  There are minor details such as
> gitweb may not allow browsing refs/remotes and git-fetch may not
> let you fetch from refs/remotes (I haven't checked), but they
> are fixable as needed.
> 

This is what I do with the 2 attached, poorly named, scripts. Very 
convenient for watching some of my favorite projects. ;-)

[-- Attachment #2: fred.sh --]
[-- Type: application/x-shellscript, Size: 432 bytes --]

[-- Attachment #3: fred_history --]
[-- Type: text/plain, Size: 1188 bytes --]

export GIT_COMMITTER_NAME="@"
export GIT_AUTHOR_NAME="@"

./fred.sh git://www.kernel.org/pub/scm/git/git.git git/git

./fred.sh git://www.kernel.org/pub/scm/gitk/gitk.git gitk/gitk

./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.11.y.git stable/linux-2.6.11.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.12.y.git stable/linux-2.6.12.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.13.y.git stable/linux-2.6.13.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.14.y.git stable/linux-2.6.14.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.15.y.git stable/linux-2.6.15.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git stable/linux-2.6.16.y
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.17.y.git stable/linux-2.6.17.y

./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/tglx/history.git tglx/history

./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git torvalds/linux-2.6
./fred.sh git://www.kernel.org/pub/scm/linux/kernel/git/torvalds/sparse.git torvalds/sparse

^ permalink raw reply

* Re: [PATCH] gitweb: Support for custom per-project owner string
From: Jakub Narebski @ 2006-09-20 19:01 UTC (permalink / raw)
  To: git
In-Reply-To: <20060920154208.GN8259@pasky.or.cz>

Petr Baudis wrote:

> Dear diary, on Wed, Sep 20, 2006 at 05:02:21PM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> Petr Baudis <pasky@suse.cz> writes:
>> 
>>> Also, ideally this would be in the configfile but calling repoconfig for
>>> each repository in the index would slow things down way too much.
>> 
>> Hmph.  I wonder why.  We do read description already from a file
>> so maybe we would want a faster way to access the config file to
>> grab gitweb.* variables in a single call?
> 
> Still, opening files is _much_ faster than executing a tool, I'd say.

Perhaps we should just parse $GIT_DIR/config ourself in gitweb
(this is thing that I certainly would want to see in Git.pm).
There are many modules which parse ini-like config files, but it
shouldn't be that difficult to write parser which _reads_ and parses
config file (without support for setting variables).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Subversion-style incrementing revision numbers
From: Junio C Hamano @ 2006-09-20 19:07 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060920182203.GS8259@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Wed, Sep 20, 2006 at 07:28:45PM CEST, I got a letter
> where Robin Rosenberg <robin.rosenberg.lists@dewire.com> said that...
>> Typing those numbers is really insane, so the idea never occurred to me. Most 
>> environments have simple methods to copy and paste strings using a clipboard,
>> even in a console. So typically you can left-doubleclick on a string and paste 
>> with the middle button in *nix like systems (right button in cygwin/windows 
>> command line).
>
>   ...which is certainly appealing for the Plan9 fans, but I'm not so
> excited from having to grab my mouse all the time. :-)

I am so used to live always in "screen" so that is not much of a
problem for me.

Sorry for the noise ;-).

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Jeff Garzik @ 2006-09-20 19:24 UTC (permalink / raw)
  To: Linus Torvalds, Stephen Hemminger; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0609200816400.4388@g5.osdl.org>

Linus Torvalds wrote:
> So you could either mark _all_ the remote branches with the extra "+" (to 
> say that you always want to fetch that exact state for whatever branch 
> you're tracking), or you can ask Jeff which branches he expects to do 
> strange things and just mark those individual ones.


Actually, I only rebase rarely, because it's a pain for downstream people.

Stephen just caught one of the rare occasions where I rebased 
'e100-sbit'.  Note that other branches did not get rebased.

Sorry about that Stephen, I should have posted to netdev.

	Jeff

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Jakub Narebski @ 2006-09-20 19:58 UTC (permalink / raw)
  To: git
In-Reply-To: <20060920155431.GO8259@pasky.or.cz>

Petr Baudis wrote:

> Dear diary, on Wed, Sep 20, 2006 at 05:28:08PM CEST, I got a letter
> where Linus Torvalds <torvalds@osdl.org> said that...

>> However, you can tell git that Jeff is being difficult by marking such 
>> branches individually as being rebased.
> 
> This is really a wrong way of describing the problem - I'd say that Git
> is being difficult here. The point is, the subsystem maintainers need to
> maintain stacks of patches and rebase against the main kernel branch
> regularily, and they want to still publish their current state. So it's
> not really any of them being strange or difficult, but Git being so
> because it has no seamless support for tracking those branches.

There was idea around moving remotes configuration to config file to have
some per branch configureation, including readonly for protecting tracking
branches, marking default branch for merge with (and which tracking
branch(es) to merge)...

...and that included marking branch _on the server side_ as being rebased,
i.e. without preserved history. Unfortunately, the discussion petered out
without changes to git. Branch marked as pu-like would either get '+'
in appropriate Pull line in remotes file generated during clone, or they
wouldn't need '+'.

By the way, there is '--force' option to git-pull/git-fetch...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: git pull for update of netdev fails.
From: Jakub Narebski @ 2006-09-20 20:01 UTC (permalink / raw)
  To: git
In-Reply-To: <20060920161825.GR8259@pasky.or.cz>

Petr Baudis wrote:
 
>> Seamless support is there and Linus described how without
>> breaking the usual "if not fast forward you may lose some
>> patches so be extra careful" safety valve.
> 
>   I argue that this safety valve is useless for most people (and
> actually I have hard time imagining a plausible scenario in which it
> actually _is_ useful). The support is not really seamless since you have
> to make manual changes to refspecs, while most people probably don't
> understand them (and shouldn't be required to if they are just tracking
> someone else anyway).

Or you can always pull with the --force option...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Enable the packed refs file format
From: Phil Richards @ 2006-09-20 20:19 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0609191407340.4388@g5.osdl.org>

On 2006-09-19, Linus Torvalds <torvalds@osdl.org> wrote:
>  Too bad that we can't get gcc to warn on these things. We do mark it as 
>  "format(printf)", but I don't know of any way to tell gcc that it _has_ to 
>  have that initial constant string.

Not sure if it just a gcc 4.x-ism, but -Wformat-nonliteral or -Wformat-security
might be what you are looking for.

`-Wformat-nonliteral'
     If `-Wformat' is specified, also warn if the format string is not a
     string literal and so cannot be checked, unless the format function
     takes its format arguments as a `va_list'.

`-Wformat-security'
     If `-Wformat' is specified, also warn about uses of format
     functions that represent possible security problems.  At present,
     this warns about calls to `printf' and `scanf' functions where the
     format string is not a string literal and there are no format
     arguments, as in `printf (foo);'.  This may be a security hole if
     the format string came from untrusted input and contains `%n'.
     (This is currently a subset of what `-Wformat-nonliteral' warns
     about, but in future warnings may be added to `-Wformat-security'
     that are not included in `-Wformat-nonliteral'.)


phil
-- 
change name before "@" to "phil" for email

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox