Git development
 help / color / mirror / Atom feed
* Re: git cherry to find equivalent commit IDs?
From: Avery Pennarun @ 2009-08-13 21:37 UTC (permalink / raw)
  To: skillzero; +Cc: git
In-Reply-To: <2729632a0908131413w1a2efca8t31ac8cc43e1d6888@mail.gmail.com>

On Thu, Aug 13, 2009 at 9:13 PM, <skillzero@gmail.com> wrote:
> For example, commit 3642151 on branch A was a cherry pick of a commit
> 460050c on master:
>
> $ git branch -a --contains 3642151
>  A
>
> $ git branch -a --contains 460050c
> * master
>
> $ git cherry -v master 3642151
> - 3642151435ce5737debc1213de46dd556475bfad1 fixed bug
>
> I assume that means an equivalent change to 3642151 is already in
> master (which it is, as commit 460050c). But I want to find out the
> commit ID on master that's equivalent to 3642151 (i.e. something that
> tells me it's 460050c).

git show 3642151 | git patch-id

You should get a line with two hashes; the first is the patchid (call
it PATCHID_FROM_ABOVE)

git log -p | git patch-id | grep PATCHID_FROM_ABOVE

This should give you a list of all commits that correspond to that patchid.

Note that if there were conflicts when applying the patch, the patchid
probably changed.

Have fun,

Avery

^ permalink raw reply

* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Nicolas Pitre @ 2009-08-13 21:28 UTC (permalink / raw)
  To: George Spelvin; +Cc: git, gitster, torvalds
In-Reply-To: <20090813201542.25431.qmail@science.horizon.com>

On Thu, 13 Aug 2009, George Spelvin wrote:

> > Wow.  Is it now faster than the arm/ and ppc/ hand-tweaked assembly?
> 
> It's probably faster than the ARM, which was tuned for size rather
> than speed, but if you want to rework the assembly for speed, the ARM's
> rotate-and-add operations allow tricks which I doubt GCC can pick up on.
> (You have to notice that the F(b,c,d) function is bitwise, so you can
> do it on rotated data and do the rotate when you add the result to e.)

gcc is not too bad at merging ALU and shift/rotate operations into the 
same instruction.  However, to make a really optimal ARM version, some 
custom SHA_ROUND macros with inline assembly could be made.  I suspect 
that wouldn't gain much though, as the pure shift/rotate mov 
instructions really aren't that many in the generated code.

> I'd be surprised if it were faster than PPC code, especially on the
> in-order G3 and G4 cores where careful scheduling really pays off.
> But maybe I just get to be surprised...

Given that PPC has enough register to hold the entire state, it is then 
only a matter of proper instruction scheduling which modern gcc ought to 
do right.  If not then this is a good test case for gcc people to fix 
the PPC machine pipeline description.

> For automatic assembly tuning, I was thinking of having a .c file that
> has a bunch of #ifdef __PPC__ statements that gets run through $(CC) -E.
> That should be a fairly portable way to 

??

> The other question about unaligned access is whether it's beneficial
> to make the fetch loop work like this:
> 
> 	char const *in;
> 	uint32_t *out
> 	unsigned lsb = (unsigned)p & 3;
> 	uint32_t const *p32 = (uint32_t const *)(in - lsb);
> 	uint32_t t = ntohl(*p32);
> 
> 	switch (lsb) {
> 
> 	case 0:
> 		*out++ = t;
> 		for (i = 1; i < 16; i++)
> 			*out++ = ntohl(*++p32);
> 		break;
> 	case 1:
> 		for (i = 0; i < 16; i++) {
> 			uint32_t s = t << 8;
> 			t = ntohl(*++p32);
> 			*out++ = s | t >> 24;
> 		}
> 		break;
> 	case 1:
> 		for (i = 0; i < 16; i++) {
> 			uint32_t s = t << 16;
> 			t = ntohl(*++p32);
> 			*out++ = s | t >> 16;
> 		}
> 		break;
> 	case 1:
> 		for (i = 0; i < 16; i++) {
> 			uint32_t s = t << 24;
> 			t = ntohl(*++p32);
> 			*out++ = s | t >> 8;
> 		}
> 		break;
> 	}
> 
> On the ARM, at least, ntohl() isn't particularly cheap, so loading 4
> bytes and assembling them turns out to be cheaper.  But it's a thought.

Well, that would have to be tested.  This could possibly only be a gain 
if you have a fast ntohl() though.

And the other question is also to decide when this is good enough for a 
generic version.  Gaining 5% speedup on raw SHA1 throughput with ugly 
code might not be worth the maintenance hassle.  At that point you might 
as well go back to a purely asm version if you really want to get the 
extra edge.


Nicolas

^ permalink raw reply

* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13 21:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, Git List
In-Reply-To: <7vhbwb5tul.fsf@alter.siamese.dyndns.org>

Heya,

On Thu, Aug 13, 2009 at 13:42, Junio C Hamano<gitster@pobox.com> wrote:
> Heh, thanks; it appears I lagged behind by about 2 hours?

So it would seem, not a bad latency at all though.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* git cherry to find equivalent commit IDs?
From: skillzero @ 2009-08-13 21:13 UTC (permalink / raw)
  To: git

Is there a way to get the commit ID of a patch that was cherry picked
(or via a manually applied patch)? I'm trying to get an equivalent to
git branch --contains, but instead of comparing the commit ID, it
compares patch IDs so it works for cherry picks and manual patches. It
seems like git cherry almost does what I want, but it only seems to
show the commit ID of the commit on the other branch rather than the
branch I specify to git cherry. I may just be using git cherry
incorrectly though.

For example, commit 3642151 on branch A was a cherry pick of a commit
460050c on master:

$ git branch -a --contains 3642151
  A

$ git branch -a --contains 460050c
* master

$ git cherry -v master 3642151
- 3642151435ce5737debc1213de46dd556475bfad1 fixed bug

I assume that means an equivalent change to 3642151 is already in
master (which it is, as commit 460050c). But I want to find out the
commit ID on master that's equivalent to 3642151 (i.e. something that
tells me it's 460050c).

I'm basically looking for something like git branch --contains, but
that searches by patch ID so it can find cherry picked or manually
applied patches:

$ git branch -a --contains-equivalent 3642151
* master (via commit 460050c)
  A (via commit 3642151)

Is there a way to do that?

^ permalink raw reply

* Re: [PATCH 1/5] port --ignore-unmatch to "git add"
From: Thomas Rast @ 2009-08-13 21:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Luke Dashjr, git
In-Reply-To: <7vy6pna4lu.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> 
> Chould you refresh my memory a bit?
> 
> In what circumstance is "rm --ignore-unmatch" useful to begin with?

Not sure about add --ignore-unmatch myself, but there's even an
example of rm --ignore-unmatch in man git-filter-branch, along the
lines of

  git filter-branch --index-filter '
    rm --ignore-unmach some_file_that_shouldnt_be_in_history
  ' -- --all

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* [PATCH] Document --ignore-unmatch in git-add.txt
From: Luke Dashjr @ 2009-08-13 21:03 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <7vr5vfa4ha.fsf@alter.siamese.dyndns.org>

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 Documentation/git-add.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index e67b7e8..6e30ee9 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
-	  [--refresh] [--ignore-errors] [--] [<filepattern>...]
+	  [--refresh] [--ignore-errors] [--ignore-unmatch] [--]
+	  [<filepattern>...]
 
 DESCRIPTION
 -----------
@@ -119,6 +120,9 @@ apply.
 	them, do not abort the operation, but continue adding the
 	others. The command shall still exit with non-zero status.
 
+--ignore-unmatch::
+	Exit with a zero status even if no files matched.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH] port --ignore-unmatch to "git add"
From: Luke Dashjr @ 2009-08-13 21:02 UTC (permalink / raw)
  To: git; +Cc: Luke Dashjr
In-Reply-To: <7vr5vfa4ha.fsf@alter.siamese.dyndns.org>

"git rm" has a --ignore-unmatch option that is also applicable to "git add"
and may be useful for persons wanting to ignore unmatched arguments, but not
all errors.

Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
---
 builtin-add.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..3882482 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -19,6 +19,7 @@ static const char * const builtin_add_usage[] = {
 };
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
+static int ignore_unmatch = 0;
 
 static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 {
@@ -60,13 +61,18 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
-	fill_pathspec_matches(pathspec, seen, specs);
 
-	for (i = 0; i < specs; i++) {
-		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
-			die("pathspec '%s' did not match any files",
-					pathspec[i]);
+	if (!ignore_unmatch)
+	{
+		fill_pathspec_matches(pathspec, seen, specs);
+
+		for (i = 0; i < specs; i++) {
+			if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
+				die("pathspec '%s' did not match any files",
+						pathspec[i]);
+		}
 	}
+
         free(seen);
 }
 
@@ -107,9 +113,12 @@ static void refresh(int verbose, const char **pathspec)
 	seen = xcalloc(specs, 1);
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
-	for (i = 0; i < specs; i++) {
-		if (!seen[i])
-			die("pathspec '%s' did not match any files", pathspec[i]);
+	if (!ignore_unmatch)
+	{
+		for (i = 0; i < specs; i++) {
+			if (!seen[i])
+				die("pathspec '%s' did not match any files", pathspec[i]);
+		}
 	}
         free(seen);
 }
@@ -226,6 +235,8 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+				"exit with a zero status even if nothing matched"),
 	OPT_END(),
 };
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors
From: Luke-Jr @ 2009-08-13 20:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Luke Dashjr, git
In-Reply-To: <7vr5vfa4ha.fsf@alter.siamese.dyndns.org>

On Thursday 13 August 2009 02:38:57 pm Junio C Hamano wrote:
> Luke Dashjr <luke-jr+git@utopios.org> writes:
> > Unmatched files are errors, and should be ignored with the rest of them.
>
> Why is this a "fix"?
>
> I would understand if it were "Make --ignore-errors imply --ignore-unmatch
> unconditionally".  But then I do not think I would necessarily agree it is
> a good change.
>
> The user may know that some files in the work tree are unreadable and
> cannot be indexed (hence he gives --ignore-errors) but he still may want
> to catch a typo on the command line.
>
> I do not think it is wise to make --ignore-errors imply --ignore-unmatch
> unconditionally like this patch does without any escape hatch.

Are unmatched files not errors? Perhaps the old flag should be renamed to
--ignore-read-errors and a new --ignore-errors that implies both added. Or 
maybe just a documentation change to preserve compatibility with anything that 
might assume that...

^ permalink raw reply

* Re: [PATCH 1/5] port --ignore-unmatch to "git add"
From: Luke-Jr @ 2009-08-13 20:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Luke Dashjr, git
In-Reply-To: <7vy6pna4lu.fsf@alter.siamese.dyndns.org>

On Thursday 13 August 2009 02:36:13 pm Junio C Hamano wrote:
> Luke Dashjr <luke-jr+git@utopios.org> writes:
> > "git rm" has a --ignore-unmatch option that is also applicable to "git
> > add" and may be useful for persons wanting to ignore unmatched arguments,
> > but not all errors.
>
> Chould you refresh my memory a bit?
>
> In what circumstance is "rm --ignore-unmatch" useful to begin with?
> A similar question for "add --ignore-unmatch".

Not sure on its purpose for "rm", but for "add"...
Avoiding a race condition in automation. In particular, if the file is deleted 
between the time the argument list is built until git scans for matches.

> Now the obligatory design level question is behind us, let's take a brief
> look at the codde.
>
> > +static int ignore_unmatch = 0;
>
> Drop " = 0" and let the language initialize this to zero.

Does C define a default initialisation of zero? My understanding was that 
uninitialised variables are always undefined until explicitly assigned a 
value.

> >  static void fill_pathspec_matches(const char **pathspec, char *seen, int
> > specs) {
> > @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir,
> > const char **pathspec, int p fill_pathspec_matches(pathspec, seen,
> > specs);
> >
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> > +		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) &&
> > !ignore_unmatch) die("pathspec '%s' did not match any files",
> >  					pathspec[i]);
> >  	}
> > @@ -108,7 +109,7 @@ static void refresh(int verbose, const char
> > **pathspec) refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED :
> > REFRESH_QUIET, pathspec, seen);
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i])
> > +		if (!seen[i] && !ignore_unmatch)
> >  			die("pathspec '%s' did not match any files", pathspec[i]);
> >  	}
> >          free(seen);
>
> What's the point of these two loops if under ignore_unmatch everything
> becomes no-op?
>
> That is, wouldn't it be much more clear if you wrote like this?

I'm not overly familiar with the git codebase, but wouldn't a null 'seen' 
variable break the refresh_index call? The loops themselves can be avoided, I 
suppose. I'll submit a new patch to optimise the changes (and rebase)...

^ permalink raw reply

* Re: [PATCH 2/4] fast-import: define a new option command
From: Junio C Hamano @ 2009-08-13 20:42 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
In-Reply-To: <fabb9a1e0908131301g4361a06es98fbf3c256c25300@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Thu, Aug 13, 2009 at 12:26, Junio C Hamano<gitster@pobox.com> wrote:
>> How about making the option parser get and keep the _name_ of the file
>> until option parsing session (i.e. read the stream until initial run of
>> "option" command runs out and then parse the command line to override),
>> and then finally open the file and read it?
>
> On Thu, Aug 13, 2009 at 10:44, Sverre Rabbelier<srabbelier@gmail.com> wrote:
>> Ah, then how about in option_import_marks() we only store the name of
>> the file, like in option_export_marks, and at the end, when we reach
>> the first non-option command (and we've parsed argv), we read the
>> file. That way it's only read once, and it deals with the above
>> scenario.
>
> Which is exactly what the latest version does :).

Heh, thanks; it appears I lagged behind by about 2 hours?

^ permalink raw reply

* Re: rebase-with-history -- a technique for rebasing without trashing  your repo history
From: Abderrahim Kitouni @ 2009-08-13 20:31 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Bazaar, mercurial mailing list, Git Mailing List
In-Reply-To: <4A840B0F.9060003@alum.mit.edu>

2009/8/13 Michael Haggerty <mhagger@alum.mit.edu>:
> I've been thinking a lot about the problems of tracking upstream changes
> while developing a feature branch.
Isn't this the purpose of pbranch [1] (and I beleive bzr's loom[2] and
git's topgit[3])?

Peace,
Abderrahim

[1] http://arrenbrecht.ch/mercurial/pbranch/
[2] https://launchpad.net/bzr-loom
[3]http://repo.or.cz/w/topgit.git

^ permalink raw reply

* merging individual files
From: Chris Marshall @ 2009-08-13 20:16 UTC (permalink / raw)
  To: git

Suppose that merging branch dev1 into master would result in three files, f1,
f2, and f3 being changed, and that I only want to merge the changes for f1 and
f2 and not the changes for f3 currently.  Later on, I want to accept the f3
changes.  Suppose further that the changes to f1, f2, and f3 occurred in a
single commit to branch dev1.

What is the simplest way to use git to achieve that effect?

More generally, I need a way to accept the changes for one or two files while
rejecting the changes for a potentially large number of files, then later on 
accepting the changes for the large number of files.

I work at a company where perforce is currently used for all development and am
trying to work out the git equivalents to all of the perforce flows we use. 
This workflow is the only one that I am stumped on.

One solution that occurs to me is to create a temporary branch off of the (most
recent) common ancestor of master and br1, let's say br2, checkout the files
from br1 that I want to merge into master and commit those to br2, then merge
br2 into master:

git checkout common_ancestor_commit
git checkout -b br2
git checkout br1 f1 f2
git commit
git checkout master
git merge br2
git branch -d br2

This strikes me as not too bad of a procedure, as long as there is a graceful
way of determining the most recent common ancestor of br1 and master.  What's
the simplest way of doing that?

^ permalink raw reply

* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: George Spelvin @ 2009-08-13 20:15 UTC (permalink / raw)
  To: git, gitster; +Cc: linux, nico, torvalds

> Wow.  Is it now faster than the arm/ and ppc/ hand-tweaked assembly?

It's probably faster than the ARM, which was tuned for size rather
than speed, but if you want to rework the assembly for speed, the ARM's
rotate-and-add operations allow tricks which I doubt GCC can pick up on.
(You have to notice that the F(b,c,d) function is bitwise, so you can
do it on rotated data and do the rotate when you add the result to e.)

I'd be surprised if it were faster than PPC code, especially on the
in-order G3 and G4 cores where careful scheduling really pays off.
But maybe I just get to be surprised...

For automatic assembly tuning, I was thinking of having a .c file that
has a bunch of #ifdef __PPC__ statements that gets run through $(CC) -E.
That should be a fairly portable way to 


The other question about unaligned access is whether it's beneficial
to make the fetch loop work like this:

	char const *in;
	uint32_t *out
	unsigned lsb = (unsigned)p & 3;
	uint32_t const *p32 = (uint32_t const *)(in - lsb);
	uint32_t t = ntohl(*p32);

	switch (lsb) {

	case 0:
		*out++ = t;
		for (i = 1; i < 16; i++)
			*out++ = ntohl(*++p32);
		break;
	case 1:
		for (i = 0; i < 16; i++) {
			uint32_t s = t << 8;
			t = ntohl(*++p32);
			*out++ = s | t >> 24;
		}
		break;
	case 1:
		for (i = 0; i < 16; i++) {
			uint32_t s = t << 16;
			t = ntohl(*++p32);
			*out++ = s | t >> 16;
		}
		break;
	case 1:
		for (i = 0; i < 16; i++) {
			uint32_t s = t << 24;
			t = ntohl(*++p32);
			*out++ = s | t >> 8;
		}
		break;
	}

On the ARM, at least, ntohl() isn't particularly cheap, so loading 4
bytes and assembling them turns out to be cheaper.  But it's a thought.

^ permalink raw reply

* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Nicolas Pitre @ 2009-08-13 20:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7v63crbja2.fsf@alter.siamese.dyndns.org>

On Thu, 13 Aug 2009, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > As it is now, I was about to suggest:
> >
> > 	git mv block-sha1/sha1.[ch] .
> > 	rmdir block-sha1
> > 	rm -r mozilla-sha1
> > 	rm -r arm
> > 	rm -r ppc 
> >
> > and remove support for openssl's SHA1 usage, making this implementation 
> > unconditional.  After all it is faster, or so close to be faster than 
> > the alternatives, that we should probably cut on the extra dependency 
> > and simplify portability issues at the same time.
> 
> Wow.  Is it now faster than the arm/ and ppc/ hand-tweaked assembly?

It is indeed faster than the ARM assembly version by far, and faster 
than all the alternative implementations too, but with a 7x increase in 
compiled code size.  In the context of Git I think this is a good 
compromize.  Making the assembly version faster than the C version could 
be possible, but that would require quite some work and I don't expect 
the gain to be significant, certainly not worth the trouble.

Furthermore the C version can be used to generate ARM Thumb code while 
the asm version cannot without yet more work.


Nicolas

^ permalink raw reply

* [PATCH] Use "gitk: /path/to/repo" as gitk window title.
From: Zbyszek Szmek @ 2009-08-13 19:58 UTC (permalink / raw)
  To: git; +Cc: zbyszek

In case of non-bare repos, the .git suffix in the path is skipped.

Previously, when run in a subdirectory, gitk would show the name
of this subdirectory as the title, which was misleading.
---
 gitk-git/gitk |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4604c83..e656e81 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -16,6 +16,14 @@ proc gitdir {} {
     }
 }
 
+proc reponame {} {
+    set n [file normalize [gitdir]]
+    if {[string match "*/.git" $n]} {
+	set n [string range $n 0 end-5]
+    }
+    return $n
+}
+
 # A simple scheduler for compute-intensive stuff.
 # The aim is to make sure that event handlers for GUI actions can
 # run at least every 50-100 ms.  Unfortunately fileevent handlers are
@@ -11156,6 +11164,8 @@ set nullfile "/dev/null"
 set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
 set git_version [join [lrange [split [lindex [exec git version] end] .] 0 2] .]
 
+set appname "gitk"
+
 set runq {}
 set history {}
 set historyindex 0
@@ -11220,7 +11230,7 @@ catch {
 }
 # wait for the window to become visible
 tkwait visibility .
-wm title . "[file tail $argv0]: [file tail [pwd]]"
+wm title . "$appname: [reponame]"
 update
 readrefs
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] git-cvsimport: add support for cvs pserver password  scrambling.
From: Sverre Rabbelier @ 2009-08-13 20:04 UTC (permalink / raw)
  To: Dirk Hörner
  Cc: Johannes Schindelin, Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <4da546dc0908131219q149844abi453d8429847af1cf@mail.gmail.com>

Heya,

2009/8/13 Dirk Hörner <dirker@gmail.com>:
> sorry for the long delay, but I finally sat down, hacked two testcases
> and amended the patch after rebasing to the most recent HEAD. Find it
> attached to this mail.

I think we'd rather find it inlined, as per SubmittingPatches ;).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13 20:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, Git List
In-Reply-To: <7vzla3bjn5.fsf@alter.siamese.dyndns.org>

Heya,

On Thu, Aug 13, 2009 at 12:26, Junio C Hamano<gitster@pobox.com> wrote:
> How about making the option parser get and keep the _name_ of the file
> until option parsing session (i.e. read the stream until initial run of
> "option" command runs out and then parse the command line to override),
> and then finally open the file and read it?

On Thu, Aug 13, 2009 at 10:44, Sverre Rabbelier<srabbelier@gmail.com> wrote:
> Ah, then how about in option_import_marks() we only store the name of
> the file, like in option_export_marks, and at the end, when we reach
> the first non-option command (and we've parsed argv), we read the
> file. That way it's only read once, and it deals with the above
> scenario.

Which is exactly what the latest version does :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Linus Torvalds @ 2009-08-13 19:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7v63crbja2.fsf@alter.siamese.dyndns.org>



On Thu, 13 Aug 2009, Junio C Hamano wrote:
> 
> Wow.  Is it now faster than the arm/ and ppc/ hand-tweaked assembly?

For the good cases, yes.

For POWER, with gcc-4.4, the C code apparently outperforms the asm code on 
POWER6. The asm code is scheduled for POWER4, and I think outperforms the 
C code there. Also, when compiling in 64-bit mode (with "-m64"), at least 
some versions of gcc seem to do some stupid things and add extra zero 
extension stuff, and that performed suboptimally at least on a PPC G5.

So it's certainly not a clear case of "the C code outperforms the asm 
code", but in BenH's tests, the best numbers really did come from the C 
version. With some silly cases of at least some versions gcc screwing up 
(not reload, but zero extension), and making it noticeably slower.

IOW, the PPC situation really isn't that different from x86. 

			Linus

^ permalink raw reply

* Re: [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors
From: Junio C Hamano @ 2009-08-13 19:38 UTC (permalink / raw)
  To: Luke Dashjr; +Cc: git
In-Reply-To: <1250133624-2272-2-git-send-email-luke-jr+git@utopios.org>

Luke Dashjr <luke-jr+git@utopios.org> writes:

> Unmatched files are errors, and should be ignored with the rest of them.

Why is this a "fix"?

I would understand if it were "Make --ignore-errors imply --ignore-unmatch
unconditionally".  But then I do not think I would necessarily agree it is
a good change.

The user may know that some files in the work tree are unreadable and
cannot be indexed (hence he gives --ignore-errors) but he still may want
to catch a typo on the command line.

I do not think it is wise to make --ignore-errors imply --ignore-unmatch
unconditionally like this patch does without any escape hatch.

> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
> ---
>  builtin-add.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-add.c b/builtin-add.c
> index 0597fb9..e3132c8 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  		add_interactive = 1;
>  	if (add_interactive)
>  		exit(interactive_add(argc - 1, argv + 1, prefix));
> +	if (ignore_add_errors)
> +		ignore_unmatch = 1;
>  
>  	if (edit_interactive)
>  		return(edit_patch(argc, argv, prefix));
> -- 
> 1.6.3.3

^ permalink raw reply

* Re: [PATCH 1/5] port --ignore-unmatch to "git add"
From: Junio C Hamano @ 2009-08-13 19:36 UTC (permalink / raw)
  To: Luke Dashjr; +Cc: git
In-Reply-To: <1250133624-2272-1-git-send-email-luke-jr+git@utopios.org>

Luke Dashjr <luke-jr+git@utopios.org> writes:

> "git rm" has a --ignore-unmatch option that is also applicable to "git add"
> and may be useful for persons wanting to ignore unmatched arguments, but not
> all errors.
>
> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>

Chould you refresh my memory a bit?

In what circumstance is "rm --ignore-unmatch" useful to begin with?
A similar question for "add --ignore-unmatch".

Now the obligatory design level question is behind us, let's take a brief
look at the codde.

> +static int ignore_unmatch = 0;

Drop " = 0" and let the language initialize this to zero.

>  static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
>  {
> @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
>  	fill_pathspec_matches(pathspec, seen, specs);
>  
>  	for (i = 0; i < specs; i++) {
> -		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> +		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) && !ignore_unmatch)
>  			die("pathspec '%s' did not match any files",
>  					pathspec[i]);
>  	}
> @@ -108,7 +109,7 @@ static void refresh(int verbose, const char **pathspec)
>  	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
>  		      pathspec, seen);
>  	for (i = 0; i < specs; i++) {
> -		if (!seen[i])
> +		if (!seen[i] && !ignore_unmatch)
>  			die("pathspec '%s' did not match any files", pathspec[i]);
>  	}
>          free(seen);

What's the point of these two loops if under ignore_unmatch everything
becomes no-op?

That is, wouldn't it be much more clear if you wrote like this?

 builtin-add.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..49576b4 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -41,16 +41,25 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 	}
 }
 
+static char *alloc_seen(const char **pathspec, int *specs_)
+{
+	int specs;
+
+	if (ignore_unmatch)
+		return NULL;
+	for (specs = 0; pathspec[specs];  specs++)
+		; /* nothing */
+	*specs_ = specs;
+	return xcalloc(specs, 1);
+}
+
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 {
 	char *seen;
 	int i, specs;
 	struct dir_entry **src, **dst;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
-
+	seen = alloc_seen(pathspec, &specs);
 	src = dst = dir->entries;
 	i = dir->nr;
 	while (--i >= 0) {
@@ -60,6 +69,8 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
+	if (!seen)
+		return;
 	fill_pathspec_matches(pathspec, seen, specs);
 
 	for (i = 0; i < specs; i++) {
@@ -102,11 +113,11 @@ static void refresh(int verbose, const char **pathspec)
 	char *seen;
 	int i, specs;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = alloc_seen(pathspec, &specs);
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
+	if (!seen)
+		return;
 	for (i = 0; i < specs; i++) {
 		if (!seen[i])
 			die("pathspec '%s' did not match any files", pathspec[i]);

^ permalink raw reply related

* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Junio C Hamano @ 2009-08-13 19:33 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Linus Torvalds, git
In-Reply-To: <alpine.LFD.2.00.0908131304520.10633@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> As it is now, I was about to suggest:
>
> 	git mv block-sha1/sha1.[ch] .
> 	rmdir block-sha1
> 	rm -r mozilla-sha1
> 	rm -r arm
> 	rm -r ppc 
>
> and remove support for openssl's SHA1 usage, making this implementation 
> unconditional.  After all it is faster, or so close to be faster than 
> the alternatives, that we should probably cut on the extra dependency 
> and simplify portability issues at the same time.

Wow.  Is it now faster than the arm/ and ppc/ hand-tweaked assembly?

^ permalink raw reply

* [PATCH] Fix "unpack-objects --strict"
From: Junio C Hamano @ 2009-08-13 19:33 UTC (permalink / raw)
  To: Frank Lichtenheld; +Cc: git, Martin Koegler
In-Reply-To: <20090813111933.GZ14475@mail-vs.djpig.de>

When unpack-objects is run under the --strict option, objects that have
pointers to other objects are verified for the reachability at the end, by
calling check_object() on each of them, and letting check_object to walk
the reachable objects from them using fsck_walk() recursively.

The function however misunderstands the semantics of fsck_walk() function
when it makes a call to it, setting itself as the callback.  fsck_walk()
expects the callback function to return a non-zero value to signal an
error (negative value causes an immediate abort, positive value is still
an error but allows further checks on sibling objects) and return zero to
signal a success.  The function however returned 1 on some non error
cases, and to cover up this mistake, complained only when fsck_walk() did
not detect any error.

To fix this double-bug, make the function return zero on all success
cases, and also check for non-zero return from fsck_walk() for an error.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

Caused by b41860b (unpack-objects: prevent writing of inconsistent
objects, 2008-02-25), which introduced these checks and also the code to
keep unverified objects in core until check_objects() verifies their
reachability.  While I think it is a good idea to check for incomplete
pack data, I do not think it is necessary to keep them in core.  We can
simply error out to signal the caller not to update the refs.

We probably should write everything as they become unpackable (i.e. as
their delta bases becomes available) while keeping track of object names
(but not data) of structured objects that we received, and running only
one level of reachability check on them at the end.  That would certainly
reduce the memory consumption and may simplify the complexity of the code
at the same time.

But I'll leave that to other people.  Hint, hint...

 builtin-unpack-objects.c       |    8 ++++----
 t/t5531-deep-submodule-push.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 557148a..109b7c8 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -184,7 +184,7 @@ static int check_object(struct object *obj, int type, void *data)
 		return 0;
 
 	if (obj->flags & FLAG_WRITTEN)
-		return 1;
+		return 0;
 
 	if (type != OBJ_ANY && obj->type != type)
 		die("object type mismatch");
@@ -195,15 +195,15 @@ static int check_object(struct object *obj, int type, void *data)
 		if (type != obj->type || type <= 0)
 			die("object of unexpected type");
 		obj->flags |= FLAG_WRITTEN;
-		return 1;
+		return 0;
 	}
 
 	if (fsck_object(obj, 1, fsck_error_function))
 		die("Error in object");
-	if (!fsck_walk(obj, check_object, NULL))
+	if (fsck_walk(obj, check_object, NULL))
 		die("Error on reachable objects of %s", sha1_to_hex(obj->sha1));
 	write_cached_object(obj);
-	return 1;
+	return 0;
 }
 
 static void write_rest(void)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
new file mode 100755
index 0000000..13b8e40
--- /dev/null
+++ b/t/t5531-deep-submodule-push.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description='unpack-objects'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	git init --bare pub.git &&
+	GIT_DIR=pub.git git config receive.fsckobjects true &&
+	git init work &&
+	(
+		cd work &&
+		git init gar/bage &&
+		(
+			cd gar/bage &&
+			>junk &&
+			git add junk &&
+			git commit -m "Initial junk"
+		) &&
+		git add gar/bage &&
+		git commit -m "Initial superproject"
+	)
+'
+
+test_expect_failure push '
+	(
+		cd work &&
+		git push ../pub.git master
+	)
+'
+
+test_done

^ permalink raw reply related

* [PATCH] git submodule summary: add --files option
From: Jens Lehmann @ 2009-08-13 19:32 UTC (permalink / raw)
  To: git, hjemli; +Cc: gitster

git submodule summary is providing similar functionality for submodules as
git diff-index does for a git project (including the meaning of --cached).
But the analogon to git diff-files is missing, so add a --files option to
summarize the differences between the index of the super project and the
last commit checked out in the working tree of the submodule.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-submodule.txt |   13 +++++++++++--
 git-submodule.sh                |   19 ++++++++++++++++---
 t/t7401-submodule-summary.sh    |   22 ++++++++++++++++++++++
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 7dd73ae..145802a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--] [<path>...]
-'git submodule' [--quiet] summary [--cached] [--summary-limit <n>] [commit] [--] [<path>...]
+'git submodule' [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach <command>
 'git submodule' [--quiet] sync [--] [<path>...]

@@ -127,7 +127,11 @@ summary::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
 	in the submodule between the given super project commit and the
-	index or working tree (switched by --cached) are shown.
+	index or working tree (switched by --cached) are shown. If the option
+	--files is given, show the series of commits in the submodule between
+	the index of super project the and the working tree of the submodule
+	(this option doesn't allow to use the --cached option or to provide an
+	explicit commit).

 foreach::
 	Evaluates an arbitrary shell command in each checked out submodule.
@@ -169,6 +173,11 @@ OPTIONS
 	commands typically use the commit found in the submodule HEAD, but
 	with this option, the commit stored in the index is used instead.

+--files::
+	This option is only valid for the summary command. This command
+	compares the commit in the index with that in the submodule HEAD
+	when this option is used.
+
 -n::
 --summary-limit::
 	This option is only valid for the summary command.
diff --git a/git-submodule.sh b/git-submodule.sh
index ebed711..9bdd6ea 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,7 +4,7 @@
 #
 # Copyright (c) 2007 Lars Hjemli

-USAGE="[--quiet] [--cached] \
+USAGE="[--quiet] [--cached|--files] \
 [add [-b branch] <repo> <path>]|[status|init|update [-i|--init] [-N|--no-fetch] [--rebase|--merge]|summary [-n|--summary-limit <n>] [<commit>]] \
 [--] [<path>...]|[foreach <command>]|[sync [--] [<path>...]]"
 OPTIONS_SPEC=
@@ -16,6 +16,7 @@ command=
 branch=
 reference=
 cached=
+files=
 nofetch=
 update=

@@ -460,6 +461,7 @@ set_name_rev () {
 cmd_summary() {
 	summary_limit=-1
 	for_status=
+	diff_cmd=diff-index

 	# parse $args after "submodule ... summary".
 	while test $# -ne 0
@@ -468,6 +470,9 @@ cmd_summary() {
 		--cached)
 			cached="$1"
 			;;
+		--files)
+			files="$1"
+			;;
 		--for-status)
 			for_status="$1"
 			;;
@@ -504,9 +509,17 @@ cmd_summary() {
 		head=HEAD
 	fi

+	if [ -n "$files" ]
+	then
+		test -n "$cached" &&
+		die "--cached cannot be used with --files"
+		diff_cmd=diff-files
+		head=
+	fi
+
 	cd_to_toplevel
 	# Get modified modules cared by user
-	modules=$(git diff-index $cached --raw $head -- "$@" |
+	modules=$(git $diff_cmd $cached --raw $head -- "$@" |
 		egrep '^:([0-7]* )?160000' |
 		while read mod_src mod_dst sha1_src sha1_dst status name
 		do
@@ -520,7 +533,7 @@ cmd_summary() {

 	test -z "$modules" && return

-	git diff-index $cached --raw $head -- $modules |
+	git $diff_cmd $cached --raw $head -- $modules |
 	egrep '^:([0-7]* )?160000' |
 	cut -c2- |
 	while read mod_src mod_dst sha1_src sha1_dst status name
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 6149829..6cc16c3 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -56,6 +56,15 @@ test_expect_success 'modified submodule(forward)' "
 EOF
 "

+test_expect_success 'modified submodule(forward), --files' "
+	git submodule summary --files >actual &&
+	diff actual - <<-EOF
+* sm1 $head1...$head2 (1):
+  > Add foo3
+
+EOF
+"
+
 commit_file sm1 &&
 cd sm1 &&
 git reset --hard HEAD~2 >/dev/null &&
@@ -114,6 +123,15 @@ test_expect_success 'typechanged submodule(submodule->blob), --cached' "
 EOF
 "

+test_expect_success 'typechanged submodule(submodule->blob), --files' "
+    git submodule summary --files >actual &&
+    diff actual - <<-EOF
+* sm1 $head5(blob)->$head4(submodule) (3):
+  > Add foo5
+
+EOF
+"
+
 rm -rf sm1 &&
 git checkout-index sm1
 test_expect_success 'typechanged submodule(submodule->blob)' "
@@ -205,4 +223,8 @@ test_expect_success '--for-status' "
 EOF
 "

+test_expect_success 'fail when using --files together with --cached' "
+    test_must_fail git submodule summary --files --cached
+"
+
 test_done
-- 
1.6.4.114.gefd1

^ permalink raw reply related

* Re: msysGit and SCons: broken?
From: Dirk Süsserott @ 2009-08-13 19:32 UTC (permalink / raw)
  To: Dirk Süsserott; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <4A7B32EA.2080505@dirk.my1.cc>

Am 06.08.2009 21:45 schrieb Dirk Süsserott:
> Am 04.08.2009 00:13 schrieb Johannes Schindelin:
>> On Mon, 3 Aug 2009, Dirk Süsserott wrote:
>>
>> How does your SCons call relate to Git?  Do you call it from the Git 
>> Bash?  Do you call it from cmd.exe directly?  Is Git/bash in your PATH?
> 
> I used to call SCons from Git-bash and it worked. After Git's upgrade 
> (or some other unknown change) I did the same and it didn't work from 
> Git-bash, but it still worked from cmd.exe. Git-bash ist not in my PATH 
> when I run cmd.exe.

If someone had the same or a similar problem: I tracked it down and
found a solution. The problem was that I tried to run a Windows program
from git-bash. The Windows program then faces the bash's $PATH with a
different separator (':' vs. ';') and a different root directory ('/c/'
vs. 'C:/'). Scons tries to split the PATH apart to figure out which
tools are installed. ActivePython thinks ';' is the right separator and
then fails. Thus, I wrote a wrapper to call Scons after manipulating the
$PATH variable by first exchanging the separator and then exchanging
'/c/' with 'c:/'.

Funny, though, that my things worked a few weeks ago *without* this
wrapper. Dunno why. At least it hasn't anything to do with my Git
update. I proved that by installing earlier versions of Git.

	Dirk

^ permalink raw reply

* Re: [PATCH 2/4] fast-import: define a new option command
From: Junio C Hamano @ 2009-08-13 19:26 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Sverre Rabbelier, Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <20090813172508.GO1033@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> On Thu, Aug 13, 2009 at 10:07, Johannes
>> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>> > ... and will import the marks twice?
>> 
>> Ah, you're right :(. What's the best way to do this? Should we dump
>> any previous marks when importing new ones?
>
> Uh, well, yes.  We shouldn't define :5 if it was in the file that
> appeared in the stream, but isn't in the file on the command line.
>
> Worse, what happens if we do this:
>
>   echo "option import-marks=/not/found" \
>   | git fast-import --import-marks=my.marks
>
> I want this to work, even though /not/found does not exist, but
> my.marks does.  So that does complicate things...

How about making the option parser get and keep the _name_ of the file
until option parsing session (i.e. read the stream until initial run of
"option" command runs out and then parse the command line to override),
and then finally open the file and read it?

^ permalink raw reply


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