Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3] convert filter: supply path to external driver
From: Jonathan Nieder @ 2010-12-21 18:19 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: Junio C Hamano, git, Jeff King, Johannes Sixt
In-Reply-To: <20101221134403.GA10401@honk.padd.com>

Hi,

Pete Wyckoff wrote:

> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh

Nitpicks (some silly, some not):

> @@ -93,4 +93,51 @@ test_expect_success expanded_in_repo '
>  	cmp expanded-keywords expected-output
>  '
>  
> +cat <<EOF >argc.sh
> +#!$SHELL_PATH
> +echo argc: \$# "\$@"
> +echo argc running >&2
> +EOF
> +chmod +x argc.sh

You can embed this in a test_expect_success stanza (like the next
one or the earlier "setup") like so:

	cat <<-EOF >argc.sh &&
	#!$SHELL_PATH
	...
	EOF
	chmod +x argc.sh &&

This way if the "chmod" fails on some platform the test would
catch that.

> +
> +#
> +# The use of %f in a filter definition is expanded to the path to
> +# the filename being smudged or cleaned.  It must be shell escaped.
> +#

I'd even prefer to see this comment inside the test_expect_success
assertion so it can be printed when running the test with "-v".
But I suppose consistency with the other test in the script suggests
otherwise.

[...]
> +    echo some test text > test
> +    cat test > $norm &&
> +    cat test > "$spec" &&

Missing && after "> test".  Probably best to remove the space
after > (just for consistency[1]).  Also, please use tabs to indent.

[...]
> +    # make sure argc.sh counted the right number of args
> +    echo "argc: 1 $norm" > res &&
> +    cmp res $norm &&

test_cmp?  (for nicer output)  See t/README.

[...]
> +    # %f with other args
> +    git config filter.argc.smudge "./argc.sh %f --myword" &&
> +    rm $norm "$spec" &&
> +    git checkout -- $norm "$spec" &&
> +
> +    # make sure argc.sh counted the right number of args
> +    echo "argc: 2 $norm --myword" > res &&
> +    cmp res $norm &&
> +    echo "argc: 2 $spec --myword" > res &&
> +    cmp res "$spec" &&

Probably would be clearer if this were a separate test assertion.

> +    :
> +'
> +
>  test_done

Thanks for the tests.  I haven't looked at the substance, alas,
but hope that helps nonetheless.

Jonathan

[1] Trumped up justification for the "no space after >" style: if I
always include a space after, I would be tempted to use

	noisy_command > /dev/null 2> &1

But that does not work because >& is recognized as a single token.

^ permalink raw reply

* Re: Dangerous "git am --abort" behavior
From: Junio C Hamano @ 2010-12-21 18:29 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <7vtyi8arxp.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> ...
>> Maybe "git am" should actually save the last commit ID that it did,
>> and only do the "reset" if the current HEAD matches the rebase-apply
>> state and warns if it doesn't? Or maybe we could just introduce a new
>> "git am --clean" that just flushes any old pending state (ie does that
>> "clean_abort" thing, which is basically just the "rm -rf" I've done by
>> hand). Or both?
> ...
> Back then my tentative conclusion was actually to get rid of "am --abort"
> and give "am --clean", making the final "reset HEAD~$n" the responsiblity
> of the user.  But I forgot to pursue it.

So here is the first step in that direction.  I suspect that stop_here
should also record what the current branch is, and safe_to_abort should
check it (the potentially risky sequence is "after a failed am, check out
a different branch and then realize you need to 'am --abort'"), but that
is left to interested others ;-) or a later round.

-- >8 --
Subject: am --abort: keep unrelated commits since the last failure and warn

After making commits (either by pulling or doing their own work) after a
failed "am", the user will be reminded by next "am" invocation that there
was a failed "am" that the user needs to decide to resolve or to get rid
of the old "am" attempt.  The "am --abort" option was meant to help the
latter.  However, it rewinded the HEAD back to the beginning of the failed
"am" attempt, discarding commits made (perhaps by mistake) since.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh           |   27 +++++++++++++++++++++++++--
 t/t4151-am-abort.sh |    9 +++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index df09b42..cf1f64b 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -68,9 +68,31 @@ sq () {
 
 stop_here () {
     echo "$1" >"$dotest/next"
+    git rev-parse --verify -q HEAD >"$dotest/abort-safety"
     exit 1
 }
 
+safe_to_abort () {
+	if test -f "$dotest/dirtyindex"
+	then
+		return 1
+	fi
+
+	if ! test -s "$dotest/abort-safety"
+	then
+		return 0
+	fi
+
+	abort_safety=$(cat "$dotest/abort-safety")
+	if test "z$(git rev-parse --verify -q HEAD)" = "z$abort_safety"
+	then
+		return 0
+	fi
+	echo >&2 "You seem to have moved HEAD since the last 'am' failure."
+	echo >&2 "Not rewinding to ORIG_HEAD"
+	return 1
+}
+
 stop_here_user_resolve () {
     if [ -n "$resolvemsg" ]; then
 	    printf '%s\n' "$resolvemsg"
@@ -419,10 +441,11 @@ then
 			exec git rebase --abort
 		fi
 		git rerere clear
-		test -f "$dotest/dirtyindex" || {
+		if safe_to_abort
+		then
 			git read-tree --reset -u HEAD ORIG_HEAD
 			git reset ORIG_HEAD
-		}
+		fi
 		rm -fr "$dotest"
 		exit ;;
 	esac
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index b55c411..001b1e3 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -62,4 +62,13 @@ do
 
 done
 
+test_expect_success 'am --abort will keep the local commits' '
+	test_must_fail git am 0004-*.patch &&
+	test_commit unrelated &&
+	git rev-parse HEAD >expect &&
+	git am --abort &&
+	git rev-parse HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_done

^ permalink raw reply related

* Re: Dangerous "git am --abort" behavior
From: Linus Torvalds @ 2010-12-21 18:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vsjxr7zdn.fsf@alter.siamese.dyndns.org>

On Tue, Dec 21, 2010 at 10:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> So here is the first step in that direction.  I suspect that stop_here
> should also record what the current branch is, and safe_to_abort should
> check it (the potentially risky sequence is "after a failed am, check out
> a different branch and then realize you need to 'am --abort'"), but that
> is left to interested others ;-) or a later round.

Yeah, this patch looks good to me.

And if you've switched branches, and do a "git am --abort" which still
sees the expected commit, I actually think your patch does the right
thing: we will rewind that new branch to ORIG_HEAD, and I think that
is actually the semantics we want.

So what you can do with this is:

 - "git am <mbox-file>" fails in the middle

 - you go "hmm. I'm happy with what we did so far, but let's go back
to check what's up"

 - "git checkout -b test-branch ; git am --abort"

 - work on the original base and maybe try to re-apply the mbox with
soem manual editing or whatever...

and that's exactly the semantics that your patch allows, which seems
to be very flexible and useful. No?

So the only thing it disallows is having "git am --abort" actually
abort some unrelated commit, which is I think the exact behavior we
want. In fact, if somebody has done a "git pull" or something, then
"ORIG_HEAD" really doesn't mean what git am thinks it means. So I
wonder if we should check ORIG_HEAD against "beginning of 'git am'"
too, the way you check HEAD against the "abort-safely" point?

Again, if ORIG_HEAD doesn't match (for whatever reason - maybe
somebody switched branches and did a 'git reset --hard" in that other
branch, and then switched back?), then "git am --abort" shouldn't
abort to some random point that came from some non-am workflow, no?

But with the HEAD check, you'd really have to _work_ at screwing up,
so the ORIG_HEAD check seems to be much less important.

                              Linus

^ permalink raw reply

* Re: Dangerous "git am --abort" behavior
From: Junio C Hamano @ 2010-12-21 18:47 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <7vsjxr7zdn.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> So here is the first step in that direction.  I suspect that stop_here
> should also record what the current branch is, and safe_to_abort should
> check it (the potentially risky sequence is "after a failed am, check out
> a different branch and then realize you need to 'am --abort'"), but that
> is left to interested others ;-) or a later round.

And here is that later round...

-- >8 --
Subject: [PATCH] am --abort: also check the current branch

If the user checks out another branch after an "am" failure, am --abort
would have rewound the tip of that branch back to where the last failed
"am" started from, which would not be fun.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh           |   10 +++++++---
 t/t4151-am-abort.sh |   17 +++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index e5671f6..ca3f910 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -68,7 +68,9 @@ sq () {
 
 stop_here () {
     echo "$1" >"$dotest/next"
-    git rev-parse --verify -q HEAD >"$dotest/abort-safety"
+    head=$(git rev-parse --verify -q HEAD)
+    branch=$(git symbolic-ref -q HEAD)
+    echo "$head,$branch" >"$dotest/abort-safety"
     exit 1
 }
 
@@ -84,11 +86,13 @@ safe_to_abort () {
 	fi
 
 	abort_safety=$(cat "$dotest/abort-safety")
-	if test "z$(git rev-parse --verify -q HEAD)" = "z$abort_safety"
+	head=$(git rev-parse --verify -q HEAD)
+	branch=$(git symbolic-ref -q HEAD)
+	if test "z$head,$branch" = "z$abort_safety"
 	then
 		return 0
 	fi
-	echo >&2 "You seem to have moved HEAD since the last 'am' failure."
+	echo >&2 "You seem to have done some other things since the last 'am' failure."
 	echo >&2 "Not rewinding to ORIG_HEAD"
 	return 1
 }
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 001b1e3..23a9fb0 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -71,4 +71,21 @@ test_expect_success 'am --abort will keep the local commits' '
 	test_cmp expect actual
 '
 
+test_expect_success 'am --abort will keep unrelated branch' '
+	git reset --hard &&
+	test_commit foo &&
+	test_must_fail git am 0004-*.patch &&
+	git checkout -b unrelated HEAD^ &&
+	(
+		git rev-parse HEAD
+		git symbolic-ref HEAD
+	) >expect &&
+	git am --abort &&
+	(
+		git rev-parse HEAD
+		git symbolic-ref HEAD
+	) >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.3.4.768.g2fa91

^ permalink raw reply related

* Re: [PATCH] trace.c: mark file-local function static
From: Johannes Sixt @ 2010-12-21 19:54 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, Drew Northup, Thiago Farina, Vasyl',
	git
In-Reply-To: <7vbp4f9gzh.fsf@alter.siamese.dyndns.org>

On Dienstag, 21. Dezember 2010, Junio C Hamano wrote:
> A more interesting topic is why the try-to-free-pack-memory logic needs to
> be disabled in the first place.  3a09425 (Do not call release_pack_memory
> in malloc wrappers when GIT_TRACE is used, 2010-05-08) explains that it is
> to avoid a race on Windows, and it looks like a workaround not a solution
> ("can be called without locking"---"why aren't we locking then?").
>
> Not that it matters in the context of "trace", which is a debugging
> facility, that this is a workaround.

Exactly. A clean implementation is not worth the effort for this debugging 
facility.

BTW, these days it is not just Windows that is affected because we use threads 
in start_async everywhere if possible.

-- Hannes

^ permalink raw reply

* [PATCH] t3419-*.sh: Fix arithmetic expansion syntax error
From: Ramsay Jones @ 2010-12-21 18:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list, drizzd


Some shells, for example dash versions older than 0.5.4, need to
spell a variable reference as '$N' rather than 'N' in an arithmetic
expansion. In order to avoid the syntax error, we change the
offending variable reference from 'i' to '$i' in function scramble.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Note that this test is unique in having an '#!/bin/bash' line (rather
than '#!/bin/sh'), which was (indirectly) responsible for me not
noticing this failure for a while. I don't see anything that would
require bash, so I suspect this is not intensional.

ATB,
Ramsay Jones

 t/t3419-rebase-patch-id.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 1aee483..6972b49 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -27,7 +27,7 @@ scramble()
 		then
 			echo "$x"
 		fi
-		i=$(((i+1) % 10))
+		i=$((($i+1) % 10))
 	done < "$1" > "$1.new"
 	mv -f "$1.new" "$1"
 }
-- 
1.7.3

^ permalink raw reply related

* Re: [PATCH 13/14] t4135-*.sh: Skip the "backslash" tests on cygwin
From: Ramsay Jones @ 2010-12-21 19:31 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, jrnieder, pclouds
In-Reply-To: <201012172254.31242.j6t@kdbg.org>

Johannes Sixt wrote:
> Yes, that's clear, and this is the reason that we must skip this test on 
> MinGW.
> 
> But you said that when the single quotes are removed, the test passes (and you 
> are right). Then git-add sees this pathspec/pattern:
> 
>     fo[ou]bar
> 
> and it matches 'foobar' when it interprets that as a pattern, but 'fo[ou]bar' 
> when it interprets that as straight file name. Even on Linux, the latter 
> happens, and *that* is suspicious. What am I missing?

Ah, sorry, I mis-understood your point. :(

So, I decided to have a quick look and ... yeah, something is not quite right!

Hmm, I *think* I have a fix, see patch attached below. This patch provoked a
failure of the unicode tests in t0050-filesystem.sh for me on Linux. However,
these tests are borked when run by the dash shell, but work fine when run by
bash. (see separate e-mail) So, all (non svn) tests pass for me when I run
the tests thus:

    $ SHELL_PATH=/bin/bash make NO_SVN_TESTS=1 test

Of course, this is no guarantee that I haven't messed up all git commands that
use match_one() to process pathspecs, but is at least promising.

The problem boils down to the call to strncmp_icase() suppressing the call to
fnmatch() when the pattern contains glob chars, but the (remaining) string is
equal to the name; thus returning an exact match (MATCHED_EXACTLY) rather than
calling fnmatch (and returning either no-match or MATCHED_FNMATCH).

Note that the test itself is not correct; the argument to git-ls-files should
be quoted the same as the git-add before it ... (well you could pass
fo\\[ou\\]bar instead!).

[BTW, I started looking at the history of this function and I think this
problem has been there for a long time!]

Hmm, I think this is all being rewritten, at the moment (in branch
nd/struct-pathspec) isn't it?

Anyway, let me know what you think...

ATB,
Ramsay Jones

--- 8< ---
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Sun, 19 Dec 2010 19:47:39 +0000
Subject: [PATCH] dir.c: Fix handling of filespecs containing glob-ing chars


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 dir.c          |    2 +-
 t/t3700-add.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 852e60f..4e8c7bf 100644
--- a/dir.c
+++ b/dir.c
@@ -139,7 +139,7 @@ static int match_one(const char *match, const char *name, int namelen)
 	 * we need to match by fnmatch
 	 */
 	matchlen = strlen(match);
-	if (strncmp_icase(match, name, matchlen))
+	if (is_glob_special(*match) || strncmp_icase(match, name, matchlen))
 		return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
 
 	if (namelen == matchlen)
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index ec71083..9140164 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -239,7 +239,7 @@ test_expect_success BSLASHPSPEC "git add 'fo\\[ou\\]bar' ignores foobar" '
 	git reset --hard &&
 	touch fo\[ou\]bar foobar &&
 	git add '\''fo\[ou\]bar'\'' &&
-	git ls-files fo\[ou\]bar | fgrep fo\[ou\]bar &&
+	git ls-files '\''fo\[ou\]bar'\'' | fgrep fo\[ou\]bar &&
 	! ( git ls-files foobar | grep foobar )
 '
 
-- 
1.7.3

^ permalink raw reply related

* t0050-filesystem.sh unicode tests borked on dash shell
From: Ramsay Jones @ 2010-12-21 19:53 UTC (permalink / raw)
  To: prohaska; +Cc: Junio C Hamano, GIT Mailing-list


I noticed recently that the unicode tests, when run by the dash shell,
have not been working as designed. (The tests *pass*, but they are
*not* testing what was intended)

In order to demonstrate, I added an "false &&" line after the touch in
test #8, so that (on Ubuntu):

    $ ./t0050-filesystem -i
    ok 1 - see what we expect
    ok 2 - detection of case insensitive filesystem during repo init
    ok 3 - detection of filesystem w/o symlink support during repo init
    ok 4 - setup case tests
    ok 5 - rename (case change)
    ok 6 - merge (case change)
    not ok 7 - add (with different case) # TODO known breakage
    not ok - 8 setup unicode normalization tests
    #	
    #	
    #	  test_create_repo unicode &&
    #	  cd unicode &&
    #	  touch "$aumlcdiar" &&
    #	false &&
    #	  git add "$aumlcdiar" &&
    #	  git commit -m initial &&
    #	  git tag initial &&
    #	  git checkout -b topic &&
    #	  git mv $aumlcdiar tmp &&
    #	  git mv tmp "$auml" &&
    #	  git commit -m rename &&
    #	  git checkout -f master
    #	
    #	
    $ ls trash\ directory.t0050-filesystem/unicode/
    \x61\xcc\x88

    $ bash t0050-filesystem -i
    ok 1 - see what we expect
    ok 2 - detection of case insensitive filesystem during repo init
    ok 3 - detection of filesystem w/o symlink support during repo init
    ok 4 - setup case tests
    ok 5 - rename (case change)
    ok 6 - merge (case change)
    not ok 7 - add (with different case) # TODO known breakage
    not ok - 8 setup unicode normalization tests
    #	
    #	
    #	  test_create_repo unicode &&
    #	  cd unicode &&
    #	  touch "$aumlcdiar" &&
    #	false &&
    #	  git add "$aumlcdiar" &&
    #	  git commit -m initial &&
    #	  git tag initial &&
    #	  git checkout -b topic &&
    #	  git mv $aumlcdiar tmp &&
    #	  git mv tmp "$auml" &&
    #	  git commit -m rename &&
    #	  git checkout -f master
    #	
    #	
    $ ls trash\ directory.t0050-filesystem/unicode/ | od -x
    0000000 cc61 0a88
    0000004

So bash works fine and I can avoid the problem by running the tests, thus:

    $ SHELL_PATH=/bin/bash make NO_SVN_TESTS=1 test

Since I have an older dash, I compiled dash from source (my dash git repo
claims:

    $ git describe --tags
    v0.5.6-24-gb61ab0b

), but the result was exactly the same.

I afraid I don't have time to investigate this further at the moment ...

ATB,
Ramsay Jones

^ permalink raw reply

* Re: [PATCH] t3419-*.sh: Fix arithmetic expansion syntax error
From: Junio C Hamano @ 2010-12-21 20:24 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list, drizzd
In-Reply-To: <4D10F707.1000206@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Note that this test is unique in having an '#!/bin/bash' line (rather
> than '#!/bin/sh'),...

Thanks for spotting, and that is idiotic for us to have such a script.
We should fix the shebang line as well.

A quick git-grep shows that this is the only instance of this problem
(outside contrib/completion/git-completion.bash, that is).

^ permalink raw reply

* Re: 'show' pretty %B without a diff
From: Martin Langhoff @ 2010-12-21 20:27 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jakub Narebski, Peter Vereshagin, Junio C Hamano, git
In-Reply-To: <20101221180459.GA25812@burratino>

On Tue, Dec 21, 2010 at 1:04 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Maybe it would be worth adding a plumbing example under the EXAMPLES
> for each porcelain?

Since porcelain went to C, one thing I often do is checkout really old
versions of git to see exactly what the shell version of a particular
command did .

That way, I get a much better understanding of how a certain action is
done at the plumbing level; complement that with the documentation for
reference, and I'm 99% done.

Maybe that helps someone .

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

^ permalink raw reply

* [PATCH] t0050: fix printf format strings for portability
From: Jonathan Nieder @ 2010-12-21 20:27 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: prohaska, Junio C Hamano, GIT Mailing-list
In-Reply-To: <4D1105B5.5070703@ramsay1.demon.co.uk>

Unlike bash and ksh, dash passes through hexadecimal \xcc escapes.
So when run with dash, these tests *pass* (since '\xcc' is a perfectly
reasonable filename) but they are not testing what was intended.

Use octal escapes instead, in the spirit of v1.6.1-rc1~55^2
(2008-11-09).

Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Ramsay Jones wrote:

> I noticed recently that the unicode tests, when run by the dash shell,
> have not been working as designed. (The tests *pass*, but they are
> *not* testing what was intended)
> 
> In order to demonstrate, I added an "false &&" line after the touch in
> test #8, so that (on Ubuntu):
> 
>     $ ./t0050-filesystem -i
[...]
>     $ ls trash\ directory.t0050-filesystem/unicode/
>     \x61\xcc\x88

Good point.  POSIX printf is not required to support \x escape
sequences.

 t/t0050-filesystem.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 057c97c..1542cf6 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -4,8 +4,8 @@ test_description='Various filesystem issues'
 
 . ./test-lib.sh
 
-auml=`printf '\xc3\xa4'`
-aumlcdiar=`printf '\x61\xcc\x88'`
+auml=$(printf '\303\244')
+aumlcdiar=$(printf '\141\314\210')
 
 case_insensitive=
 unibad=
-- 
1.7.2.3.554.gc9b5c.dirty

^ permalink raw reply related

* Re: t0050-filesystem.sh unicode tests borked on dash shell
From: Thomas Rast @ 2010-12-21 20:29 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: prohaska, Junio C Hamano, GIT Mailing-list
In-Reply-To: <4D1105B5.5070703@ramsay1.demon.co.uk>

Ramsay Jones wrote:
>     $ ls trash\ directory.t0050-filesystem/unicode/
>     \x61\xcc\x88

The printf at the top evidently does not interpolate \xAA sequences.
Since my 'man 1p printf' POSIX manpage only mandates \AAA octal
sequences, maybe we should use that instead.  Can you verify that the
patch below works for you?

Judging from

  git grep '\\x[0-9a-f][0-9a-f]' t

this is the only instance of this problem, the rest are in Perl code.

--- 8< ---
Subject: t0050: replace \xAA by \AAA in printf

POSIX does not mandate the hex escape sequences, and thus dash's
built-in printf does not expand them.  Use octal escapes instead.

diff --git c/t/t0050-filesystem.sh i/t/t0050-filesystem.sh
index 057c97c..87bf1ff 100755
--- c/t/t0050-filesystem.sh
+++ i/t/t0050-filesystem.sh
@@ -4,8 +4,8 @@ test_description='Various filesystem issues'
 
 . ./test-lib.sh
 
-auml=`printf '\xc3\xa4'`
-aumlcdiar=`printf '\x61\xcc\x88'`
+auml=`printf '\303\244'`
+aumlcdiar=`printf '\141\314\210'`
 
 case_insensitive=
 unibad=

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

^ permalink raw reply related

* [PATCH v4] convert filter: supply path to external driver
From: Pete Wyckoff @ 2010-12-21 20:33 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Jeff King, Johannes Sixt
In-Reply-To: <20101221181924.GB25812@burratino>

Filtering to support keyword expansion may need the name of
the file being filtered.  In particular, to support p4 keywords
like

    $File: //depot/product/dir/script.sh $

the smudge filter needs to know the name of the file it is
smudging.

Add a "%f" conversion specifier to the gitattribute for filter.
It will be expanded with the path name to the file when invoking
the external filter command.  The path name is quoted and
special characters are escaped to prevent the shell from splitting
incorrectly.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
jrnieder@gmail.com wrote on Tue, 21 Dec 2010 12:19 -0600:
> [detailed review]

Thanks for the nitpicks.  I put the argc.sh and chmod +x into a
setup test.  Tried to put some more in there, and to break up the
test in two, but did not want to duplicate the complex
calculation of "norm" and "spec" variables.  So ended up with the
small setup, and just one big test still.

I couldn't quite bring myself to delete the nice spaces in
redirections like "> test".  Rest of the usage in t/ seems
to be about 1/3 for space, 2/3 against.

Got the test_cmp, tabs and missing &&, too, thanks for finding
those.

		-- Pete

 Documentation/gitattributes.txt |   12 +++++++++
 convert.c                       |   22 +++++++++++++++++-
 t/t0021-conversion.sh           |   48 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 1 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 564586b..1afcf01 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -317,6 +317,18 @@ command is "cat").
 	smudge = cat
 ------------------------
 
+If your filter needs the path of the file it is working on,
+you can use the "%f" conversion specification.  It will be
+replaced with the relative path to the file.  This is important
+for keyword substitution that depends on the name of the
+file.  Like this:
+
+------------------------
+[filter "p4"]
+	clean = git-p4-filter --clean %f
+	smudge = git-p4-filter --smudge %f
+------------------------
+
 
 Interaction between checkin/checkout attributes
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/convert.c b/convert.c
index e41a31e..8f020bc 100644
--- a/convert.c
+++ b/convert.c
@@ -317,6 +317,7 @@ struct filter_params {
 	const char *src;
 	unsigned long size;
 	const char *cmd;
+	const char *path;
 };
 
 static int filter_buffer(int in, int out, void *data)
@@ -329,7 +330,23 @@ static int filter_buffer(int in, int out, void *data)
 	int write_err, status;
 	const char *argv[] = { NULL, NULL };
 
-	argv[0] = params->cmd;
+	/* apply % substitution to cmd */
+	struct strbuf cmd = STRBUF_INIT;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf_expand_dict_entry dict[] = {
+	    "f", NULL,
+	    NULL, NULL,
+	};
+
+	/* quote the path to preserve spaces, etc. */
+	sq_quote_buf(&path, params->path);
+	dict[0].value = path.buf;
+
+	/* expand all %f with the quoted path */
+	strbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);
+	strbuf_release(&path);
+
+	argv[0] = cmd.buf;
 
 	memset(&child_process, 0, sizeof(child_process));
 	child_process.argv = argv;
@@ -349,6 +366,8 @@ static int filter_buffer(int in, int out, void *data)
 	status = finish_command(&child_process);
 	if (status)
 		error("external filter %s failed %d", params->cmd, status);
+
+	strbuf_release(&cmd);
 	return (write_err || status);
 }
 
@@ -376,6 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len,
 	params.src = src;
 	params.size = len;
 	params.cmd = cmd;
+	params.path = path;
 
 	fflush(NULL);
 	if (start_async(&async))
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 828e35b..69c22a6 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -93,4 +93,52 @@ test_expect_success expanded_in_repo '
 	cmp expanded-keywords expected-output
 '
 
+test_expect_success 'filter shell-escaped filenames setup' '
+	cat >argc.sh <<-EOF &&
+	#!$SHELL_PATH
+	echo argc: \$# "\$@"
+	EOF
+	chmod +x argc.sh
+'
+
+# The use of %f in a filter definition is expanded to the path to
+# the filename being smudged or cleaned.  It must be shell escaped.
+# First, set up some interesting file names and pet them in
+# .gitattributes.
+test_expect_success 'filter shell-escaped filenames test' '
+	norm=name-no-magic &&
+	spec=$(echo name:sgl\"dbl\ spc!bang | tr : \\047) &&
+	echo some test text > test &&
+	cat test > $norm &&
+	cat test > "$spec" &&
+	git add $norm &&
+	git add "$spec" &&
+	git commit -q -m "add files" &&
+	echo "name* filter=argc" > .gitattributes &&
+
+	# delete the files and check them out again, using a smudge filter
+	# that will count the args and echo the command-line back to us
+	git config filter.argc.smudge "./argc.sh %f" &&
+	rm $norm "$spec" &&
+	git checkout -- $norm "$spec" &&
+
+	# make sure argc.sh counted the right number of args
+	echo "argc: 1 $norm" > res &&
+	test_cmp res $norm &&
+	echo "argc: 1 $spec" > res &&
+	test_cmp res "$spec" &&
+
+	# do the same thing, but with more args in the filter expression
+	git config filter.argc.smudge "./argc.sh %f --myword" &&
+	rm $norm "$spec" &&
+	git checkout -- $norm "$spec" &&
+
+	# make sure argc.sh counted the right number of args
+	echo "argc: 2 $norm --myword" > res &&
+	test_cmp res $norm &&
+	echo "argc: 2 $spec --myword" > res &&
+	test_cmp res "$spec" &&
+	:
+'
+
 test_done
-- 
1.7.2.3

^ permalink raw reply related

* Re: 'show' pretty %B without a diff
From: Jakub Narebski @ 2010-12-21 20:40 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Jonathan Nieder, Peter Vereshagin, Junio C Hamano, git
In-Reply-To: <AANLkTi=BJ0NdKrANuXKObNQJbchqdSUhpnttsdU_NnQe@mail.gmail.com>

Martin Langhoff <martin.langhoff@gmail.com> writes:

> On Tue, Dec 21, 2010 at 1:04 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Maybe it would be worth adding a plumbing example under the EXAMPLES
> > for each porcelain?
> 
> Since porcelain went to C, one thing I often do is checkout really old
> versions of git to see exactly what the shell version of a particular
> command did .

You can always browse 'contrib/examples/' instead.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: t0050-filesystem.sh unicode tests borked on dash shell
From: Junio C Hamano @ 2010-12-21 20:43 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: prohaska, GIT Mailing-list
In-Reply-To: <4D1105B5.5070703@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

>     $ ls trash\ directory.t0050-filesystem/unicode/
>     \x61\xcc\x88

The built-in printf in dash seems to lack understanding of '\xXX'.
It is tempting to patch it by using /usr/bin/printf but it is unclear how
portable it would be.

 t/t0050-filesystem.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 41df6bc..8ad102b 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -4,8 +4,8 @@ test_description='Various filesystem issues'
 
 . ./test-lib.sh
 
-auml=`printf '\xc3\xa4'`
-aumlcdiar=`printf '\x61\xcc\x88'`
+auml=$(/usr/bin/printf '\xc3\xa4')
+aumlcdiar=$(/usr/bin/printf '\x61\xcc\x88')
 
 case_insensitive=
 unibad=

^ permalink raw reply related

* Re: [PATCH v4] convert filter: supply path to external driver
From: Junio C Hamano @ 2010-12-21 21:24 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: Jonathan Nieder, git, Jeff King, Johannes Sixt
In-Reply-To: <20101221203322.GA13868@honk.padd.com>

Pete Wyckoff <pw@padd.com> writes:

> Filtering to support keyword expansion may need the name of
> the file being filtered.  In particular, to support p4 keywords
> like
>
>     $File: //depot/product/dir/script.sh $
>
> the smudge filter needs to know the name of the file it is
> smudging.
>
> Add a "%f" conversion specifier to the gitattribute for filter.
> It will be expanded with the path name to the file when invoking
> the external filter command.  The path name is quoted and
> special characters are escaped to prevent the shell from splitting
> incorrectly.

You do not specify the filter in attributes file, and this is not just
about splitting incorrectly (think $var substitutions).  I rephrased the
last paragraph when I tentatively queued v3 (and then I had to discard it
after seeing this round) like this:

    Allow "%f" in the custom filter command line specified in the
    configuration.  This will be substituted by the filename inside a
    single-quote pair to be passed to the shell.

> I couldn't quite bring myself to delete the nice spaces in
> redirections like "> test".  Rest of the usage in t/ seems
> to be about 1/3 for space, 2/3 against.

While existing mistakes by others do not make a good excuse to throw
unnecessary code-churn patches at me, it is not an excuse to introduce
more mistakes of the same kind in new code.

>  Documentation/gitattributes.txt |   12 +++++++++
>  convert.c                       |   22 +++++++++++++++++-
>  t/t0021-conversion.sh           |   48 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 564586b..1afcf01 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -317,6 +317,18 @@ command is "cat").
>  	smudge = cat
>  ------------------------
>  
> +If your filter needs the path of the file it is working on,
> +you can use the "%f" conversion specification.  It will be
> +replaced with the relative path to the file.  This is important
> +for keyword substitution that depends on the name of the
> +file.  Like this:

Maybe "important" to you, but not really.  Just let the reader decide the
importance.  I rephrased this when I tentatively queued v3 (and then I had
to discard it after seeing this round) like this:

    Sequence "%f" on the filter command line is replaced with the name of
    the file the filter is working on (a filter can use this in keyword
    substitution, for example):

> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index 828e35b..69c22a6 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -93,4 +93,52 @@ test_expect_success expanded_in_repo '
>  	cmp expanded-keywords expected-output
>  '
>  
> +test_expect_success 'filter shell-escaped filenames setup' '
> +	cat >argc.sh <<-EOF &&
> +	#!$SHELL_PATH
> +	echo argc: \$# "\$@"
> +	EOF
> +	chmod +x argc.sh
> +'
>
> +# The use of %f in a filter definition is expanded to the path to
> +# the filename being smudged or cleaned.  It must be shell escaped.
> +# First, set up some interesting file names and pet them in
> +# .gitattributes.
> +test_expect_success 'filter shell-escaped filenames test' '
> +	norm=name-no-magic &&
> +	spec=$(echo name:sgl\"dbl\ spc!bang | tr : \\047) &&

I think this is going overboard.  The correctness of sq_quote_buf() is not
what we are really testing with this test; we are primarily interested in
testing that '%f' is substituted here.  You can and should drop at least
dq from the funny pathname, so that this can be run on Windows as well.
Perhaps (note two SPs between "name" and "with"):

	special="name  with '\''sq'\'' and \$x" &&

Do not over-abbreviate variable names unnecessarily; it will only confuse
the readers.  "spec" typically stands for "specification", but you don't
mean that here.

Also if you define the filter as "sh ./argc.sh %f", you should be able to
stop worrying about the "chmod +x" failing, no?

> +	echo some test text > test &&
> +	cat test > $norm &&
> +	cat test > "$spec" &&

Quote both for consistency.

	cat test >"$norm"
        cat test >"$spec"

> +	git add $norm &&
> +	git add "$spec" &&

Why add them separately?

> +	echo "name* filter=argc" > .gitattributes &&
> +
> +	# delete the files and check them out again, using a smudge filter
> +	# that will count the args and echo the command-line back to us
> +	git config filter.argc.smudge "./argc.sh %f" &&
> +	rm $norm "$spec" &&
> +	git checkout -- $norm "$spec" &&
> +
> +	# make sure argc.sh counted the right number of args
> +	echo "argc: 1 $norm" > res &&

Perhaps "res" stands for "result", but both are misleading as it is
unclear if that is an expected result or an actual result.

	echo "argc: 1 $norm" >expect &&

Thanks.

^ permalink raw reply

* Re: [PATCH] t0050: fix printf format strings for portability
From: Junio C Hamano @ 2010-12-21 21:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ramsay Jones, prohaska, GIT Mailing-list
In-Reply-To: <20101221202755.GA27214@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Unlike bash and ksh, dash passes through hexadecimal \xcc escapes.
> So when run with dash, these tests *pass* (since '\xcc' is a perfectly
> reasonable filename) but they are not testing what was intended.
>
> Use octal escapes instead, in the spirit of v1.6.1-rc1~55^2
> (2008-11-09).

Thanks.

^ permalink raw reply

* Re: cvsimport still not working with cvsnt
From: Emil Medve @ 2010-12-21 22:09 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Guy Rouillier, git, Pascal Obry, Clemens Buchacher,
	Martin Langhoff
In-Reply-To: <20101220213654.GA24628@burratino>

Hello Guy,


On 12/20/10 15:36, Jonathan Nieder wrote:
> (+cc: Emil, some cvsimport people)
> 
> Guy Rouillier wrote:

Sometimes, on some particularly nasty CVS repos, I noticed better
results when using http://cvs2svn.tigris.org

>> I'm going to try sending this blind, as the mailing list has sent me
>> the promised authorization key after 24 hrs.
> 
> No problem.  Actually a subscription is not required --- the
> convention on this list is to always reply-to-all.
> 
>> I finally found the problems, both of which were reported in 2008
>> here:
>>
>> http://kerneltrap.org/mailarchive/git/2008/3/13/1157364
> 
> Seems to have received no replies[1].

I don't remember why, but that patch didn't get enough interest

>> I do see one possible issue with the supplied modifications.  At
>> work, we upgraded from CVS to CVSNT.  So, my home directory has both
>> .cvspass (from the original CVS) and .cvs/cvspass (after the
>> conversion to CVSNT.)  Sloppy housekeeping on my part, I admit, but
>> probably not uncommon.  The supplied patch would pick up the
>> original CVS file and would fail.  (BTW, this is true only of the
>> git-cvsimport.perl script itself; cvsps must shell out to the
>> installed CVS client (in my case, cvsnt), because when I invoked
>> that manually, it worked.)
>>
>> So, I would advise checking to see if both files exist, and if so
>> exit with an error.  Unless cvsimport wants to get real fancy and
>> shell out to the installed cvs client to try to figure out what is
>> installed, there is no way to tell which cvspass file is actively
>> being used.  I don't recommend trying to figure this out, as the
>> user's intent is unclear.
> 
> Thanks, sounds sane to me.  Care to write a patch?

If you care enough about this scenario, how about search for the
relevant <CVSROOT, password> in both files. If you find just one pair or
if you find a pair in both files and they are "equal" then just use it.
If you find two pairs, one in each file, use the one from the file with
a newer modified time-stamp. In a migration scenario such as this, you'd
imaging the "old" file will get stale after a while. Not perfect, but
some informational messages in case of a duplicate would help the user
clarify their intentions

Additionally/Alternatively just add a command line parameter to allow
the user to explicitly specify a cvspass file


Cheers,
Emil.

^ permalink raw reply

* Re: [PATCH] completion: Add PS1 configuration for submodules
From: Scott Kyle @ 2010-12-21 22:56 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Jonathan Nieder, Kevin Ballard, Ævar Arnfjörð, git
In-Reply-To: <4D06621F.6010101@web.de>

On Mon, Dec 13, 2010 at 10:12 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 12.12.2010 07:38, schrieb Jonathan Nieder:
>> Scott Kyle wrote:
>>> On Tue, Dec 7, 2010 at 1:29 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>>>> Scott Kyle wrote:
>>
>>>>> If I set the "submodule.<name>.ignore" then diffing around inside my
>>>>> history will not show the changes to that particular submodule.
>>>>
>>>> Even if you set it to "dirty"?
>>>
>>> Setting it to "dirty" is far less disruptive, you're right, but that
>>> wouldn't do me much good since my submodules are often on different
>>> branches while developing.
>>
>> Ah, I see now.  How about something like this?  Untested, just a
>> vague sketch to show the idea.
>
> Me thinks your proposal of a new "worktree" option makes sense. Let's
> hear what Scott says ...
>

I mostly really like how 'worktree' can let me focus in on only the
submodules I care about.  The drawback is that git status would no
longer list my true status.  I know that may sound hypocritical, but I
intended for this patch to only affect my PS1.  At the same time, I
would like to see the 'worktree' patch taken, regardless of whether
you guys find mine useful.

^ permalink raw reply

* [PATCH] attr.c: Use ALLOC_GROW instead of alloc_nr and xrealloc.
From: Thiago Farina @ 2010-12-22  0:35 UTC (permalink / raw)
  To: git

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 attr.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/attr.c b/attr.c
index 6aff695..fdc0515 100644
--- a/attr.c
+++ b/attr.c
@@ -305,12 +305,8 @@ static void handle_attr_line(struct attr_stack *res,
 	a = parse_attr_line(line, src, lineno, macro_ok);
 	if (!a)
 		return;
-	if (res->alloc <= res->num_matches) {
-		res->alloc = alloc_nr(res->num_matches);
-		res->attrs = xrealloc(res->attrs,
-				      sizeof(struct match_attr *) *
-				      res->alloc);
-	}
+
+	ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
 	res->attrs[res->num_matches++] = a;
 }
 
-- 
1.7.3.2.343.g7d43d

^ permalink raw reply related

* Re: [PATCH] attr.c: Use ALLOC_GROW instead of alloc_nr and xrealloc.
From: Junio C Hamano @ 2010-12-22  0:59 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <3c6870c390110bd1bf5c5f59a99928afc86cf188.1292978127.git.tfransosi@gmail.com>

Instead of trickling these down, can you give a single patch to convert
the remaining ones you have been finding with "git grep -e alloc_nr"?

^ permalink raw reply

* Re: [PATCH] attr.c: Use ALLOC_GROW instead of alloc_nr and xrealloc.
From: Thiago Farina @ 2010-12-22  1:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1v5a7hc4.fsf@alter.siamese.dyndns.org>

On Tue, Dec 21, 2010 at 10:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Instead of trickling these down, can you give a single patch to convert
> the remaining ones you have been finding with "git grep -e alloc_nr"?
>

$ git grep -w alloc_nr | grep -v cache.h | wc -l

Sorry, but I wouldn't feel comfortable fixing 19 files now. At least
not exactly now. Perhaps on the weekend.

^ permalink raw reply

* FIX/COMMENT: git remote manual page
From: Michel Briand @ 2010-12-22  1:15 UTC (permalink / raw)
  To: git

Hello,

I tried the example given at the bottom if the manual page of git
remote.

·   Imitate git clone but track only selected branches

$ mkdir project.git
$ cd project.git
$ git init
$ git remote add -f -t master -m master origin git://example.com/git.git/
$ git merge origin

It works like it is written.

But it seems this does not work with my special setup:
- I use GIT_DIR and GIT_WORK_TREE to specify another location for my
  repository, and to work from another directory,
- I name my remote with a custom name (not origin).

It fails at the last command :

    fatal: <my name> - not something we can merge

But if I try the command :

    git merge <my name>/master

the error message is different :

    fatal: This operation must be run in a work tree
    fatal: read-tree failed

I cd to the work tree and issue the same last command.
Then it works.

I suspect the first error message is related to the remote name. And
the second to the work tree not being the current directory.

Cheers,
Michel

^ permalink raw reply

* Re: [PATCH 13/14] t4135-*.sh: Skip the "backslash" tests on cygwin
From: Nguyen Thai Ngoc Duy @ 2010-12-22  1:44 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Johannes Sixt, Junio C Hamano, GIT Mailing-list, jrnieder
In-Reply-To: <4D1100A3.2010309@ramsay1.demon.co.uk>

On Wed, Dec 22, 2010 at 2:31 AM, Ramsay Jones
<ramsay@ramsay1.demon.co.uk> wrote:
> The problem boils down to the call to strncmp_icase() suppressing the call to
> fnmatch() when the pattern contains glob chars, but the (remaining) string is
> equal to the name; thus returning an exact match (MATCHED_EXACTLY) rather than
> calling fnmatch (and returning either no-match or MATCHED_FNMATCH).

I think that's expected behavior. Wildcard pathspecs are fixed
pathspecs will additional wildcard matching support and can match both
ways. See 186d604 (glossary: define pathspec)

> [BTW, I started looking at the history of this function and I think this
> problem has been there for a long time!]

Not only in this function. pathspec_matches() in builtin/grep.c
behaves the same (I think).

> Hmm, I think this is all being rewritten, at the moment (in branch
> nd/struct-pathspec) isn't it?

Yes. Thanks for pulling me in. I didn't know recent match_one() has
case-insensitive support.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 36/47] rev-parse: prints --git-dir relative to user's cwd
From: Junio C Hamano @ 2010-12-22  1:56 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1290785563-15339-37-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> git_dir variable in environment.c is relative to git's cwd, not user's
> cwd. Convert the relative path (actualy by making it absolute path)
> before printing out.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/rev-parse.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index a5a1c86..65c287b 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -647,7 +647,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  				static char cwd[PATH_MAX];
>  				int len;
>  				if (gitdir) {
> -					puts(gitdir);
> +					if (is_absolute_path(gitdir) || !prefix) {
> +						puts(gitdir);
> +						continue;
> +					}
> +					puts(make_absolute_path(gitdir));
>  					continue;
>  				}
>  				if (!prefix) {

I do not quite understand this change.  I can obtain GIT_DIR in a relative
form without this patch already:

    $ cd t/
    $ git --git-dir=../.git rev-parse --git-dir HEAD
    ../.git
    c7511731675da8b50c0d5243aa04a98c8a5ee316

Could we please have a new test case to demonstrate what is broken without
this patch?

^ permalink raw reply


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