Git development
 help / color / mirror / Atom feed
* Re: Slow fetches of tags
From: Johannes Schindelin @ 2006-07-28 10:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ralf Baechle, git, Linus Torvalds
In-Reply-To: <7v4px4osjv.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 26 Jul 2006, Junio C Hamano wrote:

> I think the attached patch is safe in general, but somebody may
> want to give an extra set of eyeballs to double check the logic
> is sane.

The only gripe I have with it is that reachable() is relatively expensive, 
and it might be misused by a nasty client, making the server go down the 
whole history. I have no idea, though, how to prevent that.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Teach the git wrapper about --name-rev and --name-rev-by-tags
From: Johannes Schindelin @ 2006-07-28 11:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4px4osjv.fsf@assigned-by-dhcp.cox.net>


Now you can say

	git --name-rev log

instead of

	git log | git name-rev --stdin | less

with the benefit that diff.color=auto still works.

There is also a shortcut "-n" for --name-rev. The option 
--name-rev-by-tags (or -t) tries to name the revs by tags instead of all 
refs, which is nicer when talking to other people, since their heads may 
be different from yours (I feel like talking to Zaphod ;-).

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

---

	On Wed, 26 Jul 2006, Junio C Hamano wrote:

	>  * Passing the standard error from "fetch-pack -v" to "name-rev
	>    --stdin" makes it a bit more pleasant to see what is going on.

	This patch makes it even easier.

 Documentation/git.txt |   12 ++++++++++--
 cache.h               |    1 +
 git.c                 |    9 +++++++--
 pager.c               |   41 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7310a2b..eae930f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -9,7 +9,8 @@ git - the stupid content tracker
 SYNOPSIS
 --------
 'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate]
-	[--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]
+	[-n|--name-rev] [-t|--name-rev-by-tags] [--bare]
+	[--git-dir=GIT_DIR] [--help] COMMAND [ARGS]
 
 DESCRIPTION
 -----------
@@ -45,12 +46,19 @@ OPTIONS
 -p|--paginate::
 	Pipe all output into 'less' (or if set, $PAGER).
 
+-n|--name-rev:
+	Try naming all SHA1s, and page the result (see
+	link:git-name-rev[1] for a detailed explanation).
+
+-t|--name-rev-by-tags:
+	Same as '--name-rev', but try to name the SHA1s by tags.
+
 --git-dir=<path>::
 	Set the path to the repository. This can also be controlled by
 	setting the GIT_DIR environment variable.
 
 --bare::
-	Same as --git-dir=`pwd`.
+	Same as  '--git-dir=`pwd`'.
 
 FURTHER DOCUMENTATION
 ---------------------
diff --git a/cache.h b/cache.h
index 8891073..d6c5edb 100644
--- a/cache.h
+++ b/cache.h
@@ -391,6 +391,7 @@ extern int receive_keep_pack(int fd[2], 
 
 /* pager.c */
 extern void setup_pager(void);
+extern void setup_name_rev_pager(int by_tags);
 extern int pager_in_use;
 
 /* base85 */
diff --git a/git.c b/git.c
index 4ea5efb..4206b43 100644
--- a/git.c
+++ b/git.c
@@ -63,9 +63,14 @@ static int handle_options(const char*** 
 				puts(git_exec_path());
 				exit(0);
 			}
-		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
+		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate"))
 			setup_pager();
-		} else if (!strcmp(cmd, "--git-dir")) {
+		else if (!strcmp(cmd, "-n") || !strcmp(cmd, "--name-rev"))
+			setup_name_rev_pager(0);
+		else if (!strcmp(cmd, "-t") ||
+				!strcmp(cmd, "--name-rev-by-tags"))
+			setup_name_rev_pager(1);
+		else if (!strcmp(cmd, "--git-dir")) {
 			if (*argc < 1)
 				return -1;
 			setenv("GIT_DIR", (*argv)[1], 1);
diff --git a/pager.c b/pager.c
index 280f57f..48b2467 100644
--- a/pager.c
+++ b/pager.c
@@ -53,3 +53,44 @@ void setup_pager(void)
 	die("unable to execute pager '%s'", pager);
 	exit(255);
 }
+
+void setup_name_rev_pager(int by_tags)
+{
+	pid_t pid;
+	int fd[2];
+
+	if (!isatty(1))
+		return;
+
+	pager_in_use = 1; /* means we are emitting to terminal */
+
+	if (pipe(fd) < 0)
+		return;
+	pid = fork();
+	if (pid < 0) {
+		close(fd[0]);
+		close(fd[1]);
+		return;
+	}
+
+	/* return in the child */
+	if (!pid) {
+		dup2(fd[1], 1);
+		close(fd[0]);
+		close(fd[1]);
+		return;
+	}
+
+	/* The original process turns into paging name-rev */
+	dup2(fd[0], 0);
+	close(fd[0]);
+	close(fd[1]);
+
+	setup_pager();
+	if (by_tags)
+		execl("git", "git", "name-rev", "--tags", "--stdin", NULL);
+	else
+		execl("git", "git", "name-rev", "--stdin", NULL);
+	die("unable to execute git-name-rev");
+	exit(255);
+}
-- 
1.4.2.rc2.g61d8

^ permalink raw reply related

* RE: Git clone stalls at a read(3, ...) saw using strace
From: Ribeiro, Humberto Plinio @ 2006-07-28 13:36 UTC (permalink / raw)
  To: André Goddard Rosa, Pavel Roskin; +Cc: Git Mailing List, Linus Torvalds
In-Reply-To: <b8bf37780607280258s421faf65o11c5dd241e7a27c6@mail.gmail.com>

Hi, Andre.

After a "ps -ef" I saw two instances of the script defined by GIT_PROXY_COMMAND. Strangely one of those instances was child of the other. A strace showed the scripts were blocked in a "waitpid(-1,". I've killed the child script and the git clone resumed the process. 

I didn't understand why this blocking on git happened though. The creation of these two instances (one child of the other) was also strange. 

Regards,
Humberto 

---
Obrigado/Best regards/Mit freundlichen Grüßen/Pozdrawiam
Humberto Ribeiro, MSc.
SIEMENS Home and Office Communication
Phone: +55 92 2129 6205
Mobile: +55 92 8146 3147

-----Original Message-----
From: André Goddard Rosa [mailto:andre.goddard@gmail.com] 
Sent: sexta-feira, 28 de julho de 2006 05:58
To: Pavel Roskin
Cc: Git Mailing List; Linus Torvalds; Ribeiro, Humberto Plinio
Subject: Re: Git clone stalls at a read(3, ...) saw using strace

On 7/28/06, André Goddard Rosa <andre.goddard@gmail.com> wrote:
> On 7/27/06, André Goddard Rosa <andre.goddard@gmail.com> wrote:
> > On 7/27/06, Pavel Roskin <proski@gnu.org> wrote:
> > > On Thu, 2006-07-27 at 10:50 -0700, Linus Torvalds wrote:
> > > > Nope. I have a fairly constant 120kbps, and:
> > > >
> > > > [torvalds@g5 ~]$  git clone git://source.mvista.com/git/linux-davinci-2.6.git
> > > > Checking files out...)
> > > >  100% (19754/19754) done
> > >
> > > Same thing here.  Current git from the master branch.
> >
> > Forgot to say that we are using this script in GIT_PROXY_COMMAND
> > environment variable:
> >
> > (echo "CONNECT $1:$2 HTTP/1.0"; echo; cat ) | nc <proxy_add> <portnum>
> > | (read a; read a; cat )
> >
> > The first 'read a' removes the 'CONNECT SUCCESS HTTP RESPONSE 200' and
> > the second removes an empty line as described here:
> >
> > http://www.gelato.unsw.edu.au/archives/git/0605/20664.html
> >
> > I will try from home later again.
>
> Okey, I tried from home (without the proxy trick) and it behaved a lot
> better but my disc went full in the process and I got these messages:
> ...
> ...
> ...
> error: git-checkout-index: unable to write file drivers/scsi/mac53c94.c
> error: git-checkout-index: unable to write file drivers/scsi/mac53c94.h
> error: git-checkout-index: unable to write file drivers/scsi/mac_esp.c
> error: git-checkout-index: unable to create file
> drivers/scsi/mac_scsi.c (No space left on device)
> error: git-checkout-index: unable to create file
> drivers/scsi/mac_scsi.h (No space left on device)
> error: git-checkout-index: unable to create file
> drivers/scsi/mca_53c9x.c (No space left on device)
> error: git-checkout-index: unable to create file
> drivers/scsi/megaraid.c (No space left on device)
> error: git-checkout-index: unable to create file
> drivers/scsi/megaraid.h (No space left on device)
> fatal: cannot create directory at drivers/scsi/megaraid
>
> And it finished keeping the downloaded files, but I still cannot see
> these files listed above.
> I tried to pull but it says that I'm up-to-date:
>
> doctorture:/opt/downloads/mvista/linux-mvista # git-pull
> Already up-to-date.
>
> I remember that using CVS I just used 'cvs update' after checkout and
> it would bring the missing files to me.
>
> What I'm doing wrong here?

I'm also receiving these messages when trying to change branches:

doctorture:/opt/downloads/mvista/linux-mvista # git-checkout origin
fatal: Untracked working tree file '.gitignore' would be overwritten by merge.

Perhaps I will need to download using git-clone again?

Thanks,
-- 
[]s,
André Goddard

^ permalink raw reply

* Re: Git clone stalls at a read(3, ...) saw using strace
From: André Goddard Rosa @ 2006-07-28 13:57 UTC (permalink / raw)
  To: Ribeiro, Humberto Plinio; +Cc: Pavel Roskin, Git Mailing List, Linus Torvalds
In-Reply-To: <EA24FE8B11562848B4760FD5EB4D2E9401B3F3C2@SAO1016V.ww101.siemens.net>

On 7/28/06, Ribeiro, Humberto Plinio <humberto.ribeiro@siemens.com> wrote:
> Hi, Andre.
>
> After a "ps -ef" I saw two instances of the script defined by
> GIT_PROXY_COMMAND. Strangely one of those instances was child
> of the other. A strace showed the scripts were blocked in a
>"waitpid(-1,". I've killed the child script and the git clone resumed the
>process.
>
> I didn't understand why this blocking on git happened though. The
> creation of these two instances (one child of the other) was also
> strange.

Yes,  it worked to me too, but I also don't know why.

Let me explain exactly what we had to do so someone perhaps can
explain why we had to do this:

I configured GIT_PROXY_COMMAND pointing to a script with this content:

(echo "CONNECT $1:$2 HTTP/1.0"; echo; cat ) | nc 172.29.0.6 3128 |
(read a; read a; cat )

Later:

git clone git://source.mvista.com/git/linux-davinci-2.6.git

After some time (downloading 117 Mb) it stalled (git-fetch on a pipe
read) and we had to do this to resume and let it finish:

[root@mao2wx23 tmp]# ps -ef | grep git | grep -v grep
opb694    9975  1451  0 01:39 pts/11   00:00:00 /bin/sh
/usr/bin/git-clone git://source.mvista.com/git/linux-davinci-2.6.git
opb694    9985  9975  3 01:39 pts/11   00:01:56 git-fetch-pack --all
-k git //source.mvista.com git/linux-davinci-2.6.git
[root@mao2wx23 tmp]# ps -ef | grep proxy | grep -v grep
opb694    9986  9985  0 01:39 pts/11   00:00:00 /bin/sh
/home/opb694/proxy-cmd.sh source.mvista.com 9418
opb694    9987  9986  0 01:39 pts/11   00:00:00 /bin/sh
/home/opb694/proxy-cmd.sh source.mvista.com 9418
[root@mao2wx23 tmp]# kill 9987

Thanks again for the patience,
-- 
[]s,
André Goddard

^ permalink raw reply

* Re: [PATCH] Allow fetching from multiple repositories at once
From: Petr Baudis @ 2006-07-28 14:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, alp
In-Reply-To: <Pine.LNX.4.63.0607281045430.29667@wbgn013.biozentrum.uni-wuerzburg.de>

  Hi,

Dear diary, on Fri, Jul 28, 2006 at 10:51:56AM CEST, I got a letter
where Johannes Schindelin <Johannes.Schindelin@gmx.de> said that...
> So the scenario is: one remote repository (probably shared), and multiple 
> local repositories, all tracking different branches?
> 
> So, why not setup a single local (master) repository, setup all the other 
> repos with the local master as alternate, and write a simple script which 
> first fetches all branches into the master, and then pulls into the other 
> local repos from that master?

  Nope, the scenario is many remote repositories and any number of local
repositories. Look at http://gitweb.freedesktop.org/ - as far as I know,
many people have many (most?) of those repositories cloned and fetching
updates is just a huge pain. This makes no assumptions about the number
of local repositories, you can even fetch into various branch of a
single local repository - only if you fetch into multiple repositories,
all of them must see the objects you fetched.

  The alternative of squashing all the remote repositories into a single
one is probably not very attractive since you get a huge branches tree
instead, varying hooks will get impractical, it won't look good in
gitweb, git clone will by default clone all the stuff, you will need to
impose tag namespaces, permissions might get tricky and so on and so on.

> The beauty of it is: you can still pull/push directly from the remote 
> repo, if you want.

  This should let you do that too.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* Re: [PATCH] Allow fetching from multiple repositories at once
From: Petr Baudis @ 2006-07-28 14:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, alp
In-Reply-To: <20060728054341.15864.35862.stgit@machine>

Dear diary, on Fri, Jul 28, 2006 at 07:44:21AM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> @@ -461,11 +465,52 @@ static int send_ref(const char *refname,
>  
>  static int upload_pack(void)
>  {
> -	reset_timeout();
> -	head_ref(send_ref);
> -	for_each_ref(send_ref);
> -	packet_flush(1);
> -	receive_needs();
> +	int multirepo = 0;
> +
> +	while (1) {
> +		char *repo;
> +		char cwd[PATH_MAX];
> +
> +		reset_timeout();
> +		head_ref(send_ref);
> +		for_each_ref(send_ref);
> +		packet_flush(1);
> +		repo = receive_needs();
> +		if (!repo)
> +			break;
> +		multirepo++;
> +
> +		fprintf(stderr, "git-upload-pack: switching to repo %s", repo);
> +
> +		/* So that we still find objects of the original repository... */
> +		getcwd(cwd, PATH_MAX);
> +		if (strlen(cwd) < PATH_MAX - 8)
> +			strcat(cwd, "/objects");
> +		link_alt_odb_entry(cwd, strlen(cwd), NULL, 0, 1);
> +
> +		if (!enter_repo(repo, strict) || !security_repo_check(!check_export))
> +			die("git-upload-pack: security violation");
> +	}
> +
> +	if (multirepo) {
> +#define ALTENV_SIZE 65536
> +		/* Propagate all the repositories to the children */
> +		char altenv[ALTENV_SIZE], *p = altenv;
> +		struct alternate_object_database *alt;
> +		strcpy(p, ALTERNATE_DB_ENVIRONMENT "=");
> +		p += sizeof(ALTERNATE_DB_ENVIRONMENT);
> +		for (alt = alt_odb_list; alt; alt = alt->next) {
> +			strncpy(p, alt->base, alt->name - alt->base);
> +			p += alt->name - alt->base;
> +			if (p - altenv < ALTENV_SIZE)
> +				*p++ = ':';
> +			if (p - altenv >= ALTENV_SIZE)
> +				die("fetching too many repositories");
> +		}
> +		p[-1] = '\0';
> +		putenv(altenv);
> +	}
> +
>  	if (!want_obj.nr)
>  		return 0;
>  	get_common_commits();

  Note that you need to be more careful about ALTENV_SIZE checking here,
and I'm not sure if we even need to abuse the alternates database here;
only later I added setting up the alternates variable since I realized
we are executing external tools here, and things would be simpler if we
could get away by just doing that.

  I'm sorry, I don't have time to send the updated patch anymore. :-(

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* Re: [PATCH] Allow fetching from multiple repositories at once
From: Johannes Schindelin @ 2006-07-28 14:13 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git, alp
In-Reply-To: <20060728140058.GM13776@pasky.or.cz>

Hi,

On Fri, 28 Jul 2006, Petr Baudis wrote:

> Dear diary, on Fri, Jul 28, 2006 at 10:51:56AM CEST, I got a letter
> where Johannes Schindelin <Johannes.Schindelin@gmx.de> said that...
> > So the scenario is: one remote repository (probably shared), and multiple 
> > local repositories, all tracking different branches?
> > 
> > So, why not setup a single local (master) repository, setup all the other 
> > repos with the local master as alternate, and write a simple script which 
> > first fetches all branches into the master, and then pulls into the other 
> > local repos from that master?
> 
>   Nope, the scenario is many remote repositories and any number of local
> repositories.

Ah, that clarifies it for me.

> Look at http://gitweb.freedesktop.org/ - as far as I know, many people 
> have many (most?) of those repositories cloned and fetching updates is 
> just a huge pain. This makes no assumptions about the number of local 
> repositories, you can even fetch into various branch of a single local 
> repository - only if you fetch into multiple repositories, all of them 
> must see the objects you fetched.
> 
>   The alternative of squashing all the remote repositories into a single
> one is probably not very attractive since you get a huge branches tree
> instead, varying hooks will get impractical, it won't look good in
> gitweb, git clone will by default clone all the stuff, you will need to
> impose tag namespaces, permissions might get tricky and so on and so on.

You misunderstood me: I was talking about a (hidden) local multiplexer.

But in the scenario you painted, wouldn't it be practical to have one 
consolidated repo _in addition_ to the small ones? Of course, the refs in 
the consolidated one would have to be prefixed with the name of the repo 
they come from.

> > The beauty of it is: you can still pull/push directly from the remote 
> > repo, if you want.
> 
>   This should let you do that too.

Of course. But the solution I proposed does not need a patch, and _still_ 
lets you do said operations. Just wanted to point that out.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Allow fetching from multiple repositories at once
From: Petr Baudis @ 2006-07-28 14:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, alp
In-Reply-To: <Pine.LNX.4.63.0607281608520.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Hi!

Dear diary, on Fri, Jul 28, 2006 at 04:13:28PM CEST, I got a letter
where Johannes Schindelin <Johannes.Schindelin@gmx.de> said that...
> You misunderstood me: I was talking about a (hidden) local multiplexer.

Yes; I just wanted to cover another obvious alternative.  :-)

> But in the scenario you painted, wouldn't it be practical to have one 
> consolidated repo _in addition_ to the small ones? Of course, the refs in 
> the consolidated one would have to be prefixed with the name of the repo 
> they come from.

You still have problems with tags, for example, and pushing may get
tricky. It's also non-intuitive for users (I'm seeing some repositories
in gitweb but fetching from something completely different, and look at
those weird things I have to do with the branches, or git will clone
_all_ the xorg projects at once for me, eeek).

Of course you could hide all that elaborately in the porcelain but I
think this turns to be eventually much less-impact solution which is
significantly easier to maintain for the repository admins, intuitive
for users and also simpler for porcelains supporting at least
light-weight subprojects.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* Re: [PATCH] Teach git-apply about '-R'
From: Linus Torvalds @ 2006-07-28 15:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Petr Baudis, git, junkio
In-Reply-To: <Pine.LNX.4.63.0607281213250.29667@wbgn013.biozentrum.uni-wuerzburg.de>



On Fri, 28 Jul 2006, Johannes Schindelin wrote:
>
> +/* a and b may not overlap! */
> +static void memswap(void *a, void *b, unsigned int len)

This is disgusting.

Especially since it's also slow as hell.

> +		memswap(p->new_name, p->old_name, sizeof(char *));
> +		memswap(&p->new_mode, &p->old_mode, sizeof(unsigned int));
> +		memswap(&p->is_new, &p->is_delete, sizeof(int));
> +		memswap(&p->lines_added, &p->lines_deleted, sizeof(int));
> +		memswap(p->old_sha1_prefix, p->new_sha1_prefix, 41);
> +
> +		for (; frag; frag = frag->next) {
> +			memswap(&frag->newpos, &frag->oldpos, sizeof(int));
> +			memswap(&frag->newlines, &frag->oldlines, sizeof(int));

All but one of those are register sizes, so doing a horribly ugly 
"memswap()"to do them is truly nasty, when you could have done

	#define swap(a,b) myswap((a),(b),sizeof(a))

	#define myswap(a,b,size) do {		\
		unsigned char mytmp[size];	\
		memcpy(tmp, &a, size);		\
		memcpy(&a, &b, size);		\
		memcpy(&b, mytmp, size);	\
	} while (0)

and it would have worked MUCH more efficiently, since any sane compiler 
would immediately have noticed that you're doing word-sized copies, and 
optimized the hell out of it.

(Untested, of course).

		Linus

^ permalink raw reply

* Re: [PATCH] Teach git-apply about '-R'
From: Junio C Hamano @ 2006-07-28 15:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Petr Baudis, git
In-Reply-To: <Pine.LNX.4.63.0607281213250.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

A quick comment without looking much at the code.  Do you sanely
bail out when asked to reverse-apply a binary patch?

Also what was the reason to change an existing test vector in
4102?  That does not look like related to -R flag.

^ permalink raw reply

* Re: [PATCH] Teach the git wrapper about --name-rev and --name-rev-by-tags
From: Junio C Hamano @ 2006-07-28 15:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0607281308280.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	On Wed, 26 Jul 2006, Junio C Hamano wrote:
>
> 	>  * Passing the standard error from "fetch-pack -v" to "name-rev
> 	>    --stdin" makes it a bit more pleasant to see what is going on.
>
> 	This patch makes it even easier.

Probably wouldn't for that particular one, since what I wanted
to do was "git fetch-pack -v 2>&1 | git name-rev >/var/tmp/1",
so isatty(1) check in setup_name_rev_pager() is defeated by
redirection, and the information I wanted to pass name-rev would
not have passed it anyway.

But this _might_ be useful for other more general cases.  I'm
not sure -- it feels somewhat like a hack, though.

^ permalink raw reply

* [PATCH v2] Teach git-apply about '-R'
From: Johannes Schindelin @ 2006-07-28 15:46 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, git, junkio
In-Reply-To: <Pine.LNX.4.64.0607280809550.4168@g5.osdl.org>


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

On Fri, 28 Jul 2006, Linus Torvalds wrote:

> On Fri, 28 Jul 2006, Johannes Schindelin wrote:
> >
> > +/* a and b may not overlap! */
> > +static void memswap(void *a, void *b, unsigned int len)
> 
> This is disgusting.

Yes, it is. Sorry, was just meant for prototyping. I had in mind to 
replace it with temporary variables, but your solution seems way 
nicer.

 builtin-apply.c         |   65 ++++++++++++++++++++++++++++++++++++++++-------
 t/t4102-apply-rename.sh |   24 ++++++++++++++++-
 2 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index d924ac3..6b38a8a 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -120,7 +120,7 @@ struct fragment {
 struct patch {
 	char *new_name, *old_name, *def_name;
 	unsigned int old_mode, new_mode;
-	int is_rename, is_copy, is_new, is_delete, is_binary;
+	int is_rename, is_copy, is_new, is_delete, is_binary, is_reverse;
 #define BINARY_DELTA_DEFLATED 1
 #define BINARY_LITERAL_DEFLATED 2
 	unsigned long deflate_origlen;
@@ -1119,6 +1119,34 @@ static int parse_chunk(char *buffer, uns
 	return offset + hdrsize + patchsize;
 }
 
+#define swap(a,b) myswap((a),(b),sizeof(a))
+
+#define myswap(a, b, size) do {		\
+	unsigned char mytmp[size];	\
+	memcpy(mytmp, &a, size);		\
+	memcpy(&a, &b, size);		\
+	memcpy(&b, mytmp, size);		\
+} while (0)
+
+static void reverse_patches(struct patch *p)
+{
+	for (; p; p = p->next) {
+		struct fragment *frag = p->fragments;
+
+		swap(p->new_name, p->old_name);
+		swap(p->new_mode, p->old_mode);
+		swap(p->is_new, p->is_delete);
+		swap(p->lines_added, p->lines_deleted);
+		swap(p->old_sha1_prefix, p->new_sha1_prefix);
+
+		for (; frag; frag = frag->next) {
+			swap(frag->newpos, frag->oldpos);
+			swap(frag->newlines, frag->oldlines);
+		}
+		p->is_reverse = !p->is_reverse;
+	}
+}
+
 static const char pluses[] = "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++";
 static const char minuses[]= "----------------------------------------------------------------------";
 
@@ -1336,7 +1364,7 @@ static int apply_line(char *output, cons
 }
 
 static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag,
-	int inaccurate_eof)
+	int reverse, int inaccurate_eof)
 {
 	int match_beginning, match_end;
 	char *buf = desc->buffer;
@@ -1350,6 +1378,7 @@ static int apply_one_fragment(struct buf
 	int pos, lines;
 
 	while (size > 0) {
+		char first;
 		int len = linelen(patch, size);
 		int plen;
 
@@ -1366,16 +1395,23 @@ static int apply_one_fragment(struct buf
 		plen = len-1;
 		if (len < size && patch[len] == '\\')
 			plen--;
-		switch (*patch) {
+		first = *patch;
+		if (reverse) {
+			if (first == '-')
+				first = '+';
+			else if (first == '+')
+				first = '-';
+		}
+		switch (first) {
 		case ' ':
 		case '-':
 			memcpy(old + oldsize, patch + 1, plen);
 			oldsize += plen;
-			if (*patch == '-')
+			if (first == '-')
 				break;
 		/* Fall-through for ' ' */
 		case '+':
-			if (*patch != '+' || !no_add)
+			if (first != '+' || !no_add)
 				newsize += apply_line(new + newsize, patch,
 						      plen);
 			break;
@@ -1615,7 +1651,8 @@ static int apply_fragments(struct buffer
 		return apply_binary(desc, patch);
 
 	while (frag) {
-		if (apply_one_fragment(desc, frag, patch->inaccurate_eof) < 0)
+		if (apply_one_fragment(desc, frag, patch->is_reverse,
+					patch->inaccurate_eof) < 0)
 			return error("patch failed: %s:%ld",
 				     name, frag->oldpos);
 		frag = frag->next;
@@ -2142,7 +2179,8 @@ static int use_patch(struct patch *p)
 	return 1;
 }
 
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(int fd, const char *filename,
+		int reverse, int inaccurate_eof)
 {
 	unsigned long offset, size;
 	char *buffer = read_patch_file(fd, &size);
@@ -2162,6 +2200,8 @@ static int apply_patch(int fd, const cha
 		nr = parse_chunk(buffer + offset, size, patch);
 		if (nr < 0)
 			break;
+		if (reverse)
+			reverse_patches(patch);
 		if (use_patch(patch)) {
 			patch_stats(patch);
 			*listp = patch;
@@ -2226,6 +2266,7 @@ int cmd_apply(int argc, const char **arg
 {
 	int i;
 	int read_stdin = 1;
+	int reverse = 0;
 	int inaccurate_eof = 0;
 
 	const char *whitespace_option = NULL;
@@ -2236,7 +2277,7 @@ int cmd_apply(int argc, const char **arg
 		int fd;
 
 		if (!strcmp(arg, "-")) {
-			apply_patch(0, "<stdin>", inaccurate_eof);
+			apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 			read_stdin = 0;
 			continue;
 		}
@@ -2313,6 +2354,10 @@ int cmd_apply(int argc, const char **arg
 			parse_whitespace_option(arg + 13);
 			continue;
 		}
+		if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
+			reverse = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--inaccurate-eof")) {
 			inaccurate_eof = 1;
 			continue;
@@ -2333,12 +2378,12 @@ int cmd_apply(int argc, const char **arg
 			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		apply_patch(fd, arg, inaccurate_eof);
+		apply_patch(fd, arg, reverse, inaccurate_eof);
 		close(fd);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		apply_patch(0, "<stdin>", inaccurate_eof);
+		apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
diff --git a/t/t4102-apply-rename.sh b/t/t4102-apply-rename.sh
index fbb508d..22da6a0 100755
--- a/t/t4102-apply-rename.sh
+++ b/t/t4102-apply-rename.sh
@@ -13,8 +13,8 @@ # setup
 cat >test-patch <<\EOF
 diff --git a/foo b/bar
 similarity index 47%
-copy from foo
-copy to bar
+rename from foo
+rename to bar
 --- a/foo
 +++ b/bar
 @@ -1 +1 @@
@@ -39,4 +39,24 @@ else
 	    'test -f bar && ls -l bar | grep "^-..x......"'
 fi
 
+test_expect_success 'apply reverse' \
+    'git-apply -R --index --stat --summary --apply test-patch &&
+     test "$(cat foo)" = "This is foo"'
+
+cat >test-patch <<\EOF
+diff --git a/foo b/bar
+similarity index 47%
+copy from foo
+copy to bar
+--- a/foo
++++ b/bar
+@@ -1 +1 @@
+-This is foo
++This is bar
+EOF
+
+test_expect_success 'apply copy' \
+    'git-apply --index --stat --summary --apply test-patch &&
+     test "$(cat bar)" = "This is bar" -a "$(cat foo)" = "This is foo"'
+
 test_done
-- 
1.4.2.rc2.g8b063-dirty

^ permalink raw reply related

* Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/
From: Petr Baudis @ 2006-07-28 15:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr706mj0u.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Fri, Jul 28, 2006 at 06:48:49AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> > Well, at once? I can do (iv) by adding --index but that contradicts (v).
> > But maybe I'm missing something.
> 
> What should the semantics of such operation be?  Apply to index
> on paths that are clean while leave the index entries untouched
> for paths that are dirty?  What should happen on renamed paths
> that are dirty?

Keep the original sha1 but change the name. If an entry with the new
name already exists, we might just leave the index alone and create a
.rej file. (Alternatively we might create two stages in the index file
but we can't do that in case of regular rejects so I'd rather stay
consistent.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

^ permalink raw reply

* Re: [PATCH] Teach git-apply about '-R'
From: Johannes Schindelin @ 2006-07-28 15:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Petr Baudis, git
In-Reply-To: <7v3bcln3m5.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 28 Jul 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> A quick comment without looking much at the code.  Do you sanely
> bail out when asked to reverse-apply a binary patch?

Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
that is enough?

> Also what was the reason to change an existing test vector in
> 4102?  That does not look like related to -R flag.

I changed "copy" to "rename", so that I could reuse that patch to test -R, 
too. Note that in the end, the original test vector is written, and 
tested.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Teach the git wrapper about --name-rev and --name-rev-by-tags
From: Linus Torvalds @ 2006-07-28 16:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0607281308280.29667@wbgn013.biozentrum.uni-wuerzburg.de>



On Fri, 28 Jul 2006, Johannes Schindelin wrote:
> 
> Now you can say
> 
> 	git --name-rev log

I think this is wrong.

It may be a straightforward translation of

> 	git log | git name-rev --stdin | less

but that doesn't make it any more "correct".

>From a logical standpoint, it should be an argument to the _logging_, not 
to the main git binary, so it should be

	git log --name-rev

and you should do the parsing (and the output) inside revision.c.

Also, I doubt most people want every release named. I think the common 
case would be that you want those releases named that match heads (and 
tags in particular) _exactly_. If you want everything named, maybe you 
want to do "--name-rev-all" or something.

Hmm?

(That would also likely perform a lot better)

		Linus

^ permalink raw reply

* [PATCH] cg-commit --review may permanently delete changes
From: Dennis Stosberg @ 2006-07-28 17:11 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

If the patch is changed in the editor in such a way that cg-patch
can not apply it, all changes made since the last commit are
irrecoverably lost, which is _really_ bad.

This patch lets cg-commit reapply the old patch and keep the edited
patch for manual fix-up.

Signed-off-by: Dennis Stosberg <dennis@stosberg.net>
---
 cg-commit |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/cg-commit b/cg-commit
index 0cec58f..9604ad7 100755
--- a/cg-commit
+++ b/cg-commit
@@ -524,8 +524,11 @@ if [ "$review" ]; then
 		fi
 		echo "Applying the edited patch..."
 		if ! cg-patch < "$PATCH2"; then
-			rm "$PATCH" "$PATCH2" "$LOGMSG"
-			die "unable to apply the edited patch"
+			echo "The edited patch does not apply. Reapplying old patch."
+			cg-patch <"$PATCH" >/dev/null
+			edited_patch="$(mktemp -t edited-patch.XXXXXX)"
+			mv "$PATCH2" "$edited_patch"
+			die "You can find the edited patch in \"$edited_patch\" for manual review."
 		fi
 	fi
 fi
-- 
1.4.1

^ permalink raw reply related

* Re: [PATCH] Makefile: ssh-pull.o depends on ssh-fetch.c
From: Junio C Hamano @ 2006-07-28 18:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0607281117240.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>  Makefile |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 636679f..e8037ad 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -661,6 +661,7 @@ git-%$X: %.o $(GITLIBS)
>  	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIB_FILE) $(SIMPLE_LIB)
>  
> +ssh-pull.o: ssh-fetch.c
>  git-local-fetch$X: fetch.o
>  git-ssh-fetch$X: rsh.o fetch.o
>  git-ssh-upload$X: rsh.o

My personal preference would be to deprecate these commit
walkers ;-) but in any case we would also need to make
ssh-push.o depend on ssh-upload.c for the same logic.

^ permalink raw reply

* Re: [PATCH] Teach the git wrapper about --name-rev and --name-rev-by-tags
From: Johannes Schindelin @ 2006-07-28 18:53 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0607280952200.4168@g5.osdl.org>

Hi,

On Fri, 28 Jul 2006, Linus Torvalds wrote:

> On Fri, 28 Jul 2006, Johannes Schindelin wrote:
> > 
> > Now you can say
> > 
> > 	git --name-rev log
> 
> I think this is wrong.

I think it is not wrong. :-)

> It may be a straightforward translation of
> 
> > 	git log | git name-rev --stdin | less
> 
> but that doesn't make it any more "correct".

I use it also for other git commands, so this was very much on purpose.

> Also, I doubt most people want every release named.

You are probably right. But _I_ want to know that e.g. commit 
a025463bc0ec2c894a88f2dfb44cf88ba71bb712 is really tags/v1.4.0^0~27^2. 
Both are immutable, but the latter is nicer to people than to computers.

> I think the common case would be that you want those releases named that 
> match heads (and tags in particular) _exactly_. If you want everything 
> named, maybe you want to do "--name-rev-all" or something.
> 
> Hmm?
> 
> (That would also likely perform a lot better)

True. But then, you probably know which head it is, because you probably 
specified it yourself on the command line.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Makefile: ssh-pull.o depends on ssh-fetch.c
From: Johannes Schindelin @ 2006-07-28 18:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vslkllhqo.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 28 Jul 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +ssh-pull.o: ssh-fetch.c
> 
> My personal preference would be to deprecate these commit walkers ;-) 
> but in any case we would also need to make ssh-push.o depend on 
> ssh-upload.c for the same logic.

Probably. But this was a _necessary_ fix: git stopped compiling here (yes, 
I could have done "make clean && make", but that takes all the fun out of 
it...).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Teach git-apply about '-R'
From: Junio C Hamano @ 2006-07-28 19:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0607281748390.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 28 Jul 2006, Junio C Hamano wrote:
>> 
>> A quick comment without looking much at the code.  Do you sanely
>> bail out when asked to reverse-apply a binary patch?
>
> Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
> that is enough?

You would need something like this, at least for now, since both
deflated literal and deflated delta methods are irreversible.
I'll cook up another binary diff output that can go
bidirectional.

Note that --allow-binary-replacement uses the blob object name
recorded on index lines of binary patch, and uses it to cheat
(iow, when it knows your version is the old version recorded on
index line and your repository happens to have the resulting
blob, it just uses the blob without looking at the binary
contents recorded in the patch), so you have to work a bit hard
to cause it to fail in t/trash repository after you run t4103
test.  Resetting to "master", apply BF.diff, and immediately
reverse apply BF.diff would _work_, only because "master" and
"binary" branch keep both preimage and postimage.

-- >8 --
diff --git a/builtin-apply.c b/builtin-apply.c
index 6b38a8a..d4381d9 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1535,6 +1535,12 @@ static int apply_binary_fragment(struct 
 	void *data;
 	void *result;
 
+	/* Binary patch is irreversible */
+	if (patch->is_reverse)
+		return error("cannot reverse-apply a binary patch to '%s'",
+			     patch->new_name
+			     ? patch->new_name : patch->old_name);
+
 	data = inflate_it(fragment->patch, fragment->size,
 			  patch->deflate_origlen);
 	if (!data)

^ permalink raw reply related

* Re: [PATCH] Teach git-apply about '-R'
From: Johannes Schindelin @ 2006-07-28 19:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhd11leny.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 28 Jul 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Fri, 28 Jul 2006, Junio C Hamano wrote:
> >> 
> >> A quick comment without looking much at the code.  Do you sanely
> >> bail out when asked to reverse-apply a binary patch?
> >
> > Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
> > that is enough?
> 
> You would need something like this, at least for now, since both
> deflated literal and deflated delta methods are irreversible.

Somehow I had the impression that binary diff meant that you needed both 
objects in the object database. I was wrong.

Ciao,
Dscho

^ permalink raw reply

* [RFC] Add the --color-words option to the diff options family
From: Johannes Schindelin @ 2006-07-28 21:56 UTC (permalink / raw)
  To: git


With this option, the changed words are shown inline. For example,
if a file containing "This is foo" is changed to "This is bar", the diff
will now show "This is " in plain text, "foo" in red, and "bar" in green.

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

---

	I am probably the only person who finds it useful, but there
	is a high coolness factor attached to it.

	And the libxdiff library actually made it very easy to do. (Yes, 
	there are two nested calls to xdiff...)

 Documentation/diff-options.txt |    3 +
 diff.c                         |  178 +++++++++++++++++++++++++++++++++++++++-
 diff.h                         |    3 -
 3 files changed, 177 insertions(+), 7 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 47ba9a4..b5d9763 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -36,6 +36,9 @@
 	Turn off colored diff, even when the configuration file
 	gives the default to color output.
 
+--color-words::
+	Show colored word diff, i.e. color words which have changed.
+
 --no-renames::
 	Turn off rename detection, even when the configuration
 	file gives the default to do so.
diff --git a/diff.c b/diff.c
index 6198a61..e910971 100644
--- a/diff.c
+++ b/diff.c
@@ -358,12 +358,152 @@ static int fill_mmfile(mmfile_t *mf, str
 	return 0;
 }
 
+struct diff_words_buffer {
+	mmfile_t text;
+	long alloc;
+	long current; /* output pointer */
+	int suppressed_newline;
+};
+
+static void diff_words_append(char *line, unsigned long len,
+		struct diff_words_buffer *buffer)
+{
+	if (buffer->text.size + len > buffer->alloc) {
+		buffer->alloc = (buffer->text.size + len) * 3 / 2;
+		buffer->text.ptr = xrealloc(buffer->text.ptr, buffer->alloc);
+	}
+	line++;
+	len--;
+	memcpy(buffer->text.ptr + buffer->text.size, line, len);
+	buffer->text.size += len;
+}
+
+struct diff_words_data {
+	struct xdiff_emit_state xm;
+	struct diff_words_buffer minus, plus;
+};
+
+static void print_word(struct diff_words_buffer *buffer, int len, int color,
+		int suppress_newline)
+{
+	const char *ptr;
+	int eol = 0;
+
+	if (len == 0)
+		return;
+
+	ptr  = buffer->text.ptr + buffer->current;
+	buffer->current += len;
+
+	if (ptr[len - 1] == '\n') {
+		eol = 1;
+		len--;
+	}
+
+	fputs(diff_get_color(1, color), stdout);
+	fwrite(ptr, len, 1, stdout);
+	fputs(diff_get_color(1, DIFF_RESET), stdout);
+
+	if (eol) {
+		if (suppress_newline)
+			buffer->suppressed_newline = 1;
+		else
+			putchar('\n');
+	}
+}
+
+static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
+{
+	struct diff_words_data *diff_words = priv;
+
+	if (diff_words->minus.suppressed_newline) {
+		if (line[0] != '+')
+			putchar('\n');
+		diff_words->minus.suppressed_newline = 0;
+	}
+
+	len--;
+	switch (line[0]) {
+		case '-':
+			print_word(&diff_words->minus, len, DIFF_FILE_OLD, 1);
+			break;
+		case '+':
+			print_word(&diff_words->plus, len, DIFF_FILE_NEW, 0);
+			break;
+		case ' ':
+			print_word(&diff_words->plus, len, DIFF_PLAIN, 0);
+			diff_words->minus.current += len;
+			break;
+	}
+}
+
+/* this executes the word diff on the accumulated buffers */
+static void diff_words_show(struct diff_words_data *diff_words)
+{
+	xpparam_t xpp;
+	xdemitconf_t xecfg;
+	xdemitcb_t ecb;
+	mmfile_t minus, plus;
+	int i;
+
+	minus.size = diff_words->minus.text.size;
+	minus.ptr = xmalloc(minus.size);
+	memcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);
+	for (i = 0; i < minus.size; i++)
+		if (isspace(minus.ptr[i]))
+			minus.ptr[i] = '\n';
+	diff_words->minus.current = 0;
+
+	plus.size = diff_words->plus.text.size;
+	plus.ptr = xmalloc(plus.size);
+	memcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);
+	for (i = 0; i < plus.size; i++)
+		if (isspace(plus.ptr[i]))
+			plus.ptr[i] = '\n';
+	diff_words->plus.current = 0;
+
+	xpp.flags = XDF_NEED_MINIMAL;
+	xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;
+	xecfg.flags = 0;
+	ecb.outf = xdiff_outf;
+	ecb.priv = diff_words;
+	diff_words->xm.consume = fn_out_diff_words_aux;
+	xdl_diff(&minus, &plus, &xpp, &xecfg, &ecb);
+
+	free(minus.ptr);
+	free(plus.ptr);
+	diff_words->minus.text.size = diff_words->plus.text.size = 0;
+
+	if (diff_words->minus.suppressed_newline) {
+		putchar('\n');
+		diff_words->minus.suppressed_newline = 0;
+	}
+}
+
 struct emit_callback {
 	struct xdiff_emit_state xm;
 	int nparents, color_diff;
 	const char **label_path;
+	struct diff_words_data *diff_words;
 };
 
+static void free_diff_words_data(struct emit_callback *ecbdata)
+{
+	if (ecbdata->diff_words) {
+		/* flush buffers */
+		if (ecbdata->diff_words->minus.text.size ||
+				ecbdata->diff_words->plus.text.size)
+			diff_words_show(ecbdata->diff_words);
+
+		if (ecbdata->diff_words->minus.text.ptr)
+			free (ecbdata->diff_words->minus.text.ptr);
+		if (ecbdata->diff_words->plus.text.ptr)
+			free (ecbdata->diff_words->plus.text.ptr);
+		free(ecbdata->diff_words);
+		ecbdata->diff_words = NULL;
+	}
+}
+
 const char *diff_get_color(int diff_use_color, enum color_diff ix)
 {
 	if (diff_use_color)
@@ -398,12 +538,31 @@ static void fn_out_consume(void *priv, c
 	else {
 		int nparents = ecbdata->nparents;
 		int color = DIFF_PLAIN;
-		for (i = 0; i < nparents && len; i++) {
-			if (line[i] == '-')
-				color = DIFF_FILE_OLD;
-			else if (line[i] == '+')
-				color = DIFF_FILE_NEW;
-		}
+		if (ecbdata->diff_words && nparents != 1)
+			/* fall back to normal diff */
+			free_diff_words_data(ecbdata);
+		if (ecbdata->diff_words) {
+			if (line[0] == '-') {
+				diff_words_append(line, len,
+						&ecbdata->diff_words->minus);
+				return;
+			} else if (line[0] == '+') {
+				diff_words_append(line, len,
+						&ecbdata->diff_words->plus);
+				return;
+			}
+			if (ecbdata->diff_words->minus.text.size ||
+					ecbdata->diff_words->plus.text.size)
+				diff_words_show(ecbdata->diff_words);
+			line++;
+			len--;
+		} else
+			for (i = 0; i < nparents && len; i++) {
+				if (line[i] == '-')
+					color = DIFF_FILE_OLD;
+				else if (line[i] == '+')
+					color = DIFF_FILE_NEW;
+			}
 		set = diff_get_color(ecbdata->color_diff, color);
 	}
 	if (len > 0 && line[len-1] == '\n')
@@ -836,7 +995,12 @@ static void builtin_diff(const char *nam
 		ecb.outf = xdiff_outf;
 		ecb.priv = &ecbdata;
 		ecbdata.xm.consume = fn_out_consume;
+		if (o->color_diff_words)
+			ecbdata.diff_words =
+				xcalloc(1, sizeof(struct diff_words_data));
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+		if (o->color_diff_words)
+			free_diff_words_data(&ecbdata);
 	}
 
  free_ab_and_return:
@@ -1712,6 +1876,8 @@ int diff_opt_parse(struct diff_options *
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
 	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
+	else if (!strcmp(arg, "--color-words"))
+		options->color_diff = options->color_diff_words = 1;
 	else if (!strcmp(arg, "--no-renames"))
 		options->detect_rename = 0;
 	else
diff --git a/diff.h b/diff.h
index 0d32830..51c163b 100644
--- a/diff.h
+++ b/diff.h
@@ -46,7 +46,8 @@ struct diff_options {
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1,
-		 color_diff:1;
+		 color_diff:1,
+		 color_diff_words:1;
 	int context;
 	int break_opt;
 	int detect_rename;
-- 
1.4.2.rc2.g8b063-dirty

^ permalink raw reply related

* Oh, you do it very fast...
From: Lamb @ 2006-07-28 23:40 UTC (permalink / raw)
  To: glenda

Nice to see you! The good news are that this obstacle can be overcome by you, the real man. Safe, efficient and covering all aspects, Extra-Time will help you forget the premature nightmare. Any man wants to last longer and make his partner happy with that. Come on in: http://juliasterkl.com/gall/get/ You will never see her face frustrated again. No arguments and no complaints!

^ permalink raw reply

* [PATCH] Fix http-fetch
From: Johannes Schindelin @ 2006-07-29  0:10 UTC (permalink / raw)
  To: git, junkio


With the latest changes in fetch.c, http-fetch crashed accessing 
write_ref[i], where write_ref was NULL.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 fetch.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fetch.c b/fetch.c
index 2151c7b..aeb6bf2 100644
--- a/fetch.c
+++ b/fetch.c
@@ -245,7 +245,7 @@ void pull_targets_free(int targets, char
 {
 	while (targets--) {
 		free(target[targets]);
-		if (write_ref[targets])
+		if (write_ref && write_ref[targets])
 			free((char *) write_ref[targets]);
 	}
 }
@@ -263,7 +263,7 @@ int pull(int targets, char **target, con
 	track_object_refs = 0;
 
 	for (i = 0; i < targets; i++) {
-		if (!write_ref[i])
+		if (!write_ref || !write_ref[i])
 			continue;
 
 		lock[i] = lock_ref_sha1(write_ref[i], NULL, 0);
@@ -295,7 +295,7 @@ int pull(int targets, char **target, con
 		msg = NULL;
 	}
 	for (i = 0; i < targets; i++) {
-		if (!write_ref[i])
+		if (!write_ref || !write_ref[i])
 			continue;
 		ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)");
 		lock[i] = NULL;
-- 
1.4.2.rc2.g0d86-dirty

^ permalink raw reply related

* Re: log and diff family: honor config even from subdirectories
From: Linus Torvalds @ 2006-07-29  1:17 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Junio,
 I think your patch is fine as a band-aid, but I wonder if we shouldn't 
just move the "setup_git_directory()" call out of init_revisions(), and 
pass it as an argument to init_revision(). Some of the callers have 
already done setup_git_directory() earlier for their own reasons anyway.

And from a quick look it looks like you missed the same bug happening in 
cmd_format_patch(), which calls git_config(git_format_config) before 
having done the setup.

I'd actually _like_ to do the setup unconditionally inside the git wrapper 
(early - to make sure that we don't have this bug), but some things (at 
least "init-db", "clone" and "ls-remote") are obviously not supposed to do 
it, since they work outside of a git directory. So either we need to do it 
in each builtin command separately, or we'd need to add a flag in the 
command descriptor array.

Any clever ideas?

		Linus

^ 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