Git development
 help / color / mirror / Atom feed
* Re: Suggestion: add option in git-p4 to preserve user in Git repository
From: Olivier Delalleau @ 2013-01-13  4:56 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Luke Diamand
In-Reply-To: <20130112225640.GA23079@padd.com>

2013/1/12 Pete Wyckoff <pw@padd.com>:
> shish@keba.be wrote on Sat, 12 Jan 2013 14:44 -0500:
>> 2013/1/12 Pete Wyckoff <pw@padd.com>:
>> > shish@keba.be wrote on Thu, 10 Jan 2013 22:38 -0500:
>> >> I'm in a situation where I don't have P4 admin rights to use the
>> >> --preserve-user option of git-p4. However, I would like to keep user
>> >> information in the associated Git branch.
>> >>
>> >> Would it be possible to add an option for this?
>> >
>> > The --preserve-user option is used to submit somebody else's work
>> > from git to p4.  It does "p4 change -f" to edit the author of the
>> > change after it has been submitted to p4.  P4 requires admin
>> > privileges to do that.
>> >
>> > Changes that are imported _from_ p4 to git do have the correct
>> > author information.
>> >
>> > Can you explain a bit more what you're looking for?
>>
>> Sorry I wasn't clear enough. When "git p4 submit" submits changes from
>> Git to P4, it also edits the Git history and replaces the Git commits'
>> authors by the information from the Perforce account submitting the
>> changes. The advantage is that both the P4 and Git repositories share
>> the same author information, but in my case I would like to keep in
>> the Git repository the original authors (because the P4 account I'm
>> using to submit to P4 is shared by all Git users).
>
> Ah, I see what you're looking for now.  It's certainly possible
> to keep a mapping in the git side to remember who really wrote
> each change that went into p4, but there's nothing set up to do
> that now.  And it would be a fair amount of work, with many
> little details.
>
> You could put the true name in the commit message, like
> we do signed-off-by messages: "Author: Real Coder <rc@my.com>".
> That would keep the proper attribution, but not work with "git
> log --author", e.g.; you'd have to use "--grep='Real Coder'"
> instead.

Ok, thanks. I actually manage to hack my way around it, restoring the
author information with "git filter-branch" and overriding the remote
p4 tracking branch with "git update-ref". Did some limited testing and
it seems to work -- hopefully I won't have nasty surprises down the
road ;)

-=- Olivier

^ permalink raw reply

* [PATCH v2 0/3] pre-push hook support
From: Aaron Schrab @ 2013-01-13  5:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>

Main changes since the initial version:

 * The first patch converts the existing hook callers to use the new
   find_hook() function.
 * Information about what is to be pushed is now sent over a pipe rather
   than passed as command-line parameters.

Aaron Schrab (3):
  hooks: Add function to check if a hook exists
  push: Add support for pre-push hooks
  Add sample pre-push hook script

 Documentation/githooks.txt       |  29 +++++++++
 builtin/commit.c                 |   6 +-
 builtin/push.c                   |   1 +
 builtin/receive-pack.c           |  25 ++++----
 run-command.c                    |  15 ++++-
 run-command.h                    |   1 +
 t/t5571-pre-push-hook.sh         | 129 +++++++++++++++++++++++++++++++++++++++
 templates/hooks--pre-push.sample |  53 ++++++++++++++++
 transport.c                      |  60 ++++++++++++++++++
 transport.h                      |   1 +
 10 files changed, 300 insertions(+), 20 deletions(-)
 create mode 100755 t/t5571-pre-push-hook.sh
 create mode 100644 templates/hooks--pre-push.sample

-- 
1.8.1.340.g425b78d

^ permalink raw reply

* [PATCH v2 1/3] hooks: Add function to check if a hook exists
From: Aaron Schrab @ 2013-01-13  5:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>

Create find_hook() function to determine if a given hook exists and is
executable.  If it is, the path to the script will be returned,
otherwise NULL is returned.

This encapsulates the tests that are used to check for the existence of
a hook in one place, making it easier to modify those checks if that is
found to be necessary.  This also makes it simple for places that can
use a hook to check if a hook exists before doing, possibly lengthy,
setup work which would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, immediately added to an argv_array list
which would result in it being duplicated at that point, or used to
actually run the command without much intervening work.  Callers which
need to hold onto the returned value for a longer time are expected to
duplicate the return value themselves.

Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
 builtin/commit.c       |  6 ++----
 builtin/receive-pack.c | 25 +++++++++++--------------
 run-command.c          | 15 +++++++++++++--
 run-command.h          |  1 +
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..65d08d2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1327,8 +1327,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);
 }
 
-static const char post_rewrite_hook[] = "hooks/post-rewrite";
-
 static int run_rewrite_hook(const unsigned char *oldsha1,
 			    const unsigned char *newsha1)
 {
@@ -1339,10 +1337,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 	int code;
 	size_t n;
 
-	if (access(git_path(post_rewrite_hook), X_OK) < 0)
+	argv[0] = find_hook("post-rewrite");
+	if (!argv[0])
 		return 0;
 
-	argv[0] = git_path(post_rewrite_hook);
 	argv[1] = "amend";
 	argv[2] = NULL;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ff781fe..e8878de 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -182,9 +182,6 @@ struct command {
 	char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static const char pre_receive_hook[] = "hooks/pre-receive";
-static const char post_receive_hook[] = "hooks/post-receive";
-
 static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
@@ -242,10 +239,10 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
 	const char *argv[2];
 	int code;
 
-	if (access(hook_name, X_OK) < 0)
+	argv[0] = find_hook(hook_name);
+	if (!argv[0])
 		return 0;
 
-	argv[0] = hook_name;
 	argv[1] = NULL;
 
 	memset(&proc, 0, sizeof(proc));
@@ -331,15 +328,14 @@ static int run_receive_hook(struct command *commands, const char *hook_name,
 
 static int run_update_hook(struct command *cmd)
 {
-	static const char update_hook[] = "hooks/update";
 	const char *argv[5];
 	struct child_process proc;
 	int code;
 
-	if (access(update_hook, X_OK) < 0)
+	argv[0] = find_hook("update");
+	if (!argv[0])
 		return 0;
 
-	argv[0] = update_hook;
 	argv[1] = cmd->ref_name;
 	argv[2] = sha1_to_hex(cmd->old_sha1);
 	argv[3] = sha1_to_hex(cmd->new_sha1);
@@ -532,24 +528,25 @@ static const char *update(struct command *cmd)
 	}
 }
 
-static char update_post_hook[] = "hooks/post-update";
-
 static void run_update_post_hook(struct command *commands)
 {
 	struct command *cmd;
 	int argc;
 	const char **argv;
 	struct child_process proc;
+	char *hook;
 
+	hook = find_hook("post-update");
 	for (argc = 0, cmd = commands; cmd; cmd = cmd->next) {
 		if (cmd->error_string || cmd->did_not_exist)
 			continue;
 		argc++;
 	}
-	if (!argc || access(update_post_hook, X_OK) < 0)
+	if (!argc || !hook)
 		return;
+
 	argv = xmalloc(sizeof(*argv) * (2 + argc));
-	argv[0] = update_post_hook;
+	argv[0] = hook;
 
 	for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {
 		char *p;
@@ -704,7 +701,7 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 				       0, &cmd))
 		set_connectivity_errors(commands);
 
-	if (run_receive_hook(commands, pre_receive_hook, 0)) {
+	if (run_receive_hook(commands, "pre-receive", 0)) {
 		for (cmd = commands; cmd; cmd = cmd->next) {
 			if (!cmd->error_string)
 				cmd->error_string = "pre-receive hook declined";
@@ -994,7 +991,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 			unlink_or_warn(pack_lockfile);
 		if (report_status)
 			report(commands, unpack_status);
-		run_receive_hook(commands, post_receive_hook, 1);
+		run_receive_hook(commands, "post-receive", 1);
 		run_update_post_hook(commands);
 		if (auto_gc) {
 			const char *argv_gc_auto[] = {
diff --git a/run-command.c b/run-command.c
index 0471219..12d4ddb 100644
--- a/run-command.c
+++ b/run-command.c
@@ -735,6 +735,15 @@ int finish_async(struct async *async)
 #endif
 }
 
+char *find_hook(const char *name)
+{
+	char *path = git_path("hooks/%s", name);
+	if (access(path, X_OK) < 0)
+		path = NULL;
+
+	return path;
+}
+
 int run_hook(const char *index_file, const char *name, ...)
 {
 	struct child_process hook;
@@ -744,11 +753,13 @@ int run_hook(const char *index_file, const char *name, ...)
 	va_list args;
 	int ret;
 
-	if (access(git_path("hooks/%s", name), X_OK) < 0)
+	p = find_hook(name);
+	if (!p)
 		return 0;
 
+	argv_array_push(&argv, p);
+
 	va_start(args, name);
-	argv_array_push(&argv, git_path("hooks/%s", name));
 	while ((p = va_arg(args, const char *)))
 		argv_array_push(&argv, p);
 	va_end(args);
diff --git a/run-command.h b/run-command.h
index 850c638..221ce33 100644
--- a/run-command.h
+++ b/run-command.h
@@ -45,6 +45,7 @@ int start_command(struct child_process *);
 int finish_command(struct child_process *);
 int run_command(struct child_process *);
 
+extern char *find_hook(const char *name);
 extern int run_hook(const char *index_file, const char *name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
-- 
1.8.1.340.g425b78d

^ permalink raw reply related

* [PATCH v2 2/3] push: Add support for pre-push hooks
From: Aaron Schrab @ 2013-01-13  5:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>

Add support for a pre-push hook which can be used to determine if the
set of refs to be pushed is suitable for the target repository.  The
hook is run with two arguments specifying the name and location of the
destination repository.

Information about what is to be pushed is provided by sending lines of
the following form to the hook's standard input:

  <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF

If the hook exits with a non-zero status, the push will be aborted.

This will allow the script to determine if the push is acceptable based
on the target repository and branch(es), the commits which are to be
pushed, and even the source branches in some cases.

Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
 Documentation/githooks.txt |  29 ++++++++++
 builtin/push.c             |   1 +
 t/t5571-pre-push-hook.sh   | 129 +++++++++++++++++++++++++++++++++++++++++++++
 transport.c                |  60 +++++++++++++++++++++
 transport.h                |   1 +
 5 files changed, 220 insertions(+)
 create mode 100755 t/t5571-pre-push-hook.sh

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..d839233 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -176,6 +176,35 @@ save and restore any form of metadata associated with the working tree
 (eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+pre-push
+~~~~~~~~
+
+This hook is called by 'git push' and can be used to prevent a push from taking
+place.  The hook is called with two parameters which provide the name and
+location of the destination remote, if a named remote is not being used both
+values will be the same.
+
+Information about what is to be pushed is provided on the hook's standard
+input with lines of the form:
+
+  <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF
+
+For instance, if the command +git push origin master:foreign+ were run the
+hook would receive a line like the following:
+
+  refs/heads/master 67890 refs/heads/foreign 12345
+
+although the full, 40-character SHA1s would be supplied.  If the foreign ref
+does not yet exist the `<remote SHA1>` will be 40 `0`.  If a ref is to be
+deleted, the `<local ref>` will be supplied as `(delete)` and the `<local
+SHA1>` will be 40 `0`.  If the local commit was specified by something other
+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be
+supplied as it was originally given.
+
+If this hook exits with a non-zero status, 'git push' will abort without
+pushing anything.  Information about why the push is rejected may be sent
+to the user by writing to standard error.
+
 [[pre-receive]]
 pre-receive
 ~~~~~~~~~~~
diff --git a/builtin/push.c b/builtin/push.c
index 8491e43..b158028 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "progress", &progress, N_("force progress reporting")),
 		OPT_BIT(0, "prune", &flags, N_("prune locally removed refs"),
 			TRANSPORT_PUSH_PRUNE),
+		OPT_BIT(0, "no-verify", &flags, N_("bypass pre-push hook"), TRANSPORT_PUSH_NO_HOOK),
 		OPT_END()
 	};
 
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
new file mode 100755
index 0000000..d68fed7
--- /dev/null
+++ b/t/t5571-pre-push-hook.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='check pre-push hooks'
+. ./test-lib.sh
+
+# Setup hook that always succeeds
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/pre-push"
+mkdir -p "$HOOKDIR"
+write_script "$HOOK" <<EOF
+exit 0
+EOF
+
+test_expect_success 'setup' '
+	git config push.default upstream &&
+	git init --bare repo1 &&
+	git remote add parent1 repo1 &&
+	test_commit one &&
+	git push parent1 HEAD:foreign
+'
+write_script "$HOOK" <<EOF
+exit 1
+EOF
+
+COMMIT1="$(git rev-parse HEAD)"
+export COMMIT1
+
+test_expect_success 'push with failing hook' '
+	test_commit two &&
+	test_must_fail git push parent1 HEAD
+'
+
+test_expect_success '--no-verify bypasses hook' '
+	git push --no-verify parent1 HEAD
+'
+
+COMMIT2="$(git rev-parse HEAD)"
+export COMMIT2
+
+write_script "$HOOK" <<'EOF'
+echo "$1" >actual
+echo "$2" >>actual
+cat >>actual
+EOF
+
+cat >expected <<EOF
+parent1
+repo1
+refs/heads/master $COMMIT2 refs/heads/foreign $COMMIT1
+EOF
+
+test_expect_success 'push with hook' '
+	git push parent1 master:foreign &&
+	diff expected actual
+'
+
+test_expect_success 'add a branch' '
+	git checkout -b other parent1/foreign &&
+	test_commit three
+'
+
+COMMIT3="$(git rev-parse HEAD)"
+export COMMIT3
+
+cat >expected <<EOF
+parent1
+repo1
+refs/heads/other $COMMIT3 refs/heads/foreign $COMMIT2
+EOF
+
+test_expect_success 'push to default' '
+	git push &&
+	diff expected actual
+'
+
+cat >expected <<EOF
+parent1
+repo1
+refs/tags/one $COMMIT1 refs/tags/tag1 $_z40
+HEAD~ $COMMIT2 refs/heads/prev $_z40
+EOF
+
+test_expect_success 'push non-branches' '
+	git push parent1 one:tag1 HEAD~:refs/heads/prev &&
+	diff expected actual
+'
+
+cat >expected <<EOF
+parent1
+repo1
+(delete) $_z40 refs/heads/prev $COMMIT2
+EOF
+
+test_expect_success 'push delete' '
+	git push parent1 :prev &&
+	diff expected actual
+'
+
+cat >expected <<EOF
+repo1
+repo1
+HEAD $COMMIT3 refs/heads/other $_z40
+EOF
+
+test_expect_success 'push to URL' '
+	git push repo1 HEAD &&
+	diff expected actual
+'
+
+# Test that filling pipe buffers doesn't cause failure
+# Too slow to leave enabled for general use
+if false
+then
+	printf 'parent1\nrepo1\n' >expected
+	nr=1000
+	while test $nr -lt 2000
+	do
+		nr=$(( $nr + 1 ))
+		git branch b/$nr $COMMIT3
+		echo "refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40" >>expected
+	done
+
+	test_expect_success 'push many refs' '
+		git push parent1 "refs/heads/b/*:refs/heads/b/*" &&
+		diff expected actual
+	'
+fi
+
+test_done
diff --git a/transport.c b/transport.c
index 2673d27..0750a5f 100644
--- a/transport.c
+++ b/transport.c
@@ -1034,6 +1034,62 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing)
 	die("Aborting.");
 }
 
+static int run_pre_push_hook(struct transport *transport,
+			     struct ref *remote_refs)
+{
+	int ret = 0, x;
+	struct ref *r;
+	struct child_process proc;
+	struct strbuf buf;
+	const char *argv[4];
+
+	if (!(argv[0] = find_hook("pre-push")))
+		return 0;
+
+	argv[1] = transport->remote->name;
+	argv[2] = transport->url;
+	argv[3] = NULL;
+
+	memset(&proc, 0, sizeof(proc));
+	proc.argv = argv;
+	proc.in = -1;
+
+	if (start_command(&proc)) {
+		finish_command(&proc);
+		return -1;
+	}
+
+	strbuf_init(&buf, 256);
+
+	for (r = remote_refs; r; r = r->next) {
+		if (!r->peer_ref) continue;
+		if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
+		if (r->status == REF_STATUS_UPTODATE) continue;
+
+		strbuf_reset(&buf);
+		strbuf_addf( &buf, "%s %s %s %s\n",
+			 r->peer_ref->name, sha1_to_hex(r->new_sha1),
+			 r->name, sha1_to_hex(r->old_sha1));
+
+		if (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {
+			ret = -1;
+			break;
+		}
+	}
+
+	strbuf_release(&buf);
+
+	x = close(proc.in);
+	if (!ret)
+		ret = x;
+
+	x = finish_command(&proc);
+	if (!ret)
+		ret = x;
+
+	return ret;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   unsigned int *reject_reasons)
@@ -1074,6 +1130,10 @@ int transport_push(struct transport *transport,
 			flags & TRANSPORT_PUSH_MIRROR,
 			flags & TRANSPORT_PUSH_FORCE);
 
+		if (!(flags & TRANSPORT_PUSH_NO_HOOK))
+			if (run_pre_push_hook(transport, remote_refs))
+				return -1;
+
 		if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
 			struct ref *ref = remote_refs;
 			for (; ref; ref = ref->next)
diff --git a/transport.h b/transport.h
index bfd2df5..ac5a9f5 100644
--- a/transport.h
+++ b/transport.h
@@ -104,6 +104,7 @@ struct transport {
 #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
 #define TRANSPORT_PUSH_PRUNE 128
 #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256
+#define TRANSPORT_PUSH_NO_HOOK 512
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
-- 
1.8.1.340.g425b78d

^ permalink raw reply related

* [PATCH v2 3/3] Add sample pre-push hook script
From: Aaron Schrab @ 2013-01-13  5:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>

Create a sample of a script for a pre-push hook.  The main purpose is to
illustrate how a script may parse the information which is supplied to
such a hook.  The script may also be useful to some people as-is for
avoiding to push commits which are marked as a work in progress.

Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
 templates/hooks--pre-push.sample | 53 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 templates/hooks--pre-push.sample

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
new file mode 100644
index 0000000..15ab6d8
--- /dev/null
+++ b/templates/hooks--pre-push.sample
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+# An example hook script to verify what is about to be pushed.  Called by "git
+# push" after it has checked the remote status, but before anything has been
+# pushed.  If this script exits with a non-zero status nothing will be pushed.
+#
+# This hook is called with the following parameters:
+#
+# $1 -- Name of the remote to which the push is being done
+# $2 -- URL to which the push is being done
+#
+# If pushing without using a named remote those arguments will be equal.
+#
+# Information about the commits which are being pushed is supplied as lines to
+# the standard input in the form:
+#
+#   <local ref> <local sha1> <remote ref> <remote sha1>
+#
+# This sample shows how to prevent push of commits where the log message starts
+# with "WIP" (work in progress).
+
+remote="$1"
+url="$2"
+
+z40=0000000000000000000000000000000000000000
+
+IFS=' '
+while read local_ref local_sha remote_ref remote_sha
+do
+	if [ "$local_sha" = $z40 ]
+	then
+		# Handle delete
+	else
+		if [ "$remote_sha" = $z40 ]
+		then
+			# New branch, examine all commits
+			range="$local_sha"
+		else
+			# Update to existing branch, examine new commits
+			range="$remote_sha..$local_sha"
+		fi
+
+		# Check for WIP commit
+		commit=`git rev-list -n 1 --grep '^WIP' "$range"`
+		if [ -n "$commit" ]
+		then
+			echo "Found WIP commit in $local_ref, not pushing"
+			exit 1
+		fi
+	fi
+done
+
+exit 0
-- 
1.8.1.340.g425b78d

^ permalink raw reply related

* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-13 10:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vvcb37xfe.fsf@alter.siamese.dyndns.org>

On 12.01.13 07:00, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> The test Makefile has a default set of lint tests which are run
>> as part of "make test".
>>
>> The macro TEST_LINT defaults to "test-lint-duplicates test-lint-executable".
>>
>> Add test-lint-shell-syntax here, to detect non-portable shell syntax early.
>>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
> 
> As I said already, I do not want to do this yet without further
> reduction of false positives.
Which reinds me that the expression fishing for "which" is really poor.

How about something like the following:

-- >8 --
Subject: [PATCH] Reduce false positive in check-non-portable-shell.pl

check-non-portable-shell.pl is using simple regular expressions to
find illegal shell syntax.
Improve the expressions and reduce the chance for false positves:

"sed -i" must be followed by 1..n whitespace and 1 non whitespace
"declare" must be followed by 1..n whitespace and 1 non whitespace
"echo -n" must be followed by 1..n whitespace and 1 non whitespace
"which" must be followed by 1..n whitespace, a string, "end of line"



diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 8b5a71d..7151dd6 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -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)';
 	/test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
 	# this resets our $. for each file
 	close ARGV if eof;

^ permalink raw reply related

* git list files
From: Стойчо Слепцов @ 2013-01-13 12:05 UTC (permalink / raw)
  To: git

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:

----------------
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
d53a3503 pclouds  2012-06-07 19:05 Remove-i18n-legos-in-notifying-n branch.c
f9a482e6 peff     2012-03-26 19:51 checkout-suppress-tracking-messa branch.h
c566ea13 gitster  2013-01-11 18:34 Merge-branch-jc-merge-blobs      builtin
cf6c52fc gitster  2013-01-10 13:46 Merge-branch-jc-maint-fmt-merge- builtin.h
568508e7 gitster  2011-10-28 14:48 bulk-checkin-replace-fast-import
bulk-checkin.c
568508e7 gitster  2011-10-28 14:48 bulk-checkin-replace-fast-import
bulk-checkin.h
8c3710fd gitster  2012-06-04 11:51 tweak-bundle-verify-of-a-complet bundle.c
b76c561a gitster  2011-10-21 16:04 Merge-branch-jc-unseekable-bundl bundle.h
----------------

(pretty the same idea as what we see in github when reviewing a
repository under the "Files" tab.)

Unfortunately I couldn't find any suitable.

As suggested at http://git-scm.com/community I asked my question at
the "Git user mailing list on Google Groups which is a nice place for
beginners to ask about anything",
and one of the valuable answers was:

"Also I wouldn't hesitate to ask this question on the main Git list as
this question appears to be hard-core enough to warrant assisting of
someone knowledgeable about Git internals."

So here I am...

So is there such a command, or I have to build my own script, starting
from, lets say git-rev-list in addition with some diff?

At the beginning I was hoping that $git rev-list HEAD --no-walk
--all-match -- <paths> + some git status --porcelain could do the job
for me,
but seems git rev-list, same as git log stops at the first found
matching commit, without to take care that there are still more files
unsatisfied in the list...

isn't it supposed to satisfy all the files in the list when
--all-match -- <paths> are given?

Thank you in advance,
Blind.

^ permalink raw reply

* Re: missing objects -- prevention
From: Jeff King @ 2013-01-13 12:30 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Git Mailing List
In-Reply-To: <CAMK1S_iKARYqi_Dv90og0No7NN=WxFg+ixmRvnkvfdrcOi1r=Q@mail.gmail.com>

On Sun, Jan 13, 2013 at 06:26:53AM +0530, Sitaram Chamarty wrote:

> > Right, I meant if you have receive.fsckObjects on. It won't help this
> > situation at all, as we already do a connectivity check separate from
> > the fsck. But I do recommend it in general, just because it helps catch
> > bad objects before they gets disseminated to a wider audience (at which
> > point it is often infeasible to rewind history). And it has found git
> > bugs (e.g., null sha1s in tree entries).
> 
> I will add this.  Any idea if there's a significant performance hit?

Not usually; we are already resolving all of the sent deltas as a
precaution, anyway. I do notice after a push to GitHub there is
sometimes a second or two of pause from the server before the push
status is shown. But I haven't narrowed it down to fsck (versus
connectivity check, versus our post-receive hook).

So you may want to keep an eye on the effects (and if you have numbers,
please share :) ).

> That's always the hard part.  System admins (at the Unix level) insist
> there's nothing wrong and no disk errors and so on...  that is why I
> was interested in network errors causing problems and so on.

Yeah, I feel bad saying "well, this repo is totally corrupted, but it
couldn't possibly be git's fault, because that's not what its failure
modes look like". But luckily our Ops people are very understanding, and
most of the problems I have seen have turned out to be fs corruption
after all (the pack-refs things is the big exception).

> Thanks once again for your patient replies!

No problem. There aren't many people dealing with large-scale
server-side issues, so it's something that doesn't come up much on the
list. I'm happy to talk about it.

-Peff

^ permalink raw reply

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

On Sun, Jan 13, 2013 at 12:41:30AM +0000, John Keeping wrote:
> 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.

Having thought about this a bit more, another possibility would be to
apply this transformation once using something like this (completely
untested, I haven't looked up the keys of interest):

-- >8 --

def _noop(s):
    return s

def _decode(s):
    return s.decode('utf-8')

CONVERSION_MAP = {
    'user': _decode,
    'data': _decode
}

d = marshal.load(p4.stdout)
retval = {}
for k, v in d.items():
    key = k.decode('utf-8')
    retval[key] = CONVERSION_MAP.get(key, _noop)(v)
return retval

-- 8< --

Obviously this isn't ideal but without p4 gaining a Python 3 output mode
I suspect this would be the best we could do.


John

^ permalink raw reply

* [PATCH v3 00/31] nd/parse-pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Changes from v2 (it's hard to keep track of after the rebase, so I may
be missing something here):

 - rebased on top of recent master, incorporate changes in
   init_pathspec from jk/pathspec-literal and nd/pathspec-wildcard to
   parse_pathspec

 - kill strip_trailing_slash_from_submodules and treat_gitlinks
   (pretty sure it'll cause conflicts with as/check-ignore)

 - kill init_pathspec, match_pathspec, diff_tree_setup_paths and
   diff_tree_release_paths
 
 - check points for future pathspec development

As far as I understand the "pathspec unification", I'd say we are
there, with a few exceptions like "mv", external commands.. But those
are pretty much isolated.

I'll send another WIP series implementing :(icase) and :(glob), mainly
to show (me) how future pathspec feature development looks like after
this.

Nguyễn Thái Ngọc Duy (31):
  clean: remove unused variable "seen"
  Add copy_pathspec
  Add parse_pathspec() that converts cmdline args to struct pathspec
  parse_pathspec: save original pathspec for reporting
  Export parse_pathspec() and convert some get_pathspec() calls
  Guard against new pathspec magic in pathspec matching code
  clean: convert to use parse_pathspec
  parse_pathspec: add PATHSPEC_EMPTY_MATCH_ALL
  commit: convert to use parse_pathspec
  status: convert to use parse_pathspec
  rerere: convert to use parse_pathspec
  checkout: convert to use parse_pathspec
  rm: convert to use parse_pathspec
  parse_pathspec: support stripping submodule trailing slashes
  ls-files: convert to use parse_pathspec
  archive: convert to use parse_pathspec
  parse_pathspec: support stripping/checking submodule paths
  add: convert to use parse_pathspec
  Convert read_cache_preload() to take struct pathspec
  Convert unmerge_cache to take struct pathspec
  checkout: convert read_tree_some to take struct pathspec
  Convert report_path_error to take struct pathspec
  Convert refresh_index to take struct pathspec
  Convert {read,fill}_directory to take struct pathspec
  Convert add_files_to_cache to take struct pathspec
  Convert common_prefix() to use struct pathspec
  Remove diff_tree_{setup,release}_paths
  Remove init_pathspec() in favor of parse_pathspec()
  Remove match_pathspec() in favor of match_pathspec_depth()
  tree-diff: remove the use of pathspec's raw[] in follow-rename codepath
  Rename field "raw" to "_raw" in struct pathspec

 archive.c              |  18 +++--
 archive.h              |   2 +-
 builtin/add.c          | 155 +++++++++++++++----------------------
 builtin/blame.c        |  12 +--
 builtin/checkout.c     |  44 ++++++-----
 builtin/clean.c        |  21 ++---
 builtin/commit.c       |  37 +++++----
 builtin/diff-files.c   |   2 +-
 builtin/diff-index.c   |   2 +-
 builtin/diff.c         |   6 +-
 builtin/grep.c         |   6 +-
 builtin/log.c          |   2 +-
 builtin/ls-files.c     |  72 +++++++-----------
 builtin/ls-tree.c      |  10 ++-
 builtin/mv.c           |  13 ++--
 builtin/rerere.c       |   6 +-
 builtin/reset.c        |   4 +-
 builtin/rm.c           |  23 +++---
 builtin/update-index.c |   3 +-
 cache.h                |  36 +++++++--
 diff-lib.c             |   2 +-
 diff.h                 |   2 -
 dir.c                  | 202 ++++++++-----------------------------------------
 dir.h                  |   9 ++-
 merge-recursive.c      |   2 +-
 notes-merge.c          |   4 +-
 preload-index.c        |  20 ++---
 read-cache.c           |   5 +-
 rerere.c               |   6 +-
 rerere.h               |   4 +-
 resolve-undo.c         |   4 +-
 resolve-undo.h         |   2 +-
 revision.c             |  11 +--
 setup.c                | 149 ++++++++++++++++++++++++++++++------
 tree-diff.c            |  47 +++++++-----
 tree-walk.c            |   2 +
 tree.c                 |   4 +-
 tree.h                 |   2 +-
 wt-status.c            |  17 ++---
 wt-status.h            |   2 +-
 40 files changed, 460 insertions(+), 510 deletions(-)

-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply

* [PATCH v3 01/31] clean: remove unused variable "seen"
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clean.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 69c1cda..4cdabe0 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -46,7 +46,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	struct strbuf buf = STRBUF_INIT;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	const char *qname;
-	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("do not print names of files removed")),
 		OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -105,9 +104,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	fill_directory(&dir, pathspec);
 
-	if (pathspec)
-		seen = xmalloc(argc > 0 ? argc : 1);
-
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		int len, pos;
@@ -141,11 +137,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		if (lstat(ent->name, &st))
 			continue;
 
-		if (pathspec) {
-			memset(seen, 0, argc > 0 ? argc : 1);
+		if (pathspec)
 			matches = match_pathspec(pathspec, ent->name, len,
-						 0, seen);
-		}
+						 0, NULL);
 
 		if (S_ISDIR(st.st_mode)) {
 			strbuf_addstr(&directory, ent->name);
@@ -184,7 +178,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			}
 		}
 	}
-	free(seen);
 
 	strbuf_release(&directory);
 	string_list_clear(&exclude_list, 0);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 02/31] Add copy_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 13 +++++++------
 cache.h      |  1 +
 dir.c        |  8 ++++++++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 034fec9..16ce99b 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -15,8 +15,9 @@ static const char * const builtin_mv_usage[] = {
 	NULL
 };
 
-static const char **copy_pathspec(const char *prefix, const char **pathspec,
-				  int count, int base_name)
+static const char **internal_copy_pathspec(const char *prefix,
+					   const char **pathspec,
+					   int count, int base_name)
 {
 	int i;
 	const char **result = xmalloc((count + 1) * sizeof(const char *));
@@ -81,17 +82,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	if (read_cache() < 0)
 		die(_("index file corrupt"));
 
-	source = copy_pathspec(prefix, argv, argc, 0);
+	source = internal_copy_pathspec(prefix, argv, argc, 0);
 	modes = xcalloc(argc, sizeof(enum update_mode));
-	dest_path = copy_pathspec(prefix, argv + argc, 1, 0);
+	dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);
 
 	if (dest_path[0][0] == '\0')
 		/* special case: "." was normalized to "" */
-		destination = copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
 	else if (!lstat(dest_path[0], &st) &&
 			S_ISDIR(st.st_mode)) {
 		dest_path[0] = add_slash(dest_path[0]);
-		destination = copy_pathspec(dest_path[0], argv, argc, 1);
+		destination = internal_copy_pathspec(dest_path[0], argv, argc, 1);
 	} else {
 		if (argc != 1)
 			die("destination '%s' is not a directory", dest_path[0]);
diff --git a/cache.h b/cache.h
index c257953..72675a1 100644
--- a/cache.h
+++ b/cache.h
@@ -491,6 +491,7 @@ struct pathspec {
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 
diff --git a/dir.c b/dir.c
index e883a91..12a76d7 100644
--- a/dir.c
+++ b/dir.c
@@ -1559,6 +1559,14 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 	return 0;
 }
 
+void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
+{
+	*dst = *src;
+	dst->items = xmalloc(sizeof(struct pathspec_item) * dst->nr);
+	memcpy(dst->items, src->items,
+	       sizeof(struct pathspec_item) * dst->nr);
+}
+
 void free_pathspec(struct pathspec *pathspec)
 {
 	free(pathspec->items);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 03/31] Add parse_pathspec() that converts cmdline args to struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

Currently to fill a struct pathspec, we do:

   const char **paths;
   paths = get_pathspec(prefix, argv);
   ...
   init_pathspec(&pathspec, paths);

"paths" can only carry bare strings, which loses information from
command line arguments such as pathspec magic or the prefix part's
length for each argument.

parse_pathspec() is introduced to combine the two calls into one. The
plan is gradually replace all get_pathspec() and init_pathspec() with
parse_pathspec(). get_pathspec() now becomes a thin wrapper of
parse_pathspec().

parse_pathspec() allows the caller to reject the pathspec magics that
it does not support. When a new pathspec magic is introduced, we can
enable it per command after making sure that all underlying code has no
problem with the new magic.

"flags" parameter is currently unused. But it would allow callers to
pass certain instructions to parse_pathspec, for example forcing
literal pathspec when no magic is used.

With the introduction of parse_pathspec, there are now two functions
that can initialize struct pathspec: init_pathspec and
parse_pathspec. Any semantic changes in struct pathspec must be
reflected in both functions. init_pathspec() will be phased out in
favor of parse_pathspec().

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

diff --git a/cache.h b/cache.h
index 72675a1..759c62a 100644
--- a/cache.h
+++ b/cache.h
@@ -481,9 +481,11 @@ struct pathspec {
 	int nr;
 	unsigned int has_wildcard:1;
 	unsigned int recursive:1;
+	unsigned magic;
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
+		unsigned magic;
 		int len;
 		int nowildcard_len;
 		int flags;
diff --git a/dir.c b/dir.c
index 12a76d7..8454c13 100644
--- a/dir.c
+++ b/dir.c
@@ -323,7 +323,7 @@ int match_pathspec_depth(const struct pathspec *ps,
 /*
  * Return the length of the "simple" part of a path match limiter.
  */
-static int simple_length(const char *match)
+int simple_length(const char *match)
 {
 	int len = -1;
 
@@ -335,7 +335,7 @@ static int simple_length(const char *match)
 	}
 }
 
-static int no_wildcard(const char *string)
+int no_wildcard(const char *string)
 {
 	return string[simple_length(string)] == '\0';
 }
diff --git a/dir.h b/dir.h
index ae1bc46..0cf5ccf 100644
--- a/dir.h
+++ b/dir.h
@@ -88,6 +88,8 @@ struct dir_struct {
 #define MATCHED_RECURSIVELY 1
 #define MATCHED_FNMATCH 2
 #define MATCHED_EXACTLY 3
+extern int simple_length(const char *match);
+extern int no_wildcard(const char *string);
 extern char *common_prefix(const char **pathspec);
 extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);
 extern int match_pathspec_depth(const struct pathspec *pathspec,
diff --git a/setup.c b/setup.c
index f108c4b..92adefc 100644
--- a/setup.c
+++ b/setup.c
@@ -174,7 +174,7 @@ static struct pathspec_magic {
 
 /*
  * Take an element of a pathspec and check for magic signatures.
- * Append the result to the prefix.
+ * Append the result to the prefix. Return the magic bitmap.
  *
  * For now, we only parse the syntax and throw out anything other than
  * "top" magic.
@@ -185,10 +185,14 @@ static struct pathspec_magic {
  * the prefix part must always match literally, and a single stupid
  * string cannot express such a case.
  */
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
+static unsigned prefix_pathspec(struct pathspec_item *item,
+				const char **raw, unsigned flags,
+				const char *prefix, int prefixlen,
+				const char *elt)
 {
 	unsigned magic = 0;
 	const char *copyfrom = elt;
+	char *match;
 	int i;
 
 	if (elt[0] != ':') {
@@ -241,39 +245,95 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
 	}
 
 	if (magic & PATHSPEC_FROMTOP)
-		return xstrdup(copyfrom);
+		match = xstrdup(copyfrom);
 	else
-		return prefix_path(prefix, prefixlen, copyfrom);
+		match = prefix_path(prefix, prefixlen, copyfrom);
+	*raw = item->match = match;
+	item->len = strlen(item->match);
+	item->flags = 0;
+	if (limit_pathspec_to_literal())
+		item->nowildcard_len = item->len;
+	else
+		item->nowildcard_len = simple_length(item->match);
+	if (item->nowildcard_len < item->len &&
+	    item->match[item->nowildcard_len] == '*' &&
+	    no_wildcard(item->match + item->nowildcard_len + 1))
+		item->flags |= PATHSPEC_ONESTAR;
+	return magic;
 }
 
-const char **get_pathspec(const char *prefix, const char **pathspec)
+static int pathspec_item_cmp(const void *a_, const void *b_)
 {
-	const char *entry = *pathspec;
-	const char **src, **dst;
-	int prefixlen;
+	struct pathspec_item *a, *b;
 
-	if (!prefix && !entry)
-		return NULL;
+	a = (struct pathspec_item *)a_;
+	b = (struct pathspec_item *)b_;
+	return strcmp(a->match, b->match);
+}
+
+/*
+ * Given command line arguments and a prefix, convert the input to
+ * pathspec. die() if any magic other than ones in magic_mask.
+ */
+static void parse_pathspec(struct pathspec *pathspec,
+			   unsigned magic_mask, unsigned flags,
+			   const char *prefix, const char **argv)
+{
+	struct pathspec_item *item;
+	const char *entry = *argv;
+	int i, n, prefixlen;
+
+	memset(pathspec, 0, sizeof(*pathspec));
+
+	/* No arguments, no prefix -> no pathspec */
+	if (!entry && !prefix)
+		return;
 
+	/* No arguments with prefix -> prefix pathspec */
 	if (!entry) {
-		static const char *spec[2];
-		spec[0] = prefix;
-		spec[1] = NULL;
-		return spec;
+		static const char *raw[2];
+
+		pathspec->items = item = xmalloc(sizeof(*item));
+		item->match = prefix;
+		item->nowildcard_len = item->len = strlen(prefix);
+		raw[0] = prefix;
+		raw[1] = NULL;
+		pathspec->nr = 1;
+		pathspec->raw = raw;
+		return;
 	}
 
-	/* Otherwise we have to re-write the entries.. */
-	src = pathspec;
-	dst = pathspec;
+	n = 0;
+	while (argv[n])
+		n++;
+
+	pathspec->nr = n;
+	pathspec->items = item = xmalloc(sizeof(*item) * n);
+	pathspec->raw = argv;
 	prefixlen = prefix ? strlen(prefix) : 0;
-	while (*src) {
-		*(dst++) = prefix_pathspec(prefix, prefixlen, *src);
-		src++;
+
+	for (i = 0; i < n; i++) {
+		const char *arg = argv[i];
+
+		item[i].magic = prefix_pathspec(item + i, argv + i, flags,
+						prefix, prefixlen, arg);
+		if (item[i].magic & ~magic_mask)
+			die(_("pathspec magic in '%s' is not supported"
+			      " by this command"), arg);
+		if (item[i].nowildcard_len < item[i].len)
+			pathspec->has_wildcard = 1;
+		pathspec->magic |= item[i].magic;
 	}
-	*dst = NULL;
-	if (!*pathspec)
-		return NULL;
-	return pathspec;
+
+	qsort(pathspec->items, pathspec->nr,
+	      sizeof(struct pathspec_item), pathspec_item_cmp);
+}
+
+const char **get_pathspec(const char *prefix, const char **pathspec)
+{
+	struct pathspec ps;
+	parse_pathspec(&ps, PATHSPEC_FROMTOP, 0, prefix, pathspec);
+	return ps.raw;
 }
 
 /*
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 04/31] parse_pathspec: save original pathspec for reporting
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

We usually use pathspec_item's match field for pathspec error
reporting. However "match" (or "raw") does not show the magic part,
which will play more important role later on. Preserve exact user
input for reporting.

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

diff --git a/cache.h b/cache.h
index 759c62a..136e4c7 100644
--- a/cache.h
+++ b/cache.h
@@ -485,6 +485,7 @@ struct pathspec {
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
+		const char *original;
 		unsigned magic;
 		int len;
 		int nowildcard_len;
diff --git a/dir.c b/dir.c
index 8454c13..beb7532 100644
--- a/dir.c
+++ b/dir.c
@@ -1538,6 +1538,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 		const char *path = paths[i];
 
 		item->match = path;
+		item->original = path;
 		item->len = strlen(path);
 		item->flags = 0;
 		if (limit_pathspec_to_literal()) {
diff --git a/setup.c b/setup.c
index 92adefc..a1ad012 100644
--- a/setup.c
+++ b/setup.c
@@ -249,6 +249,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 	else
 		match = prefix_path(prefix, prefixlen, copyfrom);
 	*raw = item->match = match;
+	item->original = elt;
 	item->len = strlen(item->match);
 	item->flags = 0;
 	if (limit_pathspec_to_literal())
@@ -295,6 +296,7 @@ static void parse_pathspec(struct pathspec *pathspec,
 
 		pathspec->items = item = xmalloc(sizeof(*item));
 		item->match = prefix;
+		item->original = prefix;
 		item->nowildcard_len = item->len = strlen(prefix);
 		raw[0] = prefix;
 		raw[1] = NULL;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 05/31] Export parse_pathspec() and convert some get_pathspec() calls
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

These call sites follow the pattern:

   paths = get_pathspec(prefix, argv);
   init_pathspec(&pathspec, paths);

which can be converted into a single parse_pathspec() call.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/grep.c         | 4 +---
 builtin/ls-tree.c      | 8 +++++++-
 builtin/update-index.c | 3 +--
 cache.h                | 6 ++++++
 revision.c             | 4 ++--
 setup.c                | 7 +++----
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 0e1b6c8..705f9ff 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -630,7 +630,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	const char *show_in_pager = NULL, *default_pager = "dummy";
 	struct grep_opt opt;
 	struct object_array list = OBJECT_ARRAY_INIT;
-	const char **paths = NULL;
 	struct pathspec pathspec;
 	struct string_list path_list = STRING_LIST_INIT_NODUP;
 	int i;
@@ -857,8 +856,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			verify_filename(prefix, argv[j], j == i);
 	}
 
-	paths = get_pathspec(prefix, argv + i);
-	init_pathspec(&pathspec, paths);
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv + i);
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
 
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fb76e38..e03aaaf 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -166,7 +166,13 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	if (get_sha1(argv[0], sha1))
 		die("Not a valid object name %s", argv[0]);
 
-	init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
+	/*
+	 * show_recursive() rolls its own matching code and is
+	 * generally ignorant of 'struct pathspec'. The magic mask
+	 * cannot be lifted until it is converted to use
+	 * match_pathspec_depth() or tree_entry_interesting()
+	 */
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv + 1);
 	for (i = 0; i < pathspec.nr; i++)
 		pathspec.items[i].nowildcard_len = pathspec.items[i].len;
 	pathspec.has_wildcard = 0;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index ada1dff..6728e59 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -546,10 +546,9 @@ static int do_reupdate(int ac, const char **av,
 	 */
 	int pos;
 	int has_head = 1;
-	const char **paths = get_pathspec(prefix, av + 1);
 	struct pathspec pathspec;
 
-	init_pathspec(&pathspec, paths);
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, av + 1);
 
 	if (read_ref("HEAD", head_sha1))
 		/* If there is no HEAD, that means it is an initial
diff --git a/cache.h b/cache.h
index 136e4c7..858c7e4 100644
--- a/cache.h
+++ b/cache.h
@@ -474,6 +474,9 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 
+/* Pathspec magic */
+#define PATHSPEC_FROMTOP    (1<<0)
+
 #define PATHSPEC_ONESTAR 1	/* the pathspec pattern sastisfies GFNM_ONESTAR */
 
 struct pathspec {
@@ -494,6 +497,9 @@ struct pathspec {
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern void parse_pathspec(struct pathspec *pathspec, unsigned magic_mask,
+			   unsigned flags, const char *prefix,
+			   const char **args);
 extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
diff --git a/revision.c b/revision.c
index 95d21e6..a044242 100644
--- a/revision.c
+++ b/revision.c
@@ -1851,8 +1851,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		 */
 		ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
 		prune_data.path[prune_data.nr++] = NULL;
-		init_pathspec(&revs->prune_data,
-			      get_pathspec(revs->prefix, prune_data.path));
+		parse_pathspec(&revs->prune_data, PATHSPEC_FROMTOP, 0,
+			       revs->prefix, prune_data.path);
 	}
 
 	if (revs->def == NULL)
diff --git a/setup.c b/setup.c
index a1ad012..0c9fc75 100644
--- a/setup.c
+++ b/setup.c
@@ -162,7 +162,6 @@ void verify_non_filename(const char *prefix, const char *arg)
  *	{ PATHSPEC_REGEXP, '\0', "regexp" },
  *
  */
-#define PATHSPEC_FROMTOP    (1<<0)
 
 static struct pathspec_magic {
 	unsigned bit;
@@ -276,9 +275,9 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
  * Given command line arguments and a prefix, convert the input to
  * pathspec. die() if any magic other than ones in magic_mask.
  */
-static void parse_pathspec(struct pathspec *pathspec,
-			   unsigned magic_mask, unsigned flags,
-			   const char *prefix, const char **argv)
+void parse_pathspec(struct pathspec *pathspec,
+		    unsigned magic_mask, unsigned flags,
+		    const char *prefix, const char **argv)
 {
 	struct pathspec_item *item;
 	const char *entry = *argv;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 06/31] Guard against new pathspec magic in pathspec matching code
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

GUARD_PATHSPEC() marks pathspec-sensitive code (basically anything in
'struct pathspec' except fields "nr" and "original"). GUARD_PATHSPEC()
is not supposed to fail. The steps for a new pathspec magic or
optimization would be:

 - update parse_pathspec, add extra information to struct pathspec

 - grep GUARD_PATHSPEC() and update all relevant code (or note those
   that won't work with your new stuff). Update GUARD_PATHSPEC mask
   accordingly.

 - update parse_pathspec calls to allow new magic. Make sure
   parse_pathspec() catches unsupported syntax early, not until
   GUARD_PATHSPEC catches it.

 - add tests to verify supported/unsupported commands both work as
   expected.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/diff.c |  2 ++
 cache.h        |  7 +++++++
 dir.c          |  2 ++
 tree-diff.c    | 19 +++++++++++++++++++
 tree-walk.c    |  2 ++
 5 files changed, 32 insertions(+)

diff --git a/builtin/diff.c b/builtin/diff.c
index 8c2af6c..d237e0a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -371,6 +371,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		die(_("unhandled object '%s' given."), name);
 	}
 	if (rev.prune_data.nr) {
+		/* builtin_diff_b_f() */
+		GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
 		if (!path)
 			path = rev.prune_data.items[0].match;
 		paths += rev.prune_data.nr;
diff --git a/cache.h b/cache.h
index 858c7e4..1f51423 100644
--- a/cache.h
+++ b/cache.h
@@ -496,6 +496,13 @@ struct pathspec {
 	} *items;
 };
 
+#define GUARD_PATHSPEC(ps, mask) \
+	do { \
+		if ((ps)->magic & ~(mask))	       \
+			die("BUG:%s:%d: unsupported magic %x",	\
+			    __FILE__, __LINE__, (ps)->magic & ~(mask)); \
+	} while (0)
+
 extern int init_pathspec(struct pathspec *, const char **);
 extern void parse_pathspec(struct pathspec *pathspec, unsigned magic_mask,
 			   unsigned flags, const char *prefix,
diff --git a/dir.c b/dir.c
index beb7532..37280c8 100644
--- a/dir.c
+++ b/dir.c
@@ -282,6 +282,8 @@ int match_pathspec_depth(const struct pathspec *ps,
 {
 	int i, retval = 0;
 
+	GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP);
+
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
 			return MATCHED_RECURSIVELY;
diff --git a/tree-diff.c b/tree-diff.c
index ba01563..68a9e7c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -199,6 +199,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	const char *paths[1];
 	int i;
 
+	/*
+	 * follow-rename code is very specific, we need exactly one
+	 * path. Magic that matches more than one path is not
+	 * supported.
+	 */
+	GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP);
+#if 0
+	/*
+	 * We should reject wildcards as well. Unfortunately we
+	 * haven't got a reliable way to detect that 'foo\*bar' in
+	 * fact has no wildcards. nowildcard_len is merely a hint for
+	 * optimization. Let it slip for now until wildmatch is taught
+	 * about dry-run mode and returns wildcard info.
+	 */
+	if (opt->pathspec.has_wildcard)
+		die("BUG:%s:%d: wildcards are not supported",
+		    __FILE__, __LINE__);
+#endif
+
 	/* Remove the file creation entry from the diff queue, and remember it */
 	choice = q->queue[0];
 	q->nr = 0;
diff --git a/tree-walk.c b/tree-walk.c
index 6e30ef9..dd03750 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -635,6 +635,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 	enum interesting never_interesting = ps->has_wildcard ?
 		entry_not_interesting : all_entries_not_interesting;
 
+	GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP);
+
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
 			return all_entries_interesting;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 07/31] clean: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clean.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 4cdabe0..fb0fe9a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -42,7 +42,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
 	struct strbuf directory = STRBUF_INIT;
 	struct dir_struct dir;
-	static const char **pathspec;
+	struct pathspec pathspec;
 	struct strbuf buf = STRBUF_INIT;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	const char *qname;
@@ -100,9 +100,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		add_exclude(exclude_list.items[i].string, "", 0,
 			    &dir.exclude_list[EXC_CMDL]);
 
-	pathspec = get_pathspec(prefix, argv);
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
 
-	fill_directory(&dir, pathspec);
+	fill_directory(&dir, pathspec.raw);
 
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
@@ -137,9 +137,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		if (lstat(ent->name, &st))
 			continue;
 
-		if (pathspec)
-			matches = match_pathspec(pathspec, ent->name, len,
-						 0, NULL);
+		if (pathspec.nr)
+			matches = match_pathspec_depth(&pathspec, ent->name,
+						       len, 0, NULL);
 
 		if (S_ISDIR(st.st_mode)) {
 			strbuf_addstr(&directory, ent->name);
@@ -163,7 +163,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			}
 			strbuf_reset(&directory);
 		} else {
-			if (pathspec && !matches)
+			if (pathspec.nr && !matches)
 				continue;
 			qname = quote_path_relative(ent->name, -1, &buf, prefix);
 			if (show_only) {
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 08/31] parse_pathspec: add PATHSPEC_EMPTY_MATCH_ALL
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

We have two ways of dealing with empty pathspec:

1. limit it to current prefix
2. match the entire working directory

Some commands go with #1, some with #2. get_pathspec() and
parse_pathspec() only supports #1. Make it support #2 too via
PATHSPEC_EMPTY_MATCH_ALL flag.

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

diff --git a/cache.h b/cache.h
index 1f51423..32231d8 100644
--- a/cache.h
+++ b/cache.h
@@ -503,6 +503,9 @@ struct pathspec {
 			    __FILE__, __LINE__, (ps)->magic & ~(mask)); \
 	} while (0)
 
+/* parse_pathspec flags */
+#define PATHSPEC_EMPTY_MATCH_ALL (1<<0) /* No args means match everything */
+
 extern int init_pathspec(struct pathspec *, const char **);
 extern void parse_pathspec(struct pathspec *pathspec, unsigned magic_mask,
 			   unsigned flags, const char *prefix,
diff --git a/setup.c b/setup.c
index 0c9fc75..d0b1d1f 100644
--- a/setup.c
+++ b/setup.c
@@ -289,6 +289,9 @@ void parse_pathspec(struct pathspec *pathspec,
 	if (!entry && !prefix)
 		return;
 
+	if (!*argv && (flags & PATHSPEC_EMPTY_MATCH_ALL))
+		return;
+
 	/* No arguments with prefix -> prefix pathspec */
 	if (!entry) {
 		static const char *raw[2];
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 09/31] commit: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


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

diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..444ae1d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -277,17 +277,17 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 {
 	int fd;
 	struct string_list partial;
-	const char **pathspec = NULL;
+	struct pathspec pathspec;
 	char *old_index_env = NULL;
 	int refresh_flags = REFRESH_QUIET;
 
 	if (is_status)
 		refresh_flags |= REFRESH_UNMERGED;
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP,
+		       PATHSPEC_EMPTY_MATCH_ALL,
+		       prefix, argv);
 
-	if (*argv)
-		pathspec = get_pathspec(prefix, argv);
-
-	if (read_cache_preload(pathspec) < 0)
+	if (read_cache_preload(pathspec.raw) < 0)
 		die(_("index file corrupt"));
 
 	if (interactive) {
@@ -329,9 +329,9 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	 * (A) if all goes well, commit the real index;
 	 * (B) on failure, rollback the real index.
 	 */
-	if (all || (also && pathspec && *pathspec)) {
+	if (all || (also && pathspec.nr)) {
 		fd = hold_locked_index(&index_lock, 1);
-		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
+		add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0);
 		refresh_cache_or_die(refresh_flags);
 		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
@@ -350,7 +350,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	 * and create commit from the_index.
 	 * We still need to refresh the index here.
 	 */
-	if (!only && (!pathspec || !*pathspec)) {
+	if (!only && !pathspec.nr) {
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
 		if (active_cache_changed) {
@@ -395,7 +395,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 
 	memset(&partial, 0, sizeof(partial));
 	partial.strdup_strings = 1;
-	if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, pathspec))
+	if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, pathspec.raw))
 		exit(1);
 
 	discard_cache();
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 10/31] status: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c |  9 +++++----
 wt-status.c      | 17 +++++++----------
 wt-status.h      |  2 +-
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 444ae1d..196dfab 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1205,11 +1205,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	handle_untracked_files_arg(&s);
 	if (show_ignored_in_status)
 		s.show_ignored_files = 1;
-	if (*argv)
-		s.pathspec = get_pathspec(prefix, argv);
+	parse_pathspec(&s.pathspec, PATHSPEC_FROMTOP,
+		       PATHSPEC_EMPTY_MATCH_ALL,
+		       prefix, argv);
 
-	read_cache_preload(s.pathspec);
-	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);
+	read_cache_preload(s.pathspec.raw);
+	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL);
 
 	fd = hold_locked_index(&index_lock, 0);
 	if (0 <= fd)
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..76edadc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -434,7 +434,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
-	init_pathspec(&rev.prune_data, s->pathspec);
+	copy_pathspec(&rev.prune_data, &s->pathspec);
 	run_diff_files(&rev, 0);
 }
 
@@ -459,22 +459,20 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = 200;
 	rev.diffopt.break_opt = 0;
-	init_pathspec(&rev.prune_data, s->pathspec);
+	copy_pathspec(&rev.prune_data, &s->pathspec);
 	run_diff_index(&rev, 1);
 }
 
 static void wt_status_collect_changes_initial(struct wt_status *s)
 {
-	struct pathspec pathspec;
 	int i;
 
-	init_pathspec(&pathspec, s->pathspec);
 	for (i = 0; i < active_nr; i++) {
 		struct string_list_item *it;
 		struct wt_status_change_data *d;
 		struct cache_entry *ce = active_cache[i];
 
-		if (!ce_path_match(ce, &pathspec))
+		if (!ce_path_match(ce, &s->pathspec))
 			continue;
 		it = string_list_insert(&s->change, ce->name);
 		d = it->util;
@@ -489,7 +487,6 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
 		else
 			d->index_status = DIFF_STATUS_ADDED;
 	}
-	free_pathspec(&pathspec);
 }
 
 static void wt_status_collect_untracked(struct wt_status *s)
@@ -505,11 +502,11 @@ static void wt_status_collect_untracked(struct wt_status *s)
 			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
 	setup_standard_excludes(&dir);
 
-	fill_directory(&dir, s->pathspec);
+	fill_directory(&dir, s->pathspec.raw);
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		if (cache_name_is_other(ent->name, ent->len) &&
-		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+		    match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL))
 			string_list_insert(&s->untracked, ent->name);
 		free(ent);
 	}
@@ -517,11 +514,11 @@ static void wt_status_collect_untracked(struct wt_status *s)
 	if (s->show_ignored_files) {
 		dir.nr = 0;
 		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
-		fill_directory(&dir, s->pathspec);
+		fill_directory(&dir, s->pathspec.raw);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
 			if (cache_name_is_other(ent->name, ent->len) &&
-			    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+			    match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL))
 				string_list_insert(&s->ignored, ent->name);
 			free(ent);
 		}
diff --git a/wt-status.h b/wt-status.h
index 236b41f..dd8df41 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -44,7 +44,7 @@ struct wt_status {
 	int is_initial;
 	char *branch;
 	const char *reference;
-	const char **pathspec;
+	struct pathspec pathspec;
 	int verbose;
 	int amend;
 	enum commit_whence whence;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 11/31] rerere: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


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

diff --git a/builtin/rerere.c b/builtin/rerere.c
index dc1708e..a573c4a 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -68,11 +68,11 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		return rerere(flags);
 
 	if (!strcmp(argv[0], "forget")) {
-		const char **pathspec;
+		struct pathspec pathspec;
 		if (argc < 2)
 			warning("'git rerere forget' without paths is deprecated");
-		pathspec = get_pathspec(prefix, argv + 1);
-		return rerere_forget(pathspec);
+		parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv + 1);
+		return rerere_forget(&pathspec);
 	}
 
 	fd = setup_rerere(&merge_rr, flags);
diff --git a/rerere.c b/rerere.c
index a6a5cd5..f8ddf85 100644
--- a/rerere.c
+++ b/rerere.c
@@ -655,7 +655,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 	return 0;
 }
 
-int rerere_forget(const char **pathspec)
+int rerere_forget(struct pathspec *pathspec)
 {
 	int i, fd;
 	struct string_list conflict = STRING_LIST_INIT_DUP;
@@ -666,12 +666,12 @@ int rerere_forget(const char **pathspec)
 
 	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
 
-	unmerge_cache(pathspec);
+	unmerge_cache(pathspec->raw);
 	find_conflict(&conflict);
 	for (i = 0; i < conflict.nr; i++) {
 		struct string_list_item *it = &conflict.items[i];
-		if (!match_pathspec(pathspec, it->string, strlen(it->string),
-				    0, NULL))
+		if (!match_pathspec_depth(pathspec, it->string, strlen(it->string),
+					  0, NULL))
 			continue;
 		rerere_forget_one_path(it->string, &merge_rr);
 	}
diff --git a/rerere.h b/rerere.h
index 156d2aa..4aa06c9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -3,6 +3,8 @@
 
 #include "string-list.h"
 
+struct pathspec;
+
 #define RERERE_AUTOUPDATE   01
 #define RERERE_NOAUTOUPDATE 02
 
@@ -16,7 +18,7 @@ extern void *RERERE_RESOLVED;
 extern int setup_rerere(struct string_list *, int);
 extern int rerere(int);
 extern const char *rerere_path(const char *hex, const char *file);
-extern int rerere_forget(const char **);
+extern int rerere_forget(struct pathspec *);
 extern int rerere_remaining(struct string_list *);
 extern void rerere_clear(struct string_list *);
 extern void rerere_gc(struct string_list *);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 12/31] checkout: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

This commit introduces a subtle bug:

- when match_pathspec() returns seen[], it follows the order of the
  input "const char **pathspec", which is now pathspec.raw[]

- when match_pathspec() returns seen[], it follows the order of
  pathspec.items[]

- due to 86e4ca6 (tree_entry_interesting(): fix depth limit with
  overlapping pathspecs - 2010-12-15), pathspec.items[] is sorted, but
  pathspec.raw[] is NOT.

by converting from match_pathspec() to match_pathspec_depth(), we also
have to switch the original path array. Unfortunately we can't because
this array is processed by report_path_error() and it's also used by
builtin/ls-files.c, which still uses the old indexing.

The bug causes wrong error messages (e.g. if the first pathspec is
faulty, it may report the second..) The bug will be dealt with after
ls-files is converted.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..3e60f2e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -45,7 +45,7 @@ struct checkout_opts {
 
 	int branch_exists;
 	const char *prefix;
-	const char **pathspec;
+	struct pathspec pathspec;
 	struct tree *source_tree;
 };
 
@@ -256,39 +256,37 @@ static int checkout_paths(const struct checkout_opts *opts,
 
 	if (opts->patch_mode)
 		return run_add_interactive(revision, "--patch=checkout",
-					   opts->pathspec);
+					   opts->pathspec.raw);
 
 	lock_file = xcalloc(1, sizeof(struct lock_file));
 
 	newfd = hold_locked_index(lock_file, 1);
-	if (read_cache_preload(opts->pathspec) < 0)
+	if (read_cache_preload(opts->pathspec.raw) < 0)
 		return error(_("corrupt index file"));
 
 	if (opts->source_tree)
-		read_tree_some(opts->source_tree, opts->pathspec);
+		read_tree_some(opts->source_tree, opts->pathspec.raw);
 
-	for (pos = 0; opts->pathspec[pos]; pos++)
-		;
-	ps_matched = xcalloc(1, pos);
+	ps_matched = xcalloc(1, opts->pathspec.nr);
 
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
 		if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
 			continue;
-		match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
+		match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
-	if (report_path_error(ps_matched, opts->pathspec, opts->prefix))
+	if (report_path_error(ps_matched, opts->pathspec.raw, opts->prefix))
 		return 1;
 
 	/* "checkout -m path" to recreate conflicted state */
 	if (opts->merge)
-		unmerge_cache(opts->pathspec);
+		unmerge_cache(opts->pathspec.raw);
 
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
+		if (match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce))
 				continue;
 			if (opts->force) {
@@ -315,7 +313,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		struct cache_entry *ce = active_cache[pos];
 		if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
 			continue;
-		if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
+		if (match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce)) {
 				errs |= checkout_entry(ce, &state, NULL);
 				continue;
@@ -960,7 +958,7 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
 static int checkout_branch(struct checkout_opts *opts,
 			   struct branch_info *new)
 {
-	if (opts->pathspec)
+	if (opts->pathspec.nr)
 		die(_("paths cannot be used with switching branches"));
 
 	if (opts->patch_mode)
@@ -1110,9 +1108,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	if (argc) {
-		opts.pathspec = get_pathspec(prefix, argv);
+		/*
+		 * In patch mode (opts.patch_mode != 0), we pass the
+		 * pathspec to an external program, git-add--interactive.
+		 * Do not accept any kind of magic that that program
+		 * cannot handle. Magic mask is pretty safe to be
+		 * lifted for new magic when opts.patch_mode == 0.
+		 */
+		parse_pathspec(&opts.pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
 
-		if (!opts.pathspec)
+		if (!opts.pathspec.nr)
 			die(_("invalid path specification"));
 
 		/*
@@ -1144,7 +1149,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		strbuf_release(&buf);
 	}
 
-	if (opts.patch_mode || opts.pathspec)
+	if (opts.patch_mode || opts.pathspec.nr)
 		return checkout_paths(&opts, new.name);
 	else
 		return checkout_branch(&opts, &new);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 13/31] rm: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/rm.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index dabfcf6..1a2c932 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -216,7 +216,7 @@ static struct option builtin_rm_options[] = {
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	const char **pathspec;
+	struct pathspec pathspec;
 	char *seen;
 
 	git_config(git_default_config, NULL);
@@ -249,31 +249,30 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	pathspec = get_pathspec(prefix, argv);
-	refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
+	refresh_index(&the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL);
 
 	seen = NULL;
-	for (i = 0; pathspec[i] ; i++)
-		/* nothing */;
-	seen = xcalloc(i, 1);
+	seen = xcalloc(pathspec.nr, 1);
 
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce = active_cache[i];
-		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
+		if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
 		ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
 		list.entry[list.nr].name = ce->name;
 		list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
 	}
 
-	if (pathspec) {
-		const char *match;
+	if (pathspec.nr) {
+		const char *original;
 		int seen_any = 0;
-		for (i = 0; (match = pathspec[i]) != NULL ; i++) {
+		for (i = 0; i < pathspec.nr; i++) {
+			original = pathspec.items[i].original;
 			if (!seen[i]) {
 				if (!ignore_unmatch) {
 					die(_("pathspec '%s' did not match any files"),
-					    match);
+					    original);
 				}
 			}
 			else {
@@ -281,7 +280,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die(_("not removing '%s' recursively without -r"),
-				    *match ? match : ".");
+				    *original ? original : ".");
 		}
 
 		if (! seen_any)
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 14/31] parse_pathspec: support stripping submodule trailing slashes
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

This flag is equivalent to builtin/ls-files.c:strip_trailing_slashes()
and is intended to replace that function when ls-files is converted to
use parse_pathspec.

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

diff --git a/cache.h b/cache.h
index 32231d8..611a410 100644
--- a/cache.h
+++ b/cache.h
@@ -505,6 +505,7 @@ struct pathspec {
 
 /* parse_pathspec flags */
 #define PATHSPEC_EMPTY_MATCH_ALL (1<<0) /* No args means match everything */
+#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<1)
 
 extern int init_pathspec(struct pathspec *, const char **);
 extern void parse_pathspec(struct pathspec *pathspec, unsigned magic_mask,
diff --git a/setup.c b/setup.c
index d0b1d1f..a1aabc2 100644
--- a/setup.c
+++ b/setup.c
@@ -250,6 +250,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 	*raw = item->match = match;
 	item->original = elt;
 	item->len = strlen(item->match);
+
+	if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
+	    (item->len >= 1 && item->match[item->len - 1] == '/') &&
+	    (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
+	    S_ISGITLINK(active_cache[i]->ce_mode)) {
+		item->len--;
+		match[item->len] = '\0';
+	}
+
 	item->flags = 0;
 	if (limit_pathspec_to_literal())
 		item->nowildcard_len = item->len;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v3 15/31] ls-files: convert to use parse_pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-13 12:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1358080539-17436-1-git-send-email-pclouds@gmail.com>

strip_trailing_slash_from_submodules() modifies pathspec and is moved
to dir.c, close to other pathspec code. It'll be removed later when
parse_pathspec() learns to take over its job.

This commit introduces a subtle bug:

- when match_pathspec() returns seen[], it follows the order of the
  input "const char **pathspec", which is now pathspec.raw[]

- when match_pathspec() returns seen[], it follows the order of
  pathspec.items[]

- due to 86e4ca6 (tree_entry_interesting(): fix depth limit with
  overlapping pathspecs - 2010-12-15), pathspec.items[] is sorted, but
  pathspec.raw[] is NOT.

by converting from match_pathspec() to match_pathspec_depth(), we also
have to switch the original path array. The bug causes wrong error
messages (e.g. if the first pathspec is faulty, it may report the
second..). report_path_error will be fixed in a separate patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/ls-files.c | 45 ++++++++++++---------------------------------
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 373c573..8905bd3 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -30,7 +30,7 @@ static int debug_mode;
 static const char *prefix;
 static int max_prefix_len;
 static int prefix_len;
-static const char **pathspec;
+static struct pathspec pathspec;
 static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
@@ -58,7 +58,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 	if (len >= ent->len)
 		die("git ls-files: internal error - directory entry not superset of prefix");
 
-	if (!match_pathspec(pathspec, ent->name, ent->len, len, ps_matched))
+	if (!match_pathspec_depth(&pathspec, ent->name, ent->len, len, ps_matched))
 		return;
 
 	fputs(tag, stdout);
@@ -133,7 +133,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	if (len >= ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
-	if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), len, ps_matched))
+	if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), len, ps_matched))
 		return;
 
 	if (tag && *tag && show_valid_bit &&
@@ -187,7 +187,7 @@ static void show_ru_info(void)
 		len = strlen(path);
 		if (len < max_prefix_len)
 			continue; /* outside of the prefix */
-		if (!match_pathspec(pathspec, path, len, max_prefix_len, ps_matched))
+		if (!match_pathspec_depth(&pathspec, path, len, max_prefix_len, ps_matched))
 			continue; /* uninterested */
 		for (i = 0; i < 3; i++) {
 			if (!ui->mode[i])
@@ -216,7 +216,7 @@ static void show_files(struct dir_struct *dir)
 
 	/* For cached/deleted files we don't need to even do the readdir */
 	if (show_others || show_killed) {
-		fill_directory(dir, pathspec);
+		fill_directory(dir, pathspec.raw);
 		if (show_others)
 			show_other_files(dir);
 		if (show_killed)
@@ -287,21 +287,6 @@ static void prune_cache(const char *prefix)
 	active_nr = last;
 }
 
-static void strip_trailing_slash_from_submodules(void)
-{
-	const char **p;
-
-	for (p = pathspec; *p != NULL; p++) {
-		int len = strlen(*p), pos;
-
-		if (len < 1 || (*p)[len - 1] != '/')
-			continue;
-		pos = cache_name_pos(*p, len - 1);
-		if (pos >= 0 && S_ISGITLINK(active_cache[pos]->ce_mode))
-			*p = xstrndup(*p, len - 1);
-	}
-}
-
 /*
  * Read the tree specified with --with-tree option
  * (typically, HEAD) into stage #1 and then
@@ -549,23 +534,17 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv);
-
-	/* be nice with submodule paths ending in a slash */
-	if (pathspec)
-		strip_trailing_slash_from_submodules();
+	parse_pathspec(&pathspec, PATHSPEC_FROMTOP,
+		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+		       prefix, argv);
 
 	/* Find common prefix for all pathspec's */
-	max_prefix = common_prefix(pathspec);
+	max_prefix = common_prefix(pathspec.raw);
 	max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 
 	/* Treat unmatching pathspec elements as errors */
-	if (pathspec && error_unmatch) {
-		int num;
-		for (num = 0; pathspec[num]; num++)
-			;
-		ps_matched = xcalloc(1, num);
-	}
+	if (pathspec.nr && error_unmatch)
+		ps_matched = xcalloc(1, pathspec.nr);
 
 	if ((dir.flags & DIR_SHOW_IGNORED) && !exc_given)
 		die("ls-files --ignored needs some exclude pattern");
@@ -592,7 +571,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 
 	if (ps_matched) {
 		int bad;
-		bad = report_path_error(ps_matched, pathspec, prefix);
+		bad = report_path_error(ps_matched, pathspec.raw, prefix);
 		if (bad)
 			fprintf(stderr, "Did you forget to 'git add'?\n");
 
-- 
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