Git development
 help / color / mirror / Atom feed
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-13 22:38 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Torsten Bögershausen, git
In-Reply-To: <20130113173207.GC5973@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Hi,
>
> Torsten Bögershausen wrote:
>
>> -	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
>> +	/^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable (please use type)';
>
> Hmm.  Neither the old version nor the new one matches what seem to
> be typical uses of 'which', based on a quick code search:
>
> 	if which sl >/dev/null 2>&1
> 	then
> 		sl -l
> 		...
> 	fi
>
> or
>
> 	if test -x "$(which sl 2>/dev/null)"
> 	then
> 		sl -l
> 		...
> 	fi

Yes, these two misuses are what we want it to trigger on, so the
test is very easy to trigger and produce a false positive, but does
not trigger on what we really want to catch.

That does not sound like a good benefit/cost ratio to me.

^ permalink raw reply

* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-13 22:20 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Michael Haggerty, Eric S. Raymond, git, Chris Rorvick
In-Reply-To: <20130112182649.GC4624@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Michael Haggerty wrote:
>
>> Regarding your claim that "within a few months the Perl git-cvsimport is
>> going to cease even pretending to work": It might be that the old
>> git-cvsimport will stop working *for people who upgrade to cvsps 3.x*.
>> But it is not realistic to expect people to synchronize their git and
>> cvsps version upgrades.  It is even quite possible that this or that
>> Linux distribution will package incompatible versions of the two packages.
>
> Moreover, I feel an obligation to point the following out:
>
> In a hypothetical world where cvsps 3.x simply breaks "git cvsimport"
> it is likely that some distributions would just stick to the existing
> cvsps and not upgrade to 3.x.  Maybe that's a wrong choice, but that's
> a choice some would make.  An even more likely outcome in that
> hypothetical world is that they would ship it renamed to something
> like "cvsps3" alongside the existing cvsps.  Or they could rename the
> old version to "cvsps2".  If we were the last holdout, we could even
> bundle it as compat/cvsps.
>
> So please do not act as though the cvsps upgrade is a crisis that we
> need to break ourselves for at threat of no longer working at all.
> The threat doesn't hold water.
>
> Luckily you have already written patches to make "git cvsimport" work
> with cvsps 3.x, and through your work you are making a better
> argument: "The new cvsimport + cvsps will work better, at least for
> some users, than the old tool."
>
> Just don't pretend you have the power to force a change for a less
> sensible reason than that!

After a quick survey of various distros, I think it is very unlikely
that we will see "distros move on to newer cvsps, leaving cvsimport
broken" situation. If anything, it is more like "distros decide to
ignore the new cvsps, until it is made to work with cvsimport" [*1*].

I think it is probably sensible to rename the current cvsimport to
cvsimport-2, write a small wrapper git-cvsimport.sh which is
something like this:

----- >8 -----
#!/bin/sh

if test -z "$GIT_CVSPS_VERSION"
then
	case "$(cvsps -h 2>&1 | grep 'cvsps version')" in
        2.*)
		GIT_CVSPS_VERSION=2
                ;;
	3.*)
		GIT_CVSPS_VERSION=3
                ;;
	esac
fi

if test -z "$GIT_CVSPS_VERSION" 
then
	echo >&2 "No supported cvsps available"
	exit 1
fi

exec git cvsimport-$GIT_CVSPS_VERSION "$@"
----- 8< -----

and put Eric's one as git-cvsimport-3 (after ripping out the code to
fallback to the old cvsimport).  The longer term trend will be to
move away from cvsimport-2, as it is unlikely cvsps-2.x will gain
improvements, if any; keeping fallback code outside cvsimport-3 will
be a better first step in the healthier long term code evolution.

We will keep the current t96xx series of tests, and have them export
GIT_CVSPS_VERSION=2 at the beginning, protect them with test prereq
that requires presence of cvsps 2.x; this will still make sure that
the current cvsimport users will not see any regressions.

Eric's one should be polished enough to produce good results on the
simpler sample CVS histories t96xx deal with soonish if not right
now, so we can use a method similar to how we shared tests between
blame and annotate while both were _different_ implementations to
make sure the newer blame did not inroduce regression by running the
same set of tests.  Where the result _ought_ to differ, we should
also add tests that work only with the new cvsimport, of course.

I could help getting the ball rolling on this, if everybody agrees
that the above is a sensible direction to go, given the real world
constraints of distro inertia.

Agreed?


[References]

*1* Fedora, Debian and Ubuntu do not even have cvsps 3.x in their
bleeding edges, OpenBSD and NetBSD do not seem to have it either,
and Gentoo and ArchLinux have the cvsps 3.x blocked due to
incompatiblity.

http://pkgs.fedoraproject.org/cgit/cvsps.git/
http://packages.debian.org/search?keywords=cvsps
http://packages.ubuntu.com/search?keywords=cvsps

http://packages.gentoo.org/package/dev-vcs/cvsps
https://bugs.gentoo.org/show_bug.cgi?id=450424

https://bugs.archlinux.org/task/33363?project=1&cat%5B0%5D=2&string=cvsps

^ permalink raw reply

* [PATCH] t0050: mark TC merge (case change) as success
From: Torsten Bögershausen @ 2013-01-13 20:38 UTC (permalink / raw)
  To: git; +Cc: tboegi

The test "merge (case change)" passes on a case ignoring file system

Use test_expect_success to remove the "known breakage vanished"

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t0050-filesystem.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 78816d9..ccd685d 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -77,7 +77,7 @@ $test_case 'rename (case change)' '
 
 '
 
-$test_case 'merge (case change)' '
+test_expect_success 'merge (case change)' '
 
 	rm -f CamelCase &&
 	rm -f camelcase &&
-- 
1.8.0.197.g5a90748

^ permalink raw reply related

* [PATCH] t0050: mark TC merge (case change) as success
From: Torsten Bögershausen @ 2013-01-13 20:38 UTC (permalink / raw)
  To: git; +Cc: tboegi

The test "merge (case change)" passes on a case ignoring file system

Use test_expect_success to remove the "known breakage vanished"

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t0050-filesystem.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 78816d9..ccd685d 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -77,7 +77,7 @@ $test_case 'rename (case change)' '
 
 '
 
-$test_case 'merge (case change)' '
+test_expect_success 'merge (case change)' '
 
 	rm -f CamelCase &&
 	rm -f camelcase &&
-- 
1.8.0.197.g5a90748

^ permalink raw reply related

* Re: git list files
From: Jeff King @ 2013-01-13 20:10 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Стойчо Слепцов,
	git, Jakub Narębski, Matthieu Moy
In-Reply-To: <20130113175602.GD5973@elie.Belkin>

On Sun, Jan 13, 2013 at 09:56:02AM -0800, Jonathan Nieder wrote:

> > lets, say the equivalent of the $ls -d b* within git.git root directory
> > would look like:
> >
> > ----------------
> > 98746061 jrnieder 2010-08-12 17:11 Standardize-do-.-while-0-style   base85.c
> > c43cb386 pclouds  2012-10-26 22:53 Move-estimate_bisect_steps-to-li bisect.c
> > efc7df45 pclouds  2012-10-26 22:53 Move-print_commit_list-to-libgit bisect.h
> > 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.c
> > 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.h
> > ebcfa444 gitster  2012-07-23 20:56 Merge-branch-jn-block-sha1       block-sha1
> 
> You might like Peff's or Jakub's tree blame script.  The newest version
> I can find is
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/168323

As far as I recall, that script works. However, I have a pure-C
blame-tree implementation that is much faster, which may also be of
interest. I need to clean up and put a few finishing touches on it to
send it to the list, but it has been in production at GitHub for a few
months. You can find it here:

  git://github.com/peff/git jk/blame-tree

It's built on the regular diff traversal, just like the perl script you
linked, but doing it all in-process makes things fast.  I also added a
"--max-depth" parameter for diff, so you can do:

  git blame-tree --max-depth=1 -- Documentation

to recurse into the Documentation subdir, but not go into its
subdirectories. One of the things I need to clean up is that my counting
of --max-depth is different from that used by "git grep", and we would
probably want reconcile that.

-Peff

^ permalink raw reply

* Re: [PATCH] archive-tar: fix sanity check in config parsing
From: Jeff King @ 2013-01-13 20:00 UTC (permalink / raw)
  To: René Scharfe; +Cc: git discussion list, Junio C Hamano
In-Reply-To: <50F2F1E9.1040700@lsrfire.ath.cx>

On Sun, Jan 13, 2013 at 06:42:01PM +0100, René Scharfe wrote:

> When parsing these config variable names, we currently check that
> the second dot is found nine characters into the name, disallowing
> filter names with a length of five characters.  Additionally,
> git archive crashes when the second dot is omitted:
> 
> 	$ ./git -c tar.foo=bar archive HEAD >/dev/null
> 	fatal: Data too large to fit into virtual memory space.
> 
> Instead we should check if the second dot exists at all, or if
> we only found the first one.

Eek. Thanks for finding it. Your fix is obviously correct.

> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -335,7 +335,7 @@ static int tar_filter_config(const char *var, const char *value, void *data)
>  	if (prefixcmp(var, "tar."))
>  		return 0;
>  	dot = strrchr(var, '.');
> -	if (dot == var + 9)
> +	if (dot == var + 3)
>  		return 0;

For the curious, the original version of the patch[1] read:

+       if (prefixcmp(var, "tarfilter."))
+               return 0;
+       dot = strrchr(var, '.');
+       if (dot == var + 9)
+               return 0;

and when I shortened the config section to "tar" in a re-roll of the
series, I missed the corresponding change to the offset.

-Peff

[1] http://thread.gmane.org/gmane.comp.version-control.git/175785/focus=175858

^ permalink raw reply

* Re: git list files
From: Стойчо Слепцов @ 2013-01-13 19:18 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20130113175602.GD5973@elie.Belkin>

thanks alot, Jonathan,
I'll try to search through those scripts.

Blind.

2013/1/13 Jonathan Nieder <jrnieder@gmail.com>:
> Hi,
>
> Стойчо Слепцов wrote:
>
>> lets, say the equivalent of the $ls -d b* within git.git root directory
>> would look like:
>>
>> ----------------
>> 98746061 jrnieder 2010-08-12 17:11 Standardize-do-.-while-0-style   base85.c
>> c43cb386 pclouds  2012-10-26 22:53 Move-estimate_bisect_steps-to-li bisect.c
>> efc7df45 pclouds  2012-10-26 22:53 Move-print_commit_list-to-libgit bisect.h
>> 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.c
>> 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.h
>> ebcfa444 gitster  2012-07-23 20:56 Merge-branch-jn-block-sha1       block-sha1
>
> You might like Peff's or Jakub's tree blame script.  The newest version
> I can find is
>
>  http://thread.gmane.org/gmane.comp.version-control.git/168323
>
> If you use it, let us know how it goes.
>
> Thanks for some food for thought,
> Jonathan

^ permalink raw reply

* Re: git list files
From: Стойчо Слепцов @ 2013-01-13 19:16 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqhaml9pr3.fsf@grenoble-inp.fr>

not really,
ls-tree provides the hash of blobs and trees,
what I am searching for is"the last commit"who introduced the blob or tree.

but, hey, thanks for the answer!
Blind

2013/1/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Стойчо Слепцов <stoycho.sleptsov@gmail.com> writes:
>
>> Hi,
>>
>> I was searching for some git- command to provide me a list of files
>> (in a git directory), same as ls,
>> but showing information from the last commit of the file instead.
>>
>> lets, say the equivalent of the $ls -d b* within git.git root directory
>> would look like:
>
> git ls-tree HEAD
>
> ?
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-13 18:58 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Junio C Hamano, Jason Pyeron, git, Jonathan Nieder,
	Torsten Bögershausen, Stephen & Linda Smith, Eric Blake
In-Reply-To: <CALxABCavvW77djKQnbQsjCBcahmMfrP24SDz609NG-94_ifZ9Q@mail.gmail.com>

On 01/11/2013 03:17 PM, Alex Riesen wrote:
> On Fri, Jan 11, 2013 at 9:08 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
>> This short discussion on GitHub (file git-compat-util.h) might be relevant:
>>
>> https://github.com/msysgit/git/commit/435bdf8c7ffa493f8f6f2e8f329f8cc22db16ce6#commitcomment-2407194
>>
>> The change suggested there (to remove an inclusion of windows.h in
>> git-compat-util.h) might simplify the solution a little. Might even
>> remove the need for auto-configuration in Makefile (worked for me).
> Just to be clear, the change is this:
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 4a1979f..780a919 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -85,12 +85,6 @@
>   #define _NETBSD_SOURCE 1
>   #define _SGI_SOURCE 1
>
> -#ifdef WIN32 /* Both MinGW and MSVC */
> -#define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
> -#include <winsock2.h>
> -#include <windows.h>
> -#endif
> -
>   #include <unistd.h>
>   #include <stdio.h>
>   #include <sys/stat.h>
>
That change alone seems fine, no apparent change building on current 
cygwin. However, with that change the build still fails if 
CYGWIN_V15_WIN32API is defined, so unless someone can show the 
compilation works on cygwin1.5 WITHOUT defining CYGWIN_V15_WIN32API this 
change does not help. I do not have an older installation available, so 
cannot test. Frankly, assuming you can compile with that macro defined, 
I would suggest leaving well enough alone - an unsupported configuration 
is unsupported :^)

Mark

^ permalink raw reply

* Re: git list files
From: Jonathan Nieder @ 2013-01-13 17:56 UTC (permalink / raw)
  To: Стойчо Слепцов
  Cc: git, Jeff King, Jakub Narębski, Matthieu Moy
In-Reply-To: <CAGL0X-rfrwtbtdN7O0=iMhVRYv1m0_czW8zmgT5QA3irkaeu5Q@mail.gmail.com>

Hi,

Стойчо Слепцов wrote:

> lets, say the equivalent of the $ls -d b* within git.git root directory
> would look like:
>
> ----------------
> 98746061 jrnieder 2010-08-12 17:11 Standardize-do-.-while-0-style   base85.c
> c43cb386 pclouds  2012-10-26 22:53 Move-estimate_bisect_steps-to-li bisect.c
> efc7df45 pclouds  2012-10-26 22:53 Move-print_commit_list-to-libgit bisect.h
> 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.c
> 837d395a barkalow 2010-01-18 13:06 Replace-parse_blob-with-an-expla blob.h
> ebcfa444 gitster  2012-07-23 20:56 Merge-branch-jn-block-sha1       block-sha1

You might like Peff's or Jakub's tree blame script.  The newest version
I can find is

 http://thread.gmane.org/gmane.comp.version-control.git/168323

If you use it, let us know how it goes.

Thanks for some food for thought,
Jonathan

^ permalink raw reply

* Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: John Keeping @ 2013-01-13 17:52 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130113171402.GA1307@padd.com>

On Sun, Jan 13, 2013 at 12:14:02PM -0500, Pete Wyckoff wrote:
> john@keeping.me.uk wrote on Sun, 13 Jan 2013 16:26 +0000:
>> On Sat, Jan 12, 2013 at 06:30:44PM -0500, Pete Wyckoff wrote:
>> > john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
>> >> When different version of python are used to build via distutils, the
>> >> behaviour can change.  Detect changes in version and pass --force in
>> >> this case.
>> >[..]
>> >> diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
>> >[..]
>> >> +py_version=$(shell $(PYTHON_PATH) -c \
>> >> +	'import sys; print("%i.%i" % sys.version_info[:2])')
>> >> +
>> >>  all: $(pysetupfile)
>> >> -	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
>> >> +	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
>> >> +	flags=--force; \
>> >> +	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
>> >> +	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
>> > 
>> > Can you depend on ../GIT-PYTHON-VARS instead?  It comes from
>> > 96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
>> > It doesn't check version, just path, but hopefully that's good
>> > enough.  I'm imagining a rule that would do "clean" if
>> > ../GIT-PYTHON-VARS changed, then build without --force.
>> 
>> I was trying to keep the git_remote_helpers directory self contained.  I
>> can't see how to depend on ../GIT-PYTHON-VARS in a way that is as simple
>> as this and keeps "make -C git_remote_helpers" working in a clean tree.
>> 
>> Am I missing something obvious here?
> 
> Not if it wants to stay self-contained; you're right.
> 
> I'm not thrilled with how git_remote_helpers/Makefile always
> runs setup.py, and always generates PYLIBDIR, and now always
> invokes python a third time to see if its version changed.

I don't think PYLIBDIR will be calculated unless it's used ('=' not
':=' means its a deferred variable).

I wonder if the version check should move into setup.py - it would be
just as easy to check the file there and massage sys.args, although
possibly not as neat.


John

^ permalink raw reply

* [PATCH] archive-tar: fix sanity check in config parsing
From: René Scharfe @ 2013-01-13 17:42 UTC (permalink / raw)
  To: git discussion list; +Cc: Jeff King, Junio C Hamano

git archive supports passing generated tar archives through filter
commands like gzip.  Additional filters can be set up using the
configuration variables tar.<name>.command and tar.<name>.remote.

When parsing these config variable names, we currently check that
the second dot is found nine characters into the name, disallowing
filter names with a length of five characters.  Additionally,
git archive crashes when the second dot is omitted:

	$ ./git -c tar.foo=bar archive HEAD >/dev/null
	fatal: Data too large to fit into virtual memory space.

Instead we should check if the second dot exists at all, or if
we only found the first one.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive-tar.c       | 2 +-
 t/t5000-tar-tree.sh | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index d1cce46..093d10e 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -335,7 +335,7 @@ static int tar_filter_config(const char *var, const char *value, void *data)
 	if (prefixcmp(var, "tar."))
 		return 0;
 	dot = strrchr(var, '.');
-	if (dot == var + 9)
+	if (dot == var + 3)
 		return 0;
 
 	name = var + 4;
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e7c240f..3fbd366 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -212,7 +212,8 @@ test_expect_success 'git-archive --prefix=olde-' '
 test_expect_success 'setup tar filters' '
 	git config tar.tar.foo.command "tr ab ba" &&
 	git config tar.bar.command "tr ab ba" &&
-	git config tar.bar.remote true
+	git config tar.bar.remote true &&
+	git config tar.invalid baz
 '
 
 test_expect_success 'archive --list mentions user filter' '
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH 0/8] Initial support for Python 3
From: John Keeping @ 2013-01-13 17:35 UTC (permalink / raw)
  To: Pete Wyckoff
  Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier,
	Sebastian Morr
In-Reply-To: <20130113164045.GA30371@padd.com>

On Sun, Jan 13, 2013 at 11:40:45AM -0500, Pete Wyckoff wrote:
> john@keeping.me.uk wrote on Sun, 13 Jan 2013 00:41 +0000:
>> On Sat, Jan 12, 2013 at 06:43:04PM -0500, Pete Wyckoff wrote:
>> > Can you give me some hints about the byte/unicode string issues
>> > in git-p4.py?  There's really only one place that does:
>> > 
>> >     p4 = subprocess.Popen("p4 -G ...")
>> >     marshal.load(p4.stdout)
>> > 
>> > If that's the only issue, this might not be too paniful.
>> 
>> The problem is that what gets loaded there is a dictionary (encoded by
>> p4) that maps byte strings to byte strings, so all of the accesses to
>> that dictionary need to either:
>> 
>>    1) explicitly call encode() on a string constant
>> or 2) use a byte string constant with a "b" prefix
>> 
>> Or we could re-write the dictionary once, which handles the keys... but
>> some of the values are also used as strings and we can't handle that as
>> a one-off conversion since in other places we really do want the byte
>> string (think content of binary files).
>> 
>> Basically a thorough audit of all access to variables that come from p4
>> would be needed, with explicit decode()s for authors, dates, etc.
> 
> Your auto-conversion snippet in the follow-up mail would work
> fine for most keys and values.  A few perforce docs and some
> playing around convince me that it is mostly utf-8, except for
> file data for particular types.
> 
> I'd still rather handle each command separately, and think about
> the conversions, to do it right in the long run.

I sent that on the assumption that the same key would have similar
semantics wherever its used, but I don't use git-p4 or know much about
perforce.

It would be interesting to know whether there is any likelihood of p4
gaining a Python 3 output mode (since the documentation currently say
not to use "p4 -G" with Python 3).  If it does then I would assume that
it will make a sensible choice about unicode/bytes such that the
existing git-p4 would Just Work with only a small change to the
invocation of p4 to add the new argument.

>> > I hesitated to take Sebastian's changes due to the huge number of
>> > print() lines, but maybe a 2to3 approach would make that aspect
>> > of python3 support not too onerous.
>> 
>> I think we'd want to change to print() eventually and having a single
>> codebase for 2 and 3 would be nicer for development, but I think we need
>> to be able to say "no one is using Python 2.5 or earlier" before we can
>> do that and I'm not sure we're there yet.  From where we are at the
>> moment I think 2to3 is a good answer, particularly where we're already
>> using distutils to generate a release image.
> 
> Agreed.  The 2to3 diff is large but straightforward.  But these
> p4 -G interface errors require a lot of thought and work.  I'm
> not too eager to work on this yet.

Fair enough.  As I don't use git-p4, it's not something I intend to
tackle either (given the scale of the changes involved).

Given the minimal scope of the changes needed for everything else, I
sent this series wondering whether it's sensible to move forward on the
basis of "Python scripts except git-p4 work with Python 3.  You must use
Python 2 if you want to use git-p4".


John

^ permalink raw reply

* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Jonathan Nieder @ 2013-01-13 17:32 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Junio C Hamano, git
In-Reply-To: <50F28BB5.9080607@web.de>

Hi,

Torsten Bögershausen wrote:

> -	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
> +	/^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable (please use type)';

Hmm.  Neither the old version nor the new one matches what seem to
be typical uses of 'which', based on a quick code search:

	if which sl >/dev/null 2>&1
	then
		sl -l
		...
	fi

or

	if test -x "$(which sl 2>/dev/null)"
	then
		sl -l
		...
	fi

^ permalink raw reply

* Re: [PATCH] t/lib-cvs: cvsimport no longer works without Python >= 2.7
From: John Keeping @ 2013-01-13 17:17 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, Eric S. Raymond, git
In-Reply-To: <50F180E8.8010907@alum.mit.edu>

On Sat, Jan 12, 2013 at 04:27:36PM +0100, Michael Haggerty wrote:
>>  Even if we were to rip out the fallback code that uses the 2.7-only
>>  subprocess.check_output() on "cvsps -V", the function is also used
>>  for doing the real work interacting with cvsps-3.x, so I think this
>>  patch will be necessary.  Unless new cvsimport is tweaked not to
>>  use the method, that is.
>> 
>>  A suggestion for a better alternative is of course very much
>>  appreciated.
> 
> If the only reason to require Python 2.7 is subprocess.check_output(),
> it would be easy to reimplement it (it is only 12 lines of
> straightforward code, plus a few lines to define the exception type
> CalledProcessError).  According to [1], the Python license is
> GPL-compatible; therefore these lines could even be copied into
> git-cvsimport.

Note that this has already be done in git_remote_helpers.util.  Is there
any reason not to just reference that?


John

^ permalink raw reply

* Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: Pete Wyckoff @ 2013-01-13 17:14 UTC (permalink / raw)
  To: John Keeping; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130113162605.GL4574@serenity.lan>

john@keeping.me.uk wrote on Sun, 13 Jan 2013 16:26 +0000:
> On Sat, Jan 12, 2013 at 06:30:44PM -0500, Pete Wyckoff wrote:
> > john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
> >> When different version of python are used to build via distutils, the
> >> behaviour can change.  Detect changes in version and pass --force in
> >> this case.
> >[..]
> >> diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
> >[..]
> >> +py_version=$(shell $(PYTHON_PATH) -c \
> >> +	'import sys; print("%i.%i" % sys.version_info[:2])')
> >> +
> >>  all: $(pysetupfile)
> >> -	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
> >> +	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
> >> +	flags=--force; \
> >> +	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
> >> +	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
> > 
> > Can you depend on ../GIT-PYTHON-VARS instead?  It comes from
> > 96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
> > It doesn't check version, just path, but hopefully that's good
> > enough.  I'm imagining a rule that would do "clean" if
> > ../GIT-PYTHON-VARS changed, then build without --force.
> 
> I was trying to keep the git_remote_helpers directory self contained.  I
> can't see how to depend on ../GIT-PYTHON-VARS in a way that is as simple
> as this and keeps "make -C git_remote_helpers" working in a clean tree.
> 
> Am I missing something obvious here?

Not if it wants to stay self-contained; you're right.

I'm not thrilled with how git_remote_helpers/Makefile always
runs setup.py, and always generates PYLIBDIR, and now always
invokes python a third time to see if its version changed.

		-- Pete

^ permalink raw reply

* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Matt Kraai @ 2013-01-13 16:50 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Junio C Hamano, git
In-Reply-To: <50F28BB5.9080607@web.de>

On Sun, Jan 13, 2013 at 11:25:57AM +0100, Torsten Bögershausen wrote:
> @@ -16,10 +16,10 @@ sub err {
>  
>  while (<>) {
>  	chomp;
> -	/^\s*sed\s+-i/ and err 'sed -i is not portable';
> -	/^\s*echo\s+-n/ and err 'echo -n is not portable (please use printf)';
> -	/^\s*declare\s+/ and err 'arrays/declare not portable';
> -	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
> +	/^\s*sed\s+-i\s+\S/ and err 'sed -i is not portable';
> +	/^\s*echo\s+-n\s+\S/ and err 'echo -n is not portable (please use printf)';
> +	/^\s*declare\s+\S/ and err 'arrays/declare not portable';
> +	/^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable (please use type)';

The "[^#]" appears to ensure that there's at least one character
before the which and that it's not a pound sign.  Why is this done?
Why isn't it done for the other commands?

^ permalink raw reply

* Re: [PATCH 0/8] Initial support for Python 3
From: Pete Wyckoff @ 2013-01-13 16:40 UTC (permalink / raw)
  To: John Keeping
  Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier,
	Sebastian Morr
In-Reply-To: <20130113004129.GH4574@serenity.lan>

john@keeping.me.uk wrote on Sun, 13 Jan 2013 00:41 +0000:
> On Sat, Jan 12, 2013 at 06:43:04PM -0500, Pete Wyckoff wrote:
> > Can you give me some hints about the byte/unicode string issues
> > in git-p4.py?  There's really only one place that does:
> > 
> >     p4 = subprocess.Popen("p4 -G ...")
> >     marshal.load(p4.stdout)
> > 
> > If that's the only issue, this might not be too paniful.
> 
> The problem is that what gets loaded there is a dictionary (encoded by
> p4) that maps byte strings to byte strings, so all of the accesses to
> that dictionary need to either:
> 
>    1) explicitly call encode() on a string constant
> or 2) use a byte string constant with a "b" prefix
> 
> Or we could re-write the dictionary once, which handles the keys... but
> some of the values are also used as strings and we can't handle that as
> a one-off conversion since in other places we really do want the byte
> string (think content of binary files).
> 
> Basically a thorough audit of all access to variables that come from p4
> would be needed, with explicit decode()s for authors, dates, etc.

Your auto-conversion snippet in the follow-up mail would work
fine for most keys and values.  A few perforce docs and some
playing around convince me that it is mostly utf-8, except for
file data for particular types.

I'd still rather handle each command separately, and think about
the conversions, to do it right in the long run.

> > I hesitated to take Sebastian's changes due to the huge number of
> > print() lines, but maybe a 2to3 approach would make that aspect
> > of python3 support not too onerous.
> 
> I think we'd want to change to print() eventually and having a single
> codebase for 2 and 3 would be nicer for development, but I think we need
> to be able to say "no one is using Python 2.5 or earlier" before we can
> do that and I'm not sure we're there yet.  From where we are at the
> moment I think 2to3 is a good answer, particularly where we're already
> using distutils to generate a release image.

Agreed.  The 2to3 diff is large but straightforward.  But these
p4 -G interface errors require a lot of thought and work.  I'm
not too eager to work on this yet.

Thanks.

		-- Pete

^ permalink raw reply

* Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: John Keeping @ 2013-01-13 16:26 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130112233044.GB23079@padd.com>

On Sat, Jan 12, 2013 at 06:30:44PM -0500, Pete Wyckoff wrote:
> john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
>> When different version of python are used to build via distutils, the
>> behaviour can change.  Detect changes in version and pass --force in
>> this case.
>[..]
>> diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
>[..]
>> +py_version=$(shell $(PYTHON_PATH) -c \
>> +	'import sys; print("%i.%i" % sys.version_info[:2])')
>> +
>>  all: $(pysetupfile)
>> -	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
>> +	$(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
>> +	flags=--force; \
>> +	$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
>> +	$(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
> 
> Can you depend on ../GIT-PYTHON-VARS instead?  It comes from
> 96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
> It doesn't check version, just path, but hopefully that's good
> enough.  I'm imagining a rule that would do "clean" if
> ../GIT-PYTHON-VARS changed, then build without --force.

I was trying to keep the git_remote_helpers directory self contained.  I
can't see how to depend on ../GIT-PYTHON-VARS in a way that is as simple
as this and keeps "make -C git_remote_helpers" working in a clean tree.

Am I missing something obvious here?


John

^ permalink raw reply

* Re: [PATCH 2/8] git_remote_helpers: fix input when running under Python 3
From: John Keeping @ 2013-01-13 16:17 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <50F2296F.8030909@alum.mit.edu>

On Sun, Jan 13, 2013 at 04:26:39AM +0100, Michael Haggerty wrote:
> On 01/12/2013 08:23 PM, John Keeping wrote:
>> Although 2to3 will fix most issues in Python 2 code to make it run under
>> Python 3, it does not handle the new strict separation between byte
>> strings and unicode strings.  There is one instance in
>> git_remote_helpers where we are caught by this.
>> 
>> Fix it by explicitly decoding the incoming byte string into a unicode
>> string.  In this instance, use the locale under which the application is
>> running.
>> 
>> Signed-off-by: John Keeping <john@keeping.me.uk>
>> ---
>>  git_remote_helpers/git/importer.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
>> index e28cc8f..6814003 100644
>> --- a/git_remote_helpers/git/importer.py
>> +++ b/git_remote_helpers/git/importer.py
>> @@ -20,7 +20,7 @@ class GitImporter(object):
>>          """Returns a dictionary with refs.
>>          """
>>          args = ["git", "--git-dir=" + gitdir, "for-each-ref", "refs/heads"]
>> -        lines = check_output(args).strip().split('\n')
>> +        lines = check_output(args).decode().strip().split('\n')
>>          refs = {}
>>          for line in lines:
>>              value, name = line.split(' ')
>> 
> 
> Won't this change cause an exception if the branch names are not all
> valid strings in the current locale's encoding?  I don't see how this
> assumption is justified (e.g., see git-check-ref-format(1) for the rules
> governing reference names).

Yes it will.  The problem is that for Python 3 we need to decode the
byte string into a unicode string, which means we need to know what
encoding it is.

I don't think we can just say "git-for-each-ref will print refs in
UTF-8" since AFAIK git doesn't care what encoding the refs are in - I
suspect that's determined by the filesystem which in the end probably
maps to whatever bytes the shell fed git when the ref was created.

That's why I chose the current locale in this case.  I'm hoping someone
here will correct me if we can do better, but I don't see any way of
avoiding choosing some encoding here if we want to support Python 3
(which I think we will, even if we don't right now).


John

^ permalink raw reply

* Re: git list files
From: Matthieu Moy @ 2013-01-13 13:28 UTC (permalink / raw)
  To: Стойчо Слепцов
  Cc: git
In-Reply-To: <CAGL0X-rfrwtbtdN7O0=iMhVRYv1m0_czW8zmgT5QA3irkaeu5Q@mail.gmail.com>

Стойчо Слепцов <stoycho.sleptsov@gmail.com> writes:

> Hi,
>
> I was searching for some git- command to provide me a list of files
> (in a git directory), same as ls,
> but showing information from the last commit of the file instead.
>
> lets, say the equivalent of the $ls -d b* within git.git root directory
> would look like:

git ls-tree HEAD

?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* [PATCH/WIP 10/10] Enable ls-files and ls-tree for testing PATHSPEC_ICASE
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:49 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1358081379-17752-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c      | 6 ++++--
 builtin/ls-files.c | 3 ++-
 builtin/ls-tree.c  | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index a3ffa9d..b9a5432 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -421,12 +421,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		GUARD_PATHSPEC(&pathspec,
 			       PATHSPEC_FROMTOP |
 			       PATHSPEC_LITERAL |
-			       PATHSPEC_GLOB);
+			       PATHSPEC_GLOB |
+			       PATHSPEC_ICASE);
 
 		for (i = 0; i < pathspec.nr; i++) {
 			const char *path = pathspec.items[i].match;
 			if (!seen[i] &&
-			    ((pathspec.items[i].magic & PATHSPEC_GLOB) ||
+			    ((pathspec.items[i].magic &
+			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
 			     !file_exists(path))) {
 				if (ignore_missing) {
 					int dtype = DT_UNKNOWN;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index feb4220..53b222d 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -538,7 +538,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	parse_pathspec(&pathspec,
 		       PATHSPEC_FROMTOP |
 		       PATHSPEC_LITERAL |
-		       PATHSPEC_GLOB,
+		       PATHSPEC_GLOB |
+		       PATHSPEC_ICASE,
 		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 		       prefix, argv);
 
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 25d0590..cf943dd 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -175,7 +175,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	parse_pathspec(&pathspec,
 		       PATHSPEC_FROMTOP |
 		       PATHSPEC_LITERAL |
-		       PATHSPEC_GLOB,
+		       PATHSPEC_GLOB |
+		       PATHSPEC_ICASE,
 		       0, prefix, argv + 1);
 	for (i = 0; i < pathspec.nr; i++)
 		pathspec.items[i].nowildcard_len = pathspec.items[i].len;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH/WIP 09/10] pathspec: support icase in match_pathspec_depth and tree_entry_interesting
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:49 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1358081379-17752-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h     | 17 +++++++++++++++++
 dir.c       | 18 +++++++++++-------
 tree-walk.c | 30 +++++++++++++++++++-----------
 3 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index c3b5585..216f87c 100644
--- a/cache.h
+++ b/cache.h
@@ -522,6 +522,23 @@ extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 
 extern int limit_pathspec_to_literal(void);
+static inline int ps_strncmp(const struct pathspec_item *item,
+			     const char *s1, const char *s2, size_t n)
+{
+	if (item->magic & PATHSPEC_ICASE)
+		return strncasecmp(s1, s2, n);
+	else
+		return strncmp(s1, s2, n);
+}
+
+static inline int ps_strcmp(const struct pathspec_item *item,
+			    const char *s1, const char *s2)
+{
+	if (item->magic & PATHSPEC_ICASE)
+		return strcasecmp(s1, s2);
+	else
+		return strcmp(s1, s2);
+}
 
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
diff --git a/dir.c b/dir.c
index d0e7ca8..e9edb65 100644
--- a/dir.c
+++ b/dir.c
@@ -42,7 +42,7 @@ inline int git_fnmatch(const struct pathspec_item *item,
 		       int prefix)
 {
 	if (prefix > 0) {
-		if (strncmp(pattern, string, prefix))
+		if (ps_strncmp(item, pattern, string, prefix))
 			return FNM_NOMATCH;
 		pattern += prefix;
 		string += prefix;
@@ -51,14 +51,16 @@ inline int git_fnmatch(const struct pathspec_item *item,
 		int pattern_len = strlen(++pattern);
 		int string_len = strlen(string);
 		return string_len < pattern_len ||
-		       strcmp(pattern,
-			      string + string_len - pattern_len);
+			ps_strcmp(item, pattern,
+				  string + string_len - pattern_len);
 	}
 	if (item->magic & PATHSPEC_GLOB)
-		return wildmatch(pattern, string, 0);
+		return wildmatch(pattern, string,
+				 item->magic & PATHSPEC_ICASE ? FNM_CASEFOLD : 0);
 	else
 		/* wildmatch has not learned no FNM_PATHNAME mode yet */
-		return fnmatch(pattern, string, 0);
+		return fnmatch(pattern, string,
+			       item->magic & PATHSPEC_ICASE ? FNM_CASEFOLD : 0);
 }
 
 static size_t common_prefix_len(const struct pathspec *pathspec)
@@ -162,7 +164,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
 	if (!*match)
 		return MATCHED_RECURSIVELY;
 
-	if (matchlen <= namelen && !strncmp(match, name, matchlen)) {
+	if (matchlen <= namelen && !ps_strncmp(item, match, name, matchlen)) {
 		if (matchlen == namelen)
 			return MATCHED_EXACTLY;
 
@@ -192,10 +194,12 @@ int match_pathspec_depth(const struct pathspec *ps,
 {
 	int i, retval = 0;
 
+	/* BUG: we should not match icase on the prefix part */
 	GUARD_PATHSPEC(ps,
 		       PATHSPEC_FROMTOP |
 		       PATHSPEC_LITERAL |
-		       PATHSPEC_GLOB);
+		       PATHSPEC_GLOB |
+		       PATHSPEC_ICASE);
 
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
diff --git a/tree-walk.c b/tree-walk.c
index 1679ce7..3d9c2ba 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -488,7 +488,8 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch
 	return retval;
 }
 
-static int match_entry(const struct name_entry *entry, int pathlen,
+static int match_entry(const struct pathspec_item *item,
+		       const struct name_entry *entry, int pathlen,
 		       const char *match, int matchlen,
 		       enum interesting *never_interesting)
 {
@@ -504,8 +505,8 @@ static int match_entry(const struct name_entry *entry, int pathlen,
 		 * Does match sort strictly earlier than path
 		 * with their common parts?
 		 */
-		m = strncmp(match, entry->path,
-			    (matchlen < pathlen) ? matchlen : pathlen);
+		m = ps_strncmp(item, match, entry->path,
+			       (matchlen < pathlen) ? matchlen : pathlen);
 		if (m < 0)
 			return 0;
 
@@ -540,7 +541,7 @@ static int match_entry(const struct name_entry *entry, int pathlen,
 		 * we cheated and did not do strncmp(), so we do
 		 * that here.
 		 */
-		m = strncmp(match, entry->path, pathlen);
+		m = ps_strncmp(item, match, entry->path, pathlen);
 
 	/*
 	 * If common part matched earlier then it is a hit,
@@ -553,10 +554,11 @@ static int match_entry(const struct name_entry *entry, int pathlen,
 	return 0;
 }
 
-static int match_dir_prefix(const char *base,
+static int match_dir_prefix(const struct pathspec_item *item,
+			    const char *base,
 			    const char *match, int matchlen)
 {
-	if (strncmp(base, match, matchlen))
+	if (ps_strncmp(item, base, match, matchlen))
 		return 0;
 
 	/*
@@ -593,7 +595,7 @@ static int match_wildcard_base(const struct pathspec_item *item,
 		 */
 		if (baselen >= matchlen) {
 			*matched = matchlen;
-			return !strncmp(base, match, matchlen);
+			return !ps_strncmp(item, base, match, matchlen);
 		}
 
 		dirlen = matchlen;
@@ -606,7 +608,7 @@ static int match_wildcard_base(const struct pathspec_item *item,
 		 * base ends with '/' so we are sure it really matches
 		 * directory
 		 */
-		if (strncmp(base, match, baselen))
+		if (ps_strncmp(item, base, match, baselen))
 			return 0;
 		*matched = baselen;
 	} else
@@ -635,10 +637,16 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 	enum interesting never_interesting = ps->has_wildcard ?
 		entry_not_interesting : all_entries_not_interesting;
 
+	/*
+	 * BUG: we should not match icase on the prefix part. The
+	 * prefix length is in 'ps'. Although using it won't be easy
+	 * as the pattern is cut into pieces...
+	 */
 	GUARD_PATHSPEC(ps,
 		       PATHSPEC_FROMTOP |
 		       PATHSPEC_LITERAL |
-		       PATHSPEC_GLOB);
+		       PATHSPEC_GLOB |
+		       PATHSPEC_ICASE);
 
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
@@ -659,7 +667,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 
 		if (baselen >= matchlen) {
 			/* If it doesn't match, move along... */
-			if (!match_dir_prefix(base_str, match, matchlen))
+			if (!match_dir_prefix(item, base_str, match, matchlen))
 				goto match_wildcards;
 
 			if (!ps->recursive || ps->max_depth == -1)
@@ -674,7 +682,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 
 		/* Either there must be no base, or the base must match. */
 		if (baselen == 0 || !strncmp(base_str, match, baselen)) {
-			if (match_entry(entry, pathlen,
+			if (match_entry(item, entry, pathlen,
 					match + baselen, matchlen - baselen,
 					&never_interesting))
 				return entry_interesting;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH/WIP 08/10] common_prefix/read_directory: treat PATHSPEC_ICASE like wildcards
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:49 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1358081379-17752-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 dir.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/dir.c b/dir.c
index 760b776..d0e7ca8 100644
--- a/dir.c
+++ b/dir.c
@@ -66,15 +66,22 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
 	int n;
 	size_t max = 0;
 
+	/*
+	 * ":(icase)path" is treated as a pathspec full of wildcard
+	 */
 	GUARD_PATHSPEC(pathspec,
 		       PATHSPEC_FROMTOP |
 		       PATHSPEC_LITERAL |
-		       PATHSPEC_GLOB);
+		       PATHSPEC_GLOB |
+		       PATHSPEC_ICASE);
 
 	for (n = 0; n < pathspec->nr; n++) {
-		size_t i = 0, len = 0;
-		while (i < pathspec->items[n].nowildcard_len &&
-		       (n == 0 || i < max)) {
+		size_t i = 0, len = 0, item_len;
+		if (pathspec->items[n].magic & PATHSPEC_ICASE)
+			item_len = pathspec->items[n].prefix;
+		else
+			item_len = pathspec->items[n].nowildcard_len;
+		while (i < item_len && (n == 0 || i < max)) {
 			char c = pathspec->items[n].match[i];
 			if (c != pathspec->items[0].match[i])
 				break;
@@ -1240,7 +1247,8 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru
 		GUARD_PATHSPEC(pathspec,
 			       PATHSPEC_FROMTOP |
 			       PATHSPEC_LITERAL |
-			       PATHSPEC_GLOB);
+			       PATHSPEC_GLOB |
+			       PATHSPEC_ICASE);
 
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH/WIP 07/10] parse_pathspec: accept :(icase)path syntax
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:49 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1358081379-17752-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h | 1 +
 setup.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 9c27f18..c3b5585 100644
--- a/cache.h
+++ b/cache.h
@@ -480,6 +480,7 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct
 #define PATHSPEC_FROMTOP    (1<<0)
 #define PATHSPEC_LITERAL    (1<<1)
 #define PATHSPEC_GLOB       (1<<2)
+#define PATHSPEC_ICASE      (1<<3)
 
 #define PATHSPEC_ONESTAR 1	/* the pathspec pattern sastisfies GFNM_ONESTAR */
 
diff --git a/setup.c b/setup.c
index b3e146d..e22abf1 100644
--- a/setup.c
+++ b/setup.c
@@ -157,7 +157,6 @@ void verify_non_filename(const char *prefix, const char *arg)
  *
  * Possible future magic semantics include stuff like:
  *
- *	{ PATHSPEC_ICASE, '\0', "icase" },
  *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
  *	{ PATHSPEC_REGEXP, '\0', "regexp" },
  *
@@ -171,6 +170,7 @@ static struct pathspec_magic {
 	{ PATHSPEC_FROMTOP, '/', "top" },
 	{ PATHSPEC_LITERAL,   0, "literal" },
 	{ PATHSPEC_GLOB,   '\0', "glob" },
+	{ PATHSPEC_ICASE,  '\0', "icase" },
 };
 
 /*
-- 
1.8.0.rc2.23.g1fb49df

^ 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