Git development
 help / color / mirror / Atom feed
* git-svn: cloning from svn.apache.org
From: Florian Weimer @ 2009-01-25 10:49 UTC (permalink / raw)
  To: git

What can be done to speed up cloning from svn.apache.org?  It's a
Subversion repository with over 700,000 revisions.  Many projects
stored there started their life in some of the later revisions, and
the forward-scanning part of git-svn takes ages.

I wonder if it were faster to search history backwards, starting with
the specified path, similar to what "svn log" does?

^ permalink raw reply

* Re: [PATCH 1/2] tree.c: allow read_tree_recursive() to traverse gitlink entries
From: Johannes Schindelin @ 2009-01-25 11:43 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232844726-14902-2-git-send-email-hjemli@gmail.com>

Hi,

On Sun, 25 Jan 2009, Lars Hjemli wrote:

> When the callback function invoked from read_tree_recursive() returns
> the value `READ_TREE_RECURSIVE` for a gitlink entry, the traversal will
> now continue into the tree connected to the gitlinked commit.

\n

> This functionality can be used to allow inter-repository operations, but 
> since the current users of read_tree_recursive() does not yet support 
> such operations, they have been modified where necessary to make sure 
> that they never return READ_TREE_RECURSIVE for gitlink entries (hence no 
> change in behaviour should be introduces by this patch alone).

s/\(introduce\)s/\1d/

> diff --git a/archive.c b/archive.c
> index 9ac455d..e6de039 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
>  		err = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);
>  		if (err)
>  			return err;
> -		return READ_TREE_RECURSIVE;
> +		return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);

You do not need the parentheses around the conditional:

	$ git grep 'return (.*?' *.c | wc -l
	14
	gene099@racer:~/git (rebase-i-p)$ git grep 'return [^(]*?' *.c | wc -l
	41

Note that the 14 matches include 9 false positives.

> diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
> index 5b63e6e..fca4631 100644
> --- a/builtin-ls-tree.c
> +++ b/builtin-ls-tree.c
> @@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
>  		 *
>  		 * Something similar to this incomplete example:
>  		 *
> -		if (show_subprojects(base, baselen, pathname)) {
> -			struct child_process ls_tree;
> -
> -			ls_tree.dir = base;
> -			ls_tree.argv = ls-tree;

I wondered how that could ever have compiled...

Until I inspected the file (which is different in junio/next from what you 
based your patch on; your patch is vs junio/master).

> @@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,
>  			if (retval)
>  				return -1;
>  			continue;
> +		} else if (S_ISGITLINK(entry.mode)) {
> +			int retval;
> +			struct strbuf path;

s/;/ = STRBUF_INIT;/

> +			unsigned int entrylen;
> +			struct commit *commit;
> +
> +			entrylen = tree_entry_len(entry.path, entry.sha1);
> +			strbuf_init(&path, baselen + entrylen + 1);
> +			strbuf_add(&path, base, baselen);
> +			strbuf_add(&path, entry.path, entrylen);
> +			strbuf_addch(&path, '/');

Why not
			strbuf_addf(&path, "%.*s%.*s/", baselen, base, 
				entrylen, entry.path);

> +
> +			commit = lookup_commit(entry.sha1);
> +			if (!commit)
> +				die("Commit %s in submodule path %s not found",
> +				    sha1_to_hex(entry.sha1), path.buf);
> +
> +			if (parse_commit(commit))
> +				die("Invalid commit %s in submodule path %s",
> +				    sha1_to_hex(entry.sha1), path.buf);
> +
> +			retval = read_tree_recursive(commit->tree,
> +						     path.buf, path.len,
> +						     stage, match, fn, context);
> +			strbuf_release(&path);
> +			if (retval)
> +				return -1;
> +			continue;

I'd also place a comment above read_tree_recursive() stating that this 
function tries to traverse into submodules when READ_TREE_RECURSIVE is 
returned for submodule entries, but no attempt is made at including 
alternate object directories.  (And it must be that way: think bare 
repositories -- they cannot just try to include a subdirectory's 
.git/objects/..)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]
From: Johannes Schindelin @ 2009-01-25 11:57 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232844726-14902-3-git-send-email-hjemli@gmail.com>

Hi,

On Sun, 25 Jan 2009, Lars Hjemli wrote:

> The --submodules option uses the enhanced read_tree_recursive() to
> enable inclusion of submodules in the generated archive.
> 
> When invoked with `--submodules=all` all gitlink entries will be
> traversed, and when invoked with --submodules=checkedout (the default
> option) only gitlink entries with a git repo (i.e. checked out sub-
> modules) will be traversed.

                               In bare repositories, this means: none.

My reasoning for "*" instead of "all" and "" instead for "checkedout" was 
that you could allow "<name1>,<name2>" at some stage, where <name> would 
first be interpreted as a submodule group, and if that fails, as submodule 
name.

Thinking about that more, "" seems illogical, that should rather mean 
"none", i.e. the same as --no-submodules.  The "checkedout" could be "." 
then, perhaps?  As in "what we have checked out in ./, the current 
directory"?

> When a gitlink has been selected for traversal, it is required that all
> objects necessary to perform this traversal are available in either the
> primary odb or through an alternate odb. To this end, git archive will
> insert the object database of the selected gitlink (when checked out)
> as an alternate odb, using the new function add_alt_odb().

> And since alternates now can be added after parsing of 
> objects/info/alternates, the error message in link_alt_odb_entry() has 
> been updated to not mention this file.

Could you split that part into its own patch again, please?

> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)
>  	check[1].attr = attr_export_subst;
>  }
>  
> +static int include_repository(const char *path)
> +{
> +	struct stat st;
> +	const char *tmp;
> +
> +	/* Return early if the path does not exist since it is OK to not
> +	 * checkout submodules.
> +	 */
> +	if (stat(path, &st) && errno == ENOENT)
> +		return 1;
> +
> +	tmp = read_gitfile_gently(path);

This will leak memory, no?

> +	if (tmp) {
> +		path = tmp;
> +		if (stat(path, &st))
> +			die("Unable to stat submodule gitdir %s: %s (%d)",
> +			    path, strerror(errno), errno);
> +	}
> +
> +	if (!S_ISDIR(st.st_mode))
> +		die("Submodule gitdir %s is not a directory", path);
> +
> +	if (add_alt_odb(mkpath("%s/objects", path)))
> +		die("submodule odb %s could not be added as an alternate",
> +		    path);
> +
> +	return 0;
> +}

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] diff-options.txt: Fix asciidoc markup issue
From: Johannes Schindelin @ 2009-01-25 11:58 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: gitster, git
In-Reply-To: <1232860721-6174-1-git-send-email-tlikonen@iki.fi>

Hi,

On Sun, 25 Jan 2009, Teemu Likonen wrote:

> Must be "--patience::", not "--patience:".

Thanks.  If you would add your Sign-off, I'd add my Acked-by.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Johannes Schindelin @ 2009-01-25 12:06 UTC (permalink / raw)
  To: Kyle Moffett; +Cc: Hannu Koivisto, Felipe Contreras, Junio C Hamano, git
In-Reply-To: <f73f7ab80901242332x7de1f856j51181c3c05cf2b11@mail.gmail.com>

Hi,

On Sun, 25 Jan 2009, Kyle Moffett wrote:

> Actually, if msysgit really wanted to be a "native" windows program,

... it would

- not use MSys, but MSVC++,

- have a backing by a startup company,

- not use any of Git's source code at all, but reimplement everything in 
  closed source,

- come with a shrink wrapped box and a huge manual,

- do a lot of BS advertisement,

- offer overprized courses for management, in fancy locations,

- would cost around 50 dollars for a single license,

- would cost around 1500 dollars for a site-wide license with free 
  upgrades for 3 years and 2 free training courses,

- be dog slow, eating resources like mad, and

- crash frequently.

Other questions?

Ciao,
Dscho

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #06; Sat, 24)
From: Johannes Schindelin @ 2009-01-25 12:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wp0kmj4.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 24 Jan 2009, Junio C Hamano wrote:

> ----------------------------------------------------------------
> [Actively cooking]
> 
> * js/valgrind (Wed Jan 21 02:36:40 2009 +0100) 2 commits
>  - valgrind: ignore ldso errors
>  - Add valgrind support in test scripts
> 
> Dscho seems to have some updates out of discussion with Peff, which is 
> not queued here.

Yep.  I got sidetracked with the rebase -i -p stuff.  Will try to submit 
something usable later today.

BTW a test run on my machine resulted in a few koku of valgrind errors; 
This was done in my personal tree which contains dozens of extra patches, 
so I want to repeat the exercize with 'next' first, but I think we will 
get quite some patches due to the valgrind support...

Ciao,
Dscho

^ permalink raw reply

* [PATCH v2] diff-options.txt: Fix asciidoc markup issue
From: Teemu Likonen @ 2009-01-25 12:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <alpine.DEB.1.00.0901251257510.14855@racer>

Must be "--patience::", not "--patience:".

Signed-off-by: Teemu Likonen <tlikonen@iki.fi>
---

Johannes Schindelin (2009-01-25 12:58 +0100) wrote:

> Thanks.  If you would add your Sign-off, I'd add my Acked-by.

Here's a new version. Though it feels a bit funny to add a S-o-b for a
patch that does nothing but adds a single character. :-)


 Documentation/diff-options.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 3816a83..813a7b1 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -36,7 +36,7 @@ endif::git-format-patch[]
 --patch-with-raw::
 	Synonym for "-p --raw".
 
---patience:
+--patience::
 	Generate a diff using the "patience diff" algorithm.
 
 --stat[=width[,name-width]]::
-- 
1.6.1.295.g75c4eae

^ permalink raw reply related

* Re: [PATCH 1/2] tree.c: allow read_tree_recursive() to traverse  gitlink entries
From: Lars Hjemli @ 2009-01-25 12:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901251225250.14855@racer>

On Sun, Jan 25, 2009 at 12:43, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Sun, 25 Jan 2009, Lars Hjemli wrote:
>
>> This functionality can be used to allow inter-repository operations, but
>> since the current users of read_tree_recursive() does not yet support
>> such operations, they have been modified where necessary to make sure
>> that they never return READ_TREE_RECURSIVE for gitlink entries (hence no
>> change in behaviour should be introduces by this patch alone).
>
> s/\(introduce\)s/\1d/

Thanks

>
>> diff --git a/archive.c b/archive.c
>> index 9ac455d..e6de039 100644
>> --- a/archive.c
>> +++ b/archive.c
>> @@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
>>               err = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);
>>               if (err)
>>                       return err;
>> -             return READ_TREE_RECURSIVE;
>> +             return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
>
> You do not need the parentheses around the conditional:
>
>        $ git grep 'return (.*?' *.c | wc -l
>        14
>        gene099@racer:~/git (rebase-i-p)$ git grep 'return [^(]*?' *.c | wc -l
>        41
>
> Note that the 14 matches include 9 false positives.

Ok, will fix.

>
>> diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
>> index 5b63e6e..fca4631 100644
>> --- a/builtin-ls-tree.c
>> +++ b/builtin-ls-tree.c
>> @@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
>>                *
>>                * Something similar to this incomplete example:
>>                *
>> -             if (show_subprojects(base, baselen, pathname)) {
>> -                     struct child_process ls_tree;
>> -
>> -                     ls_tree.dir = base;
>> -                     ls_tree.argv = ls-tree;
>
> I wondered how that could ever have compiled...
>
> Until I inspected the file (which is different in junio/next from what you
> based your patch on; your patch is vs junio/master).

Yes, sorry for not mentioning that.


>
>> @@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,
>>                       if (retval)
>>                               return -1;
>>                       continue;
>> +             } else if (S_ISGITLINK(entry.mode)) {
>> +                     int retval;
>> +                     struct strbuf path;
>
> s/;/ = STRBUF_INIT;/

I skipped the STRBUF_INIT since I used strbuf_init() below, but...


>
>> +                     unsigned int entrylen;
>> +                     struct commit *commit;
>> +
>> +                     entrylen = tree_entry_len(entry.path, entry.sha1);
>> +                     strbuf_init(&path, baselen + entrylen + 1);
>> +                     strbuf_add(&path, base, baselen);
>> +                     strbuf_add(&path, entry.path, entrylen);
>> +                     strbuf_addch(&path, '/');
>
> Why not
>                        strbuf_addf(&path, "%.*s%.*s/", baselen, base,
>                                entrylen, entry.path);

...with this cute fix the STRBUF_INIT is required. Will fix.

Thanks for the review.

--
larsh

^ permalink raw reply

* Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]
From: Lars Hjemli @ 2009-01-25 13:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901251247040.14855@racer>

On Sun, Jan 25, 2009 at 12:57, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 25 Jan 2009, Lars Hjemli wrote:
>
>> The --submodules option uses the enhanced read_tree_recursive() to
>> enable inclusion of submodules in the generated archive.
>>
>> When invoked with `--submodules=all` all gitlink entries will be
>> traversed, and when invoked with --submodules=checkedout (the default
>> option) only gitlink entries with a git repo (i.e. checked out sub-
>> modules) will be traversed.
>
>                               In bare repositories, this means: none.
>
> My reasoning for "*" instead of "all" and "" instead for "checkedout" was
> that you could allow "<name1>,<name2>" at some stage, where <name> would
> first be interpreted as a submodule group, and if that fails, as submodule
> name.
>
> Thinking about that more, "" seems illogical, that should rather mean
> "none", i.e. the same as --no-submodules.  The "checkedout" could be "."
> then, perhaps?  As in "what we have checked out in ./, the current
> directory"?

Yes, I think that makes sense, i.e. '--submodules' will include _all_
submodules (making the option behave identically for bare and non-bare
repositories), '--submodules=.' will include checked out submodules
(making the option a no-op in bare repos, which also makes sense) and
'--submodules=<name>[,<name>...]' will include the named submodules,
where "named" could mean groupname, submodule name or submodule path,
in that order.

But then we probably also want some (optional) syntax to specify the
kind of name, e.g. '--submodules=g:foo,n:bar,p:lib/baz' for group foo,
name bar and path lib/baz. Agree?

>
>> When a gitlink has been selected for traversal, it is required that all
>> objects necessary to perform this traversal are available in either the
>> primary odb or through an alternate odb. To this end, git archive will
>> insert the object database of the selected gitlink (when checked out)
>> as an alternate odb, using the new function add_alt_odb().
>
>> And since alternates now can be added after parsing of
>> objects/info/alternates, the error message in link_alt_odb_entry() has
>> been updated to not mention this file.
>
> Could you split that part into its own patch again, please?

Sure.


>
>> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)
>>       check[1].attr = attr_export_subst;
>>  }
>>
>> +static int include_repository(const char *path)
>> +{
>> +     struct stat st;
>> +     const char *tmp;
>> +
>> +     /* Return early if the path does not exist since it is OK to not
>> +      * checkout submodules.
>> +      */
>> +     if (stat(path, &st) && errno == ENOENT)
>> +             return 1;
>> +
>> +     tmp = read_gitfile_gently(path);
>
> This will leak memory, no?

I don't think so: read_gitfile_gently() returns a value obtained by
calling make_absolute_path() which returns a static buffer. Also, the
path argument to include_repository() is obtained by calling mkpath()
which returns another static buffer so I don't see any malloc()'s
which should be free()'d. Is my code-reading flawed?

--
larsh

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Jakub Narebski @ 2009-01-25 13:46 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.DEB.1.00.0901250324320.14855@racer>

Johannes Schindelin wrote:

>> Hmm.  You're right, that is not really intuitive.  How about
>> 
>>       merge (B) A # Merge...
>> 
>> instead?
> 
> Or even better:
> 
>         merge B parent A' # Merge...

merge B with A' # Merge... 

;-)

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]
From: Johannes Schindelin @ 2009-01-25 13:55 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <8c5c35580901250500s667db3f0j608a30541321ac0a@mail.gmail.com>

Hi,

On Sun, 25 Jan 2009, Lars Hjemli wrote:

> On Sun, Jan 25, 2009 at 12:57, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > My reasoning for "*" instead of "all" and "" instead for "checkedout" 
> > was that you could allow "<name1>,<name2>" at some stage, where <name> 
> > would first be interpreted as a submodule group, and if that fails, as 
> > submodule name.
> >
> > Thinking about that more, "" seems illogical, that should rather mean 
> > "none", i.e. the same as --no-submodules.  The "checkedout" could be 
> > "." then, perhaps?  As in "what we have checked out in ./, the current 
> > directory"?
> 
> Yes, I think that makes sense, i.e. '--submodules' will include _all_ 
> submodules (making the option behave identically for bare and non-bare 
> repositories), '--submodules=.' will include checked out submodules 
> (making the option a no-op in bare repos, which also makes sense) and 
> '--submodules=<name>[,<name>...]' will include the named submodules, 
> where "named" could mean groupname, submodule name or submodule path, in 
> that order.

Well, I can live with the default of all submodules, even if I think that 
"git-submodule.sh" uses the checked out submodules by default.

> But then we probably also want some (optional) syntax to specify the 
> kind of name, e.g. '--submodules=g:foo,n:bar,p:lib/baz' for group foo, 
> name bar and path lib/baz. Agree?

IMO that is overkill.  Anybody naming a submodule group identically to a 
submodule deserves what she gets, anyway.


> >> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)
> >>       check[1].attr = attr_export_subst;
> >>  }
> >>
> >> +static int include_repository(const char *path)
> >> +{
> >> +     struct stat st;
> >> +     const char *tmp;
> >> +
> >> +     /* Return early if the path does not exist since it is OK to not
> >> +      * checkout submodules.
> >> +      */
> >> +     if (stat(path, &st) && errno == ENOENT)
> >> +             return 1;
> >> +
> >> +     tmp = read_gitfile_gently(path);
> >
> > This will leak memory, no?
> 
> I don't think so: read_gitfile_gently() returns a value obtained by
> calling make_absolute_path() which returns a static buffer. Also, the
> path argument to include_repository() is obtained by calling mkpath()
> which returns another static buffer so I don't see any malloc()'s
> which should be free()'d. Is my code-reading flawed?

No, your code reading is good.  And you spared me having to read the code 
myself ;-)  Now, maybe a code comment is in order, to spare others, too?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Markus Heidelberg @ 2009-01-25 14:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, René Scharfe, git
In-Reply-To: <7vk58ko8k7.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, 24.01.2009:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > So with the following diff it works:
> >
> > -	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
> > +	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")
> > +				       || !strcmp(var, "color.ui")) {
> 
> Why should format-patch need to even worry about protecting itself from
> "color.ui" to begin with?

That's the reason, why color handling needs another rework than my
patch, which only was originated from the color.ui git_use_color_default
workarounds. Call it shortsighted, if you want.

> If your patch is making color handling saner, I would expect that
> format-patch can *lose* the existing "ignore diff.color or color.diff"
> workaround as a result of that.  If you need to add even *more* workaround
> code like that, there's something wrong, don't you think?

That's the reason, why it doesn't make sense to continue work on my
patch base.

> > format-patch is perhaps the only place where the commit has broken
> > things, because I didn't find other places,...
> 
> You did not find the breakage in format-patch either to begin with; so
> your not finding does not give us much confidence that there is no other
> breakage, does it?

Of course not.

> Grumble...

Why grumble? That was just a personal suspicion. I didn't say: "I think
this is the only breakage place, please reapply".

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-25 14:17 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <glhqdi$tec$1@ger.gmane.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1040 bytes --]

Hi,

[please do not forget to Cc: me; today is a slow day, so I did not miss 
 your mail, but that is definitely not true on other days.]

On Sun, 25 Jan 2009, Jakub Narebski wrote:

> Johannes Schindelin wrote:
> 
> >> Hmm.  You're right, that is not really intuitive.  How about
> >> 
> >>       merge (B) A # Merge...
> >> 
> >> instead?
> > 
> > Or even better:
> > 
> >         merge B parent A' # Merge...
> 
> merge B with A' # Merge... 

No, that does not catch the meaning.

B is the _original_ merge commit.  So it actually knows what parents it 
has, but we want to give the user the freedom to change those parents.

The first parent is easy: this will be HEAD at that stage.

The other parents will be relatively easy: just replace A' by something 
else.

_However_ now that the merge commit B will be _redone_, we _still_ want to 
be able to refer to it later in the rebase script.  Therefore, rebase has 
to know that we _redid_ B at this stage.

Another idea:

	merge B Merge bla/blub
	parent A' bla/blub

Hmm?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: Thomas Rast @ 2009-01-25 14:21 UTC (permalink / raw)
  To: Vitaly "_Vi" Shukela; +Cc: git, normalperson
In-Reply-To: <1232864842-8841-1-git-send-email-public_vi@tut.by>

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

Vitaly "_Vi" Shukela wrote:
> 
> Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>

This would be a good place to explain why this is useful, and (if
applicable) why you chose to implement it the way you did.

> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -96,6 +96,10 @@ COMMANDS
>  	Store Git commit times in the local timezone instead of UTC.  This
>  	makes 'git-log' (even without --date=local) show the same times
>  	that `svn log` would in the local timezone.
> +--ignore-paths=<regex>;;
> +	This allows one to specify regular expression that will
> +	cause skipping of all matching paths from checkout from SVN.
> +	Example: --ignore-paths='^doc'
>  
>  This doesn't interfere with interoperating with the Subversion
>  repository you cloned from, but if you wish for your local Git

You put the --ignore-paths explanation in the middle of the
--localtime documentation (the last paragraph quoted still talks about
--localtime).

> @@ -3245,6 +3246,15 @@ use warnings;
>  use Carp qw/croak/;
>  use File::Temp qw/tempfile/;
>  use IO::File qw//;
> +use vars qw/ $ignoreRegex/;
> +
> +# 0 -- don't ignore, 1 -- ignore
> +sub isPathIgnored($) {
> +    return 0 unless defined($ignoreRegex);
> +    my $path = shift;
> +    return 1 if $path =~ m!^$ignoreRegex!o;
> +    return 0;
> +}

This is the first function in git-svn.perl using camelCase.  Consider
sticking to the current style and spelling it is_path_ignored().

> @@ -3372,11 +3384,14 @@ sub add_file {
>  	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
>  	my $mode;
>  
> +	goto out if isPathIgnored($path);
> +
>  	if (!in_dot_git($path)) {
>  		my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
>  		delete $self->{empty}->{$dir};
>  		$mode = '100644';
>  	}
> +out:
>  	{ path => $path, mode_a => $mode, mode_b => $mode,
>  	  pool => SVN::Pool->new, action => 'A' };
>  }

You broke the symmetry here, while all other hunks just add an
equivalent check to the existing in_dot_git().

However, the latter makes me wonder if it would be cleaner to move the
in_dot_git() test to isPathIgnored (er, is_path_ignored) too?


FWIW, I like the feature; it seems a good way to exclude subtrees with
large blobs, and I know several SVN repos that have such a directory.

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

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

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Sverre Rabbelier @ 2009-01-25 15:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jakub Narebski, git
In-Reply-To: <alpine.DEB.1.00.0901251509550.14855@racer>

Heya,

On Sun, Jan 25, 2009 at 15:17, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> >>       merge (B) A # Merge...
>> >         merge B parent A' # Merge...
>> merge B with A' # Merge...
>        merge B Merge bla/blub
>        parent A' bla/blub

Oh goody, more painting! I was wondering when the next pick-a-word
contest would be!

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-25 15:24 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Jakub Narebski, git
In-Reply-To: <bd6139dc0901250707m5e1898cdu530a0d7566ca2da5@mail.gmail.com>

Hi,

On Sun, 25 Jan 2009, Sverre Rabbelier wrote:

> On Sun, Jan 25, 2009 at 15:17, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >> >>       merge (B) A # Merge...
> >> >         merge B parent A' # Merge...
> >> merge B with A' # Merge...
> >        merge B Merge bla/blub
> >        parent A' bla/blub
> 
> Oh goody, more painting! I was wondering when the next pick-a-word
> contest would be!

In this case, it is not about painting.  I will gladly ignore all those 
who think they must submit a new word for this action.

The thing is: I want the command to be intuitive, so I need a syntax where 
even the most idiotic dullard will understand what are the parents, and 
why we need the original merge's commit name, too.

So maybe I answered my question myself:

	merge parents $sha1 [$sha1...] original $sha1 $msg

Ciao,
Dscho

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Jakub Narebski @ 2009-01-25 16:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Thomas Rast
In-Reply-To: <alpine.DEB.1.00.0901251509550.14855@racer>

On Sun, 25 Jan 2009, Johannes Schindelin wrote:
> Hi,
> 
> [please do not forget to Cc: me; today is a slow day, so I did not miss 
>  your mail, but that is definitely not true on other days.]

This was spur of the moment idea, one that I wouldn't mind if you
would miss it.

But now that you have something interesting to say, I'll re-added
CC list for this thread.

> On Sun, 25 Jan 2009, Jakub Narebski wrote:
>> Johannes Schindelin wrote:
>> 
>>>> Hmm.  You're right, that is not really intuitive.  How about
>>>> 
>>>>       merge (B) A # Merge...
>>>> 
>>>> instead?
>>> 
>>> Or even better:
>>> 
>>>         merge B parent A' # Merge...
>> 
>> merge B with A' # Merge... 
> 
> No, that does not catch the meaning.

Errr... I didn't mean for 'with' to mean 'into'.
 
> B is the _original_ merge commit.  So it actually knows what parents it 
> has, but we want to give the user the freedom to change those parents.
> 
> The first parent is easy: this will be HEAD at that stage.
> 
> The other parents will be relatively easy: just replace A' by something 
> else.
> 
> _However_ now that the merge commit B will be _redone_, we _still_ want to 
> be able to refer to it later in the rebase script.  Therefore, rebase has 
> to know that we _redid_ B at this stage.
> 
> Another idea:
> 
> 	merge B Merge bla/blub
> 	parent A' bla/blub

It would be good idea... even better if 'p' shortcut was not taken
by 'pick'...

This is similar to your earlier idea:

        merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
        #   \ 9383af1 Revert previous two commits


Or perhaps:

	merge A' D' into B Merge bla/blub

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: public_vi @ 2009-01-25 16:29 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, normalperson
In-Reply-To: <200901251521.15591.trast@student.ethz.ch>

It is useful for me, but I think I can be useful for others too.
May be it's better to make in inclusive, i.e. "--only-matching=<regex>"?
> This would be a good place to explain why this is useful, and (if
> applicable) why you chose to implement it the way you did.
>   

OK, fixing them.
> You put the --ignore-paths explanation in the middle of the
> --localtime documentation (the last paragraph quoted still talks about
> --localtime).
>   
> This is the first function in git-svn.perl using camelCase.  Consider
> sticking to the current style and spelling it is_path_ignored().
>
> You broke the symmetry here, while all other hunks just add an
> equivalent check to the existing in_dot_git().
>
> However, the latter makes me wonder if it would be cleaner to move the
> in_dot_git() test to isPathIgnored (er, is_path_ignored) too?
>   

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Björn Steinbrink @ 2009-01-25 17:18 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Johannes Schindelin, git, Thomas Rast
In-Reply-To: <200901251722.53392.jnareb@gmail.com>

On 2009.01.25 17:22:52 +0100, Jakub Narebski wrote:
> Or perhaps:
> 
> 	merge A' D' into B Merge bla/blub

That would be confusing. I'd read it as:

git checkout B
git merge A' D'

But the point of B is just to tell rebase that the original merge commit
B is replaced by this new merge commit, so that B' works later.

Björn

^ permalink raw reply

* Translations in Git release?
From: Dill @ 2009-01-25 17:41 UTC (permalink / raw)
  To: git

Is there a plan to include translations of the Documentation within
Git or should they exist outside of the project?

^ permalink raw reply

* [PATCH v1 0/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 17:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git

The next three patches introduce a way to specify diff options
git always obeys. Then use the new feature to
enhance git-gui with white space ignore settings.  The fastest
way to see this patch in action is: apply all three patches,
fire up git-gui, modify a file, then right-click on the diff
panel and look for the new "White Space" sub-menu.

Future work: Extend the gitattributes mechanism so it supports
all [diff] config variables, including e.g. diff.mnemonicprefix
and diff.primer.

Keith Cascio (3):
 Introduce config variable "diff.primer"
 Test functionality of new config variable "diff.primer"
 git-gui hooks for new config variable "diff.primer"

 Documentation/config.txt       |   14 +++++
 Documentation/diff-options.txt |   13 +++++
 Makefile                       |    2 +
 builtin-log.c                  |    1 +
 diff.c                         |   83 ++++++++++++++++++++++++++----
 diff.h                         |   15 ++++-
 git-gui/git-gui.sh             |   51 ++++++++++++++++++
 git-gui/lib/diff.tcl           |    8 ++-
 git-gui/lib/option.tcl         |   57 +++++++++++++++++++--
 gitk-git/gitk                  |   16 +++---
 t/t4033-diff-primer.sh         |  111 ++++++++++++++++++++++++++++++++++++++++
 11 files changed, 343 insertions(+), 28 deletions(-)
 create mode 100755 t/t4033-diff-primer.sh

^ permalink raw reply

* [PATCH v1 2/3] Test functionality of new config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 17:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-2-git-send-email-keith@cs.ucla.edu>

Test functionality of new config variable "diff.primer"

Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
---
 t/t4033-diff-primer.sh |  111 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 111 insertions(+), 0 deletions(-)
 create mode 100755 t/t4033-diff-primer.sh

diff --git a/t/t4033-diff-primer.sh b/t/t4033-diff-primer.sh
new file mode 100755
index 0000000..116d6ad
--- /dev/null
+++ b/t/t4033-diff-primer.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Keith G. Cascio
+#
+# based on t4015-diff-whitespace.sh by Johannes E. Schindelin
+#
+
+test_description='Ensure diff engine honors config variable "diff.primer".
+
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+tr 'Q' '\015' << EOF > x
+whitespace at beginning
+whitespace change
+whitespace in the middle
+whitespace at end
+unchanged line
+CR at endQ
+EOF
+
+git add x
+git commit -m '1.0' >/dev/null 2>&1
+
+tr '_' ' ' << EOF > x
+	whitespace at beginning
+whitespace 	 change
+white space in the middle
+whitespace at end__
+unchanged line
+CR at end
+EOF
+
+test_expect_success 'ensure diff.primer begins with empty value' '
+[ -z $(git config --get diff.primer) ]
+'
+
+tr 'Q_' '\015 ' << EOF > expect
+diff --git a/x b/x
+index d99af23..8b32fb5 100644
+--- a/x
++++ b/x
+@@ -1,6 +1,6 @@
+-whitespace at beginning
+-whitespace change
+-whitespace in the middle
+-whitespace at end
++	whitespace at beginning
++whitespace 	 change
++white space in the middle
++whitespace at end__
+ unchanged line
+-CR at endQ
++CR at end
+EOF
+git diff > out
+test_expect_success 'test diff with empty value of diff.primer' 'test_cmp expect out'
+
+git config diff.primer '-w'
+
+test_expect_success 'ensure diff.primer value set' '
+[ $(git config --get diff.primer) = "-w" ]
+'
+
+git diff --no-primer > out
+test_expect_success 'test git diff --no-primer' 'test_cmp expect out'
+
+cat << EOF > expect
+diff --git a/x b/x
+index d99af23..8b32fb5 100644
+EOF
+git diff > out
+test_expect_success 'test with diff.primer = -w' 'test_cmp expect out'
+
+git add x
+git commit -m 'whitespace changes' >/dev/null 2>&1
+
+git config diff.primer '-w --color'
+
+tr 'Q_' '\015 ' << EOF > expect
+Subject: [PATCH] whitespace changes
+
+---
+ x |   10 +++++-----
+ 1 files changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/x b/x
+index d99af23..8b32fb5 100644
+--- a/x
++++ b/x
+@@ -1,6 +1,6 @@
+-whitespace at beginning
+-whitespace change
+-whitespace in the middle
+-whitespace at end
++	whitespace at beginning
++whitespace 	 change
++white space in the middle
++whitespace at end__
+ unchanged line
+-CR at endQ
++CR at end
+--_
+EOF
+
+git format-patch --stdout HEAD^..HEAD 2>&1 | sed -re '1,3d;$d' | sed -re '$d' > out
+test_expect_success 'ensure git format-patch not affected by diff.primer' 'test_cmp expect out'
+
+test_done
+
-- 
1.6.1

^ permalink raw reply related

* [PATCH v1 3/3] git-gui hooks for new config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 17:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-3-git-send-email-keith@cs.ucla.edu>

git-gui hooks for new config variable "diff.primer".
Add three checkboxes to both sides of
options panel (local/global).
Add a sub-menu named "White Space" to
diff-panel right-click context menu, with
three checkboxes.

Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
---
 git-gui/git-gui.sh     |   51 ++++++++++++++++++++++++++++++++++++++++++
 git-gui/lib/option.tcl |   57 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e018e07..5d93351 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -3075,10 +3075,43 @@ $ui_diff tag conf d>>>>>>> \
 
 $ui_diff tag raise sel
 
+proc mirror_diff_state {} {
+	global  diff__ignore_space_at_eol diff__ignore_space_change diff__ignore_all_space
+
+	set key  "diff.primer"
+	set ddo [git config --get $key]
+	set diff__ignore_space_at_eol [expr {[string match "*--ignore-space-at-eol*" $ddo] ? "true" : "false"}]
+	set diff__ignore_space_change [expr {[string match "*--ignore-space-change*" $ddo] ? "true" : "false"}]
+	set diff__ignore_all_space    [expr {[string match "*--ignore-all-space*"    $ddo] ? "true" : "false"}]
+}
+
+proc adjust_command_line { flag value str } {
+	if {$value eq "true"} {
+	  if { ! [string match "*$flag*" $str ] } {
+	    set              str [concat $str $flag] }
+	} else { regsub       -- $flag   $str "" str }
+	return                           $str
+}
+
+proc record_diff_state {} {
+	global  diff__ignore_space_at_eol diff__ignore_space_change diff__ignore_all_space
+
+	set key  "diff.primer"
+	set ddo [git config --get $key]
+	set ddo [adjust_command_line --ignore-space-at-eol $diff__ignore_space_at_eol $ddo]
+	set ddo [adjust_command_line --ignore-space-change $diff__ignore_space_change $ddo]
+	set ddo [adjust_command_line --ignore-all-space    $diff__ignore_all_space    $ddo]
+
+	git config $key $ddo
+	reshow_diff
+}
+
 # -- Diff Body Context Menu
 #
 
 proc create_common_diff_popup {ctxm} {
+	global  diff__ignore_space_at_eol diff__ignore_space_change diff__ignore_all_space
+
 	$ctxm add command \
 		-label [mc "Show Less Context"] \
 		-command show_less_context
@@ -3087,6 +3120,24 @@ proc create_common_diff_popup {ctxm} {
 		-label [mc "Show More Context"] \
 		-command show_more_context
 	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	mirror_diff_state
+	set whitespacemenu $ctxm.ws
+	menu $whitespacemenu -postcommand mirror_diff_state
+	$ctxm add cascade \
+		-label [mc "White Space"] \
+		-menu $whitespacemenu
+	$whitespacemenu add checkbutton \
+		-label [mc "--ignore-space-at-eol"] \
+		-variable diff__ignore_space_at_eol -onvalue "true" -offvalue "false" \
+		-command record_diff_state
+	$whitespacemenu add checkbutton \
+		-label [mc "--ignore-space-change"] \
+		-variable diff__ignore_space_change -onvalue "true" -offvalue "false" \
+		-command record_diff_state
+	$whitespacemenu add checkbutton \
+		-label [mc "--ignore-all-space"   ] \
+		-variable diff__ignore_all_space    -onvalue "true" -offvalue "false" \
+		-command record_diff_state
 	$ctxm add separator
 	$ctxm add command \
 		-label [mc Refresh] \
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index 1d55b49..fbdf4e8 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -28,6 +28,7 @@ proc save_config {} {
 	global repo_config global_config system_config
 	global repo_config_new global_config_new
 	global ui_comm_spell
+	global ddo diff_primer_global diff_primer_repo pseudovariables
 
 	foreach option $font_descs {
 		set name [lindex $option 0]
@@ -46,17 +47,40 @@ proc save_config {} {
 		unset global_config_new(gui.$font^^size)
 	}
 
+	foreach name [get_diff_primer] {
+		set diff_option [string range $name 8 [string length $name]]
+		set ifound      [lsearch     $diff_primer_global $diff_option]
+		if {$global_config_new($name) eq "true"} {
+		  if {$ifound <  0} { lappend diff_primer_global $diff_option }
+		} else {
+		  if {$ifound >= 0} { set     diff_primer_global [lreplace $diff_primer_global $ifound $ifound]}
+		}
+		set ifound      [lsearch     $diff_primer_repo   $diff_option]
+		if {  $repo_config_new($name) eq "true"} {
+		  if {$ifound <  0} { lappend diff_primer_repo   $diff_option }
+		} else {
+		  if {$ifound >= 0} { set     diff_primer_repo   [lreplace $diff_primer_repo   $ifound $ifound]}
+		}
+	}
+	array unset default_config gui.diff--ignore-*
+	set    default_config($ddo) ""
+	set global_config_new($ddo) [join    $diff_primer_global]
+	set   repo_config_new($ddo) [join    $diff_primer_repo  ]
+
 	foreach name [array names default_config] {
 		set value $global_config_new($name)
-		if {$value ne $global_config($name)} {
-			if {$value eq $system_config($name)} {
+		set value_global [expr {[info exists global_config($name)] ? $global_config($name) : ""}]
+		set value_system [expr {[info exists system_config($name)] ? $system_config($name) : ""}]
+		set value_repo   [expr {[info exists   repo_config($name)] ?   $repo_config($name) : ""}]
+		if {$value ne $value_global} {
+			if {$value eq $value_system} {
 				catch {git config --global --unset $name}
 			} else {
 				regsub -all "\[{}\]" $value {"} value
 				git config --global $name $value
 			}
 			set global_config($name) $value
-			if {$value eq $repo_config($name)} {
+			if {$value eq $value_repo} {
 				catch {git config --unset $name}
 				set repo_config($name) $value
 			}
@@ -65,8 +89,10 @@ proc save_config {} {
 
 	foreach name [array names default_config] {
 		set value $repo_config_new($name)
-		if {$value ne $repo_config($name)} {
-			if {$value eq $global_config($name)} {
+		set value_global [expr {[info exists global_config($name)] ? $global_config($name) : ""}]
+		set value_repo   [expr {[info exists   repo_config($name)] ?   $repo_config($name) : ""}]
+		if {$value ne $value_repo} {
+			if {$value eq $value_global} {
 				catch {git config --unset $name}
 			} else {
 				regsub -all "\[{}\]" $value {"} value
@@ -88,10 +114,23 @@ proc save_config {} {
 	}
 }
 
+proc get_diff_primer {} {
+	global repo_config global_config
+	global ddo diff_primer_global diff_primer_repo pseudovariables
+
+	set ddo "diff.primer"
+	set diff_primer_global [expr {[info exists global_config($ddo)] ? [split $global_config($ddo)] : [list]}]
+	set diff_primer_repo   [expr {[info exists   repo_config($ddo)] ? [split   $repo_config($ddo)] : [list]}]
+	set pseudovariables [list "gui.diff--ignore-space-at-eol" "gui.diff--ignore-space-change" "gui.diff--ignore-all-space"]
+
+	return $pseudovariables
+}
+
 proc do_options {} {
 	global repo_config global_config font_descs
 	global repo_config_new global_config_new
 	global ui_comm_spell
+	global ddo diff_primer_global diff_primer_repo pseudovariables
 
 	array unset repo_config_new
 	array unset global_config_new
@@ -108,6 +147,11 @@ proc do_options {} {
 	foreach name [array names global_config] {
 		set global_config_new($name) $global_config($name)
 	}
+	foreach name [get_diff_primer] {
+		set diff_option [string range $name 8 [string length $name]]
+		set global_config_new($name) [expr {[lsearch $diff_primer_global $diff_option] < 0 ? "false" : "true"}]
+		set   repo_config_new($name) [expr {[lsearch $diff_primer_repo   $diff_option] < 0 ? "false" : "true"}]
+	}
 
 	set w .options_editor
 	toplevel $w
@@ -150,6 +194,9 @@ proc do_options {} {
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..300 gui.blamehistoryctx {mc "Blame History Context Radius (days)"}}
 		{i-1..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
+		{b gui.diff--ignore-space-at-eol {mc "Diff Ignore Trailing White Space"            }}
+		{b gui.diff--ignore-space-change {mc "Diff Ignore Changes In Amount Of White Space"}}
+		{b gui.diff--ignore-all-space    {mc "Diff Ignore All White Space"                 }}
 		{i-0..99 gui.commitmsgwidth {mc "Commit Message Text Width"}}
 		{t gui.newbranchtemplate {mc "New Branch Name Template"}}
 		{c gui.encoding {mc "Default File Contents Encoding"}}
-- 
1.6.1

^ permalink raw reply related

* [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 17:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-1-git-send-email-keith@cs.ucla.edu>

Introduce config variable "diff.primer".
Allows user to specify arbitrary options
to pass to diff on every invocation,
including internal invocations from other
programs, e.g. git-gui.
Introduce diff command-line options:
--no-primer, --machine-friendly
Protect git-format-patch, git-apply,
git-am, git-rebase, git-gui and gitk
from inapplicable options.

Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
---
 Documentation/config.txt       |   14 +++++++
 Documentation/diff-options.txt |   13 ++++++
 Makefile                       |    2 +
 builtin-log.c                  |    1 +
 diff.c                         |   83 +++++++++++++++++++++++++++++++++++-----
 diff.h                         |   15 ++++++-
 git-gui/lib/diff.tcl           |    8 +++-
 gitk-git/gitk                  |   16 ++++----
 8 files changed, 129 insertions(+), 23 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 290cb48..dd00f98 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -601,6 +601,20 @@ diff.autorefreshindex::
 	affects only 'git-diff' Porcelain, and not lower level
 	'diff' commands, such as 'git-diff-files'.
 
+diff.primer::
+	Whitespace-separated list of options to pass to 'git-diff'
+	on every invocation, including internal invocations from
+	linkgit:git-gui[1] and linkgit:gitk[1],
+	e.g. `"--color --ignore-space-at-eol --exit-code"`.
+	See linkgit:git-diff[1]. You can override these at run time with the
+	diff option --no-primer.  Supports a subset of
+	'git-diff'\'s many options, at least:
+	`-b --binary --color --color-words --cumulative --dirstat-by-file
+--exit-code --ext-diff --find-copies-harder --follow --full-index
+--ignore-all-space --ignore-space-at-eol --ignore-space-change
+--ignore-submodules --no-color --no-ext-diff --no-textconv -q --quiet -R -r
+--relative -t --text --textconv -w`
+
 diff.external::
 	If this config variable is set, diff generation is not
 	performed using the internal diff machinery, but using the
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 1f8ce97..4d12359 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -240,5 +240,18 @@ endif::git-format-patch[]
 --no-prefix::
 	Do not show any source or destination prefix.
 
+--no-primer::
+	Ignore default options specified in '.git/config', i.e.
+	those that were set using a command like
+	`git config diff.primer "--color --ignore-space-at-eol --exit-code"`
+
+--machine-friendly::
+	Declaratively override and turn off all diff options that alter patch
+	output in a way not suitable for input to a program that expects
+	a canonical patch.  For example, `--color`, and the whitespace ignore
+	options `-w`, `-b` and `--ignore-space-at-eol`.  Important when
+	'git-format-patch' generates output for 'git-apply' or 'git-am', for
+	example in the context of 'git-rebase'.
+
 For more detailed explanation on these common options, see also
 linkgit:gitdiffcore[7].
diff --git a/Makefile b/Makefile
index b4d9cb4..195f984 100644
--- a/Makefile
+++ b/Makefile
@@ -1279,6 +1279,8 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+diff.h: xdiff/xdiff.h
+
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
 builtin-revert.o wt-status.o: wt-status.h
diff --git a/builtin-log.c b/builtin-log.c
index 2ae39af..b385e35 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -784,6 +784,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
 	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	DIFF_OPT_SET(&rev.diffopt, MACHINE_FRIENDLY);
 
 	rev.subject_prefix = fmt_patch_subject_prefix;
 
diff --git a/diff.c b/diff.c
index 82cff97..a8c103f 100644
--- a/diff.c
+++ b/diff.c
@@ -24,6 +24,8 @@ static int diff_rename_limit_default = 200;
 static int diff_suppress_blank_empty;
 int diff_use_color_default = -1;
 static const char *external_diff_cmd_cfg;
+static const char     *diff_primer;
+static struct diff_options *primer;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 
@@ -102,6 +104,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		diff_rename_limit_default = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.primer"))
+		return git_config_string(& diff_primer, var, value);
 
 	switch (userdiff_config(var, value)) {
 		case 0: break;
@@ -2215,6 +2219,46 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
 }
 
+static const char blank[] = " \t\r\n";
+
+void set_diff_primer(struct diff_options *options)
+{
+	char  *str1, *token, *saveptr;
+	int    len;
+
+	if((DIFF_OPT_TST(options, SUPPRESS_PRIMER)) ||
+	   (!              diff_primer            ) ||
+	   ((len = (strlen(diff_primer)+1)) < 3   )){ return; }
+
+	token = str1 = strncpy( (char*) malloc(len), diff_primer, len );
+	if( (           saveptr = strpbrk( token += strspn( token, blank ), blank )) ){ *(saveptr++) = '\0'; }
+	while( token ){
+	  if( *token == '-'      ){
+	    diff_opt_parse( options, (const char **) &token, -1 );
+	  }
+	  if( (token  = saveptr) ){
+	    if( (       saveptr = strpbrk( token += strspn( token, blank ), blank )) ){ *(saveptr++) = '\0'; }
+	  }
+	}
+
+	free( str1 );
+}
+
+struct diff_options* flatten_diff_options( struct diff_options *master, struct diff_options *slave )
+{
+	unsigned   x0 = master->flags   , x1 = master->mask    , x2 = slave->flags   , x3 = slave->mask;
+	long       w  = master->xdl_opts, x  = master->xdl_mask, y  = slave->xdl_opts, z  = slave->xdl_mask;
+
+	//minimized by Quine-McClusk
+	master->flags    = (~x1&x2&x3)|(x0&~x3)|(x0&x1);
+	master->mask     = x1 | x3;
+
+	master->xdl_opts = (~x &y &z )|(w &~z )|(w &x );
+	master->xdl_mask = x  | z ;
+
+	return master;
+}
+
 void diff_setup(struct diff_options *options)
 {
 	memset(options, 0, sizeof(*options));
@@ -2225,15 +2269,16 @@ void diff_setup(struct diff_options *options)
 	options->break_opt = -1;
 	options->rename_limit = -1;
 	options->dirstat_percent = 3;
-	DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
+	if( DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE))
+	    DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
 	options->context = 3;
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
 	if (diff_use_color_default > 0)
-		DIFF_OPT_SET(options, COLOR_DIFF);
-	else
-		DIFF_OPT_CLR(options, COLOR_DIFF);
+		 DIFF_OPT_SET(options, COLOR_DIFF);
+	else if( DIFF_OPT_TST(options, COLOR_DIFF))
+		 DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
 
 	if (!diff_mnemonic_prefix) {
@@ -2322,6 +2367,18 @@ int diff_setup_done(struct diff_options *options)
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	}
 
+	if( ! DIFF_OPT_TST(      options, SUPPRESS_PRIMER ) ){
+	  if( !                           primer ){
+	    diff_setup(                   primer = (struct diff_options *) malloc( sizeof(struct diff_options) ) );
+	    set_diff_primer(              primer );
+	  }
+	  flatten_diff_options(  options, primer );
+	}
+
+	if(   DIFF_OPT_TST(      options, MACHINE_FRIENDLY ) ){
+	  DIFF_MACHINE_FRIENDLY( options );
+	}
+
 	return 0;
 }
 
@@ -2469,13 +2526,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 
 	/* xdiff options */
 	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
+		DIFF_XDL_SET(options, IGNORE_WHITESPACE);
 	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
+		DIFF_XDL_SET(options, IGNORE_WHITESPACE_CHANGE);
 	else if (!strcmp(arg, "--ignore-space-at-eol"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
+		DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
 	else if (!strcmp(arg, "--patience"))
-		options->xdl_opts |= XDF_PATIENCE_DIFF;
+		DIFF_XDL_SET(options, PATIENCE_DIFF);
 
 	/* flags options */
 	else if (!strcmp(arg, "--binary")) {
@@ -2496,8 +2553,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--no-color"))
 		DIFF_OPT_CLR(options, COLOR_DIFF);
-	else if (!strcmp(arg, "--color-words"))
-		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
+	else if (!strcmp(arg, "--color-words")){
+		DIFF_OPT_SET(options, COLOR_DIFF);
+		DIFF_OPT_SET(options, COLOR_DIFF_WORDS);
+	}
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
@@ -2512,6 +2571,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
 	else if (!strcmp(arg, "--ignore-submodules"))
 		DIFF_OPT_SET(options, IGNORE_SUBMODULES);
+	else if (!strcmp(arg, "--no-primer"))
+		DIFF_OPT_SET(options, SUPPRESS_PRIMER);
+	else if (!strcmp(arg, "--machine-friendly"))
+		DIFF_OPT_SET(options, MACHINE_FRIENDLY);
 
 	/* misc options */
 	else if (!strcmp(arg, "-z"))
diff --git a/diff.h b/diff.h
index 4d5a327..e98c23a 100644
--- a/diff.h
+++ b/diff.h
@@ -5,6 +5,7 @@
 #define DIFF_H
 
 #include "tree-walk.h"
+#include "xdiff/xdiff.h"
 
 struct rev_info;
 struct diff_options;
@@ -66,9 +67,15 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
 #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)
-#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
-#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
-#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
+#define DIFF_OPT_SUPPRESS_PRIMER     (1 << 22)
+#define DIFF_OPT_MACHINE_FRIENDLY    (1 << 23)
+#define DIFF_OPT_TST(opts, flag)    ((opts)->flags &   DIFF_OPT_##flag)
+#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |=  DIFF_OPT_##flag), ((opts)->mask |= DIFF_OPT_##flag)
+#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag), ((opts)->mask |= DIFF_OPT_##flag)
+#define DIFF_XDL_TST(opts, flag)    ((opts)->xdl_opts &   XDF_##flag)
+#define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |=  XDF_##flag), ((opts)->xdl_mask |= XDF_##flag)
+#define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag), ((opts)->xdl_mask |= XDF_##flag)
+#define DIFF_MACHINE_FRIENDLY(opts) ((opts)->flags &= ~(DIFF_OPT_COLOR_DIFF)), ((opts)->xdl_opts &= ~(XDF_WHITESPACE_FLAGS))
 
 struct diff_options {
 	const char *filter;
@@ -77,6 +84,7 @@ struct diff_options {
 	const char *single_follow;
 	const char *a_prefix, *b_prefix;
 	unsigned flags;
+	unsigned  mask;
 	int context;
 	int interhunkcontext;
 	int break_opt;
@@ -95,6 +103,7 @@ struct diff_options {
 	int prefix_length;
 	const char *stat_sep;
 	long xdl_opts;
+	long xdl_mask;
 
 	int stat_width;
 	int stat_name_width;
diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index bbbf15c..94faf95 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -276,6 +276,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	lappend cmd -p
+	lappend cmd --exit-code
 	lappend cmd --no-color
 	if {$repo_config(gui.diffcontext) >= 1} {
 		lappend cmd "-U$repo_config(gui.diffcontext)"
@@ -310,6 +311,7 @@ proc read_diff {fd cont_info} {
 	global ui_diff diff_active
 	global is_3way_diff is_conflict_diff current_diff_header
 	global current_diff_queue
+	global errorCode
 
 	$ui_diff conf -state normal
 	while {[gets $fd line] >= 0} {
@@ -397,7 +399,9 @@ proc read_diff {fd cont_info} {
 	$ui_diff conf -state disabled
 
 	if {[eof $fd]} {
-		close $fd
+		fconfigure $fd -blocking 1
+		catch { close $fd } err
+		set diff_exit_status $errorCode
 
 		if {$current_diff_queue ne {}} {
 			advance_diff_queue $cont_info
@@ -413,7 +417,7 @@ proc read_diff {fd cont_info} {
 		}
 		ui_ready
 
-		if {[$ui_diff index end] eq {2.0}} {
+		if {$diff_exit_status eq "NONE"} {
 			handle_empty_diff
 		}
 		set callback [lindex $cont_info 1]
diff --git a/gitk-git/gitk b/gitk-git/gitk
index dc2a439..49e5cb7 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4259,7 +4259,7 @@ proc do_file_hl {serial} {
 	# must be "containing:", i.e. we're searching commit info
 	return
     }
-    set cmd [concat | git diff-tree -r -s --stdin $gdtargs]
+    set cmd [concat | git diff-tree --no-color -r -s --stdin $gdtargs]
     set filehighlight [open $cmd r+]
     fconfigure $filehighlight -blocking 0
     filerun $filehighlight readfhighlight
@@ -4753,7 +4753,7 @@ proc dodiffindex {} {
 
     if {!$showlocalchanges || !$isworktree} return
     incr lserial
-    set cmd "|git diff-index --cached HEAD"
+    set cmd "|git diff-index --no-color --cached HEAD"
     if {$vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
@@ -4782,7 +4782,7 @@ proc readdiffindex {fd serial inst} {
     }
 
     # now see if there are any local changes not checked in to the index
-    set cmd "|git diff-files"
+    set cmd "|git diff-files --no-color"
     if {$vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
@@ -7068,7 +7068,7 @@ proc diffcmd {ids flags} {
     if {$i >= 0} {
 	if {[llength $ids] > 1 && $j < 0} {
 	    # comparing working directory with some specific revision
-	    set cmd [concat | git diff-index $flags]
+	    set cmd [concat | git diff-index --no-color $flags]
 	    if {$i == 0} {
 		lappend cmd -R [lindex $ids 1]
 	    } else {
@@ -7076,13 +7076,13 @@ proc diffcmd {ids flags} {
 	    }
 	} else {
 	    # comparing working directory with index
-	    set cmd [concat | git diff-files $flags]
+	    set cmd [concat | git diff-files --no-color $flags]
 	    if {$j == 1} {
 		lappend cmd -R
 	    }
 	}
     } elseif {$j >= 0} {
-	set cmd [concat | git diff-index --cached $flags]
+	set cmd [concat | git diff-index --no-color --cached $flags]
 	if {[llength $ids] > 1} {
 	    # comparing index with specific revision
 	    if {$i == 0} {
@@ -7095,7 +7095,7 @@ proc diffcmd {ids flags} {
 	    lappend cmd HEAD
 	}
     } else {
-	set cmd [concat | git diff-tree -r $flags $ids]
+	set cmd [concat | git diff-tree --no-color -r $flags $ids]
     }
     return $cmd
 }
@@ -10657,7 +10657,7 @@ if {[catch {package require Tk 8.4} err]} {
 }
 
 # defaults...
-set wrcomcmd "git diff-tree --stdin -p --pretty"
+set wrcomcmd "git diff-tree --no-color --stdin -p --pretty"
 
 set gitencoding {}
 catch {
-- 
1.6.1

^ permalink raw reply related

* [PATCH] Add new testcase for git-merge-tree
From: Miklos Vajna @ 2009-01-25 18:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 t/t6034-merge-tree.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100755 t/t6034-merge-tree.sh

diff --git a/t/t6034-merge-tree.sh b/t/t6034-merge-tree.sh
new file mode 100755
index 0000000..42bd5e7
--- /dev/null
+++ b/t/t6034-merge-tree.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='git merge-tree
+
+Testing merging two trees without touching the index.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo c0 > c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 > c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	c1=$(git hash-object c1.c)
+	git reset --hard c0 &&
+	echo c2 > c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2
+	c2=$(git hash-object c2.c)
+'
+
+test_expect_success 'merge c1 with c2' '
+	cat <<EOF >expected &&
+added in local
+  our    100644 $c1 c1.c
+added in remote
+  their  100644 $c2 c2.c
+@@ -0,0 +1 @@
++c2
+EOF
+	git merge-tree c0 c1 c2 > actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.6.1

^ permalink raw reply related


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