Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-cherry-pick: improve description of -x.
From: Ralf Wildenhues @ 2007-10-22  5:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Frank Lichtenheld, git
In-Reply-To: <20071022051453.GM14735@spearce.org>

* Shawn O. Pearce wrote on Mon, Oct 22, 2007 at 07:14:53AM CEST:
> 
> I think you are right that the current behavior of -x *not*
> including the prior commit SHA-1 in the case of a conflict is wrong.
> The problem however is that git-commit.sh doesn't get the data
> necessary to preseve the original author name/email/date/tz unless
> you use the "-c $id" option.

But the note added by -x is even missing when I add "-c $id" to
the git-commit command!  That's the point I was trying to make,
and really the only thing that seemed weird to me.

Cheers,
Ralf

^ permalink raw reply

* Re: [PATCH] git-cherry-pick: improve description of -x.
From: Shawn O. Pearce @ 2007-10-22  5:14 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: Frank Lichtenheld, git
In-Reply-To: <20071021093618.GC12794@ins.uni-bonn.de>

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> * Shawn O. Pearce wrote on Sat, Oct 20, 2007 at 05:19:17AM CEST:
> > Frank Lichtenheld <frank@lichtenheld.de> wrote:
> > > On Fri, Oct 19, 2007 at 07:41:34PM +0200, Ralf Wildenhues wrote:
> > > > 
> > > > Is that by design (because there were conflicts) or an omission?
> > > > In case of the former, maybe the description of -x should mention this.
> > > 
> > > git commit currently doesn't know that you commit a cherry-pick. The -c
> > > only says to use the commit message of the original commit. So this is
> > > currently by design.
> > 
> > Ralf, can you submit an updated version of this patch that describes
> > the current behavior better, given the "by design" remark above
> > from Frank?
> 
> Here it goes.  Still makes me wonder whether that is the ideal mode of
> operation or not.

Thanks.

I think you are right that the current behavior of -x *not*
including the prior commit SHA-1 in the case of a conflict is wrong.
The problem however is that git-commit.sh doesn't get the data
necessary to preseve the original author name/email/date/tz unless
you use the "-c $id" option.  There's some work here to store the
necessary information into a file that git-commit.sh could pickup,
and then making sure stale versions of those files get cleaned up
properly, etc.

At least the current behavior is now documented.  Maybe someone
will be bothered enough by it to try and submit a patch that changes
the behavior.
 
-- 
Shawn.

^ permalink raw reply

* Re: .gittattributes handling has deficiencies
From: Shawn O. Pearce @ 2007-10-22  5:01 UTC (permalink / raw)
  To: david; +Cc: Steffen Prohaska, git
In-Reply-To: <Pine.LNX.4.64.0710210204580.4818@asgard>

david@lang.hm wrote:
> On Sun, 21 Oct 2007, Steffen Prohaska wrote:
> >If a .gitattributes is in the work tree and we checkout a
> >different head, the .gitattributes of the head we are switching
> >to must have precedence.
> >
> >Maybe the gitattributes of a file should be part of the per-file
> >flags in the index. Thus we could verify if the flags changed and
> >if so, adjust the work tree accordig to the new flags.  I'm
> >lacking a deeper insight into the git internals.  Therefore, I
> >can't really say if the index is the right place.  But it looks
> >to me as if changing an attribute should be treated similar to a
> >changing sha1, as far as the work tree is concerned.
> 
> the problem with this is that each attribute ends up needing it's own 
> flag, which severely limits extending things (see the discussions on file 
> permissions for examples). it's also much harder to manipulate them then 
> in a file.

Yea, you really don't want to copy .gitattributes into the per-file
records in the index.  That's not going to scale as more types of
attributes are defined.

Fortunately the .gitattributes file format was designed to be
readable even when there's merge conflicts; that is it is a
very simple line-oriented record format.  One could difference
the old .gitattributes currently found in the index against the
.gitattributes we are switching to (from the target tree-ish),
scan the lines removed/added, find which files those match against
in the target tree-ish, and just add those files to the list of
things we need to checkout.

If any of those files is dirty then we just refuse the checkout,
just as if the file was modified and we were switching branches.
The user then needs to decide how to continue (probably stash the
file and then restart the checkout).

Rather simple IMHO.  Of course I haven't gone into that part of
read-tree recently, and the .gitattribute parser reads from the
working directory, so you need to make sure you checkout the target
.gitattributes file before anything else in the "to process list".

-- 
Shawn.

^ permalink raw reply

* Re: how to deal with conflicts after "git stash apply"?
From: Scott Parish @ 2007-10-22  4:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0710212338200.25221@racer.site>

> You should have a look into Documentation/diff-format.txt (section 
> "combined diff format") to learn more.

Ah, thanks for the pointer; i hadn't realized that this wasn't
standard diff format. With this and what Shawn said, it all makes
sense now.

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: how to deal with conflicts after "git stash apply"?
From: Shawn O. Pearce @ 2007-10-22  4:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Scott Parish, git
In-Reply-To: <Pine.LNX.4.64.0710212338200.25221@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sun, 21 Oct 2007, Scott Parish wrote:
> 
> > How is the intended way to deal with "git stash apply" conflicts?
> > If i just edit the file and remove the conflict, "git diff" gives
> > some really messed up output. Documentation for other commands and
> > conflicts suggest "git commit" after cleaning up the conflict, or
> > "git add", but in the case of "stash apply" i'm not ready for a
> > commit yet, and "git add" keeps "git diff" from showing any output.
> 
> You are probably seeing combined diffs.
> 
> This show not only the differences of the working tree relative to HEAD, 
> but also of the changes stored in the stash.

The reason Scott is seeing a combined diff here is merge-recursive
left the different versions of the file in the higher order stages
of the index when it found conflicts during the apply.  You need
to use git-add to stage the resolved file and replace the higher
order stages with just the normal stage 0.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.
From: Shawn O. Pearce @ 2007-10-22  4:21 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Scott Parish, git
In-Reply-To: <Pine.LNX.4.64.0710212256270.25221@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Earlier, we tried to find the git commands in several possible exec
> dirs.  Now, if all of these failed, try to find the git command in
> PATH.
...
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..70b84b0 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
>  	int i;
>  	const char *paths[] = { current_exec_path,
>  				getenv(EXEC_PATH_ENVIRONMENT),
> -				builtin_exec_path };
> +				builtin_exec_path,
> +				"" };

So if the user sets GIT_EXEC_PATH="" and exports it we'll search
$PATH before the builtin exec path that Git was compiled with?
Are we sure we want to do that?

I'm going to throw this into pu tonight just so I don't lose it,
but I have a feeling we want to amend it before merging.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/2] Correct some sizeof(size_t) != sizeof(unsigned long)  typing errors
From: Shawn O. Pearce @ 2007-10-22  4:00 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: René Scharfe, git
In-Reply-To: <20071021103132.GA25741@artemis.corp>

Pierre Habouzit <madcoder@debian.org> wrote:
> On Sun, Oct 21, 2007 at 09:23:49AM +0000, René Scharfe wrote:
> > > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> > > ---
> > >  builtin-apply.c   |    2 +-
> > >  builtin-archive.c |    2 +-
> > >  diff.c            |    4 ++--
> > >  entry.c           |    2 +-
> > >  strbuf.h          |    8 +++++++-
> > >  test-delta.c      |    3 ++-
> > >  6 files changed, 14 insertions(+), 7 deletions(-)
> > 
> > I have a feeling this is going in then wrong direction.  Shouldn't
> > we rather use size_t everywhere?  malloc() takes a size_t, and it's
> > the basis of strbuf and also of the file content functions.
> 
>   I agree, Junio was working on a patch that generalized use of size_t's
> when unsigned long where used and size_t meant, I suppose he didn't had
> the time to push it.

Yea, you guys convinced me to go with René's patch.  I'm
replacing mine and will put it into next tonight.

I actually had started with what René wrote but changed it to
what you saw before posting it to the list.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: On Tabs and Spaces
From: Miles Bader @ 2007-10-22  3:39 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Petr Baudis, Jari Aalto, git
In-Reply-To: <dbfc82860710180439j6d02651foff9c0c84623a9cf1@mail.gmail.com>

"Nikolai Weibull" <now@bitwi.se> writes:
> To change the subject, let me point out that the percent symbol should
> be juxtaposed with the number, that is, write "99%", not "99 %", in
> English.

Hmm, but what if one uses a tab?

-miles

-- 
My books focus on timeless truths.  -- Donald Knuth

^ permalink raw reply

* [PATCH] gitweb: Provide title attributes for abbreviated author names.
From: David Symonds @ 2007-10-22  0:28 UTC (permalink / raw)
  To: pasky, spearce; +Cc: git, David Symonds

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 gitweb/gitweb.perl |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b2bae1b..119ad55 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3461,9 +3461,15 @@ sub git_shortlog_body {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
+		my $author = chop_str($co{'author_name'}, 10);
+		if ($author ne $co{'author_name'}) {
+			$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
+		} else {
+			$author = esc_html($author);
+		}
 		# git_summary() used print "<td><i>$co{'age_string'}</i></td>\n" .
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
-		      "<td><i>" . esc_html(chop_str($co{'author_name'}, 10)) . "</i></td>\n" .
+		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>";
 		print format_subject_html($co{'title'}, $co{'title_short'},
 		                          href(action=>"commit", hash=>$commit), $ref);
@@ -3511,9 +3517,15 @@ sub git_history_body {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
+	# shortlog uses      chop_str($co{'author_name'}, 10)
+		my $author = chop_str($co{'author_name'}, 15, 3);
+		if ($author ne $co{'author_name'}) {
+			"<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
+		} else {
+			$author = esc_html($author);
+		}
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
-		      # shortlog uses      chop_str($co{'author_name'}, 10)
-		      "<td><i>" . esc_html(chop_str($co{'author_name'}, 15, 3)) . "</i></td>\n" .
+		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>";
 		# originally git_history used chop_str($co{'title'}, 50)
 		print format_subject_html($co{'title'}, $co{'title_short'},
@@ -3667,8 +3679,14 @@ sub git_search_grep_body {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
+		my $author = chop_str($co{'author_name'}, 15, 5);
+		if ($author ne $co{'author_name'}) {
+			$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
+		} else {
+			$author = esc_html($author);
+		}
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
-		      "<td><i>" . esc_html(chop_str($co{'author_name'}, 15, 5)) . "</i></td>\n" .
+		      "<td><i>" . $author . "</i></td>\n" .
 		      "<td>" .
 		      $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), -class => "list subject"},
 			       esc_html(chop_str($co{'title'}, 50)) . "<br/>");
@@ -5181,8 +5199,14 @@ sub git_search {
 						print "<tr class=\"light\">\n";
 					}
 					$alternate ^= 1;
+					my $author = chop_str($co{'author_name'}, 15, 5);
+					if ($author ne $co{'author_name'}) {
+						$author = "<span title=\"" . esc_html($co{'author_name'}) . "\">" . esc_html($author) . "</span>";
+					} else {
+						$author = esc_html($author);
+					}
 					print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
-					      "<td><i>" . esc_html(chop_str($co{'author_name'}, 15, 5)) . "</i></td>\n" .
+					      "<td><i>" . $author . "</i></td>\n" .
 					      "<td>" .
 					      $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}),
 					              -class => "list subject"},
-- 
1.5.3.1

^ permalink raw reply related

* Re: [PATCH] "git help -a" should search all exec_paths and PATH
From: Scott Parish @ 2007-10-22  0:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0710212323580.25221@racer.site>

On Sun, Oct 21, 2007 at 11:25:29PM +0100, Johannes Schindelin wrote:

> > To fix this, help.c is modified to search in all the exec_paths and PATH
> > for potential git commands.
> 
> With this explanation, I would have expected that you add a loop just like 
> in exec-cmd.c.  Not anything more.  And certainly not the removal of a 
> sanity check for the length of the path name.

Well, i took a slightly different approach where that sanity check
wasn't nessisary. Instead of building up a string of the path of
each file, i'm saving the original directory in a file descriptor,
and "cd"ing to the exec_path currently being listed. Because of
that i can just stat() the names returned by readdir. Much simpler
imho

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: [PATCH, take 1] Linear-time/space rename logic (exact renames only)
From: David Symonds @ 2007-10-22  0:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Junio C Hamano, Shawn O. Pearce, David Kastrup,
	Jeff King
In-Reply-To: <alpine.LFD.0.999.0710211603200.10525@woody.linux-foundation.org>

On 22/10/2007, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> diff --git a/Makefile b/Makefile
> index 8db4dbe..17c31ba 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -291,7 +291,7 @@ LIB_H = \
>         run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
>         tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
>         utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
> -       mailmap.h remote.h
> +       mailmap.h remote.h hash.o

I assume that should be "hash.h", not "hash.o"?


Dave.

^ permalink raw reply

* Re: .gittattributes handling has deficiencies
From: david @ 2007-10-22  0:05 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0710211645210.12998@asgard.lang.hm>

On Sun, 21 Oct 2007, david@lang.hm wrote:

>>>> 
>>>> What do you mean by "checking out everything"?
>>>> Which command do you propose?
>>> 
>>> something like git checkout -f
>> 
>> I suspected this. I see two problems:
>> 
>> 1) it's too dangerous: I throws away _all_ changes, not only
>> changes that are related to gitattributes.
>
> this is true, the question of if this is 'too dangerous' depends on what 
> workflow you teach as safe. if you teach that checking out a new version will 
> loose any modifications you have made (which is useually the sane thing to do 
> by default anyway) then this is just more of the same
>
>> 2) it doesn't work reliably. git checkout -f will only update
>> files that git detects as changed. But you could have files that
>> should have crlf in the working copy but actually have only lf.
>> Those would not be updated.
>
> ok, you could do rm -r * before doing the checkout -f (or there's probably a 
> option to git to tell it not to preserve changes to the working area, I am 
> not a git guru.
>
>> I'll not recommend this. Not using .gitattributes is the only
>> sane solution.
>
> it may be the best thing to do for you and your users, that's not the same 
> thing as saying that it's the only sane solution.

by the way, I am not saying that my suggestion is the right way for things 
to be (especially long term), but I'm trying to figure out a work-around 
for the short term.

I'm very interested to see the logn-term suggestions, becouse I suspect 
that modt of them could be leveraged for the metastore jobs.

David Lang

^ permalink raw reply

* [PATCH, take 1] Linear-time/space rename logic (exact renames only)
From: Linus Torvalds @ 2007-10-21 23:59 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano, Shawn O. Pearce
  Cc: David Kastrup, Jeff King


This is a first effort at avoiding various O(n*m) effects in the rename 
detection. Right now it does so only for the exact renames, which is 
admittedly a rather easier case to handle, but having emailed a bit with 
Andy Chu, I think it's possible to do even the non-exact renames using a 
similar approach.

This depends on the previous diffcore-rename() cleanup, and introduces a 
new set of helpers for doing hash tables (hash.[ch]). We could probably 
move some of the other of our hash table users over to this (it's designed 
to be fairly generic), but that's a separate issue.

What it does is to rather than iterate over all sources and destinations 
and checking if they are identical (which is O(src*dst)), it hashes each 
of the sources and destinations into a hash table, using the SHA1 hash of 
the contents as the hash. That's O(n+m). It then walks the hash table 
(which is also O(m+n) in size), and only pairs up files for comparison 
that hashed to the same spot.

Doing this for more than just the exact same contents would be basically 
the same thing, except it starts hashing up fingerprints of the contents 
and linking up file pairs that get linked up by those fingerprints. More 
involved, but not impossible.

I tried a trivial case where I moved 100,000 files from one directory to 
another, and this patch speeds that up from ~13s to just under 2s for me.

However! Please note:
 - it looks ok, and I've tested it some, but this needs more people 
   looking at it.
 - because it only helps the exact rename case, it by no means "solves" 
   the rename cost issue. It just makes one particular case go much 
   faster.
 - in fact, the big optimization isn't the actual hash table, but the 
   independent and much simpler "diff_filespec->used" optimization for a 
   deleted filename that was used for a rename/copy.

But I'd like to have people give it a look,

		Linus

---

 Makefile          |    4 +-
 diffcore-rename.c |  214 ++++++++++++++++++++++++++++++++++------------------
 diffcore.h        |    1 +
 hash.c            |  110 +++++++++++++++++++++++++++
 hash.h            |   43 +++++++++++
 5 files changed, 296 insertions(+), 76 deletions(-)

diff --git a/Makefile b/Makefile
index 8db4dbe..17c31ba 100644
--- a/Makefile
+++ b/Makefile
@@ -291,7 +291,7 @@ LIB_H = \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
 	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
-	mailmap.h remote.h
+	mailmap.h remote.h hash.o
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -301,7 +301,7 @@ DIFF_OBJS = \
 LIB_OBJS = \
 	blob.o commit.o connect.o csum-file.o cache-tree.o base85.o \
 	date.o diff-delta.o entry.o exec_cmd.o ident.o \
-	interpolate.o \
+	interpolate.o hash.o \
 	lockfile.o \
 	patch-ids.o \
 	object.o pack-check.o pack-write.o patch-delta.o path.o pkt-line.o \
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 2077a9b..05d39db 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -4,6 +4,7 @@
 #include "cache.h"
 #include "diff.h"
 #include "diffcore.h"
+#include "hash.h"
 
 /* Table of rename/copy destinations */
 
@@ -96,29 +97,6 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
 	return &(rename_src[first]);
 }
 
-static int is_exact_match(struct diff_filespec *src,
-			  struct diff_filespec *dst,
-			  int contents_too)
-{
-	if (src->sha1_valid && dst->sha1_valid &&
-	    !hashcmp(src->sha1, dst->sha1))
-		return 1;
-	if (!contents_too)
-		return 0;
-	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
-		return 0;
-	if (src->size != dst->size)
-		return 0;
-	if (src->sha1_valid && dst->sha1_valid)
-	    return !hashcmp(src->sha1, dst->sha1);
-	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
-		return 0;
-	if (src->size == dst->size &&
-	    !memcmp(src->data, dst->data, src->size))
-		return 1;
-	return 0;
-}
-
 static int basename_same(struct diff_filespec *src, struct diff_filespec *dst)
 {
 	int src_len = strlen(src->path), dst_len = strlen(dst->path);
@@ -216,6 +194,7 @@ static void record_rename_pair(int dst_index, int src_index, int score)
 		die("internal error: dst already matched.");
 
 	src = rename_src[src_index].one;
+	src->used = 1;
 	one = alloc_filespec(src->path);
 	fill_filespec(one, src->sha1, src->mode);
 
@@ -262,56 +241,152 @@ static int compute_stays(struct diff_queue_struct *q,
 	return 1;
 }
 
+struct file_similarity {
+	int src_dst, index;
+	struct diff_filespec *filespec;
+	struct file_similarity *next;
+};
+
+static int find_identical_files(struct file_similarity *src,
+				struct file_similarity *dst)
+{
+	int renames = 0;
+	do {
+		struct diff_filespec *one = src->filespec;
+		struct file_similarity *p, *best;
+		int i = 100;
+
+		best = NULL;
+		for (p = dst; p; p = p->next) {
+			struct diff_filespec *two = p->filespec;
+
+			/* Already picked as a destination? */
+			if (!p->src_dst)
+				continue;
+			/* False hash collission? */
+			if (hashcmp(one->sha1, two->sha1))
+				continue;
+			best = p;
+			if (basename_same(one, two))
+				break;
+
+			/* Too many identical alternatives? Pick one */
+			if (!--i)
+				break;
+		}
+		if (best) {
+			best->src_dst = 0;
+			record_rename_pair(best->index, src->index, MAX_SCORE);
+			renames++;
+		}
+	} while ((src = src->next) != NULL);
+	return renames;
+}
+
+static int find_same_files(void *ptr)
+{
+	struct file_similarity *p = ptr;
+	struct file_similarity *src = NULL, *dst = NULL;
+
+	/* Split the hash list up into sources and destinations */
+	do {
+		struct file_similarity *entry = p;
+		p = p->next;
+		if (entry->src_dst < 0) {
+			entry->next = src;
+			src = entry;
+		} else {
+			entry->next = dst;
+			dst = entry;
+		}
+	} while (p);
+
+	/*
+	 * If we have both sources *and* destinations, see if
+	 * we can match them up
+	 */
+	return (src && dst) ? find_identical_files(src, dst) : 0;
+}
+
+/*
+ * Note: the rest of the rename logic depends on this
+ * phase also populating all the filespecs for any
+ * entry that isn't matched up with an exact rename.
+ */
+static int free_file_table(void *ptr)
+{
+	struct file_similarity *p = ptr;
+	do {
+		struct file_similarity *entry = p;
+		p = p->next;
+
+		/* Stupid special case, see note above! */
+		diff_populate_filespec(entry->filespec, 0);
+		free(entry);
+	} while (p);
+	return 0;
+}
+
+static unsigned int hash_filespec(struct diff_filespec *filespec)
+{
+	unsigned int hash;
+	if (!filespec->sha1_valid) {
+		if (diff_populate_filespec(filespec, 0))
+			return 0;
+		hash_sha1_file(filespec->data, filespec->size, "blob", filespec->sha1);
+	}
+	memcpy(&hash, filespec->sha1, sizeof(hash));
+	return hash;
+}
+
+static void insert_file_table(struct hash_table *table, int src_dst, int index, struct diff_filespec *filespec)
+{
+	void **pos;
+	unsigned int hash;
+	struct file_similarity *entry = xmalloc(sizeof(*entry));
+
+	entry->src_dst = src_dst;
+	entry->index = index;
+	entry->filespec = filespec;
+	entry->next = NULL;
+
+	hash = hash_filespec(filespec);
+	pos = insert_hash(hash, entry, table);
+
+	/* We already had an entry there? */
+	if (pos) {
+		entry->next = *pos;
+		*pos = entry;
+	}
+}
+
 /*
  * Find exact renames first.
  *
  * The first round matches up the up-to-date entries,
  * and then during the second round we try to match
  * cache-dirty entries as well.
- *
- * Note: the rest of the rename logic depends on this
- * phase also populating all the filespecs for any
- * entry that isn't matched up with an exact rename,
- * see "is_exact_match()".
  */
 static int find_exact_renames(void)
 {
-	int rename_count = 0;
-	int contents_too;
-
-	for (contents_too = 0; contents_too < 2; contents_too++) {
-		int i;
-
-		for (i = 0; i < rename_dst_nr; i++) {
-			struct diff_filespec *two = rename_dst[i].two;
-			int j;
-
-			if (rename_dst[i].pair)
-				continue; /* dealt with an earlier round */
-			for (j = 0; j < rename_src_nr; j++) {
-				int k;
-				struct diff_filespec *one = rename_src[j].one;
-				if (!is_exact_match(one, two, contents_too))
-					continue;
-
-				/* see if there is a basename match, too */
-				for (k = j; k < rename_src_nr; k++) {
-					one = rename_src[k].one;
-					if (basename_same(one, two) &&
-						is_exact_match(one, two,
-							contents_too)) {
-						j = k;
-						break;
-					}
-				}
-
-				record_rename_pair(i, j, (int)MAX_SCORE);
-				rename_count++;
-				break; /* we are done with this entry */
-			}
-		}
-	}
-	return rename_count;
+	int i;
+	struct hash_table file_table;
+
+	init_hash(&file_table);
+	for (i = 0; i < rename_src_nr; i++)
+		insert_file_table(&file_table, -1, i, rename_src[i].one);
+
+	for (i = 0; i < rename_dst_nr; i++)
+		insert_file_table(&file_table, 1, i, rename_dst[i].two);
+
+	/* Find the renames */
+	i = for_each_hash(&file_table, find_same_files);
+
+	/* .. and free the hash data structures */
+	for_each_hash(&file_table, free_file_table);
+	free_hash(&file_table);
+
+	return i;
 }
 
 void diffcore_rename(struct diff_options *options)
@@ -474,16 +549,7 @@ void diffcore_rename(struct diff_options *options)
 					pair_to_free = p;
 			}
 			else {
-				for (j = 0; j < rename_dst_nr; j++) {
-					if (!rename_dst[j].pair)
-						continue;
-					if (strcmp(rename_dst[j].pair->
-						   one->path,
-						   p->one->path))
-						continue;
-					break;
-				}
-				if (j < rename_dst_nr)
+				if (p->one->used)
 					/* this path remains */
 					pair_to_free = p;
 			}
diff --git a/diffcore.h b/diffcore.h
index eb618b1..a58d345 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -40,6 +40,7 @@ struct diff_filespec {
 	unsigned should_munmap : 1; /* data should be munmap()'ed */
 	unsigned checked_attr : 1;
 	unsigned is_binary : 1; /* data should be considered "binary" */
+	unsigned used : 1;	/* this pathspec was used for copy/delete */
 };
 
 extern struct diff_filespec *alloc_filespec(const char *);
diff --git a/hash.c b/hash.c
new file mode 100644
index 0000000..7b492d4
--- /dev/null
+++ b/hash.c
@@ -0,0 +1,110 @@
+/*
+ * Some generic hashing helpers.
+ */
+#include "cache.h"
+#include "hash.h"
+
+/*
+ * Look up a hash entry in the hash table. Return the pointer to
+ * the existing entry, or the empty slot if none existed. The caller
+ * can then look at the (*ptr) to see whether it existed or not.
+ */
+static struct hash_table_entry *lookup_hash_entry(unsigned int hash, struct hash_table *table)
+{
+	unsigned int size = table->size, nr = hash % size;
+	struct hash_table_entry *array = table->array;
+
+	while (array[nr].ptr) {
+		if (array[nr].hash == hash)
+			break;
+		nr++;
+		if (nr >= size)
+			nr = 0;
+	}
+	return array + nr;
+}
+
+
+/*
+ * Insert a new hash entry pointer into the table.
+ *
+ * If that hash entry already existed, return the pointer to
+ * the existing entry (and the caller can create a list of the
+ * pointers or do anything else). If it didn't exist, return
+ * NULL (and the caller knows the pointer has been inserted).
+ */
+static void **insert_hash_entry(unsigned int hash, void *ptr, struct hash_table *table)
+{
+	struct hash_table_entry *entry = lookup_hash_entry(hash, table);
+
+	if (!entry->ptr) {
+		entry->ptr = ptr;
+		entry->hash = hash;
+		table->nr++;
+		return NULL;
+	}
+	return &entry->ptr;
+}
+
+static void grow_hash_table(struct hash_table *table)
+{
+	unsigned int i;
+	unsigned int old_size = table->size, new_size;
+	struct hash_table_entry *old_array = table->array, *new_array;
+
+	new_size = alloc_nr(old_size);
+	new_array = xcalloc(sizeof(struct hash_table_entry), new_size);
+	table->size = new_size;
+	table->array = new_array;
+	table->nr = 0;
+	for (i = 0; i < old_size; i++) {
+		unsigned int hash = old_array[i].hash;
+		void *ptr = old_array[i].ptr;
+		if (ptr)
+			insert_hash_entry(hash, ptr, table);
+	}
+	free(old_array);
+}
+
+void *lookup_hash(unsigned int hash, struct hash_table *table)
+{
+	if (!table->array)
+		return NULL;
+	return &lookup_hash_entry(hash, table)->ptr;
+}
+
+void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table)
+{
+	unsigned int nr = table->nr;
+	if (nr >= table->size/2)
+		grow_hash_table(table);
+	return insert_hash_entry(hash, ptr, table);
+}
+
+int for_each_hash(struct hash_table *table, int (*fn)(void *))
+{
+	int sum = 0;
+	unsigned int i;
+	unsigned int size = table->size;
+	struct hash_table_entry *array = table->array;
+
+	for (i = 0; i < size; i++) {
+		void *ptr = array->ptr;
+		array++;
+		if (ptr) {
+			int val = fn(ptr);
+			if (val < 0)
+				return val;
+			sum += val;
+		}
+	}
+	return sum;
+}
+
+void free_hash(struct hash_table *table)
+{
+	free(table->array);
+	table->array = NULL;
+	table->size = 0;
+	table->nr = 0;
+}
diff --git a/hash.h b/hash.h
new file mode 100644
index 0000000..5056c9a
--- /dev/null
+++ b/hash.h
@@ -0,0 +1,43 @@
+#ifndef HASH_H
+#define HASH_H
+
+/*
+ * These are some simple generic hash table helper functions.
+ * Not necessarily suitable for all users, but good for things
+ * where you want to just keep track of a list of things, and
+ * have a good hash to use on them.
+ *
+ * It keeps the hash table at roughly 50-75% free, so the memory
+ * cost of the hash table itself is roughly
+ *
+ *	3 * 2*sizeof(void *) * nr_of_objects
+ *
+ * bytes. 
+ *
+ * FIXME: on 64-bit architectures, we waste memory. It would be
+ * good to have just 32-bit pointers, requiring a special allocator
+ * for hashed entries or something.
+ */
+struct hash_table_entry {
+	unsigned int hash;
+	void *ptr;
+};
+
+struct hash_table {
+	unsigned int size, nr;
+	struct hash_table_entry *array;
+};
+
+extern void *lookup_hash(unsigned int hash, struct hash_table *table);
+extern void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table);
+extern int for_each_hash(struct hash_table *table, int (*fn)(void *));
+extern void free_hash(struct hash_table *table);
+
+static inline void init_hash(struct hash_table *table)
+{
+	table->size = 0;
+	table->nr = 0;
+	table->array = NULL;
+}
+
+#endif

^ permalink raw reply related

* Re: .gittattributes handling has deficiencies
From: david @ 2007-10-21 23:49 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <C7F59DFB-D4E4-4F75-88F7-F1A90C7D41E8@zib.de>

On Sun, 21 Oct 2007, Steffen Prohaska wrote:

> On Oct 21, 2007, at 7:57 PM, david@lang.hm wrote:
>
>> On Sun, 21 Oct 2007, Steffen Prohaska wrote:
>> 
>>> On Oct 21, 2007, at 7:09 PM, david@lang.hm wrote:
>>> 
>>>> On Sun, 21 Oct 2007, Steffen Prohaska wrote:
>>>>> On Oct 21, 2007, at 11:19 AM, david@lang.hm wrote:
>>>>>>> But this is really hard to solve. We would need to compare
>>>>>>> attributes before and after for _all_ files that have attributes
>>>>>>> in one of the two commits and check if they changed. If so, we
>>>>>>> need to do a fresh checkout according to the new attributes.
>>>>>> if you know that you will get the new .gitattributes if it changes, 
>>>>>> setup a post-checkout hook to checkout everything if it has changed. 
>>>>>> it's far from ideal, but it should be a good, safe, first 
>>>>>> approximation.
>>>>> That's not good enough. I'll stop using .gitattributes. I
>>>>> need to teach >40 devs how to use git on Windows. I only use
>>>>> features that work flawlessly. .gitattributes doesn't. It bit
>>>>> me twice now.
>>>> why would checking everything out if .gitattributes has changed not work? 
>>>> I can see why _not_ doing so would cause problems, and I freely 
>>>> acknowledge that this approach imposes a performance hit by checking 
>>>> everything out twice, but I don't see how it would not be reliable.
>>> 
>>> What do you mean by "checking out everything"?
>>> Which command do you propose?
>> 
>> something like git checkout -f
>
> I suspected this. I see two problems:
>
> 1) it's too dangerous: I throws away _all_ changes, not only
> changes that are related to gitattributes.

this is true, the question of if this is 'too dangerous' depends on what 
workflow you teach as safe. if you teach that checking out a new version 
will loose any modifications you have made (which is useually the sane 
thing to do by default anyway) then this is just more of the same

> 2) it doesn't work reliably. git checkout -f will only update
> files that git detects as changed. But you could have files that
> should have crlf in the working copy but actually have only lf.
> Those would not be updated.

ok, you could do rm -r * before doing the checkout -f (or there's probably 
a option to git to tell it not to preserve changes to the working area, I 
am not a git guru.

> I'll not recommend this. Not using .gitattributes is the only
> sane solution.

it may be the best thing to do for you and your users, that's not the same 
thing as saying that it's the only sane solution.

David Lang

^ permalink raw reply

* Re: how to deal with conflicts after "git stash apply"?
From: Johannes Schindelin @ 2007-10-21 22:40 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071021223206.GJ16291@srparish.net>

Hi,

On Sun, 21 Oct 2007, Scott Parish wrote:

> How is the intended way to deal with "git stash apply" conflicts?
> If i just edit the file and remove the conflict, "git diff" gives
> some really messed up output. Documentation for other commands and
> conflicts suggest "git commit" after cleaning up the conflict, or
> "git add", but in the case of "stash apply" i'm not ready for a
> commit yet, and "git add" keeps "git diff" from showing any output.

You are probably seeing combined diffs.

This show not only the differences of the working tree relative to HEAD, 
but also of the changes stored in the stash.

You should have a look into Documentation/diff-format.txt (section 
"combined diff format") to learn more.

Hth,
Dscho

^ permalink raw reply

* how to deal with conflicts after "git stash apply"?
From: Scott Parish @ 2007-10-21 22:32 UTC (permalink / raw)
  To: git

How is the intended way to deal with "git stash apply" conflicts?
If i just edit the file and remove the conflict, "git diff" gives
some really messed up output. Documentation for other commands and
conflicts suggest "git commit" after cleaning up the conflict, or
"git add", but in the case of "stash apply" i'm not ready for a
commit yet, and "git add" keeps "git diff" from showing any output.

Thanks for any clarification
sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: [PATCH] "git help -a" should search all exec_paths and PATH
From: Johannes Schindelin @ 2007-10-21 22:25 UTC (permalink / raw)
  To: Scott R Parish; +Cc: git
In-Reply-To: <20071021214846.GI16291@srparish.net>

Hi,

On Sun, 21 Oct 2007, Scott R Parish wrote:

> Currently "git help -a" only searches in the highest priority exec_path, 
> meaning at worst, nothing is listed if the git commands are only 
> available from the PATH. It also makes git slightly less extensible.
> 
> To fix this, help.c is modified to search in all the exec_paths and PATH
> for potential git commands.

With this explanation, I would have expected that you add a loop just like 
in exec-cmd.c.  Not anything more.  And certainly not the removal of a 
sanity check for the length of the path name.

Ciao,
Dscho

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Johannes Schindelin @ 2007-10-21 22:15 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Jakub Narebski, Steffen Prohaska, Federico Mena Quintero, git
In-Reply-To: <471AFD07.4040606@op5.se>

Hi,

On Sun, 21 Oct 2007, Andreas Ericsson wrote:

> Johannes Schindelin wrote:
> 
> > On Sun, 21 Oct 2007, Jakub Narebski wrote:
> > 
> > > On 10/20/07, Steffen Prohaska <prohaska@zib.de> wrote:
> > > 
> > > > Maybe we could group commands into more categories?
> > > > 
> > > > plumbing: should be hidden from the 'normal' user. Porcelain
> > > >    should be sufficient for every standard task.
> > >
> > > The problem is division between what is porcelain and what is 
> > > plumbing. Some commands are right on border (git-fsck, 
> > > git-update-index, git-rev-parse comes to mind).
> > 
> > Sorry, but my impression from the latest mails was that the commands 
> > are fine.  What is lacking is a nice, _small_ collection of 
> > recommended workflows.  And when we have agreed on such a set of 
> > workflows, we optimize the hell out of them.  Only this time it is not 
> > performance, but user-friendliness.
> 
> http://www.kernel.org/pub/software/scm/git/docs/everyday.html would be a 
> good starting point, I think.

I don't think so.  Way too few authors were involved in writing this 
document, so it is not "typical" in and of itself.

I'd really like people to respond not so much with broad and general 
statements to my mail (those statements tend to be rather useless to find 
how to make git more suitable to newbies), but rather with concrete top 
ten lists of what they do daily.

My top ten list:

- git diff
- git commit
- git status
- git fetch
- git rebase
- git pull
- git cherry-pick
- git bisect
- git push
- git add

Of course, my list is somewhat skewed (because I am quite comfortable with 
the commands git provided; otherwise I would have provided -- unlike 
others, probably -- patches, and would have fought -- also unlike others 
-- to get them in, such as --color-words).

So again, I'd like people who did _not_ tweak git to their likings to tell 
the most common steps they do.  My hope is that we see things that are 
good practices, but could use an easier user interface.

Ciao,
Dscho

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: J. Bruce Fields @ 2007-10-21 22:12 UTC (permalink / raw)
  To: Steffen Prohaska
  Cc: Andreas Ericsson, Federico Mena Quintero, Johannes Schindelin,
	Jakub Narebski, git
In-Reply-To: <DE4FB702-24E8-421F-8447-04A5C7F7B5D2@zib.de>

On Sat, Oct 20, 2007 at 12:19:45PM +0200, Steffen Prohaska wrote:
> mail porcelain: the list will probably hate me for this, but
>   I think all commands needed to create and send patches per
>   mail are not essential. I suspect that I'll _never_ ask
>   my colleagues at work to send me a patch by mail. They'll
>   always push it to a shared repo.

That's not going to fly.  There are too many projects for which email is
the preferred way to submit patches (especially for new or occasional
developers).

--b.

^ permalink raw reply

* Re: [PATCH] Define compat version of mkdtemp for systems lacking it
From: Johannes Schindelin @ 2007-10-21 22:03 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071021053015.GA31995@spearce.org>

Hi,

On Sun, 21 Oct 2007, Shawn O. Pearce wrote:

> Solaris 9 doesn't have mkdtemp() so we need to emulate it for the
> rsync transport implementation.

Thanks.  I've been meaning to do it for msysGit, but I never came around 
to do it...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.
From: Johannes Schindelin @ 2007-10-21 22:02 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071021182150.GG16291@srparish.net>

Hi,

On Sun, 21 Oct 2007, Scott Parish wrote:

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..374ffc9 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -99,8 +103,8 @@ int execv_git_cmd(const char **argv)
>  
>  		trace_argv_printf(argv, -1, "trace: exec:");
>  
> -		/* execve() can only ever return if it fails */
> -		execve(git_command, (char **)argv, environ);
> +		/* execvp() can only ever return if it fails */
> +		execvp(git_command, (char **)argv);

I'd rather have it conditional upon *exec_dir (see the brown paper bag 
patch I just sent).

Ciao,
Dscho

^ permalink raw reply

* [PATCH] execv_git_cmd(): also try PATH if everything else fails.
From: Johannes Schindelin @ 2007-10-21 21:59 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Scott Parish, git
In-Reply-To: <20071021023614.GB14735@spearce.org>


Earlier, we tried to find the git commands in several possible exec
dirs.  Now, if all of these failed, try to find the git command in
PATH.

This allows you to install the git programs somewhere else than
originally specified when building git, as long as you add that location
to the PATH.

Initial implementation by Scott R Parish.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sat, 20 Oct 2007, Shawn O. Pearce wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
	> > On Sat, 20 Oct 2007, Scott Parish wrote:
	> > 
	> > > Actually, i didn't test it right, execve() were using the 
	> > > files in my cwd. In addition to you patch, you'd need to use 
	> > > execvp() instead of execve().
	> > 
	> > Ah, right.  I missed that one ;-)
	> > 
	> > How about this instead?
	> 
	> Uhhh.  Its the same, isn't it?  Still using execve() which means 
	> we will not look at PATH in the final attempt.

	Sorry.  I forgot to --amend.

 exec_cmd.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..70b84b0 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
 	int i;
 	const char *paths[] = { current_exec_path,
 				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
+				builtin_exec_path,
+				"" };
 
 	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
 		size_t len;
@@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)
 		const char *exec_dir = paths[i];
 		const char *tmp;
 
-		if (!exec_dir || !*exec_dir) continue;
+		if (!exec_dir) continue;
 
-		if (*exec_dir != '/') {
+		if (!*exec_dir)
+			/* try PATH */
+			*git_command = '\0';
+		else if (*exec_dir != '/') {
 			if (!getcwd(git_command, sizeof(git_command))) {
 				fprintf(stderr, "git: cannot determine "
 					"current directory: %s\n",
@@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)
 
 		len = strlen(git_command);
 		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
+			      "%sgit-%s", *exec_dir ? "/" : "", argv[0]);
 		if (rc < 0 || rc >= sizeof(git_command) - len) {
 			fprintf(stderr,
 				"git: command name given is too long.\n");
@@ -100,7 +104,10 @@ int execv_git_cmd(const char **argv)
 		trace_argv_printf(argv, -1, "trace: exec:");
 
 		/* execve() can only ever return if it fails */
-		execve(git_command, (char **)argv, environ);
+		if (*exec_dir)
+			execve(git_command, (char **)argv, environ);
+		else
+			execvp(git_command, (char **)argv);
 
 		trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-- 
1.5.3.4.1289.ga1432

^ permalink raw reply related

* Re: [PATCH] Be nice with compilers that do not support runtime paths at all.
From: Benoit SIGOURE @ 2007-10-21 21:56 UTC (permalink / raw)
  To: git list
In-Reply-To: <34DAC3CA-E226-4488-8B03-FC45A6A95F78@lrde.epita.fr>

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

On Oct 4, 2007, at 5:59 PM, Benoit SIGOURE wrote:

> On Oct 4, 2007, at 1:18 AM, Junio C Hamano wrote:
>
>> Benoit Sigoure <tsuna@lrde.epita.fr> writes:
>>
>>> diff --git a/Makefile b/Makefile
>>> index a1fe443..7c6c453 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -100,6 +100,9 @@ all::
>>>  # that tells runtime paths to dynamic libraries;
>>>  # "-Wl,-rpath=/path/lib" is used instead.
>>>  #
>>> +# Define NO_RPATH if your dynamic loader doesn't support runtime  
>>> paths at
>>> +# all.
>>> +#
>>>  # Define USE_NSEC below if you want git to care about sub-second  
>>> file mtimes
>>>  # and ctimes. Note that you need recent glibc (at least 2.2.4)  
>>> for this, and
>>>  # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using  
>>> it will likely
>>
>> Thanks for this part;
>>
>>> @@ -507,6 +510,7 @@ ifeq ($(uname_S),Darwin)
>>>  			BASIC_LDFLAGS += -L/opt/local/lib
>>>  		endif
>>>  	endif
>>> +        NO_RPATH = YesPlease
>>>  endif
>>
>> I'll let Darwin users to fight the defaults for this part out.
>
> No more replies on this thread, and the Apple documentation  
> confirms that there is no rpath support in the dynamic loader of  
> OSX 10.4 and before.  I don't know about the soon-to-be-released  
> 10.5 aka Leopard.
>
>>> @@ -521,7 +525,10 @@ ifndef NO_CURL
>>>  	ifdef CURLDIR
>>>  		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
>>>  		BASIC_CFLAGS += -I$(CURLDIR)/include
>>> -		CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$ 
>>> (lib) -lcurl
>>> +		CURL_LIBCURL = -L$(CURLDIR)/$(lib) -lcurl
>>> +ifndef NO_RPATH
>>> +		CURL_LIBCURL += $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
>>> +endif
>>>  	else
>>>  		CURL_LIBCURL = -lcurl
>>>  	endif
>>
>>> @@ -539,7 +546,10 @@ endif
>>>
>>>  ifdef ZLIB_PATH
>>>  	BASIC_CFLAGS += -I$(ZLIB_PATH)/include
>>> -	EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$ 
>>> (lib)
>>> +	EXTLIBS += -L$(ZLIB_PATH)/$(lib)
>>> +ifndef NO_RPATH
>>> +	EXTLIBS += $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
>>> +endif
>>>  endif
>>>  EXTLIBS += -lz
>>>
>>
>> While these parts are ugly but correct, I think...
>>
>>> @@ -547,7 +557,10 @@ ifndef NO_OPENSSL
>>>  	OPENSSL_LIBSSL = -lssl
>>>  	ifdef OPENSSLDIR
>>>  		BASIC_CFLAGS += -I$(OPENSSLDIR)/include
>>> -		OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib) $(CC_LD_DYNPATH)$ 
>>> (OPENSSLDIR)/$(lib)
>>> +		OPENSSL_LINK = -L$(OPENSSLDIR)/$(lib)
>>> +ifndef NO_RPATH
>>> +		OPENSSL_LINK = $(CC_LD_DYNPATH)$(OPENSSLDIR)/$(lib)
>>> +endif
>>>  	else
>>>  		OPENSSL_LINK =
>>>  	endif
>>
>> this and the ICONV one are missing s/=/+=/.
>
> You're right, sorry.
>
>>
>> If we do not care about supporting too old GNU make, we can do
>> this by first adding this near the top:
>>
>>         ifndef NO_RPATH
>>         LINKER_PATH = -L$(1) $(CC_LD_DYNPATH)$(1)
>>         else
>>         LINKER_PATH = -L$(1)
>>         endif
>>
>> and then doing something like:
>>
>> 	CURL_LIBCURL = $(call LINKER_PATH,$(CURLDIR)/$(lib))
>> 	OPENSSL_LINK = $(call LINKER_PATH,$(OPENSSLDIR)/$(lib))
>>
>> to make it easier to read and less error prone.
>>
>
> Yes.  I can rework the patch, but the question is: do you care  
> about old GNU make?  Can I rewrite the patch with this feature?

I know Junio is still offline but maybe someone else has an objection  
against this?

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* [PATCH] "git help -a" should search all exec_paths and PATH
From: Scott R Parish @ 2007-10-21 21:48 UTC (permalink / raw)
  To: git

Currently "git help -a" only searches in the highest priority exec_path,
meaning at worst, nothing is listed if the git commands are only available
from the PATH. It also makes git slightly less extensible.

To fix this, help.c is modified to search in all the exec_paths and PATH
for potential git commands. So that it has access to all the exec_paths,
exec_cmd.c now exposes the various paths. "current_exec_path" is renamed
as its name is misleading.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |   26 +++++++++++---
 exec_cmd.h |    3 ++
 git.c      |    2 +-
 help.c     |  103 +++++++++++++++++++++++++++++++++++++++++++-----------------
 4 files changed, 98 insertions(+), 36 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 374ffc9..2c787a4 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -5,21 +5,35 @@
 
 extern char **environ;
 static const char *builtin_exec_path = GIT_EXEC_PATH;
-static const char *current_exec_path;
+static const char *argv_exec_path;
 
-void git_set_exec_path(const char *exec_path)
+void git_set_argv_exec_path(const char *exec_path)
 {
-	current_exec_path = exec_path;
+	argv_exec_path = exec_path;
 }
 
+const char *git_argv_exec_path(void)
+{
+	return argv_exec_path;
+}
+
+const char *git_builtin_exec_path(void)
+{
+	return builtin_exec_path;
+}
+
+const char *git_env_exec_path(void)
+{
+	return getenv(EXEC_PATH_ENVIRONMENT); 
+}
 
 /* Returns the highest-priority, location to look for git programs. */
 const char *git_exec_path(void)
 {
 	const char *env;
 
-	if (current_exec_path)
-		return current_exec_path;
+	if (argv_exec_path)
+		return argv_exec_path;
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
@@ -34,7 +48,7 @@ int execv_git_cmd(const char **argv)
 {
 	char git_command[PATH_MAX + 1];
 	int i;
-	const char *paths[] = { current_exec_path,
+	const char *paths[] = { argv_exec_path,
 				getenv(EXEC_PATH_ENVIRONMENT),
 				builtin_exec_path,
 				"" };
diff --git a/exec_cmd.h b/exec_cmd.h
index 849a839..315fe83 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,6 +3,9 @@
 
 extern void git_set_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
+extern const char* git_argv_exec_path(void);
+extern const char* git_builtin_exec_path(void);
+extern const char* git_env_exec_path(void);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index 853e66c..b67fb17 100644
--- a/git.c
+++ b/git.c
@@ -51,7 +51,7 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 		if (!prefixcmp(cmd, "--exec-path")) {
 			cmd += 11;
 			if (*cmd == '=')
-				git_set_exec_path(cmd + 1);
+				git_set_argv_exec_path(cmd + 1);
 			else {
 				puts(git_exec_path());
 				exit(0);
diff --git a/help.c b/help.c
index b0d2dd4..85b2853 100644
--- a/help.c
+++ b/help.c
@@ -93,37 +93,27 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 	}
 }
 
-static void list_commands(const char *exec_path, const char *pattern)
+static unsigned int list_commands_in_dir(const char *dir, const char *prefix)
 {
 	unsigned int longest = 0;
-	char path[PATH_MAX];
-	int dirlen;
-	DIR *dir = opendir(exec_path);
+	int start_dir = open(".", O_RDONLY, 0);
+	DIR *dirp = opendir(dir);
 	struct dirent *de;
 
-	if (!dir) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
+	if (!dirp || chdir(dir)) {
+		fchdir(start_dir);
+		close(start_dir);
+		return 0;
 	}
 
-	dirlen = strlen(exec_path);
-	if (PATH_MAX - 20 < dirlen) {
-		fprintf(stderr, "git: insanely long exec-path '%s'\n",
-			exec_path);
-		exit(1);
-	}
-
-	memcpy(path, exec_path, dirlen);
-	path[dirlen++] = '/';
-
-	while ((de = readdir(dir)) != NULL) {
+	while ((de = readdir(dirp)) != NULL) {
 		struct stat st;
 		int entlen;
-
-		if (prefixcmp(de->d_name, "git-"))
+			
+		if (prefixcmp(de->d_name, prefix))
 			continue;
-		strcpy(path+dirlen, de->d_name);
-		if (stat(path, &st) || /* stat, not lstat */
+
+		if (stat(de->d_name, &st) || /* stat, not lstat */
 		    !S_ISREG(st.st_mode) ||
 		    !(st.st_mode & S_IXUSR))
 			continue;
@@ -137,12 +127,67 @@ static void list_commands(const char *exec_path, const char *pattern)
 
 		add_cmdname(de->d_name + 4, entlen-4);
 	}
-	closedir(dir);
 
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
+	closedir(dirp);
+	fchdir(start_dir);
+	close(start_dir);
+
+	return longest;
+}
+
+static unsigned int list_commands_in_PATH(const char *prefix)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+       
+	if (!env_path)
+		return longest;
+
+	path = paths = xstrdup(env_path);
+
+	while ((char *)1 != path) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(path, prefix);
+		longest = MAX(longest, len);
+
+		path = colon + 1;
+	}
+
+	free(paths);
+	return longest;
+}
+
+static void list_commands(const char *prefix)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *paths[] = { git_argv_exec_path(),
+				git_env_exec_path(),
+				git_builtin_exec_path(),
+				"" };
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(paths); i++) {
+		if (!paths[i])
+			continue;
+
+		if (!*paths[i]) {
+			/* try PATH */
+			len = list_commands_in_PATH(prefix);
+			longest = MAX(longest, len);
+		}
+		else {
+			len = list_commands_in_dir(paths[i], prefix);
+			longest = MAX(longest, len);
+		}
+	}
+
+	printf("available git commands\n");
+	printf("----------------------\n");
 	pretty_print_string_list(cmdname, longest - 4);
 	putchar('\n');
 }
@@ -158,7 +203,7 @@ static void list_common_cmds_help(void)
 
 	puts("The most commonly used git commands are:");
 	for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
-		printf("   %s   ", common_cmds[i].name);
+		printf("   %s	", common_cmds[i].name);
 		mput_char(' ', longest - strlen(common_cmds[i].name));
 		puts(common_cmds[i].help);
 	}
@@ -210,7 +255,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
-			list_commands(exec_path, "git-*");
+			list_commands("git-");
 		exit(0);
 	}
 
-- 
1.5.3.4.209.g5d1ce-dirty

^ permalink raw reply related

* [PATCH] "git help" and "git help -a" shouldn't exit(1) unless they error
From: Scott R Parish @ 2007-10-21 21:47 UTC (permalink / raw)
  To: git

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/help.c b/help.c
index 1cd33ec..b0d2dd4 100644
--- a/help.c
+++ b/help.c
@@ -204,14 +204,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
-		exit(1);
+		exit(0);
 	}
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
 			list_commands(exec_path, "git-*");
-		exit(1);
+		exit(0);
 	}
 
 	else
-- 
1.5.3.4.209.g5d1ce-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