* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Sam Ravnborg @ 2007-10-19 17:26 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Steven Grimm, Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191211210.19446@xanadu.home>
On Fri, Oct 19, 2007 at 12:12:41PM -0400, Nicolas Pitre wrote:
> On Fri, 19 Oct 2007, Steven Grimm wrote:
>
> > On 19/10/2007, Jeff King <peff@peff.net> wrote:
> > > This makes the fetch output much more terse. It is likely to
> > > be very controversial. Here's an example of the new output:
> > >
> > > Indexing objects: 100% (1061/1061), done.
> > > Resolving deltas: 100% (638/638), done.
> >
> > Those two lines are actually my beef with the fetch output. As a newbie, I had
> > no idea what "Indexing objects" actually meant. We have this thing called "the
> > index" in git so I would expect "Indexing objects" to have something to do
> > with that, but it doesn't seem to.
> >
> > How about something more descriptive of the high-level operation that's going
> > on, along the lines of:
> >
> > Gathering changes from remote: 100% (1061/1061), done.
> > Applying changes locally: 100% (638/638), done.
>
> This is even more wrong.
>
> Agreed, indexing objects might not be the best description. It probably
> will become "receiving objects" along with a bandwitth meter.
The term 'objects' here always confuses me. What is often my first
thing to check the number of individual commits being added after
a git pull. Wether a commit touches one or several files is less
important (to my way of using git).
Sam
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Schindelin @ 2007-10-19 17:19 UTC (permalink / raw)
To: Mike Hommey; +Cc: Scott Parish, Johannes Sixt, git
In-Reply-To: <20071019164816.GA24573@glandium.org>
Hi,
On Fri, 19 Oct 2007, Mike Hommey wrote:
> On Fri, Oct 19, 2007 at 04:27:39PM +0200, Johannes Schindelin wrote:
> > While reading this, I have to wonder why it is not just simpler to try
> > with builtin_exec_path first, and if that fails, just let exec() find the
> > program in the PATH?
>
> Why not try the directory where the git executable is, too ?
I commented on that. If the git command was not specified with an
absolute path, then make it absolute (and only if not even a relative path
was specified, ignore this altogether since git is in the PATH).
I was a bit terse on the issue I have to admit, though.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Mike Hommey @ 2007-10-19 16:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Scott Parish, Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0710191616490.16728@wbgn129.biozentrum.uni-wuerzburg.de>
On Fri, Oct 19, 2007 at 04:27:39PM +0200, Johannes Schindelin wrote:
> While reading this, I have to wonder why it is not just simpler to try
> with builtin_exec_path first, and if that fails, just let exec() find the
> program in the PATH?
Why not try the directory where the git executable is, too ?
Mike
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Sixt @ 2007-10-19 14:34 UTC (permalink / raw)
To: Scott Parish; +Cc: git
In-Reply-To: <20071019141805.GE1463@srparish.net>
Scott Parish schrieb:
> On Fri, Oct 19, 2007 at 03:21:12PM +0200, Johannes Sixt wrote:
>
>> Scott Parish schrieb:
>>> I have a situation where software for a distribution is installed
>>> into a fake "prefix" and then moved to one of several potential
>>> places to be used by users. Given that the final location isn't
>>> static, i can't depend on builtin_exec_path. I'd really like users
>>> to be able to get started with git as easily as possible. With the
>>> current setup, they would have to create and maintain either an
>>> GIT_EXEC_PATH or an alias for including --exec-path, as well as a
>>> MANPATH and PERL5LIB. This seem like an unnessisary burden.
>> Interesting. How does this compare to this 2-patch-series:
>>
>> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
>> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae
>>
>> which I had come up with to accomplish something very similar
>> (on Windows). Your approach looks superior, but I hadn't gone
>> into depths, yet.
>
> I know very little about what's available on windows. Looking at
> your code, it looks like the command isn't passed in in argv[0] and
> that it contains the windows style path seperators. My code currently
> assumes that PATH is a colon separated list, and that directories
> are separated with '/'. How should these assumptions change for
> windows?
The question is rather whether my patches would be sufficient to also
achieve your requirements. They turn bultin_exec_path into a non-constant
that derives exec-path from argv[0] (which on Windows happens to be
available in the global _pgmptr). Isn't this enough, or at least the essence
of what you need?
(How to get to the value of _pgmptr, ie. argv[0], on non-Windows is a
secondary matter.)
-- Hannes
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 16:12 UTC (permalink / raw)
To: Steven Grimm; +Cc: Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <4718D25A.7040109@midwinter.com>
On Fri, 19 Oct 2007, Steven Grimm wrote:
> On 19/10/2007, Jeff King <peff@peff.net> wrote:
> > This makes the fetch output much more terse. It is likely to
> > be very controversial. Here's an example of the new output:
> >
> > Indexing objects: 100% (1061/1061), done.
> > Resolving deltas: 100% (638/638), done.
>
> Those two lines are actually my beef with the fetch output. As a newbie, I had
> no idea what "Indexing objects" actually meant. We have this thing called "the
> index" in git so I would expect "Indexing objects" to have something to do
> with that, but it doesn't seem to.
>
> How about something more descriptive of the high-level operation that's going
> on, along the lines of:
>
> Gathering changes from remote: 100% (1061/1061), done.
> Applying changes locally: 100% (638/638), done.
This is even more wrong.
Agreed, indexing objects might not be the best description. It probably
will become "receiving objects" along with a bandwitth meter.
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19 15:57 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <28B30BAE-9482-4219-924B-F0EDFB2936FB@wincent.com>
On 19 Oct 2007, at 7:33:39 AM, Wincent Colaiuta wrote:
>> What you want to happen is the following:
>>
>> git show HEAD:A.txt > path/B.txt
>> git add path/B.txt
>> mv A.txt B.txt
>> git rm A.txt
>>
>> Is this correct?
>
> Here you're copying the content of A.txt as it was in the last
> (HEAD) commit, but from what the poster said he wants the content
> of A.txt as it is staged in the index (that is, there may be staged
> but uncomitted changes).
>
>> Better:
>>
>>> mv A.txt path/B.txt
>>> Point the index entry for A.txt to path/B.txt
>
> Yes, that is basically what he was asking for, as I read it.
You're right.
There is the subtlety in the first case that he's already staged
something.
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Steven Grimm @ 2007-10-19 15:53 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David Symonds, Jeff King, git
In-Reply-To: <4718D25A.7040109@midwinter.com>
(Sorry for the repeat; my mail client tried to send this as HTML at
first and the git list rejected it.)
On 19/10/2007, Jeff King <peff@peff.net> wrote:
> This makes the fetch output much more terse. It is likely to
> be very controversial. Here's an example of the new output:
>
> Indexing objects: 100% (1061/1061), done.
> Resolving deltas: 100% (638/638), done.
Those two lines are actually my beef with the fetch output. As a newbie,
I had no idea what "Indexing objects" actually meant. We have this thing
called "the index" in git so I would expect "Indexing objects" to have
something to do with that, but it doesn't seem to.
How about something more descriptive of the high-level operation that's
going on, along the lines of:
Gathering changes from remote: 100% (1061/1061), done.
Applying changes locally: 100% (638/638), done.
-Steve
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Steven Grimm @ 2007-10-19 15:50 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David Symonds, Jeff King, git
In-Reply-To: <20071019073938.GN14735@spearce.org>
On 19/10/2007, Jeff King <peff@peff.net> wrote:
> This makes the fetch output much more terse. It is likely to
> be very controversial. Here's an example of the new output:
>
> Indexing objects: 100% (1061/1061), done.
> Resolving deltas: 100% (638/638), done.
Those two lines are actually my beef with the fetch output. As a newbie,
I had no idea what "Indexing objects" actually meant. We have this thing
called "the index" in git so I would expect "Indexing objects" to have
something to do with that, but it doesn't seem to.
How about something more descriptive of the high-level operation that's
going on, along the lines of:
Gathering changes from remote: 100% (1061/1061), done.
Applying changes locally: 100% (638/638), done.
-Steve
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 15:03 UTC (permalink / raw)
To: Karl Hasselström
Cc: Theodore Tso, Santi Béjar, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <20071019143844.GB23765@diana.vm.bytemark.co.uk>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1393 bytes --]
On Fri, 19 Oct 2007, Karl Hasselström wrote:
> On 2007-10-19 07:38:22 -0400, Theodore Tso wrote:
>
> > Finally, one last question --- am I the only one who had to take a
> > second look at the whether the arrow should be <- or ->? The
> > question is whether we are saying "gitk is moving to include all of
> > spearce/gitk"; but I could also see it stated that we are assigning
> > refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
> > arrow should be reversed. Or maybe:
> >
> > ==> git://repo.or.cz/git/spearce.git
> > * branch gitk := spearce/gitk (new)
> > * branch maint := spearce/maint 1aa3d01..e7187e4
> > * branch master := spearce/master de61e42..7840ce6
> > * branch next := spearce/next 895be02..2fe5433
> > + branch pu := spearce/pu 89fa332...1e4c517
> > * branch todo := spearce/todo (new)
>
> I think the reasoning behind the "foo -> spearce/foo" syntax is that
> "(refs/heads/)foo" in the remote repository has been fetched to
> "(refs/remotes/)spearce/foo" in the local repository.
Well, the important thing is that the _content_ is moving from the
remote repository to the local one. That's how the arrow should be
interpreted conceptually. The fact that technically we end up assigning
the local ref with the remote value is a technical issue.
Nicolas
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 14:56 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Santi Béjar, Johannes Sixt, Theodore Tso, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <Pine.LNX.4.64.0710191640250.16728@wbgn129.biozentrum.uni-wuerzburg.de>
On Fri, 19 Oct 2007, Johannes Schindelin wrote:
> Better to say (forced) if need be. But I do not think so. I like Hannes'
> proposal as-is.
I'm of that opinion too. Except maybe using a space instead of * for
fast forward, so the other types stand out more.
Nicolas
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 14:54 UTC (permalink / raw)
To: Johannes Sixt
Cc: Santi Béjar, Theodore Tso, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <4718C1DE.5030708@viscovery.net>
On Fri, 19 Oct 2007, Johannes Sixt wrote:
> The '*' could go away, then the '+' is more visible.
Agreed. ' ' = fast forward, '+' = forced update, and '!' = refused.
Nicolas
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 14:52 UTC (permalink / raw)
To: Santi Béjar
Cc: Johannes Sixt, Theodore Tso, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <8aa486160710190731v67626fd8wa94ba069a17f73ce@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 900 bytes --]
On Fri, 19 Oct 2007, Santi Béjar wrote:
> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked. Something like:
>
> $ git fetch spearce
> ...
> URL: git://repo.or.cz/git/spearce.git
> * (new) spearce/gitk: new branch 'gitk'
> * 1aa3d01..e7187e4 spearce/maint: fast forward to branch 'maint'
> * de61e42..7840ce6 spearce/master: fast forward to branch 'master'
> * 895be02..2fe5433 spearce/next: fast forward to branch 'next'
> + 89fa332...1e4c517 spearce/pu: forcing update to non-fast forward branch 'pu'
> * (new) spearce/todo: new branch spearce/todo
Well, I don't like it as much. First I don't think newbies will care
much more even if the type of update is spelled out verbosely. Better
keep it short and add all the necessary information in the fetch man
page instead.
Nicolas
^ permalink raw reply
* Re: git push bug?
From: Joakim Tjernlund @ 2007-10-19 14:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Steffen Prohaska, git
In-Reply-To: <Pine.LNX.4.64.0710182259190.25221@racer.site>
On Thu, 2007-10-18 at 23:00 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 18 Oct 2007, Joakim Tjernlund wrote:
>
> > On Thu, 2007-10-18 at 17:21 +0100, Johannes Schindelin wrote:
> >
> > > On Thu, 18 Oct 2007, Joakim Tjernlund wrote:
> > >
> > > > Seems like it is a bit too easy to make mistakes here. Why can I
> > > > delete a branch with :linus but not create one with linus:linus?
> > >
> > > I wonder why you bother with the colon at all. Just
> > >
> > > git push <remote> linus
> > >
> > > and be done with it. The colon is only there to play interesting
> > > games, not something as simple as "push this branch" or "push this
> > > tag".
> >
> > First, I didn't know that I could do that. Secondly, I was also looking
> > do v2.6.23:linus refspecs
>
>
> First, then our documentation could be better. How?
Well, it isn't clear to me how all this is supposed to work and
what is bugs. Clearifying that would help.
For instances I did a push with v2.6.23:refs/heads/linus and now
I got a branch with the SHA1 of v2.6.23 tag(0b8bc8b91cf6befea20fe78b90367ca7b61cfa0d)
in it. Makes gitk display that branch as "linus^{}".
>
> Second, why not "git checkout -b linus v2.6.23 && git push origin linus"?
An extra checkout that takes time but works. Doesn't make the above
"weiredness" go away though.
Jocke
>
> Ciao,
> Dscho
>
>
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Schindelin @ 2007-10-19 14:41 UTC (permalink / raw)
To: Santi Béjar
Cc: Nicolas Pitre, Johannes Sixt, Theodore Tso, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <8aa486160710190731v67626fd8wa94ba069a17f73ce@mail.gmail.com>
Hi,
On Fri, 19 Oct 2007, Santi B?jar wrote:
> On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 19 Oct 2007, Johannes Sixt wrote:
> >
> > > Theodore Tso schrieb:
> > > > ==> git://repo.or.cz/git/spearce.git
> > > > * branch gitk -> spearce/gitk (new)
> > > > * branch maint -> spearce/maint 1aa3d01..e7187e4
> > > > * branch master -> spearce/master de61e42..7840ce6
> > > > * branch next -> spearce/next 895be02..2fe5433
> > > > + branch pu -> spearce/pu 89fa332...1e4c517
> > > > * branch todo -> spearce/todo (new)
> > >
> > > > As far as the padding, it would be a pain to figure out how to make
> > > > the right hand column be padded so that it starts 3 spaces after the
> > > > longest " * branch foo -> bar" line, but that would look the best.
> > >
> > > But this way it wouldn't be difficult at all:
> > >
> > > ==> git://repo.or.cz/git/spearce.git
> > > * (new) gitk -> spearce/gitk
> > > * 1aa3d01..e7187e4 maint -> spearce/maint
> > > * de61e42..7840ce6 master -> spearce/master
> > > * 895be02..2fe5433 next -> spearce/next
> > > + 89fa332...1e4c517 pu -> spearce/pu
> > > * (new) todo -> spearce/todo
> >
> > Actually I think this is the best format so far: one line per branch, no
> > terminal width issue (long branch names are simply wrapped), the
> > old..new info is there also with the single character marker to quickly
> > notice the type of update.
>
> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked. Something like:
>
> $ git fetch spearce
> ...
> URL: git://repo.or.cz/git/spearce.git
> * (new) spearce/gitk: new branch 'gitk'
Nah, that is just duplication.
> * 1aa3d01..e7187e4 spearce/maint: fast forward to branch 'maint'
> * de61e42..7840ce6 spearce/master: fast forward to branch 'master'
> * 895be02..2fe5433 spearce/next: fast forward to branch 'next'
> + 89fa332...1e4c517 spearce/pu: forcing update to non-fast forward branch 'pu'
Better to say (forced) if need be. But I do not think so. I like Hannes'
proposal as-is.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Karl Hasselström @ 2007-10-19 14:40 UTC (permalink / raw)
To: Santi Béjar
Cc: Nicolas Pitre, Johannes Sixt, Theodore Tso, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <8aa486160710190731v67626fd8wa94ba069a17f73ce@mail.gmail.com>
On 2007-10-19 16:31:08 +0200, Santi Béjar wrote:
> I would also put 'URL:' instead '==>'.
Seconded.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Sixt @ 2007-10-19 14:40 UTC (permalink / raw)
To: Santi Béjar
Cc: Nicolas Pitre, Theodore Tso, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <8aa486160710190731v67626fd8wa94ba069a17f73ce@mail.gmail.com>
Santi Béjar schrieb:
> On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
>> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>>> ==> git://repo.or.cz/git/spearce.git
>>> * (new) gitk -> spearce/gitk
>>> * 1aa3d01..e7187e4 maint -> spearce/maint
>>> * de61e42..7840ce6 master -> spearce/master
>>> * 895be02..2fe5433 next -> spearce/next
>>> + 89fa332...1e4c517 pu -> spearce/pu
>>> * (new) todo -> spearce/todo
>
> I like it too. I would like to add some more descripton, because I
> think for newbies the .. and ... can be overlooked.
The '*' could go away, then the '+' is more visible.
-- Hanes
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Karl Hasselström @ 2007-10-19 14:38 UTC (permalink / raw)
To: Theodore Tso
Cc: Santi Béjar, Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <20071019113822.GB16726@thunk.org>
On 2007-10-19 07:38:22 -0400, Theodore Tso wrote:
> Finally, one last question --- am I the only one who had to take a
> second look at the whether the arrow should be <- or ->? The
> question is whether we are saying "gitk is moving to include all of
> spearce/gitk"; but I could also see it stated that we are assigning
> refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
> arrow should be reversed. Or maybe:
>
> ==> git://repo.or.cz/git/spearce.git
> * branch gitk := spearce/gitk (new)
> * branch maint := spearce/maint 1aa3d01..e7187e4
> * branch master := spearce/master de61e42..7840ce6
> * branch next := spearce/next 895be02..2fe5433
> + branch pu := spearce/pu 89fa332...1e4c517
> * branch todo := spearce/todo (new)
I think the reasoning behind the "foo -> spearce/foo" syntax is that
"(refs/heads/)foo" in the remote repository has been fetched to
"(refs/remotes/)spearce/foo" in the local repository.
I might be deluded, though.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Santi Béjar @ 2007-10-19 14:31 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Johannes Sixt, Theodore Tso, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191009330.19446@xanadu.home>
On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>
> > Theodore Tso schrieb:
> > > ==> git://repo.or.cz/git/spearce.git
> > > * branch gitk -> spearce/gitk (new)
> > > * branch maint -> spearce/maint 1aa3d01..e7187e4
> > > * branch master -> spearce/master de61e42..7840ce6
> > > * branch next -> spearce/next 895be02..2fe5433
> > > + branch pu -> spearce/pu 89fa332...1e4c517
> > > * branch todo -> spearce/todo (new)
> >
> > > As far as the padding, it would be a pain to figure out how to make
> > > the right hand column be padded so that it starts 3 spaces after the
> > > longest " * branch foo -> bar" line, but that would look the best.
> >
> > But this way it wouldn't be difficult at all:
> >
> > ==> git://repo.or.cz/git/spearce.git
> > * (new) gitk -> spearce/gitk
> > * 1aa3d01..e7187e4 maint -> spearce/maint
> > * de61e42..7840ce6 master -> spearce/master
> > * 895be02..2fe5433 next -> spearce/next
> > + 89fa332...1e4c517 pu -> spearce/pu
> > * (new) todo -> spearce/todo
>
> Actually I think this is the best format so far: one line per branch, no
> terminal width issue (long branch names are simply wrapped), the
> old..new info is there also with the single character marker to quickly
> notice the type of update.
I like it too. I would like to add some more descripton, because I
think for newbies the .. and ... can be overlooked. Something like:
$ git fetch spearce
...
URL: git://repo.or.cz/git/spearce.git
* (new) spearce/gitk: new branch 'gitk'
* 1aa3d01..e7187e4 spearce/maint: fast forward to branch 'maint'
* de61e42..7840ce6 spearce/master: fast forward to branch 'master'
* 895be02..2fe5433 spearce/next: fast forward to branch 'next'
+ 89fa332...1e4c517 spearce/pu: forcing update to non-fast forward branch 'pu'
* (new) spearce/todo: new branch spearce/todo
I would also put 'URL:' instead '==>'.
Santi
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Schindelin @ 2007-10-19 14:31 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Johannes Sixt, Theodore Tso, Santi Béjar, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191009330.19446@xanadu.home>
Hi,
On Fri, 19 Oct 2007, Nicolas Pitre wrote:
> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>
> > ==> git://repo.or.cz/git/spearce.git
> > * (new) gitk -> spearce/gitk
> > * 1aa3d01..e7187e4 maint -> spearce/maint
> > * de61e42..7840ce6 master -> spearce/master
> > * 895be02..2fe5433 next -> spearce/next
> > + 89fa332...1e4c517 pu -> spearce/pu
> > * (new) todo -> spearce/todo
>
> Actually I think this is the best format so far: one line per branch, no
> terminal width issue (long branch names are simply wrapped), the
> old..new info is there also with the single character marker to quickly
> notice the type of update.
Yes. Definitely my favourite so far, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Schindelin @ 2007-10-19 14:27 UTC (permalink / raw)
To: Scott Parish; +Cc: Johannes Sixt, git
In-Reply-To: <20071019130402.GD1463@srparish.net>
Hi,
On Fri, 19 Oct 2007, Scott Parish wrote:
> + check PATH for the location of git
Okay, but better do it only if the current exec_path did not succeed to
find something, to stay as backwards compatible as possible.
> + the checking of argv[0] was restricted to absolute paths; remove
> that restriction so it also works when called with a relative
> path (eg ../../otheruser/usr/bin/git)
This will utterly break down when you try to do things in a subdirectory
of your project. git will cd up, and the relative path will no longer be
relative.
> + try to guess and set the env for the typical relative locations for
> MANPATH and PERL5LIB based off exec_path
Now this is ugly. At least make it a separate patch.
> +/* Return the first path in PATH that git is found in or NULL if not found */
> +char *git_path_from_env(void)
> +{
> + const char *env_paths = getenv("PATH");
> + const char *git = "/git";
> + int git_len = strlen(git);
> + char *paths, *path, *colon, *git_path;
> + int path_len;
> + struct stat st;
> +
> + if (!env_paths)
> + return NULL;
> +
> + path_len = strlen(env_paths);
> + path = paths = xmalloc(path_len + 1);
> + memcpy(paths, env_paths, path_len + 1);
> +
> + while ((char *)1 != path) {
> + if ((colon = strchr(path, ':')))
> + *colon = 0;
> +
> + path_len = strlen(path);
> + git_path = xmalloc(path_len + git_len + 1);
> + memcpy(git_path, path, path_len);
> + memcpy(git_path + path_len, git, git_len + 1);
> +
> + if (!stat(git_path, &st)) { /* found */
> + free(paths);
> + git_path[path_len] = 0;
> + return git_path;
> + }
> +
> + free(git_path);
> + path = colon + 1;
> + }
> +
> + free(paths);
> + return NULL;
> +}
I am convinced that this function will look a lot less ugly when you use
strbufs. And I'd call it "find_git_in_path()".
> /* Returns the highest-priority, location to look for git programs. */
> const char *git_exec_path(void)
> {
> - const char *env;
> + const char *env, *path;
>
> if (current_exec_path)
> return current_exec_path;
>
> env = getenv(EXEC_PATH_ENVIRONMENT);
> if (env && *env) {
> + current_exec_path = env;
> return env;
> }
>
> + if ((path = git_path_from_env())) {
> + current_exec_path = path;
> + return path;
> + }
> +
> + current_exec_path = builtin_exec_path;
> return builtin_exec_path;
> }
As I said, I'd rather have git try with builtin_exec_path first, and only
if that fails, search through the PATH, for the _current_ command.
> -static void prepend_to_path(const char *dir, int len)
> +static void prepend_to_env(const char *env, const char *basedir,
I do not like this rename. It makes things more obscure, rather than
clearing things up.
> + const char *subdir, const char *env_default)
> {
> - const char *old_path = getenv("PATH");
> - char *path;
> - int path_len = len;
> -
> - if (!old_path)
> - old_path = "/usr/local/bin:/usr/bin:/bin";
> -
> - path_len = len + strlen(old_path) + 1;
> -
> - path = xmalloc(path_len + 1);
> + const char *old = getenv(env);
> + int basedir_len = strlen(basedir);
> + int subdir_len = strlen(subdir);
> + char *new;
> + int old_len;
> +
> + if (!old)
> + old = env_default;
> +
> + old_len = strlen(old);
> +
> + new = xmalloc(basedir_len + subdir_len + old_len + 1);
> +
> + memcpy(new, basedir, basedir_len);
> + memcpy(new + basedir_len, subdir, subdir_len);
> + memcpy(new + basedir_len + subdir_len, old, old_len + 1);
> +
> + if (setenv(env, new, 1))
> + fprintf(stderr, "Setenv failed: %s\n", strerror(errno));
> +
> + free(new);
> +}
Again, this would be so much more elegant using strbufs.
>
> - memcpy(path, dir, len);
> - path[len] = ':';
> - memcpy(path + len + 1, old_path, path_len - len);
> +static void prepend_to_envs(const char *dir, int len)
> +{
> + char *slash;
> + char *basedir;
> +
> + /* basedir is dir with "/bin" stripped off */
> + basedir = xmalloc(len + 1);
> + memcpy(basedir, dir, len + 1);
> +
> + if ((slash = strrchr(basedir, '/'))) {
> + *slash = 0;
> + while (slash == basedir + --len) /* found trailing slash */
> + if ((slash = strrchr(basedir, '/')))
> + *slash = 0;
> + }
>
> - setenv("PATH", path, 1);
> + prepend_to_env("PATH", basedir, "/bin:",
> + "/usr/local/bin:/usr/bin:/bin");
> + prepend_to_env("MANPATH", basedir, "/share/man:",
> + "/usr/local/share/man:/usr/share/man");
> + prepend_to_env("PERL5LIB", basedir, "/lib/perl5:",
> + "/usr/lib/perl5");
>
> - free(path);
> + free(basedir);
> }
As I said, this is so controversial it belongs into an own patch.
> @@ -414,8 +444,7 @@ int main(int argc, const char **argv)
> */
> if (slash) {
> *slash++ = 0;
> - if (*cmd == '/')
> - exec_path = cmd;
> + exec_path = cmd;
As I said, this breaks down. This alone is enough reason to move it to
its own patch. And I strongly suggest the use of make_path_absolute()
(with an xstrdup()).
> @@ -453,14 +482,15 @@ int main(int argc, const char **argv)
> /*
> * We execute external git command via execv_git_cmd(),
> * which looks at "--exec-path" option, GIT_EXEC_PATH
> - * environment, and $(gitexecdir) in Makefile while built,
> - * in this order. For scripted commands, we prepend
> - * the value of the exec_path variable to the PATH.
> + * environment, PATH environment, and $(gitexecdir) in
> + * Makefile while built, in this order. For scripted
> + * commands, we prepend the value of the exec_path
> + * variable to the PATH.
While reading this, I have to wonder why it is not just simpler to try
with builtin_exec_path first, and if that fails, just let exec() find the
program in the PATH?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott Parish @ 2007-10-19 14:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4718AF48.9020607@viscovery.net>
On Fri, Oct 19, 2007 at 03:21:12PM +0200, Johannes Sixt wrote:
> Scott Parish schrieb:
> > I have a situation where software for a distribution is installed
> > into a fake "prefix" and then moved to one of several potential
> > places to be used by users. Given that the final location isn't
> > static, i can't depend on builtin_exec_path. I'd really like users
> > to be able to get started with git as easily as possible. With the
> > current setup, they would have to create and maintain either an
> > GIT_EXEC_PATH or an alias for including --exec-path, as well as a
> > MANPATH and PERL5LIB. This seem like an unnessisary burden.
>
> Interesting. How does this compare to this 2-patch-series:
>
> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae
>
> which I had come up with to accomplish something very similar
> (on Windows). Your approach looks superior, but I hadn't gone
> into depths, yet.
I know very little about what's available on windows. Looking at
your code, it looks like the command isn't passed in in argv[0] and
that it contains the windows style path seperators. My code currently
assumes that PATH is a colon separated list, and that directories
are separated with '/'. How should these assumptions change for
windows?
sRp
--
Scott Parish
http://srparish.net/
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 14:14 UTC (permalink / raw)
To: Johannes Sixt
Cc: Theodore Tso, Santi Béjar, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <4718A3AB.7090301@viscovery.net>
On Fri, 19 Oct 2007, Johannes Sixt wrote:
> Theodore Tso schrieb:
> > ==> git://repo.or.cz/git/spearce.git
> > * branch gitk -> spearce/gitk (new)
> > * branch maint -> spearce/maint 1aa3d01..e7187e4
> > * branch master -> spearce/master de61e42..7840ce6
> > * branch next -> spearce/next 895be02..2fe5433
> > + branch pu -> spearce/pu 89fa332...1e4c517
> > * branch todo -> spearce/todo (new)
>
> > As far as the padding, it would be a pain to figure out how to make
> > the right hand column be padded so that it starts 3 spaces after the
> > longest " * branch foo -> bar" line, but that would look the best.
>
> But this way it wouldn't be difficult at all:
>
> ==> git://repo.or.cz/git/spearce.git
> * (new) gitk -> spearce/gitk
> * 1aa3d01..e7187e4 maint -> spearce/maint
> * de61e42..7840ce6 master -> spearce/master
> * 895be02..2fe5433 next -> spearce/next
> + 89fa332...1e4c517 pu -> spearce/pu
> * (new) todo -> spearce/todo
Actually I think this is the best format so far: one line per branch, no
terminal width issue (long branch names are simply wrapped), the
old..new info is there also with the single character marker to quickly
notice the type of update.
Nicolas
^ permalink raw reply
* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 14:11 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019134339.GA21852@coredump.intra.peff.net>
Ahh, shoot. Forgot to reply to all.
On 10/19/07, Jeff King <peff@peff.net> wrote:
> It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
> something silly like executing an older version of git that is in your
> PATH?
Yeah, just tried that again, definitely using the right version of
git. Before I apply your patch, both my test script and your addition
to t5400 fail. After applying your patch, my test script fails but
your addition to t5400 succeeds. Could this be something where
git-push and git-send-pack are not interacting correctly?
-Dan
^ permalink raw reply
* Re: git stash apply usability issues
From: David Kastrup @ 2007-10-19 13:57 UTC (permalink / raw)
To: git
In-Reply-To: <20071019132753.GA23765@diana.vm.bytemark.co.uk>
Karl Hasselström <kha@treskal.com> writes:
> On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:
>
>> Johannes Sixt <j.sixt@viscovery.net> wrote:
>
>> > (2) when 'git stash apply' runs merge-recursive, it treats the
>> > current state as 'ours' and the stash as 'theirs'. IMHO it should
>> > be the other way round: I have stashed away changes to a binary
>> > file. Then committed a different modification to it, and now want
>> > to apply the stash. This results in a conflict that leaves the
>> > current state in the working tree, but I had preferred that the
>> > stashed binary file were in the working tree now.
>> >
>> > What do other git-stash users think about changing the order?
>>
>> The current order is the same order that git-rebase uses. I'm not
>> saying its correct, just that its the same as rebase.
>
> FWIW, StGit push works the same way. The idea being that the current
> HEAD is our current state ("ours"), and the patch we're pushing is
> some change we want to apply ("theirs"). I always felt that this was a
> very natural order of things. But I guess the philosophy in the
> "stash" case is subtly different, so maybe the change is warranted
> there.
Well, maybe one should then just name this "current" and "separate"
instead of "ours" and "theirs".
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Sixt @ 2007-10-19 13:21 UTC (permalink / raw)
To: Scott Parish; +Cc: git
In-Reply-To: <20071019130402.GD1463@srparish.net>
Scott Parish schrieb:
> I have a situation where software for a distribution is installed
> into a fake "prefix" and then moved to one of several potential
> places to be used by users. Given that the final location isn't
> static, i can't depend on builtin_exec_path. I'd really like users
> to be able to get started with git as easily as possible. With the
> current setup, they would have to create and maintain either an
> GIT_EXEC_PATH or an alias for including --exec-path, as well as a
> MANPATH and PERL5LIB. This seem like an unnessisary burden.
Interesting. How does this compare to this 2-patch-series:
http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae
which I had come up with to accomplish something very similar
(on Windows). Your approach looks superior, but I hadn't gone
into depths, yet.
-- Hannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox