Git development
 help / color / mirror / Atom feed
* Re: obstack fails to compile on OS X 10.7
From: David Aguilar @ 2011-08-29  3:18 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: Brian Gernhardt, Git List
In-Reply-To: <20110828200846.GA10754@fredrik-Q430-Q530>

On Sun, Aug 28, 2011 at 10:08:46PM +0200, Fredrik Kuivinen wrote:
> On Sat, Aug 27, 2011 at 03:14:43AM -0700, David Aguilar wrote:
> > On Sat, Aug 27, 2011 at 02:21:40AM -0400, Brian Gernhardt wrote:
> > > Some of the errors look like things I could track down, but some just confuse me.  If anyone else could take a look into this, it would be much appreciated.
> > > 
> > > ~~ Brian G.
> > > 
> > > gcc -o compat/obstack.o -c -MF compat/.depend/obstack.o.d -MMD -MP  -Wall -Wdeclaration-after-statement -Werror -Wno-deprecated-declarations -I. -DUSE_ST_TIMESPEC  -DSHA1_HEADER='"block-sha1/sha1.h"'  -DNO_MEMMEM  compat/obstack.c
> > > In file included from compat/obstack.c:30:
> > > compat/obstack.h:190: error: __block attribute can be specified on variables only
> > > compat/obstack.c:70: error: expected specifier-qualifier-list before ‘uintmax_t’
> > > compat/obstack.c:111:24: error: exitfail.h: No such file or directory
> > > cc1: warnings being treated as errors
> > > compat/obstack.c: In function ‘print_and_abort’:
> > > compat/obstack.c:436: warning: implicit declaration of function ‘gettext’
> > > compat/obstack.c:436: warning: incompatible implicit declaration of built-in function ‘gettext’
> > > compat/obstack.c:438: error: ‘exit_failure’ undeclared (first use in this function)
> > > compat/obstack.c:438: error: (Each undeclared identifier is reported only once
> > > compat/obstack.c:438: error: for each function it appears in.)
> > > compat/obstack.c:439: warning: ‘noreturn’ function does return
> > > make: *** [compat/obstack.o] Error 1
> > > 
> > > $ gcc --version
> > > i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)
> > 
> > I ran into the same thing.
> > 
> > This fixes it for me, but we might want to rearrange the
> > #includes a bit.  I think this needs more work.. including
> > compat/obstack.h from kwset.c seems wrong.
> > Should we just include obstack.h in git-compat-util instead?
> > 
> > I suspect that more exotic platforms may have problems
> > with obstack.h as well.  This probably needs some testing
> > on SunOS, AIX, IRIX, etc.
> 
> 
> How about doing something a bit simpler instead and changing obstack.c
> to not make use of exit.h and exitfail.h? Then we don't have to update
> Makefile for all platforms needing NEEDS_OBSTACK and NEEDS_EXITFAIL.

I like this much better.  Less code is better code.

> I don't understand why the ELIDE_CODE check is not sufficient. Care to
> explain?

I can't say I know.  The intention of the check is to avoid
pulling in that section of code when it is already built-in to
the C library.  Maybe that check doesn't quite mean the same
thing when the file is used alone outside of its original
context?

> -void obstack_free (struct obstack *__obstack, void *__block);
> +void obstack_free (struct obstack *obstack, void *block);

Tiny nit: I know it's just a declaration but would it be
advisable to drop the variable names altogether here?
Having a pointer and a structure with the same "obstack"
name could be confusing.

This looks good otherwise.  I was a bit iffy about my patch
when I had to bring in the extra headers.
Doing without them is much better.

I tried your patch on top of my recent
"Improve compiler header dependency check" and it worked fine.

So...
Tested-by: David Aguilar <davvid@gmail.com>
-- 
					David

^ permalink raw reply

* Re: [PATCH] t5540-http-test: shorten grep pattern
From: Junio C Hamano @ 2011-08-29  5:17 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List
In-Reply-To: <1314506520-17430-1-git-send-email-brian@gernhardtsoftware.com>

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

> On OS X, the grep pattern
>
>     "\"OP .*/objects/$x2/X38_X40 HTTP/[.0-9]*\" 20[0-9] "
>
> is far too long ($x38 and $x40 represent 38 and 40 copies of
> [0-9a-f]).  In order to still be able to match this, use the sed
> invocation to replace what we're looking for a token.
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
>  t/t5540-http-push.sh |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
> index a266ca5..5bf287d 100755
> --- a/t/t5540-http-push.sh
> +++ b/t/t5540-http-push.sh
> @@ -132,8 +132,9 @@ x38="$x5$x5$x5$x5$x5$x5$x5$x1$x1$x1"
>  x40="$x38$x2"
>  
>  test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
> -	sed -e "s/PUT /OP /" -e "s/MOVE /OP /" "$HTTPD_ROOT_PATH"/access.log |
> -	grep -e "\"OP .*/objects/$x2/${x38}_$x40 HTTP/[.0-9]*\" 20[0-9] "
> +	sed -e "s/PUT /OP /" -e "s/MOVE /OP /" -e "s/$x40/X40/" -e "s/$x38/X38/"\
> +		"$HTTPD_ROOT_PATH"/access.log |
> +	grep -e "\"OP .*/objects/$x2/X38_X40 HTTP/[.0-9]*\" 20[0-9] "
>  
>  '

Hmm...

Is it sensible to replace $x40 with X40 and $x38 with X38 on any line
anywhere for the purpose of this test? As the downstream test is only
interested in a line with HTTP request line that asks for a specific path
under objects/??/?{38}_?{40}, wouldn't it make more sense to replace
occurrences of only such line?

	sed -n \
            -e "s/PUT /OP " \
	    -e "s/MOVE /OP /" \
	    -e "s|/objects/$x2/$x38_$x40|WANTED_PATH_REQUEST|p" \
	    "$HTTPD_ROOT_PATH/access.log" |
	grep -e "\"OP .*WANTED_PATH_REQUEST HTTP/[.0-9]*\" 20[0-9] "

^ permalink raw reply

* Re: git-config: case insensitivity for subsections
From: milki @ 2011-08-29  5:42 UTC (permalink / raw)
  To: Alex Vandiver; +Cc: Jeff King, git
In-Reply-To: <1314579031.10094.19.camel@umgah.localdomain>

On 20:50 Sun 28 Aug     , Alex Vandiver wrote:
> For reference, https://github.com/bestpractical/config-gitlike/ is a
> complete parser for git config files written in perl, which passes git's
> config test suite (among other tests).  Which is not terribly
> surprising, since its parsing algorithm is strongly derived from
> config.c's.

Yes, I've been looking at it and forked a majority of it into python,
but I can't seem to replicate some of the expected quoting behaviour of
git-config, among other things (now off-topic). My implementation so
far can be seen at [0]. A user gave me a link [1] to his git-config
and I cannot correctly parse, for example, his alias.last.

-milki


[0] https://github.com/jelmer/dulwich/pull/31#issuecomment-1918904
[1] https://github.com/kergoth/homefiles/blob/master/.gitconfig#L67

^ permalink raw reply

* Re: [PATCH 2/2] Add a remote helper to interact with mediawiki (fetch & push)
From: Sverre Rabbelier @ 2011-08-29  5:42 UTC (permalink / raw)
  To: Junio C Hamano, Jonathan Nieder
  Cc: Matthieu Moy, git, Jeremie Nikaes, Arnaud Lacurie, Claire Fousse,
	David Amouyal, Matthieu Moy, Sylvain Boulmé
In-Reply-To: <7v4o14dppz.fsf@alter.siamese.dyndns.org>

Heya,

2011/8/26 Junio C Hamano <gitster@pobox.com>:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> In short, the changes since v3 are:
>>
>> * Adapt to newer Git, which seem to require a "done" command at the
>>   end of the fast-import stream. I don't understand why this is
>>   needed, since fast-import is called without the --done flag by
>>   remote-helpers, but if I don't do this, "git fetch" doesn't
>>   terminate and keeps waiting ...
>
> Hmmmm, is this a regression in fast-import? Can this be bisected if so?

We agreed that making this change is the best way to go forward, since
there's so few (read: none) remote-helpers yet, the fix is trivial,
and it solves a design mistake while the api is not yet set in stone.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCHv3 4/5] branch: introduce --list option
From: Junio C Hamano @ 2011-08-29  5:55 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <05a1a73e0b942dd94dd091c233be964fa19dfc6a.1314543252.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> +test_expect_success 'git branch --list shows local branches' '
> +	git branch --list >actual &&
> +	test_cmp expect actual
> +'
> +
> +cat >expect <<'EOF'
> +  branch-one
> +  branch-two
> +EOF
> +test_expect_success 'git branch --list pattern shows matching local branches' '
> +	git branch --list branch* >actual &&
> +	test_cmp expect actual
> +'

Does this one exclude the "* master" entry at this stage in the series?

^ permalink raw reply

* Re: [PATCH 2/2] Add a remote helper to interact with mediawiki (fetch & push)
From: Junio C Hamano @ 2011-08-29  6:05 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Jonathan Nieder, Matthieu Moy, git, Jeremie Nikaes,
	Arnaud Lacurie, Claire Fousse, David Amouyal, Matthieu Moy,
	Sylvain Boulmé
In-Reply-To: <CAGdFq_iYRkfnTbYAgmX1g4uOxWb_ZYxr+TNinVfWRV-zXrnzAg@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> 2011/8/26 Junio C Hamano <gitster@pobox.com>:
>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>>> In short, the changes since v3 are:
>>>
>>> * Adapt to newer Git, which seem to require a "done" command at the
>>>   end of the fast-import stream. I don't understand why this is
>>>   needed, since fast-import is called without the --done flag by
>>>   remote-helpers, but if I don't do this, "git fetch" doesn't
>>>   terminate and keeps waiting ...
>>
>> Hmmmm, is this a regression in fast-import? Can this be bisected if so?
>
> We agreed that making this change is the best way to go forward, since
> there's so few (read: none) remote-helpers yet, the fix is trivial,
> and it solves a design mistake while the api is not yet set in stone.

Does this exchange suggest that at least we would need an update to
documentation around "done", as Matthieu's "why is 'done' needed even
though I am not calling with --done?" sounds like a very fair question.

^ permalink raw reply

* Re: obstack fails to compile on OS X 10.7
From: Junio C Hamano @ 2011-08-29  6:07 UTC (permalink / raw)
  To: David Aguilar; +Cc: Fredrik Kuivinen, Brian Gernhardt, Git List
In-Reply-To: <20110829031806.GA58338@gmail.com>

David Aguilar <davvid@gmail.com> writes:

>> -void obstack_free (struct obstack *__obstack, void *__block);
>> +void obstack_free (struct obstack *obstack, void *block);
>
> Tiny nit: I know it's just a declaration but would it be advisable to
> drop the variable names altogether here?

I agree. In general it is easier to read if parameter names to a function
declaration is omitted, especially if it is clear from their types what
they mean [*1*]. And in this case, it is.

> Tested-by: David Aguilar <davvid@gmail.com>

Thanks.

[Footnote]

*1* It is Ok to spell them out in a case like this:

    int copy(char *dst, const char *src, size_t);

You do not have to, as "char *" vs "const char *" makes it clear which one
is which.

^ permalink raw reply

* Re: [PATCH v4 0/4] git-p4: Improve branch support
From: Junio C Hamano @ 2011-08-29  6:07 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git, Pete Wyckoff, Tor Arvid Lund
In-Reply-To: <1314568710-6472-1-git-send-email-vitor.hda@gmail.com>

Could you make this an incremental patch relative to what is already in
next?


	

^ permalink raw reply

* Re: [PATCHv3 4/5] branch: introduce --list option
From: Michael J Gruber @ 2011-08-29  6:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v8vqc92yp.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 29.08.2011 07:55:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> +test_expect_success 'git branch --list shows local branches' '
>> +	git branch --list >actual &&
>> +	test_cmp expect actual
>> +'
>> +
>> +cat >expect <<'EOF'
>> +  branch-one
>> +  branch-two
>> +EOF
>> +test_expect_success 'git branch --list pattern shows matching local branches' '
>> +	git branch --list branch* >actual &&
>> +	test_cmp expect actual
>> +'
> 
> Does this one exclude the "* master" entry at this stage in the series?

Sheesh, that rebasing mistake (flipping the order of 4 and 5) was
present in v2 already. Only recently I learned about rebase-i's "exec"
and have to make it a habit to use it for step-by-step series testing.

Sorry, I'll send out v3 a bit later when I see that there are no more
comments (and a full step-by-step series test has passed).

Michael

^ permalink raw reply

* Re: [PATCH 2/2] Add a remote helper to interact with mediawiki (fetch & push)
From: Sverre Rabbelier @ 2011-08-29  6:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Matthieu Moy, git, Jeremie Nikaes,
	Arnaud Lacurie, Claire Fousse, David Amouyal, Matthieu Moy,
	Sylvain Boulmé
In-Reply-To: <7v4o1092hn.fsf@alter.siamese.dyndns.org>

Heya,

On Mon, Aug 29, 2011 at 08:05, Junio C Hamano <gitster@pobox.com> wrote:
> Does this exchange suggest that at least we would need an update to
> documentation around "done", as Matthieu's "why is 'done' needed even
> though I am not calling with --done?" sounds like a very fair question.

No I think the documentation for fast-import is correct. If you pass
--use-done-feature or print 'feature done' in the stream the use of
'done' is required, otherwise it isn't. We did recently changed git to
pass '--use-done-feature' to the fast-import process though :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH v2] t5540-http-test: shorten grep pattern
From: Brian Gernhardt @ 2011-08-29  6:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vhb5094pm.fsf@alter.siamese.dyndns.org>

On OS X, the grep pattern

    "\"OP .*/objects/$x2/X38_X40 HTTP/[.0-9]*\" 20[0-9] "

is too long ($x38 and $x40 represent 38 and 40 copies of [0-9a-f]) for
grep to handle.  In order to still be able to match this, use the sed
invocation to replace what we're looking for with a token.

Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
 t/t5540-http-push.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

On Aug 29, 2011, at 1:17 AM, Junio C Hamano wrote:

> Is it sensible to replace $x40 with X40 and $x38 with X38 on any line
> anywhere for the purpose of this test? As the downstream test is only
> interested in a line with HTTP request line that asks for a specific path
> under objects/??/?{38}_?{40}, wouldn't it make more sense to replace
> occurrences of only such line?

Yes, of course.  The reason it ended up the way it did was that I first
tried replacing $x40 with SHA-1, but that was still too long.  Your way
makes more sense in the end, although the $x38_$x40 bit needs to be
${x38}_$x40 so the shell looks for the right variable.

diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index a266ca5..64767d8 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -132,8 +132,12 @@ x38="$x5$x5$x5$x5$x5$x5$x5$x1$x1$x1"
 x40="$x38$x2"
 
 test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
-	sed -e "s/PUT /OP /" -e "s/MOVE /OP /" "$HTTPD_ROOT_PATH"/access.log |
-	grep -e "\"OP .*/objects/$x2/${x38}_$x40 HTTP/[.0-9]*\" 20[0-9] "
+	sed \
+		-e "s/PUT /OP /" \
+		-e "s/MOVE /OP /" \
+	    -e "s|/objects/$x2/${x38}_$x40|WANTED_PATH_REQUEST|" \
+		"$HTTPD_ROOT_PATH"/access.log |
+	grep -e "\"OP .*WANTED_PATH_REQUEST HTTP/[.0-9]*\" 20[0-9] "
 
 '
 
-- 
1.7.7.rc0.308.g517a2

^ permalink raw reply related

* Re: [PATCHv3 4/5] branch: introduce --list option
From: Junio C Hamano @ 2011-08-29  6:51 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <4E5B3317.7000207@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Sheesh, that rebasing mistake (flipping the order of 4 and 5) was
> present in v2 already. Only recently I learned about rebase-i's "exec"
> and have to make it a habit to use it for step-by-step series testing.

That's Ok. Locally moved the offending part of the test to 5 when
applying.

Thanks.

^ permalink raw reply

* Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
From: Nguyen Thai Ngoc Duy @ 2011-08-29  7:41 UTC (permalink / raw)
  To: Marat Radchenko; +Cc: git
In-Reply-To: <loom.20110823T091132-107@post.gmane.org>

On Tue, Aug 23, 2011 at 2:25 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
> $ time git show branch:file | diff -u - file > /dev/null
>
> real    0m0.003s
> user    0m0.000s
> sys     0m0.000s
>
> $ time git diff branch -- file > /dev/null
>
> real    0m31.442s
> user    0m31.040s
> sys     0m0.380s
>
> What does git diff do so it takes that much time?

You said elsewhere in this thread this is private repo, so some more questions:

 - is "file" above at top repo, or is it actually very/deep/path/to/a/file?
 - how many entries in the tree that contain "file"?
 - how is "git ls-files | wc -l"?
 - how about "time git diff branch another-branch -- file >/dev/null"?
That'd remove unpack-trees code.
-- 
Duy

^ permalink raw reply

* [tig: PATCH] Decorate replaced commits
From: Michael J Gruber @ 2011-08-29  9:08 UTC (permalink / raw)
  To: git; +Cc: Jonas Fonseca

Decorate replaced commits by "replaced" analogous to git.git's
5267d29 (log: decorate "replaced" on to replaced commits, 2011-08-19).
---
Should probably follow this up with graft decorations.

 tig.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/tig.c b/tig.c
index 92091ab..c9e8d1d 100644
--- a/tig.c
+++ b/tig.c
@@ -26,6 +26,7 @@ struct ref {
 	unsigned int tag:1;	/* Is it a tag? */
 	unsigned int ltag:1;	/* If so, is the tag local? */
 	unsigned int remote:1;	/* Is it a remote ref? */
+	unsigned int replace:1;	/* Is it a replace ref? */
 	unsigned int tracked:1;	/* Is it the remote for the current HEAD? */
 	char name[1];		/* Ref name; tag or head names are shortened. */
 };
@@ -444,6 +445,7 @@ LINE(MAIN_COMMIT,  "",			COLOR_DEFAULT,	COLOR_DEFAULT,	0), \
 LINE(MAIN_TAG,     "",			COLOR_MAGENTA,	COLOR_DEFAULT,	A_BOLD), \
 LINE(MAIN_LOCAL_TAG,"",			COLOR_MAGENTA,	COLOR_DEFAULT,	0), \
 LINE(MAIN_REMOTE,  "",			COLOR_YELLOW,	COLOR_DEFAULT,	0), \
+LINE(MAIN_REPLACE, "",			COLOR_CYAN,	COLOR_DEFAULT,	0), \
 LINE(MAIN_TRACKED, "",			COLOR_YELLOW,	COLOR_DEFAULT,	A_BOLD), \
 LINE(MAIN_REF,     "",			COLOR_CYAN,	COLOR_DEFAULT,	0), \
 LINE(MAIN_HEAD,    "",			COLOR_CYAN,	COLOR_DEFAULT,	A_BOLD), \
@@ -521,6 +523,8 @@ get_line_type_from_ref(const struct ref *ref)
 		return LINE_MAIN_TRACKED;
 	else if (ref->remote)
 		return LINE_MAIN_REMOTE;
+	else if (ref->replace)
+		return LINE_MAIN_REPLACE;
 
 	return LINE_MAIN_REF;
 }
@@ -6748,6 +6752,8 @@ compare_refs(const void *ref1_, const void *ref2_)
 		return ref2->head - ref1->head;
 	if (ref1->tracked != ref2->tracked)
 		return ref2->tracked - ref1->tracked;
+	if (ref1->replace != ref2->replace)
+		return ref2->replace - ref1->replace;
 	/* Order remotes last. */
 	if (ref1->remote != ref2->remote)
 		return ref1->remote - ref2->remote;
@@ -6809,6 +6815,7 @@ read_ref(char *id, size_t idlen, char *name, size_t namelen, void *data)
 	bool tag = FALSE;
 	bool ltag = FALSE;
 	bool remote = FALSE;
+	bool replace = FALSE;
 	bool tracked = FALSE;
 	bool head = FALSE;
 	int from = 0, to = refs_size - 1;
@@ -6831,6 +6838,13 @@ read_ref(char *id, size_t idlen, char *name, size_t namelen, void *data)
 		name	+= STRING_SIZE("refs/remotes/");
 		tracked  = !strcmp(opt_remote, name);
 
+	} else if (!prefixcmp(name, "refs/replace/")) {
+		replace = TRUE;
+		id	= name + strlen("refs/replace/");
+		idlen	= namelen - strlen("refs/replace/");
+		name	= "replaced";
+		namelen	= strlen(name);
+
 	} else if (!prefixcmp(name, "refs/heads/")) {
 		namelen -= STRING_SIZE("refs/heads/");
 		name	+= STRING_SIZE("refs/heads/");
@@ -6849,7 +6863,7 @@ read_ref(char *id, size_t idlen, char *name, size_t namelen, void *data)
 	 * previous SHA1 with the resolved commit id; relies on the fact
 	 * git-ls-remote lists the commit id of an annotated tag right
 	 * before the commit id it points to. */
-	while (from <= to) {
+	while ((from <= to) && !replace) {
 		size_t pos = (to + from) / 2;
 		int cmp = strcmp(name, refs[pos]->name);
 
@@ -6881,6 +6895,7 @@ read_ref(char *id, size_t idlen, char *name, size_t namelen, void *data)
 	ref->tag = tag;
 	ref->ltag = ltag;
 	ref->remote = remote;
+	ref->replace = replace;
 	ref->tracked = tracked;
 	string_copy_rev(ref->id, id);
 
-- 
1.7.7.rc0.328.g9d6c7

^ permalink raw reply related

* [PATCH] git-p4: simple branch tests edits
From: Vitor Antunes @ 2011-08-29  9:32 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes

From: Pete Wyckoff <pw@padd.com>

More review comments.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9800-git-p4.sh |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 39e7f78..01ba041 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -413,7 +413,7 @@ test_expect_success 'add simple p4 branches' '
 	cd branch1 &&
 	echo file1 >file1 &&
 	echo file2 >file2 &&
-	p4 add file* &&
+	p4 add file1 file2 &&
 	p4 submit -d "branch1" &&
 	p4 integrate //depot/branch1/... //depot/branch2/... &&
 	p4 submit -d "branch2" &&
@@ -433,13 +433,12 @@ test_expect_success 'add simple p4 branches' '
 # Finally, make an update to branch1 on P4 side to check if it is imported
 # correctly by git-p4.
 test_expect_success 'git-p4 clone simple branches' '
-	git init "$git" &&
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
 	cd "$git" &&
 	git config git-p4.branchList branch1:branch2 &&
 	git config --add git-p4.branchList branch1:branch3 &&
-	cd "$TRASH_DIRECTORY" &&
-	"$GITP4" clone --dest="$git" --detect-branches //depot@all &&
-	cd "$git" &&
+	"$GITP4" clone --dest=. --detect-branches //depot@all &&
 	git log --all --graph --decorate --stat &&
 	git reset --hard p4/depot/branch1 &&
 	test -f file1 &&
@@ -449,7 +448,7 @@ test_expect_success 'git-p4 clone simple branches' '
 	git reset --hard p4/depot/branch2 &&
 	test -f file1 &&
 	test -f file2 &&
-	test \! -z file3 &&
+	test ! -f file3 &&
 	! grep -q update file2 &&
 	git reset --hard p4/depot/branch3 &&
 	test -f file1 &&
@@ -459,14 +458,12 @@ test_expect_success 'git-p4 clone simple branches' '
 	cd "$cli" &&
 	cd branch1 &&
 	p4 edit file2 &&
-	echo file2_ >> file2 &&
-	p4 submit -d "update file2 in branch3" &&
+	echo file2_ >>file2 &&
+	p4 submit -d "update file2 in branch1" &&
 	cd "$git" &&
 	git reset --hard p4/depot/branch1 &&
 	"$GITP4" rebase &&
-	grep -q file2_ file2 &&
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" && mkdir "$git"
+	grep -q file2_ file2
 '
 
 test_expect_success 'shutdown' '
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH v4 0/4] git-p4: Improve branch support
From: Vitor Antunes @ 2011-08-29  9:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Pete Wyckoff, Tor Arvid Lund
In-Reply-To: <7vmxes7nu2.fsf@alter.siamese.dyndns.org>

On Sun, 28 Aug 2011 23:07:17 -0700
Junio C Hamano <gitster@pobox.com> wrote:

> Could you make this an incremental patch relative to what is already in
> next?

Done.
-- 
Vitor Antunes

^ permalink raw reply

* Re: Overriding ~/.gitconfig using GIT_CONFIG
From: Richard Purdie @ 2011-08-29 12:16 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, Daniel Barkalow,
	Nguy?? n Thái Ngọc Duy, GIT Mailing-list
In-Reply-To: <20110828130555.GA56765@gmail.com>

On Sun, 2011-08-28 at 06:05 -0700, David Aguilar wrote:
> On Fri, Aug 12, 2011 at 09:44:13PM +0100, Richard Purdie wrote:
> > My problem isn't that I want to specify a specific .gitconfig file, I
> > just need it to ignore the one in $HOME. I'm happy for the .git/config
> > file to be used, in fact I need it to be.
> 
> If you're writing a tool then it should restrict itself to
> git's plumbing commands.  You should be able to do just about
> anything without needing to worry about differences in
> configuraiton.  Git commands almost always provide a way to
> override configuration through the use of flags.
> 
> The plumbing commands are listed in the main git manpage.
> See "Low-level commands (plumbing)" here:
> http://www.kernel.org/pub/software/scm/git/docs/
> 
> What is the specific problem solved by overriding the
> configuration?  It may be possible to solve it without needing
> to get too complicated.

I'm not sure writing my own porcelain makes sense in this case.

The tool in question is a build system which is primarily interested in
building software. Sometimes the software we want to build is "bleeding
edge" and hence rather than download tarballs, we want to interact
directly with SCMs like git to obtain it.

The commands I'm using are the likes of "git clone" and "git fetch"
although we do use commands listed under the plumbing section too such
as ls-remote and read-tree. We do "cache" checkouts and support
automatically noticing changes and updating/building.

What I do want to be able to say is "ignore whatever the user might have
put in their ~/.gitconfig file" since I've open bug reports about people
putting things in there that break builds.

The fetch/clone commands do what I need, apart from being influenced by
userconfig so reimplementing them myself doesn't seem like a good
approach.

Cheers,

Richard

-- 
Linux Foundation
http://www.yoctoproject.org/

^ permalink raw reply

* Re: [PATCH] .gitattributes: Enable cpp diff parsing for .[ch] files
From: Peter Zijlstra @ 2011-08-29 13:32 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Linus Torvalds, Andrew Morton, Git Mailing List, linux-kernel,
	Junio C Hamano
In-Reply-To: <4E580830.4010305@panasas.com>

On Fri, 2011-08-26 at 13:55 -0700, Boaz Harrosh wrote:

> But this patch is for all the people and distributions that are going
> to use the old git versions for a while, (Years).
> 
> It is important for the Linux mailing-list review, because you get
> patches with:
> 
> @@@ lable out:
> -	change foo
> +	to bazz
> 
> And how the hell are you suppose to know where in the file it is.
> This is such a trivial addition, that can help unify everyone's
> usage. I don't see the down sides.

Ah, I'm using:

# cat ~/.gitconfig 
[diff "default"]
                xfuncname = "^[[:alpha:]$_].*[^:]$"


For this.

^ permalink raw reply

* Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
From: Marat Radchenko @ 2011-08-29 14:48 UTC (permalink / raw)
  To: git
In-Reply-To: <CACsJy8DMqjMfb5wVMnsrZhP8yv2rF1wyQ6LM6b-PVcYY1SXkcQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds <at> gmail.com> writes:
>  - is "file" above at top repo, or is it actually very/deep/path/to/a/file?
3 levels deep. Most parent dir (one after repo root) contains 20k files.

>  - how many entries in the tree that contain "file"?
Sorry, didn't understand this.

>  - how is "git ls-files | wc -l"?
$ time git ls-files | wc -l
603137

real    0m0.417s
user    0m0.440s
sys     0m0.060s

>  - how about "time git diff branch another-branch -- file >/dev/null"?
> That'd remove unpack-trees code.
Pretty fast:
$ time git diff branch:file other_branch:file > /dev/null

real    0m0.278s
user    0m0.210s
sys     0m0.060s

One more test:
git diff HEAD branch -- file > /dev/null 

real    0m0.276s
user    0m0.240s
sys     0m0.030s

So the only troubled variant is `git diff branch -- file`.

^ permalink raw reply

* [PATCH] am: format is in $patch_format, not parse_patch
From: Giuseppe Bilotta @ 2011-08-29 15:22 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Giuseppe Bilotta

The error message given when the patch format was not recognized was
wrong, since the variable checked was $parse_patch rather than
$patch_format. Fix by checking the non-emptyness of the correct
variable.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-am.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index e78cb54..4fff195 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -312,7 +312,7 @@ split_patches ()
 		msgnum=
 		;;
 	*)
-		if test -n "$parse_patch" ; then
+		if test -n "$patch_format" ; then
 			clean_abort "$(eval_gettext "Patch format \$patch_format is not supported.")"
 		else
 			clean_abort "$(gettext "Patch format detection failed.")"
-- 
1.7.7.rc0.328.g626f7.dirty

^ permalink raw reply related

* Re: git-config: case insensitivity for subsections
From: Jeff King @ 2011-08-29 15:58 UTC (permalink / raw)
  To: milki; +Cc: Alex Vandiver, git
In-Reply-To: <20110829054240.GB94231@hal.rescomp.berkeley.edu>

On Sun, Aug 28, 2011 at 10:42:40PM -0700, milki wrote:

> A user gave me a link [1] to his git-config and I cannot correctly
> parse, for example, his alias.last.
> [...]
> [1] https://github.com/kergoth/homefiles/blob/master/.gitconfig#L67

Isn't his config somewhat broken?  It looks like this:

  last = "!f(){ since="$1"; shift; git lg --since=\"last $since\" "$@"; }; f"

Those interior double-quotes should all be backslash-escaped. I didn't
check, but git should interpret this as:

  !f(){ since=$1; shift; git lg --since="last $since" $@; }; f

which is probably not quite what he wanted (the quotes around $1 were
actually superfluous, but the ones around $@ are important).

That being said, I think it is intentional that the value is not just "a
single double-quoted chunk" but rather could consist of several quoted
(or unquoted) chunks concatenated together. What does your parser think
of:

  [foo]
    bar = "foo"bar"baz"

It should be:

  $ git config foo.bar
  foobarbaz

-Peff

^ permalink raw reply

* Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
From: Nguyen Thai Ngoc Duy @ 2011-08-29 16:09 UTC (permalink / raw)
  To: Marat Radchenko; +Cc: git
In-Reply-To: <loom.20110829T155805-331@post.gmane.org>

On Mon, Aug 29, 2011 at 9:48 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
> Nguyen Thai Ngoc Duy <pclouds <at> gmail.com> writes:
>>  - is "file" above at top repo, or is it actually very/deep/path/to/a/file?
> 3 levels deep. Most parent dir (one after repo root) contains 20k files.
>
>>  - how many entries in the tree that contain "file"?
> Sorry, didn't understand this.

You have already answered it. I was asking the size of parent dir, but
phrased poorly.

>>  - how is "git ls-files | wc -l"?
> $ time git ls-files | wc -l
> 603137
>
> real    0m0.417s
> user    0m0.440s
> sys     0m0.060s
>
>>  - how about "time git diff branch another-branch -- file >/dev/null"?
>> That'd remove unpack-trees code.
> Pretty fast:
>
> git diff HEAD branch -- file > /dev/null
>
> real    0m0.276s
> user    0m0.240s
> sys     0m0.030s

That may explain it. "git diff <ref>" walks through the index, unpacks
tree objects along the way, matches up entries with the same path from
the branch, the index then feeds matching entries to diff function. If
tree cutting is not done efficiently, it could very well walk through
every entry in the index (~600k entries in your case), unpacking all
tree objects along the way.

And it looks like to me that diff_cache() in diff-lib.c, responsible
for this case, does not do any prefix trimming. traverse_trees() also
does not seem to do "never_interesting" optimization like in
tree_interesting(), so if the traversed tree is big (~20k as you told
me), it will take some time, even though you are only interested in a
single entry.

> So the only troubled variant is `git diff branch -- file`.

No, I suspect "git diff --cached" would be also slow. "git merge"
would be definitely slow. But we can hardly improve these cases
because the commands are usually called tree-wide, no path limiting.

If you only work on a small subset of files, there's some (unfinished)
code in narrow clone implementation that cuts down index size, which
may speed up in big-index repositories like yours. Could be a good
reason for me (or someone) to extract that part and get it in before
full narrow clone is implemented.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] am: format is in $patch_format, not parse_patch
From: Junio C Hamano @ 2011-08-29 16:12 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <1314631326-11796-1-git-send-email-giuseppe.bilotta@gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> The error message given when the patch format was not recognized was
> wrong, since the variable checked was $parse_patch rather than
> $patch_format. Fix by checking the non-emptyness of the correct
> variable.

Thanks for a fix to a problem that is from more than two years ago ;-)

^ permalink raw reply

* [PATCH 0/2] am: foreign patch support fixes
From: Giuseppe Bilotta @ 2011-08-29 16:44 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Giuseppe Bilotta

Two small patches to fix/enhance support for foreign patchset.

The first patch adds support for hg patches, which have been detected
(but not supported) for a while. I've used it to import a couple of
patches successfully.

The second patch fixes a rather long-standing issue with stgit patches,
when Author was used instead of From. Apparently not many patches with
this format are encountered in the wild, since nobody had an issue with
it so far.

Giuseppe Bilotta (2):
  am: preliminary support for hg patches
  am: fix stgit patch mangling

 git-am.sh |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

-- 
1.7.7.rc0.331.g25483.dirty

^ permalink raw reply

* [PATCH 1/2] am: preliminary support for hg patches
From: Giuseppe Bilotta @ 2011-08-29 16:44 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1314636247-26125-1-git-send-email-giuseppe.bilotta@gmail.com>

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-am.sh |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 4fff195..729ee51 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -311,6 +311,40 @@ split_patches ()
 		this=
 		msgnum=
 		;;
+	hg)
+		this=0
+		for hg in "$@"
+		do
+			this=`expr "$this" + 1`
+			msgnum=`printf "%0${prec}d" $this`
+			# hg stores changeset metadata in #-commented lines preceding
+			# the commit message and diff(s). The only metadata we care about
+			# are the User and Date (Node ID and Parent are hashes which are
+			# only relevant to the hg repository and thus not useful to us)
+			# Since we cannot guarantee that the commit message is in git-friendly
+			# format, we put no Subject: line and just consume all of the message
+			# as the body
+			perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+				if ($subject) { print ; }
+				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
+				elsif (/^\# Date /) {
+					my ($hashsign, $str, $time, $tz) = split ;
+					$tz = sprintf "%+05d", (0-$tz)/36;
+					print "Date: " .
+					      strftime("%a, %d %b %Y %H:%M:%S ",
+						       localtime($time))
+					      . "$tz\n";
+				} elsif (/^\# /) { next ; }
+				else {
+					print "\n", $_ ;
+					$subject = 1;
+				}
+			' < "$hg" > "$dotest/$msgnum" || clean_abort
+		done
+		echo "$this" > "$dotest/last"
+		this=
+		msgnum=
+		;;
 	*)
 		if test -n "$patch_format" ; then
 			clean_abort "$(eval_gettext "Patch format \$patch_format is not supported.")"
-- 
1.7.7.rc0.331.g25483.dirty

^ permalink raw reply related


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