Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Shawn O. Pearce @ 2007-10-19  3:45 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710182328580.19446@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
> > Yup.  Your patches were a big improvement.  But I'm now sitting here
> > wondering if we shouldn't just allow a progress meter to overwrite
> > the prior one.  Then you only see the current task and progress,
> > or the final output if we have nothing further to say about that.
> 
> And then you've lost some diagnostic clue (the absolute numbers) about 
> the actual number of objects that were listed for "deltification" for 
> example.

Leave the "Total" line.  Add to it the number of objects we had to
consider for deltification as part of the packing.
 
> And imagine that you see the progress moving slowly because the remote 
> server is a NSLU2, but it says 80%.  Then you go for a coffee and the 
> progress says 20% when you return because it now has moved to a 
> different phase.  Rather counter intuitive.

Yea, I didn't consider that.  That's where you need to show the
number of steps and which one you are on, so the meter looks
more like:

	Step 1/3: Counting objects: .... \r
	Step 2/4: Compressing objects: ... \r
	Step 3/3: Writing objects: .... \r

only all smashed into one line of course, so only the most recent
one is being displayed.

-- 
Shawn.

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19  3:47 UTC (permalink / raw)
  To: Michael Witten; +Cc: git
In-Reply-To: <93BF5798-F1C3-48EE-8233-A0F111BF8138@MIT.EDU>

On Thu, Oct 18, 2007 at 11:40:47PM -0400, Michael Witten wrote:

> Anyway, succinctly:
>
>> 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
>
> Better:
>
>>  	mv A.txt path/B.txt
>> 	Point the index entry for A.txt to path/B.txt
>
> I hope that's right.

Hrm. So you _do_ want to do an index-only move of A to B, in which case
the suggestion of a "git-mv --cached" seems sensible. Though I'm curious
why you want that. The only workflow I can think of is:

  1. you modify a.c
  2. your boss comes in and tells you to make some unrelated change,
     which involves moving a.c to b.c
  3. You don't want to commit your changes, so you git-mv in the index
     only without involving your dirty working tree file.
  4. You commit the index (which doesn't have your changes from (1)

I think that is sort of a bogus workflow, though, since you will never
have actually compiled or tested the changes in (2). You are much better
to git-stash your current work, fulfill the boss's request, then
unstash.

-Peff

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19  3:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019033228.GA10697@coredump.intra.peff.net>

On Thu, 18 Oct 2007, Jeff King wrote:

> On Thu, Oct 18, 2007 at 11:24:41PM -0400, Nicolas Pitre wrote:
> 
> > You usually get long lines that gets wrapped, so that means 3 lines of 
> > screen space for one updated branches.  Is the "66ffb04..4fa4d23" 
> > information really useful?  Might someone ever care?
> 
> I have used it occasionally when tracking repos to see what new commits
> have happened. Usually I use a separate branch to mark "what I've seen"
> (i.e., fetch, gitk origin..master, pull), but if it's a branch that I'm
> not actively tracking, the display is useful.

Maybe we should have a shortcut notation for <ref>@{1}..<ref> instead?
I end up using that all the time since the fetch result has long 
scrolled off the screen when I want to look at what was fetched.

Usully this llooks like:

	git pull
	git log @{1}..

But using origin/master@{1}..origin/master is a bit cumbersome.

> What is really useless in that line is the fact that _every_ ref is
> going to have the name of the remote, even though we only support
> fetching from one remote at a time. Perhaps something like:
> 
> Fetching from git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
>  * refs/heads/origin: fast forward to branch 'master'
> 
> although that URL is almost a line by itself. :)

It is, therefore I'd skip "Fetching from " entirely.

> Also, why do we abbreviate "refs/heads/master" from the remote, but we
> don't abbreviate refs/heads/origin for the local? Maybe something like:
> 
>   * local heads/origin -> remote heads/master (fast forward)
> 
> or for separate remote
> 
>   * local remotes/origin/master -> remote heads/master (fast forward)

That looks fine for a push.  I'd say "remote foo -> local bar" for a 
fetch.


Nicolas

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19  3:53 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071019034704.GB11095@coredump.intra.peff.net>


On 18 Oct 2007, at 11:47:05 PM, Jeff King wrote:

> The only workflow I can think of is:
>
>   1. you modify a.c
>   2. your boss comes in and tells you to make some unrelated change,
>      which involves moving a.c to b.c
>   3. You don't want to commit your changes, so you git-mv in the index
>      only without involving your dirty working tree file.
>   4. You commit the index (which doesn't have your changes from (1)
>
> I think that is sort of a bogus workflow

Ha!

That's exactly what the original poster wanted.

It's not unreasonable.

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19  3:56 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182340550.19446@xanadu.home>

On Thu, Oct 18, 2007 at 11:50:39PM -0400, Nicolas Pitre wrote:

> Maybe we should have a shortcut notation for <ref>@{1}..<ref> instead?
> I end up using that all the time since the fetch result has long 
> scrolled off the screen when I want to look at what was fetched.

I must confess to never using reflogs in that case...for some reason
they just never come to mind. But now that you and Shawn mention them,
there's really no reason to leave the hash..hash for the fetch.

As for a shortcut notation, what about allowing '..' notation inside a
reflog. I.e., <ref>@{a..b} is the same as <ref>@{a}..<ref>@{b}? So you
could perhaps do origin/master@{1..}?

I'm not sure if there are syntactic issues with parsing out the '..' (or
'...') operator.

> > although that URL is almost a line by itself. :)
> It is, therefore I'd skip "Fetching from " entirely.

Yes, I was tempted to suggest that. I think it might need some context,
depending on what comes right before it.

> That looks fine for a push.  I'd say "remote foo -> local bar" for a 
> fetch.

Agreed.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19  3:58 UTC (permalink / raw)
  To: Michael Witten; +Cc: git
In-Reply-To: <C5184350-51A7-46B4-B0C9-E32F79214546@MIT.EDU>

On Thu, Oct 18, 2007 at 11:53:05PM -0400, Michael Witten wrote:

>>   3. You don't want to commit your changes, so you git-mv in the index
>>      only without involving your dirty working tree file.
> That's exactly what the original poster wanted.
>
> It's not unreasonable.

I guess. I really think git-stash is your friend here. But you can still
do step (3) with git-update-index (I'll leave the exact details as an
exercise for the reader).

-Peff

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19  4:02 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jeff King, git
In-Reply-To: <20071019034501.GG14735@spearce.org>

On Thu, 18 Oct 2007, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@cam.org> wrote:
> > And imagine that you see the progress moving slowly because the remote 
> > server is a NSLU2, but it says 80%.  Then you go for a coffee and the 
> > progress says 20% when you return because it now has moved to a 
> > different phase.  Rather counter intuitive.
> 
> Yea, I didn't consider that.  That's where you need to show the
> number of steps and which one you are on, so the meter looks
> more like:
> 
> 	Step 1/3: Counting objects: .... \r
> 	Step 2/4: Compressing objects: ... \r
> 	Step 3/3: Writing objects: .... \r
> 
> only all smashed into one line of course, so only the most recent
> one is being displayed.

Yet you might not know in advance how many steps there'll be.  You might 
or might not have the deltification phase (I simply can't let that term 
go...), pack indexing also have 1 or 2 steps, and if objects are 
unpacked instead then you have only one step.

Given the asynchronous nature of the sideband messages, I think that 
could only create messed up displays.  Some messages are terminated with 
\n and others with \r.


Nicolas

^ permalink raw reply

* [PATCH] Teach prune-packed to use the standard progress meter
From: Shawn O. Pearce @ 2007-10-19  4:11 UTC (permalink / raw)
  To: Nicolas Pitre, Jeff King; +Cc: git

Rather than reimplementing the progress meter logic and always
showing 100 lines of output while pruning already packed objects
we now use a delayed progress meter and only show it if there are
enough objects to make us take a little while.

Most users won't see the message anymore as it usually doesn't take
very long to delete the already packed loose objects.  This neatens
the output of a git-gc or git-repack execution, which is especially
important for a `git gc --auto` triggered from within another
command.

We perform the display_progress() call from within the very innermost
loop in case we spend more than 1 second within any single object
directory.  This ensures that a progress_update event from the
timer will still trigger in a timely fashion and allow the user to
see the progress meter.

While I'm in here I changed the message to be more descriptive of
its actual task.  "Deleting unused objects" is a little scary for
new users as they wonder where these unused objects came from and
how they should avoid them.  Truth is these objects aren't unused
in the sense of what git-prune would call a dangling object, these
are used but are just duplicates of things we have already stored
in a packfile.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 On top of np/progress topic.  I kicked around a few different
 changes for the progress title but finally settled on this one.
 Improvement suggestions welcome.

 builtin-prune-packed.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 9777300..015c8bb 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "progress.h"
 
 static const char prune_packed_usage[] =
 "git-prune-packed [-n] [-q]";
@@ -7,6 +8,8 @@ static const char prune_packed_usage[] =
 #define DRY_RUN 01
 #define VERBOSE 02
 
+static struct progress progress;
+
 static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 {
 	struct dirent *de;
@@ -23,6 +26,8 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 		if (!has_sha1_pack(sha1, NULL))
 			continue;
 		memcpy(pathname + len, de->d_name, 38);
+		if (opts == VERBOSE)
+			display_progress(&progress, i + 1);
 		if (opts & DRY_RUN)
 			printf("rm -f %s\n", pathname);
 		else if (unlink(pathname) < 0)
@@ -39,6 +44,11 @@ void prune_packed_objects(int opts)
 	const char *dir = get_object_directory();
 	int len = strlen(dir);
 
+	if (opts == VERBOSE)
+		start_progress_delay(&progress,
+			"Removing duplicate objects",
+			256, 95, 2);
+
 	if (len > PATH_MAX - 42)
 		die("impossible object directory");
 	memcpy(pathname, dir, len);
@@ -49,16 +59,13 @@ void prune_packed_objects(int opts)
 
 		sprintf(pathname + len, "%02x/", i);
 		d = opendir(pathname);
-		if (opts == VERBOSE && (d || i == 255))
-			fprintf(stderr, "Removing unused objects %d%%...\015",
-				((i+1) * 100) / 256);
 		if (!d)
 			continue;
 		prune_dir(i, d, pathname, len + 3, opts);
 		closedir(d);
 	}
 	if (opts == VERBOSE)
-		fprintf(stderr, "\nDone.\n");
+		stop_progress(&progress);
 }
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
-- 
1.5.3.4.1249.g895be

^ permalink raw reply related

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Linus Torvalds @ 2007-10-19  4:21 UTC (permalink / raw)
  To: Jeff King; +Cc: Nicolas Pitre, Shawn O. Pearce, git
In-Reply-To: <20071019035647.GA18717@coredump.intra.peff.net>



On Thu, 18 Oct 2007, Jeff King wrote:
> 
> As for a shortcut notation, what about allowing '..' notation inside a
> reflog. I.e., <ref>@{a..b} is the same as <ref>@{a}..<ref>@{b}? So you
> could perhaps do origin/master@{1..}?

I'd love it, but the way our current SHA1 parser works, I don't think it 
can really do it.

Basically, we currently assume that a SHA1 expression always expands to a 
*single* SHA1.

And then, on top of that SHA1 expression parser, we then have a totally 
separate logic (which is *not* part of the SHA1 expression parser itself) 
that handles the "a..b" and "a...b" cases.

In other words, all the magic "head@{xyz}" logic is all in sha1_name.c, 
but that never handles any ranges at all.

And then the range handling is a separate thing in revision.c and 
builtin-rev-parse.c.

So I think your syntax is wonderful, but it would require moving the 
complex range handling into sha1_name.c, and would also require that file 
to get a more complex interface (right now all the sha1_name.c routines 
just take the "fill in this one SHA1 hash" approach, but ".." and "..." 
means that you have multiple objects *and* you can mark one of them as 
being "negated" etc..)

> I'm not sure if there are syntactic issues with parsing out the '..' (or
> '...') operator.

See above: I don't think we have syntax problems per se: it's just that 
right now the "complex" parser (the one that knows about ^, ~, and @{x} 
etc) simply cannot do anything but a single SHA1 due to internal interface 
issues.

		Linus

^ permalink raw reply

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19  4:29 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Nicolas Pitre, Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.999.0710182111030.26902@woody.linux-foundation.org>

On Thu, Oct 18, 2007 at 09:21:55PM -0700, Linus Torvalds wrote:

> I'd love it, but the way our current SHA1 parser works, I don't think it 
> can really do it.
> 
> Basically, we currently assume that a SHA1 expression always expands to a 
> *single* SHA1.

Ah, right. I hadn't thought of that. While it would be a nice
convenience feature, it's probably not worth the deep internal hackery
that would be required.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: lmage11 @ 2007-10-19  4:41 UTC (permalink / raw)
  To: git

Uh... wow. Ok, this thread has really taken off. Sorry, but I can't be
involved with it right now, I'll get back into it sometime tomorrow. Try
not to get too far ahead of me! :)

Also sorry about the crazy line breaking, I'm using a crappy webmail
client at the moment...

Thanks,
   Ari

^ permalink raw reply

* Re: [PATCH 2/2] Documentation/git-gc: improve description of --auto
From: Jeff King @ 2007-10-19  4:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Steven Grimm, Brian Gernhardt
In-Reply-To: <20071019023850.GD8298@coredump.intra.peff.net>

On Thu, Oct 18, 2007 at 10:38:50PM -0400, Jeff King wrote:

> I am hunting this down right now. asciidoc _does_ generate
> XML <literal>foo</literal> for `foo`, but it looks like docbook is
> throwing that away when converting to manpages. Hopefully there is an
> easy tweak...

Ugh. Looks like Junio came up with a solution to this in 524e5ffc
(although it was for literallayout sections, I think the same technique
could be applied). However, it had problems with docbook 1.69, and was
reverted in 63c21c49.

Julian Phillips added monospacing of listingblocks in 281a53bb, but that
technique is only applicable to asciidoc "blocks", which I think won't
work in this instance.

It really seems silly that docbook doesn't monospace <literal>s when
converting to manpages. Perhaps somebody who knows more about docbook
than I do can say more.

-Peff

^ permalink raw reply

* [PATCH] Stop displaying "Pack pack-$ID created." during git-gc
From: Shawn O. Pearce @ 2007-10-19  5:03 UTC (permalink / raw)
  To: Nicolas Pitre, Jeff King; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0710182251110.19446@xanadu.home>

Discussion on the list tonight came to the conclusion that showing
the name of the packfile we just created during git-repack is not
a very useful message for any end-user.  For the really technical
folk who need to have the name of the newest packfile they can use
something such as `ls -t .git/objects/pack | head -2` to find the
most recently created packfile.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Nicolas Pitre <nico@cam.org> wrote:
 > On Thu, 18 Oct 2007, Jeff King wrote:
 > >     Can we get rid of total statistics (I think this is useful for some
 > >     power users, but perhaps there should be a verbosity level), the
 > >     name of the pack file (same deal), and the totally useless "Done."?
 >
 > Agreed for the pack name.  Certainly no one cares.

 This makes it so.

 git-repack.sh |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/git-repack.sh b/git-repack.sh
index e72adc4..7220635 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -83,9 +83,6 @@ for name in $names ; do
 	fullbases="$fullbases pack-$name"
 	chmod a-w "$PACKTMP-$name.pack"
 	chmod a-w "$PACKTMP-$name.idx"
-	if test "$quiet" != '-q'; then
-	    echo "Pack pack-$name created."
-	fi
 	mkdir -p "$PACKDIR" || exit
 
 	for sfx in pack idx
-- 
1.5.3.4.1249.g895be

^ permalink raw reply related

* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Shawn O. Pearce @ 2007-10-19  5:09 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710182340550.19446@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> On Thu, 18 Oct 2007, Jeff King wrote:
> > Also, why do we abbreviate "refs/heads/master" from the remote, but we
> > don't abbreviate refs/heads/origin for the local? Maybe something like:
> > 
> >   * local heads/origin -> remote heads/master (fast forward)
> > 
> > or for separate remote
> > 
> >   * local remotes/origin/master -> remote heads/master (fast forward)
> 
> That looks fine for a push.  I'd say "remote foo -> local bar" for a 
> fetch.

The fetch side is easy to change on top of the db/fetch-pack series
that is currently in next.

What's hard is the remote side.  receive-pack on the remote side
doesn't have the local's refname but it prints its own message upon
a successful update.

-- 
Shawn.

^ permalink raw reply

* gitk patch collection pull request
From: Shawn O. Pearce @ 2007-10-19  5:28 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:
  Paul Mackerras (1):
        gitk: Fix bug causing undefined variable error when cherry-picking

are available in the git repository at:

  git://repo.or.cz:/git/spearce.git gitk

Jonathan del Strother (2):
      gitk: Added support for OS X mouse wheel
      Fixing gitk indentation

Sam Vilain (1):
      gitk: disable colours when calling git log

 gitk |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)


I'm carrying these in my pu branch but would like to move them up
into master.  My gitk branch is actually forked off your own gitk
repository and doesn't contain the git.git history so you should
be able to do a direct pull.

-- 
Shawn.

^ permalink raw reply

* Re: git on afs
From: Shawn O. Pearce @ 2007-10-19  5:48 UTC (permalink / raw)
  To: Todd T. Fries; +Cc: git, Brandon Casey
In-Reply-To: <20071018203106.GA13518@fries.net>

There's two different issues here so I'm going to split the thread
into two to simplify the discussion.  Well, for me anyway.  ;-)

"Todd T. Fries" <todd@fries.net> wrote:
> 1) git presumes that link() works fine across subdirs; in afs land,
>    hardlinks do not succeed ever
...
> diff --git a/sha1_file.c b/sha1_file.c
> index 83a06a7..1b93322 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1961,7 +1961,7 @@ static int link_temp_to_file(const char *tmpfile, const char *filename)
>  	int ret;
>  	char *dir;
>  
> -	if (!link(tmpfile, filename))
> +	if (!rename(tmpfile, filename))
>  		return 0;
>  
>  	/*
> @@ -1980,7 +1980,7 @@ static int link_temp_to_file(const char *tmpfile, const char *filename)
>  			return -2;
>  		}
>  		*dir = '/';
> -		if (!link(tmpfile, filename))
> +		if (!rename(tmpfile, filename))
>  			return 0;
>  		ret = errno;
>  	}

These cases should already be handled lower, see l.1997-2012 of
sha1_file.c:

    /*
     * Coda hack - coda doesn't like cross-directory links,
     * so we fall back to a rename, which will mean that it
     * won't be able to check collisions, but that's not a
     * big deal.
     *
     * The same holds for FAT formatted media.
     *
     * When this succeeds, we just return 0. We have nothing
     * left to unlink.
     */
    if (ret && ret != EEXIST) {
        if (!rename(tmpfile, filename))

> Brandon Casey <casey@nrlssc.navy.mil> wrote:
> On Thu, 18 Oct 2007, Todd T. Fries wrote:
> 
> > link() returns -1 errno 17 File exists on afs.
> >
> > To further muddy the waters, linking within the same dir is ok,
> > linking outside the same dir is not:
> >
> > todd@ispdesk/p6 ~/tmp=A661$ mkdir dir
> > todd@ispdesk/p6 ~/tmp=A662$ touch a
> > todd@ispdesk/p6 ~/tmp=A663$ ln a b
> > todd@ispdesk/p6 ~/tmp=A664$ ls -l a b
> > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > todd@ispdesk/p6 ~/tmp=A665$ ls -li a b
> > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > todd@ispdesk/p6 ~/tmp=A666$ ln a dir/b
> > ln: dir/b: File exists
> > todd@ispdesk/p6 ~/tmp=A667$ echo $?
> 
> That error is just bogus on afs. If it returned a sane
> error, things would just work.
> 
> But, looks like afs only supports linking within the same
> directory: http://www.angelfire.com/hi/plutonic/afs-faq.html

So according to that error message from "ln" we really should have
fallen into that Coda hack I just mentioned.  Did we instead get
a different errno here and not use that hack?


We probably could just use rename as you do above but then we would
want to remove the unlink(tmpfile) call on l.2013 in sha1_file.c.
Otherwise we're always incurring a syscall for no reason.  I'm not
against a change here, I just want to make sure we make the right
change for AFS.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: git on afs
From: Shawn O. Pearce @ 2007-10-19  6:06 UTC (permalink / raw)
  To: Linus Torvalds, Todd T. Fries; +Cc: git
In-Reply-To: <alpine.LFD.0.999.0710181543380.26902@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > 
> > 2) git presumes that DTYPE(de) != DT_DIR .. means the dirent is not a dir
> >    this is not true for afs
> 
> That's a major bug, and has nothing to do with AFS. Oops. 
> 
> If you look just a bit lower, you'll see that just a few lines down, git 
> handles DT_UNKNOWN correctly, and just does a lstat() on it as required. I 
> guess that logic should be moved up, or alternatively the exclude logic 
> should be moved down.
> 
> Your patch looks ok, but at the same time, I don't think it's really the 
> right thing to do, since it now does that lstat() twice.

What about this instead?  It avoids the double lstat() of Todd's
original patch but seems like it would fix the issue here.  Or did
I misunderstand the problem?


--8>--
From f290fc5f6ec5042b7c0393a300e4d3738b9090b8 Mon Sep 17 00:00:00 2001
From: Shawn O. Pearce <spearce@spearce.org>
Date: Fri, 19 Oct 2007 02:03:55 -0400
Subject: [PATCH] Correctly scan directories when DTYPE(de) is unknown

We have been presuming that if DTYPE(de) != DT_DIR than the item we
obtained from readdir() isn't a directory.  This is actually not
true, it could still be a directory, especially if the underlying
system we were compiled for doesn't have the d_type member of struct
dirent and we have enabled our compatability definition of DTYPE(de)
to always be DT_UNKNOWN.

If DTYPE(de) == DT_UNKNOWN we need to fall through and perform an
actual lstat() call to determine what the type of this item is so
we know how to proceed with our scanning.

Reported by Todd T. Fries while trying to use Git on AFS, which
apparently has DTYPE(de) != DT_DIR for directories.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 dir.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..d2597ff 100644
--- a/dir.c
+++ b/dir.c
@@ -487,9 +487,10 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
 			if (exclude != dir->show_ignored) {
-				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+				if (!dir->show_ignored)
+					continue;
+				else if (DTYPE(de) != DT_DIR && DTYPE(de) != DT_UNKNOWN)
 					continue;
-				}
 			}
 
 			switch (DTYPE(de)) {
-- 
1.5.3.4.1249.g895be

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH] Avoid invoking diff drivers during git-stash
From: Johannes Sixt @ 2007-10-19  6:08 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <20071019013350.GA14020@spearce.org>

Shawn,

thanks for the fast response with a patch.

Shawn O. Pearce schrieb:
>  Johannes Sixt <j.sixt@viscovery.net> wrote:
>  > (1) Looking at git-stash.sh I see a few uses of 'git diff' in
>  > apply_stash(). Shouldn't these use one of git-diff-{tree,index,files)? The
>  > reason is that porcelain 'git diff' invokes custom diff drivers (that in my   
>  > case run a UI program), whereas the plumbing does not.
>  >
>  > Is there a particular reason to use porcelain 'git diff'?
> 
>  Does this fix the problem?

It does!

> @@ -110,7 +110,7 @@ show_stash () {
>  
>  	w_commit=$(git rev-parse --verify "$s") &&
>  	b_commit=$(git rev-parse --verify "$s^") &&
> -	git diff $flags $b_commit $w_commit
> +	git diff-tree $flags $b_commit $w_commit

However, this porcelain 'git diff' should actually remain because it's part 
of show_stash().

-- Hannes

^ permalink raw reply

* Re: [PATCH] Avoid invoking diff drivers during git-stash
From: Shawn O. Pearce @ 2007-10-19  6:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <471849D7.1020303@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Shawn O. Pearce schrieb:
> > Johannes Sixt <j.sixt@viscovery.net> wrote:
> > > (1) Looking at git-stash.sh I see a few uses of 'git diff' in
> > > apply_stash(). Shouldn't these use one of git-diff-{tree,index,files)? 
> > The
> > > reason is that porcelain 'git diff' invokes custom diff drivers (that 
> > in my   > case run a UI program), whereas the plumbing does not.
> > >
> > > Is there a particular reason to use porcelain 'git diff'?
> >
> > Does this fix the problem?
> 
> It does!
> 
> >@@ -110,7 +110,7 @@ show_stash () {
> > 
> > 	w_commit=$(git rev-parse --verify "$s") &&
> > 	b_commit=$(git rev-parse --verify "$s^") &&
> >-	git diff $flags $b_commit $w_commit
> >+	git diff-tree $flags $b_commit $w_commit
> 
> However, this porcelain 'git diff' should actually remain because it's part 
> of show_stash().

Heh.  Damn.  I was just starting to prepare my evening push and
this patch is in maint, which I just merged to master, and I just
rebased all of my pu topic branches over that.  Junio's Meta toolkit
doesn't have an "unrebase" so I can go back and amend that damn
commit before pushing.

I'm feeling lazy and don't want to create an unRB right now.
I'll probably just throw another commit into maint to fix the
above hunk.  Thanks for catching it.

-- 
Shawn.

^ permalink raw reply

* [RFC/PATCH] git-fetch: mega-terse fetch output
From: Jeff King @ 2007-10-19  6:22 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce

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.
==> git://repo.or.cz/git/spearce.git
 * branch gitk -> origin/gitk
 * branch maint -> origin/maint (fast forward)
 * branch master -> origin/master (fast forward)
 * branch next -> origin/next (fast forward)
 - branch pu -> origin/pu (non-fast forward, refused)
 * branch todo -> origin/todo (fast forward)
==> git://repo.or.cz/git/spearce.git
 * tag v1.5.3.2 -> v1.5.3.2

Particular changes include:
  - rather than each updated ref stating the remote url, the
    url is printed once before any refs
  - refs which did not update get a '-' rather than a '*'
  - the order is changed from "$to: storing $from" to
    "$from -> $to"
  - we abbreviate the local refs (chopping refs/heads,
    refs/tags, refs/remotes). This means we're losing
    information, but hopefully it is obvious when storing
    "origin/master" that it is in refs/remotes.
  - fast forward information goes at the end
  - cut out "Auto-following ..." text

What do people think? Some changes? All?

Other questions:
  - Is the "==>" too ugly? It needs to be short (many urls
    are almost 80 characters already), and it needs to stand
    out from the "resolving deltas" line, so I think some
    symbol is reasonable.
  - Should we omit "(fast forward)" since it is the usual
    case?
  - Should refs/remotes/* keep the "remotes/" part?
  - If you read the patch, there are a few cases covered
    that I don't show in the example. Are they ugly or
    better? I can't even figure out how to
    get the '==' case to show up.
  - Should tags always just say "tag $foo". Do we ever
    actually map the tags when following?
  - How annoying is the doubled '==> $url' line? It comes
    from the fact that we fetch the tags separately.

Somebody mentioned colorization on irc. I think that is reasonable but
should definitely wait for another patch.

---
 builtin-fetch.c |   73 +++++++++++++++++++++++++------------------------------
 1 files changed, 33 insertions(+), 40 deletions(-)

It drops more lines than it adds, so it _must_ be good!

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 3442f3d..4440521 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -123,12 +123,6 @@ static struct ref *get_ref_map(struct transport *transport,
 	return ref_map;
 }
 
-static void show_new(enum object_type type, unsigned char *sha1_new)
-{
-	fprintf(stderr, "  %s: %s\n", typename(type),
-		find_unique_abbrev(sha1_new, DEFAULT_ABBREV));
-}
-
 static int s_update_ref(const char *action,
 			struct ref *ref,
 			int check_old)
@@ -157,6 +151,11 @@ static int update_local_ref(struct ref *ref,
 	struct commit *current = NULL, *updated;
 	enum object_type type;
 	struct branch *current_branch = branch_get(NULL);
+	const char *pretty_ref = ref->name + (
+		!prefixcmp(ref->name, "refs/heads/") ? 11 :
+		!prefixcmp(ref->name, "refs/tags/") ? 10 :
+		!prefixcmp(ref->name, "refs/remotes/") ? 13 :
+		0);
 
 	type = sha1_object_info(ref->new_sha1, NULL);
 	if (type < 0)
@@ -164,19 +163,15 @@ static int update_local_ref(struct ref *ref,
 
 	if (!*ref->name) {
 		/* Not storing */
-		if (verbose) {
-			fprintf(stderr, "* fetched %s\n", note);
-			show_new(type, ref->new_sha1);
-		}
+		if (verbose)
+			fprintf(stderr, " * branch %s -> FETCH_HEAD\n", note);
 		return 0;
 	}
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose) {
-			fprintf(stderr, "* %s: same as %s\n",
-				ref->name, note);
-			show_new(type, ref->new_sha1);
-		}
+		if (verbose)
+			fprintf(stderr, " - %s == %s\n",
+				note, pretty_ref);
 		return 0;
 	}
 
@@ -189,30 +184,33 @@ static int update_local_ref(struct ref *ref,
 		 * the head, and the old value of the head isn't empty...
 		 */
 		fprintf(stderr,
-			" * %s: Cannot fetch into the current branch.\n",
-			ref->name);
+			" - %s: Cannot fetch into the current branch.\n",
+			pretty_ref);
 		return 1;
 	}
 
 	if (!is_null_sha1(ref->old_sha1) &&
 	    !prefixcmp(ref->name, "refs/tags/")) {
-		fprintf(stderr, "* %s: updating with %s\n",
-			ref->name, note);
-		show_new(type, ref->new_sha1);
+		fprintf(stderr, " * tag %s -> %s\n",
+			note, pretty_ref);
 		return s_update_ref("updating tag", ref, 0);
 	}
 
 	current = lookup_commit_reference(ref->old_sha1);
 	updated = lookup_commit_reference(ref->new_sha1);
 	if (!current || !updated) {
-		char *msg;
-		if (!strncmp(ref->name, "refs/tags/", 10))
+		const char *msg;
+		const char *what;
+		if (!strncmp(ref->name, "refs/tags/", 10)) {
 			msg = "storing tag";
-		else
+			what = "tag";
+		}
+		else {
 			msg = "storing head";
-		fprintf(stderr, "* %s: storing %s\n",
-			ref->name, note);
-		show_new(type, ref->new_sha1);
+			what = "branch";
+		}
+		fprintf(stderr, " * %s %s -> %s\n",
+			what, note, pretty_ref);
 		return s_update_ref(msg, ref, 0);
 	}
 
@@ -220,23 +218,19 @@ static int update_local_ref(struct ref *ref,
 	strcpy(newh, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 
 	if (in_merge_bases(current, &updated, 1)) {
-		fprintf(stderr, "* %s: fast forward to %s\n",
-			ref->name, note);
-		fprintf(stderr, "  old..new: %s..%s\n", oldh, newh);
+		fprintf(stderr, " * branch %s -> %s (fast forward)\n",
+			note, pretty_ref);
 		return s_update_ref("fast forward", ref, 1);
 	}
 	if (!force && !ref->force) {
 		fprintf(stderr,
-			"* %s: not updating to non-fast forward %s\n",
-			ref->name, note);
-		fprintf(stderr,
-			"  old...new: %s...%s\n", oldh, newh);
+			" - branch %s -> %s (non-fast forward, refused)\n",
+			note, pretty_ref);
 		return 1;
 	}
 	fprintf(stderr,
-		"* %s: forcing update to non-fast forward %s\n",
-		ref->name, note);
-	fprintf(stderr, "  old...new: %s...%s\n", oldh, newh);
+		" * branch %s -> %s (non-fast forward)\n",
+		note, pretty_ref);
 	return s_update_ref("forced-update", ref, 1);
 }
 
@@ -249,6 +243,8 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 	const char *what, *kind;
 	struct ref *rm;
 
+	fprintf(stderr, "==> %s\n", url);
+
 	fp = fopen(git_path("FETCH_HEAD"), "a");
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -308,7 +304,7 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 			note);
 
 		if (ref)
-			update_local_ref(ref, note, verbose);
+			update_local_ref(ref, what, verbose);
 	}
 	fclose(fp);
 }
@@ -368,9 +364,6 @@ static struct ref *find_non_local_tags(struct transport *transport,
 		if (!path_list_has_path(&existing_refs, ref_name) &&
 		    !path_list_has_path(&new_refs, ref_name) &&
 		    lookup_object(ref->old_sha1)) {
-			fprintf(stderr, "Auto-following %s\n",
-				ref_name);
-
 			path_list_insert(ref_name, &new_refs);
 
 			rm = alloc_ref(strlen(ref_name) + 1);
-- 
1.5.3.4.1252.g21baf-dirty

^ permalink raw reply related

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: David Symonds @ 2007-10-19  6:39 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Shawn O. Pearce
In-Reply-To: <20071019062219.GA28499@coredump.intra.peff.net>

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.
> ==> git://repo.or.cz/git/spearce.git
>  * branch gitk -> origin/gitk
>  * branch maint -> origin/maint (fast forward)
>  * branch master -> origin/master (fast forward)
>  * branch next -> origin/next (fast forward)
>  - branch pu -> origin/pu (non-fast forward, refused)
>  * branch todo -> origin/todo (fast forward)
> ==> git://repo.or.cz/git/spearce.git
>  * tag v1.5.3.2 -> v1.5.3.2

What about making it even more terse so it's even easier to visually
scan: (mainly thinking that fast-forwarding is so common it could be
considered the "default")

> ==> git://repo.or.cz/git/spearce.git
 * gitk -> origin/gitk (new)
 * maint -> origin/maint
 * master -> origin/master
 * next -> origin/next
 - pu -> origin/pu (refused)
 * todo -> origin/todo
==> git://repo.or.cz/git/spearce.git
 * tag v1.5.3.2

Also, perhaps the trailing notes (fast forward, refused, etc.) should
be significantly indented to the right to stand out even further from
branch names that might be quite long.

Dave.

^ permalink raw reply

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Jeff King @ 2007-10-19  6:46 UTC (permalink / raw)
  To: David Symonds; +Cc: git, Shawn O. Pearce
In-Reply-To: <ee77f5c20710182339g30d025f0tfe74479d672ae36e@mail.gmail.com>

On Fri, Oct 19, 2007 at 04:39:56PM +1000, David Symonds wrote:

> What about making it even more terse so it's even easier to visually
> scan: (mainly thinking that fast-forwarding is so common it could be
> considered the "default")

Reasonable. I think it would be easier to scan if the fields were
column-aligned, but that requires making a few passes, which would
change the current code quite a bit. Or we could just fake it and give
it 20 characters for a branch name, padded with spaces.

> > ==> git://repo.or.cz/git/spearce.git
>  * gitk -> origin/gitk (new)

I miss the "branch" designator, personally. I do like the "new" to
differentiate from fast-forward.

>  * maint -> origin/maint
>  * master -> origin/master
>  * next -> origin/next
>  - pu -> origin/pu (refused)

I think this needs to explain why it was refused (non-fast forward,
refused). And you may still have:

 * pu -> origin/pu (non-fast forward)

for forced updates.

> ==> git://repo.or.cz/git/spearce.git
>  * tag v1.5.3.2

I am fine with that, as long as there aren't cases where we lose
information (i.e., where the local and remote tag names differ).

> Also, perhaps the trailing notes (fast forward, refused, etc.) should
> be significantly indented to the right to stand out even further from
> branch names that might be quite long.

Again, we could probably fake that by fixing the minimum column width of
the other fields.

-Peff

^ permalink raw reply

* Re: Qgit performance and maintain CVS environment with GIT repository
From: Andreas Ericsson @ 2007-10-19  7:14 UTC (permalink / raw)
  To: pete
  Cc: Marco Costalba, Johannes Schindelin, Robin Rosenberg,
	piet.delaney, Linus Torvalds, VMiklos, free cycle, git,
	piet.delaney, Piet Delaney
In-Reply-To: <4717EF40.6000509@bluelane.com>

Pete/Piet Delaney wrote:
> Johannes:
>   I read somewhere in the past week that it was possible to maintain
>   our existing CVS environment with git. I though it was a separate
>   package to export git back to cvs but I just noticed a git-cvsserver
>   and as a std part of git and was wondering about using that.
> 
>   We have a number of build machines with flamebox perl scripts pulling
>   out CVS branches for builds. I was wondering what is the best way to
>   use git and it's nicer pull/push model and merge facility and possibly
>   maintain CVS exports for scripts doing builds if possible the cvsweb
>   and bonsai (CVS Query Form) that a number of engineers are currently
>   using. I started looking over out flamebox scripts with the intent
>   up converting them over to git but I mentioned the git to cvs
>   coexistence and we are wondering if that's a better route than
>   upgrading the flamebox scripts. Having our existing cvsweb, bonsai,
>   and gitweb along with the git utilities seems at least desirable.
>   Any thoughts or suggestions?
> 

If you do convert them to git, you can fairly easily do an automatic
bisect on build-errors, and the developer can (after some time) get
an email of what machines they broke the code on and what the bad
commit was.

Besides that, it's not a black-and-white scenario. If I were you I'd set
up git-cvsserver and make sure that works for all the scripts, and then
pick one or two auto-build things to convert to git. Preferrably on a
separate machine, so everything keeps working the same as always while
you're fiddling with the auto-build stuff.

Just my two cents.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH resend again] gitk: Do not pick up file names of "copy from" lines
From: Johannes Sixt @ 2007-10-19  7:25 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Paul Mackerras, git
In-Reply-To: <20071019052823.GI14735@spearce.org>

From: Johannes Sixt <johannes.sixt@telecom.at>

A file copy would be detected only if the original file was modified in the
same commit. This implies that there will be a patch listed under the
original file name, and we would expect that clicking the original file
name in the file list warps the patch window to that file's patch. (If the
original file was not modified, the copy would not be detected in the first
place, the copied file would be listed as "new file", and this whole matter
would not apply.)

However, if the name of the copy is sorted after the original file's patch,
then the logic introduced by commit d1cb298b0b (which picks up the link
information from the "copy from" line) would overwrite the link
information that is already present for the original file name, which was
parsed earlier. Hence, this patch reverts part of said commit.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
  Shawn O. Pearce schrieb:
  > I'm carrying these in my pu branch but would like to move them up
  > into master.

  Would you mind putting this one into your queue, too? I haven't seen it
  appear in Paul's repo.

  -- Hannes

  gitk |    3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index b3ca704..1306382 100755
--- a/gitk
+++ b/gitk
@@ -5216,8 +5216,7 @@ proc getblobdiffline {bdf ids} {
  	    set diffinhdr 0

  	} elseif {$diffinhdr} {
-	    if {![string compare -length 12 "rename from " $line] ||
-		![string compare -length 10 "copy from " $line]} {
+	    if {![string compare -length 12 "rename from " $line]} {
  		set fname [string range $line [expr 6 + [string first " from " $line] ] end]
  		if {[string index $fname 0] eq "\""} {
  		    set fname [lindex $fname 0]
-- 
1.5.3.722.gccbb1

^ permalink raw reply related

* [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott R Parish @ 2007-10-19  6:59 UTC (permalink / raw)
  To: git

 + check PATH for git_exec_path after other locations but before builtin
 + prepend MANPATH and PERL5LIB in addition to PATH

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |   50 ++++++++++++++++++++++++++++++++++++++-
 git.c      |   76 +++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..c6ecca9 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -13,19 +13,67 @@ void git_set_exec_path(const char *exec_path)
 }
 
 
+/* 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;
+}
+
+
 /* 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;
 }
 
diff --git a/git.c b/git.c
index 9eaca1d..252ee7c 100644
--- a/git.c
+++ b/git.c
@@ -6,26 +6,56 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
+static void prepend_to_env(const char *env, const char *basedir,
+			   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);
+}
 
-	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);
 }
 
 static int handle_options(const char*** argv, int* argc, int* envchanged)
@@ -414,8 +444,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
+		exec_path = cmd;
 		cmd = slash;
 	}
 
@@ -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.
 	 */
 	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
+		prepend_to_envs(exec_path, strlen(exec_path));
 	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	prepend_to_envs(exec_path, strlen(exec_path));
 
 	while (1) {
 		/* See if it's an internal command */
-- 
1.5.3.4.206.g58ba4-dirty

^ permalink raw reply related


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