* [PATCH v4 3/5] push: flag updates
From: Chris Rorvick @ 2012-11-17 20:16 UTC (permalink / raw)
To: git
Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
Felipe Contreras, Junio C Hamano
In-Reply-To: <1353183397-17719-1-git-send-email-chris@rorvick.com>
If the reference exists on the remote and the the update is not a
delete, then mark as an update. This is in preparation for handling
tags and branches differently when pushing.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
cache.h | 1 +
remote.c | 18 +++++++++++-------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 82dead1..0f53d11 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,7 @@ struct ref {
merge:1,
nonfastforward:1,
is_a_tag:1,
+ update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
diff --git a/remote.c b/remote.c
index 186814d..fe89869 100644
--- a/remote.c
+++ b/remote.c
@@ -1318,15 +1318,19 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
- ref->nonfastforward =
+ ref->update =
!ref->deletion &&
- !is_null_sha1(ref->old_sha1) &&
- (!has_sha1_file(ref->old_sha1)
- || !ref_newer(ref->new_sha1, ref->old_sha1));
+ !is_null_sha1(ref->old_sha1);
- if (ref->nonfastforward && !ref->force && !force_update) {
- ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
- continue;
+ if (ref->update) {
+ ref->nonfastforward =
+ !has_sha1_file(ref->old_sha1)
+ || !ref_newer(ref->new_sha1, ref->old_sha1);
+
+ if (ref->nonfastforward && !ref->force && !force_update) {
+ ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ continue;
+ }
}
}
}
--
1.8.0.155.g3a063ad.dirty
^ permalink raw reply related
* [PATCH v4 5/5] push: update remote tags only with force
From: Chris Rorvick @ 2012-11-17 20:16 UTC (permalink / raw)
To: git
Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
Felipe Contreras, Junio C Hamano
In-Reply-To: <1353183397-17719-1-git-send-email-chris@rorvick.com>
References are allowed to update from one commit-ish to another if the
former is a ancestor of the latter. This behavior is oriented to
branches which are expected to move with commits. Tag references are
expected to be static in a repository, though, thus an update to a
tag (lightweight and annotated) should be rejected unless the update is
forced.
To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:
1) The old and new references must be commits. If this fails,
it is not a valid update for a branch.
2) The reference name cannot start with "refs/tags/". This
catches lightweight tags which (usually) point to commits
and therefore would not be caught by (1).
If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote. This can be overridden by passing
--force to git push.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
Documentation/git-push.txt | 10 +++++-----
builtin/push.c | 2 +-
builtin/send-pack.c | 5 +++++
cache.h | 3 ++-
remote.c | 23 +++++++++++++++++++----
send-pack.c | 1 +
t/t5516-fetch-push.sh | 30 +++++++++++++++++++++++++++++-
transport-helper.c | 6 ++++++
transport.c | 8 ++++++--
9 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..479e25f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
updated.
+
The object referenced by <src> is used to update the <dst> reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward <dst>. By having the optional leading `+`,
-you can tell git to update the <dst> ref even when the update is not a
-fast-forward. This does *not* attempt to merge <src> into <dst>. See
-EXAMPLES below for details.
+on the remote side. By default this is only allowed if the update is
+a branch, and then only if it can fast-forward <dst>. By having the
+optional leading `+`, you can tell git to update the <dst> ref even when
+the update is not a branch or it is not a fast-forward. This does *not*
+attempt to merge <src> into <dst>. See EXAMPLES below for details.
+
`tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
+
diff --git a/builtin/push.c b/builtin/push.c
index 0e13e11..cd7aa3f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
static const char message_advice_ref_already_exists[] =
N_("Updates were rejected because a matching reference already exists in\n"
- "the remote and the update is not a fast-forward.");
+ "the remote.");
static void advise_pull_before_push(void)
{
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index fda28bc..1eabf42 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
msg = "non-fast forward";
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ res = "error";
+ msg = "already exists";
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = "error";
diff --git a/cache.h b/cache.h
index f410d94..127e504 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,13 +1004,14 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- is_a_tag:1,
+ forwardable:1,
update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 1e263b0..4fcd22c 100644
--- a/remote.c
+++ b/remote.c
@@ -1311,14 +1311,23 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
* to overwrite it; you would not know what you are losing
* otherwise.
*
- * (3) if both new and old are commit-ish, and new is a
- * descendant of old, it is OK.
+ * (3) if new is commit-ish and old is a commit, new is a
+ * descendant of old, and the reference is not of the
+ * refs/tags/ hierarchy it is OK.
*
* (4) regardless of all of the above, removing :B is
* always allowed.
*/
- ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
+ if (prefixcmp(ref->name, "refs/tags/")) {
+ // Additionally, disallow fast-forwarding if
+ // the old object is not a commit. This kicks
+ // out annotated tags that might pass the
+ // is_newer() test but dangle if the reference
+ // is updated.
+ struct object *obj = parse_object(ref->old_sha1);
+ ref->forwardable = !obj || obj->type == OBJ_COMMIT;
+ }
ref->update =
!ref->deletion &&
@@ -1329,7 +1338,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
!has_sha1_file(ref->old_sha1)
|| !ref_newer(ref->new_sha1, ref->old_sha1);
- if (ref->nonfastforward) {
+ if (!ref->forwardable) {
+ ref->requires_force = 1;
+ if (!force_ref_update) {
+ ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ continue;
+ }
+ } else if (ref->nonfastforward) {
ref->requires_force = 1;
if (!force_ref_update) {
ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
diff --git a/send-pack.c b/send-pack.c
index f50dfd9..1c375f0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..afb9b1b 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
git branch -D frotz
fi &&
git tag -f frotz &&
- git push testrepo frotz &&
+ git push -f testrepo frotz &&
check_push_result $the_commit tags/frotz &&
check_push_result $the_first_commit heads/frotz
@@ -929,6 +929,34 @@ test_expect_success 'push into aliased refs (inconsistent)' '
)
'
+test_expect_success 'push tag requires --force to update remote tag' '
+ mk_test heads/master &&
+ mk_child child1 &&
+ mk_child child2 &&
+ (
+ cd child1 &&
+ git tag lw_tag &&
+ git tag -a -m "message 1" ann_tag &&
+ git push ../child2 lw_tag &&
+ git push ../child2 ann_tag &&
+ >file1 &&
+ git add file1 &&
+ git commit -m "file1" &&
+ git tag -f lw_tag &&
+ git tag -f -a -m "message 2" ann_tag &&
+ test_must_fail git push ../child2 lw_tag &&
+ test_must_fail git push ../child2 ann_tag &&
+ git push --force ../child2 lw_tag &&
+ git push --force ../child2 ann_tag &&
+ git tag -f lw_tag HEAD~ &&
+ git tag -f -a -m "message 3" ann_tag &&
+ test_must_fail git push ../child2 lw_tag &&
+ test_must_fail git push ../child2 ann_tag &&
+ git push --force ../child2 lw_tag &&
+ git push --force ../child2 ann_tag
+ )
+'
+
test_expect_success 'push --porcelain' '
mk_empty &&
echo >.git/foo "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index 4713b69..965b778 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, "already exists")) {
+ status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ free(msg);
+ msg = NULL;
+ }
}
if (*ref)
@@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/transport.c b/transport.c
index 60a7421..a380ad7 100644
--- a/transport.c
+++ b/transport.c
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast-forward", porcelain);
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "already exists", porcelain);
+ break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
ref->deletion ? NULL : ref->peer_ref,
@@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs,
ref->status != REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
- if (!ref->is_a_tag)
- *reject_mask |= REJECT_ALREADY_EXISTS;
if (!strcmp(head, ref->name))
*reject_mask |= REJECT_NON_FF_HEAD;
else
*reject_mask |= REJECT_NON_FF_OTHER;
+ } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
+ *reject_mask |= REJECT_ALREADY_EXISTS;
}
}
}
--
1.8.0.155.g3a063ad.dirty
^ permalink raw reply related
* Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests
From: Felipe Contreras @ 2012-11-17 20:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, SZEDER Gábor, Jonathan Nieder
In-Reply-To: <7v7gplktdt.fsf@alter.siamese.dyndns.org>
On Sat, Nov 17, 2012 at 12:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> No need to have two versions; if a second argument is specified, use
>> that, otherwise use stdin.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> t/t9902-completion.sh | 30 +++++++++++++-----------------
>> 1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index 204c92a..59cdbfd 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -60,19 +60,15 @@ run_completion ()
>> # 2: expected completion
>> test_completion ()
>> {
>> - test $# -gt 1 && echo "$2" > expected
>> + if [ $# -gt 1 ]; then
>> + echo "$2" > expected
>> + else
>> + sed -e 's/Z$//' > expected
>> + fi &&
>
> As "$2" could begin with dash, end with \c, etc. that possibly can
> be misinterpred by echo, I'd rewrite this as
>
> printf '%s\n' "$2" >expected
>
> Otherwise looked fine; thanks.
But that was the case before. I would do that in a separate patch.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Heiko Voigt @ 2012-11-17 21:31 UTC (permalink / raw)
To: W. Trevor King
Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121117192026.GI22234@odin.tremily.us>
On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote:
> On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote:
> > > > (2) "git diff [$path]" and friends in the superproject compares the
> > > > HEAD of thecheckout of the submodule at $path with the tip of
> > > > the branch named by submodule.$name.branch in .gitmodules of
> > > > the superproject, instead of the commit that is recorded in the
> > > > index of the superproject.
> > > >
> > >
> > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a
> > > SHA for submodules), so I don't think it should care about the status
> > > of a remote branch. This sounds like you want something like:
> > >
> > > $ git submodule foreach 'git diff origin/$submodule_branch'
> > >
> > > Perhaps this is enough motivation for keeping $submodule_* exports?
> > >
> > > > and the option were called something like "--follow-branch=$branch",
> > > > ???
> >
> > I am not sure if hiding changes to the recorded SHA1 from the user is
> > such a useful thing. In the first step I would like it if it was kept
> > simple and only the submodule update machinery learned to follow a
> > branch. If that results in local changes that should be shown. The user
> > is still in charge of recording the updated SHA1 in his commit.
>
> I understand what you're warning against here, or what it has to do
> with "git diff".
Is there a not missing here? Reads somehow like that. What I am talking
about is the suggestion of Junio. Instead of showing a diff if the
SHA1 is different we show a diff if the checkout in the worktree is
different from the tip of the configured branch. That would hide the
fact that a submodule has changed during a submodule update operation.
> > From what I have heard of projects using this: They usually still have
> > something that records the SHA1s on a regular basis. Thinking further,
> > why not record them in git? We could add an option to update which
> > creates such a commit.
>
> I think it's best to have users craft their own commit messages
> explaining why the branch was updated. That said, an auto-generated
> hint (a la "git merge") would probably be a useful extra feature.
I have the same opinion. Commits should always be created by humans so
you have someone to blame/ask why. But I guess there are people that
expect this to be automatic.
One argument somehow goes along the lines:
"I already created a commit in the submodule why do I need to create
another one in the superproject? Just follow the HEAD revision!" They
think in subversions "submodules" which are merely pointers to other svn
repositories without any revision information. I am unsure if its good
to support this the same way.
Another use case is big projects that have so many submodules that
creating superproject commits would create to much maintenance work.
They want to have their integration server make those commits. That
would already be supported with update checking out the branch tips and
the commit is just one extra thing to do by the integration server.
So I think it should be fine just to teach update to checkout the
configured branch tips (or forward them to their tracking branch tips)
and leave the rest to the user.
Cheers Heiko
^ permalink raw reply
* [PATCH v4.1 5/5] push: update remote tags only with force
From: Chris Rorvick @ 2012-11-17 21:53 UTC (permalink / raw)
To: git
Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
Felipe Contreras, Junio C Hamano
In-Reply-To: <1353183397-17719-6-git-send-email-chris@rorvick.com>
References are allowed to update from one commit-ish to another if the
former is a ancestor of the latter. This behavior is oriented to
branches which are expected to move with commits. Tag references are
expected to be static in a repository, though, thus an update to a
tag (lightweight and annotated) should be rejected unless the update is
forced.
To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:
1) The old and new references must be commits. If this fails,
it is not a valid update for a branch.
2) The reference name cannot start with "refs/tags/". This
catches lightweight tags which (usually) point to commits
and therefore would not be caught by (1).
If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote. This can be overridden by passing
--force to git push.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
Fix C99 comment.
Documentation/git-push.txt | 10 +++++-----
builtin/push.c | 2 +-
builtin/send-pack.c | 5 +++++
cache.h | 3 ++-
remote.c | 24 ++++++++++++++++++++----
send-pack.c | 1 +
t/t5516-fetch-push.sh | 42 +++++++++++++++++++++++++++++++++++++++++-
transport-helper.c | 6 ++++++
transport.c | 8 ++++++--
9 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..479e25f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
updated.
+
The object referenced by <src> is used to update the <dst> reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward <dst>. By having the optional leading `+`,
-you can tell git to update the <dst> ref even when the update is not a
-fast-forward. This does *not* attempt to merge <src> into <dst>. See
-EXAMPLES below for details.
+on the remote side. By default this is only allowed if the update is
+a branch, and then only if it can fast-forward <dst>. By having the
+optional leading `+`, you can tell git to update the <dst> ref even when
+the update is not a branch or it is not a fast-forward. This does *not*
+attempt to merge <src> into <dst>. See EXAMPLES below for details.
+
`tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
+
diff --git a/builtin/push.c b/builtin/push.c
index 0e13e11..cd7aa3f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
static const char message_advice_ref_already_exists[] =
N_("Updates were rejected because a matching reference already exists in\n"
- "the remote and the update is not a fast-forward.");
+ "the remote.");
static void advise_pull_before_push(void)
{
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index fda28bc..1eabf42 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
msg = "non-fast forward";
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ res = "error";
+ msg = "already exists";
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = "error";
diff --git a/cache.h b/cache.h
index f410d94..127e504 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,13 +1004,14 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- is_a_tag:1,
+ forwardable:1,
update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 44e72d6..a723f7a 100644
--- a/remote.c
+++ b/remote.c
@@ -1311,14 +1311,24 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
* to overwrite it; you would not know what you are losing
* otherwise.
*
- * (3) if both new and old are commit-ish, and new is a
- * descendant of old, it is OK.
+ * (3) if new is commit-ish and old is a commit, new is a
+ * descendant of old, and the reference is not of the
+ * refs/tags/ hierarchy it is OK.
*
* (4) regardless of all of the above, removing :B is
* always allowed.
*/
- ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
+ if (prefixcmp(ref->name, "refs/tags/")) {
+ /* Additionally, disallow fast-forwarding if
+ * the old object is not a commit. This kicks
+ * out annotated tags that might pass the
+ * is_newer() test but dangle if the reference
+ * is updated.
+ */
+ struct object *obj = parse_object(ref->old_sha1);
+ ref->forwardable = !obj || obj->type == OBJ_COMMIT;
+ }
ref->update =
!ref->deletion &&
@@ -1329,7 +1339,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
!has_sha1_file(ref->old_sha1)
|| !ref_newer(ref->new_sha1, ref->old_sha1);
- if (ref->nonfastforward) {
+ if (!ref->forwardable) {
+ ref->requires_force = 1;
+ if (!force_ref_update) {
+ ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ continue;
+ }
+ } else if (ref->nonfastforward) {
ref->requires_force = 1;
if (!force_ref_update) {
ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
diff --git a/send-pack.c b/send-pack.c
index f50dfd9..1c375f0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..ca800b2 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
git branch -D frotz
fi &&
git tag -f frotz &&
- git push testrepo frotz &&
+ git push -f testrepo frotz &&
check_push_result $the_commit tags/frotz &&
check_push_result $the_first_commit heads/frotz
@@ -929,6 +929,46 @@ test_expect_success 'push into aliased refs (inconsistent)' '
)
'
+test_expect_success 'push requires --force to update lightweight tag' '
+ mk_test heads/master &&
+ mk_child child1 &&
+ mk_child child2 &&
+ (
+ cd child1 &&
+ git tag Tag &&
+ git push ../child2 Tag &&
+ >file1 &&
+ git add file1 &&
+ git commit -m "file1" &&
+ git tag -f Tag &&
+ test_must_fail git push ../child2 Tag &&
+ git push --force ../child2 Tag &&
+ git tag -f Tag &&
+ test_must_fail git push ../child2 Tag HEAD~ &&
+ git push --force ../child2 Tag
+ )
+'
+
+test_expect_success 'push requires --force to update annotated tag' '
+ mk_test heads/master &&
+ mk_child child1 &&
+ mk_child child2 &&
+ (
+ cd child1 &&
+ git tag -a -m "message 1" Tag &&
+ git push ../child2 Tag:refs/tmp/Tag &&
+ >file1 &&
+ git add file1 &&
+ git commit -m "file1" &&
+ git tag -f -a -m "message 2" Tag &&
+ test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+ git push --force ../child2 Tag:refs/tmp/Tag &&
+ git tag -f -a -m "message 3" Tag HEAD~ &&
+ test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+ git push --force ../child2 Tag:refs/tmp/Tag
+ )
+'
+
test_expect_success 'push --porcelain' '
mk_empty &&
echo >.git/foo "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index 4713b69..965b778 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, "already exists")) {
+ status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ free(msg);
+ msg = NULL;
+ }
}
if (*ref)
@@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/transport.c b/transport.c
index 60a7421..a380ad7 100644
--- a/transport.c
+++ b/transport.c
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast-forward", porcelain);
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "already exists", porcelain);
+ break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
ref->deletion ? NULL : ref->peer_ref,
@@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs,
ref->status != REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
- if (!ref->is_a_tag)
- *reject_mask |= REJECT_ALREADY_EXISTS;
if (!strcmp(head, ref->name))
*reject_mask |= REJECT_NON_FF_HEAD;
else
*reject_mask |= REJECT_NON_FF_OTHER;
+ } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
+ *reject_mask |= REJECT_ALREADY_EXISTS;
}
}
}
--
1.7.11.7
^ permalink raw reply related
* Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-17 22:00 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121117213130.GC7695@book.hvoigt.net>
[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]
On Sat, Nov 17, 2012 at 10:31:30PM +0100, Heiko Voigt wrote:
> On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote:
> > On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote:
> > > > > (2) "git diff [$path]" and friends in the superproject compares the
> > > > > HEAD of thecheckout of the submodule at $path with the tip of
> > > > > the branch named by submodule.$name.branch in .gitmodules of
> > > > > the superproject, instead of the commit that is recorded in the
> > > > > index of the superproject.
> > > > >
> > > >
> > > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a
> > > > SHA for submodules), so I don't think it should care about the status
> > > > of a remote branch. This sounds like you want something like:
> > > >
> > > > $ git submodule foreach 'git diff origin/$submodule_branch'
> > > >
> > > > Perhaps this is enough motivation for keeping $submodule_* exports?
> > > >
> > > > > and the option were called something like "--follow-branch=$branch",
> > > > > ???
> > >
> > > I am not sure if hiding changes to the recorded SHA1 from the user is
> > > such a useful thing. In the first step I would like it if it was kept
> > > simple and only the submodule update machinery learned to follow a
> > > branch. If that results in local changes that should be shown. The user
> > > is still in charge of recording the updated SHA1 in his commit.
> >
> > I understand what you're warning against here, or what it has to do
> > with "git diff".
>
> Is there a not missing here?
Thanks. I'd meant to say "I don't understand…".
> What I am talking about is the suggestion of Junio. Instead of
> showing a diff if the SHA1 is different we show a diff if the
> checkout in the worktree is different from the tip of the configured
> branch. That would hide the fact that a submodule has changed during
> a submodule update operation.
Ahh, now I understand. I agree that comparing to the remote tip is a
bad idea.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: Jonathan Nieder @ 2012-11-17 23:00 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Felipe Contreras, Jeff King, Junio C Hamano
In-Reply-To: <1353150353-29874-2-git-send-email-szeder@ira.uka.de>
SZEDER Gábor wrote:
> The 'basic' test uses 'grep -q' to filter the resulting possible
> completion words while looking for the presence or absence of certain
> git commands, and relies on grep's exit status to indicate a failure.
[...]
> To make testers' life easier provide some output about the failed
> condition: store the results of the filtering in a file and compare
> its contents to the expected results by the good old test_cmp helper.
Looks good. I wonder if this points to the need for a test_grep
helper more generally.
[...]
> run_completion "git f" &&
> - ! grep -q -v "^f" out
> + >expected &&
> + sed -n "/^[^f]/p" out >out2 &&
> + test_cmp expected out2
Functional change: before, this would fail if "out" contained a blank
line, while afterward it does not. I doubt that matters, though.
Thanks and hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 2/7] completion: fix args of run_completion() test helper
From: Jonathan Nieder @ 2012-11-17 23:02 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Felipe Contreras, Jeff King, Junio C Hamano
In-Reply-To: <1353150353-29874-3-git-send-email-szeder@ira.uka.de>
SZEDER Gábor wrote:
> Fix this by passing the command line to run_completion() as separate
> words.
Good catch. The change makes sense, the code looks saner after the
fix, and since this is test code any breakage should be caught
quickly, so
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
^ permalink raw reply
* Re: [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
From: Jonathan Nieder @ 2012-11-17 23:31 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Felipe Contreras, Jeff King, Junio C Hamano
In-Reply-To: <1353150353-29874-4-git-send-email-szeder@ira.uka.de>
SZEDER Gábor wrote:
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -155,6 +155,90 @@ test_expect_success '__gitcomp - suffix' '
> test_cmp expected out
> '
>
> +test_expect_success '__gitcomp_nl - trailing space' '
> + sed -e "s/Z$//" >expected <<-\EOF &&
'$' is usually a shell metacharacter, so it would be more comfortable
to read a version that escapes it:
sed -e "s/Z\$//" >expected <<-\EOF
Since '$/' is not a valid parameter expansion, if I understand
correctly then POSIX leaves the meaning of the quoted string "s/Z$//"
undefined (XCU §2.2.3). Luckily every shell I've tried, including
bash, keeps the $ unmolested.
Other parts of the file already use the same style, so I wouldn't
suggest changing it in this patch.
Thanks for some nice tests.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: Jonathan Nieder @ 2012-11-17 23:40 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Felipe Contreras, Jeff King, Junio C Hamano
In-Reply-To: <1353150353-29874-5-git-send-email-szeder@ira.uka.de>
SZEDER Gábor wrote:
> The breakage can
> be simply bogus possible completion words, but it can also be a
> failure:
>
> $ git branch '${foo.bar}'
> $ git checkout <TAB>
> bash: ${foo.bar}: bad substitution
Or arbitrary code execution:
$ git branch '$(>kilroy-was-here)'
$ git checkout <TAB>
$ ls -l kilroy-was-here
-rw-rw-r-- 1 jrn jrn 0 nov 17 15:40 kilroy-was-here
The final version of this patch should go to maint. Thanks for
catching it.
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: Felipe Contreras @ 2012-11-18 0:53 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <CAMP44s0YnoGwJEgUbXZ831_OrgO=dDf5_QHxT5JYnGUHYPuiTw@mail.gmail.com>
On Sat, Nov 17, 2012 at 8:33 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:
>>> On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>> > On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
>>> >> The functionality we use is very simple, plus, this fixes a known
>>> >> breakage 'complete tree filename with metacharacters'.
>>> >>
>>> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> >> ---
>>> >> contrib/completion/git-completion.bash | 6 +++++-
>>> >> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>>> >> index 975ae13..ad3e1fe 100644
>>> >> --- a/contrib/completion/git-completion.bash
>>> >> +++ b/contrib/completion/git-completion.bash
>>> >> @@ -227,7 +227,11 @@ fi
>>> >>
>>> >> __gitcompadd ()
>>> >> {
>>> >> - COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
>>> >> + for x in $1; do
>>> >> + if [[ "$x" = "$3"* ]]; then
>>> >> + COMPREPLY+=("$2$x$4")
>>> >> + fi
>>> >> + done
>>> >
>>> > The whole point of creating __gitcomp_nl() back then was to fill
>>> > COMPREPLY without iterating through all words in the wordlist, making
>>> > completion faster for large number of words, e.g. a lot of refs, or
>>> > later a lot of symbols for 'git grep' in a larger project.
>>> >
>>> > The loop here kills that optimization.
>>>
>>> So your solution is to move the loop to awk? I fail to see how that
>>> could bring more optimization, specially since it includes an extra
>>> fork now.
>>
>> This patch didn't aim for more optimization, but it was definitely a
>> goal not to waste what we gained by creating __gitcomp_nl() in
>> a31e6262 (completion: optimize refs completion, 2011-10-15). However,
>> as it turns out the new version with awk is actually faster than
>> current master with compgen:
>>
>> Before:
>>
>> $ refs="$(for i in {0..9999} ; do echo branch$i ; done)"
>> $ time __gitcomp_nl "$refs"
>>
>> real 0m0.242s
>> user 0m0.220s
>> sys 0m0.028s
>>
>> After:
>>
>> $ time __gitcomp_nl "$refs"
>>
>> real 0m0.109s
>> user 0m0.096s
>> sys 0m0.012s
>
> This one is even faster:
>
> while read -r x; do
> if [[ "$x" == "$3"* ]]; then
> COMPREPLY+=("$2$x$4")
> fi
> done <<< $1
>
> == 10000 ==
> one:
> real 0m0.148s
> user 0m0.134s
> sys 0m0.025s
> two:
> real 0m0.055s
> user 0m0.054s
> sys 0m0.000s
Ah, nevermind, I didn't quote the $1.
However, this one is quite close and much simpler:
local IFS=$'\n'
COMPREPLY=($(printf -- "$2%s$4\n" $1 | grep "^$2$3"))
== 10000 ==
awk 1:
real 0m0.064s
user 0m0.062s
sys 0m0.003s
printf:
real 0m0.069s
user 0m0.064s
sys 0m0.020s
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-18 0:55 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <CAMP44s1zaxey7TQxGaLtaMUwPTVTmRBn1w6=Zqefy7FJzEepBw@mail.gmail.com>
On Sat, Nov 17, 2012 at 8:28 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 17, 2012 at 8:08 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sat, Nov 17, 2012 at 3:14 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>> On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
>>>> On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>>>
>>>> > __gitcomp_nl ()
>>>> > {
>>>> > local IFS=$'\n'
>>>> > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
>>>> > + COMPREPLY=($(awk -v pfx="${2-}" -v sfx="${4- }" -v cur="${3-$cur}" '
>>>> > + BEGIN {
>>>> > + FS="\n";
>>>> > + len=length(cur);
>>>> > + }
>>>> > + {
>>>> > + if (cur == substr($1, 1, len))
>>>> > + print pfx$1sfx;
>>>> > + }' <<< "$1" ))
>>>> > }
>>>>
>>>> Does this really perform better than my alternative?
>>>>
>>>> + for x in $1; do
>>>> + if [[ "$x" = "$3"* ]]; then
>>>> + COMPREPLY+=("$2$x$4")
>>>> + fi
>>>> + done
>>>
>>> It does:
>>>
>>> My version:
>>>
>>> $ refs="$(for i in {0..9999} ; do echo branch$i ; done)"
>>> $ time __gitcomp_nl "$refs"
>>>
>>> real 0m0.109s
>>> user 0m0.096s
>>> sys 0m0.012s
>>>
>>> Yours:
>>>
>>> $ time __gitcomp_nl "$refs"
>>>
>>> real 0m0.321s
>>> user 0m0.312s
>>> sys 0m0.008s
>>
>> Yeah, for 10000 refs, which is not the common case:
>
> And this beats both in every case:
>
> while read -r x; do
> if [[ "$x" == "$3"* ]]; then
> COMPREPLY+=("$2$x$4")
> fi
> done <<< $1
Nevermind that.
Here's another:
local IFS=$'\n'
COMPREPLY=($(printf -- "$2%s$4\n" $1 | grep "^$2$3"))
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-18 0:58 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-6-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> __gitcomp_nl ()
> {
> local IFS=$'\n'
> - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
> + COMPREPLY=($(awk -v pfx="${2-}" -v sfx="${4- }" -v cur="${3-$cur}" '
> + BEGIN {
> + FS="\n";
> + len=length(cur);
> + }
> + {
> + if (cur == substr($1, 1, len))
> + print pfx$1sfx;
> + }' <<< "$1" ))
> }
This version is simpler and faster:
local IFS=$'\n'
COMPREPLY=($(awk -v cur="${3-$cur}" -v pre="${2-}" -v suf="${4- }"
'$0 ~ cur { print pre$0suf }' <<< "$1" ))
== 10000 ==
awk 1:
real 0m0.067s
user 0m0.066s
sys 0m0.001s
awk 2:
real 0m0.057s
user 0m0.055s
sys 0m0.002s
--
Felipe Contreras
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Torsten Bögershausen @ 2012-11-18 5:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <CAMP44s2yenQKSgdUXfZP+yDJJ+bdveyms=SQ+3ptPvpT6D0hsg@mail.gmail.com>
On 10.11.12 23:05, Felipe Contreras wrote:
> On Sat, Nov 10, 2012 at 8:20 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 11/10/2012 08:15 PM, Felipe Contreras wrote:
>>>
>>> Hi,
>>>
>>> On Sat, Nov 10, 2012 at 2:48 PM, Torsten Bögershausen <tboegi@web.de>
>>> wrote:
>>>
>>>> on peff/pu t5801 fails, the error is in git-remote-testgit, please see
>>>> below.
>>>>
>>>> That's on my Mac OS X box.
>>>>
>>>> I haven't digged further into the test case, but it looks as if
>>>> "[-+]A make NAMEs associative arrays"
>>>> is not supported on this version of bash.
>>>> /Torsten
>>>>
>>>>
>>>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 64: declare: -A:
>>>> invalid option
>>>> declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
>>>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 66:
>>>> refs/heads/master: division by 0 (error token is "/master")
>>>> error: fast-export died of signal 13
>>>> fatal: Error while running fast-export
>>>
>>>
>>> What is your bash --version?
>>>
>> bash --version
>> GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0)
>> Copyright (C) 2007 Free Software Foundation, Inc.
>
> I see, that version indeed doesn't have associative arrays.
>
>> On the other hand, Documentation/CodingGuidelines says:
>> - No shell arrays.
>
> Yeah, for shell code I guess, but this is bash code.
>
>> Could we use perl to have arrays?
>
> I think the code in perl would be harder to follow, and this is meant
> not only as a test, but also as a reference.
>
> I'm not exactly sure what we should do here:
>
> a) remove the code (would not be so good as a reference)
> b) enable the code conditionally based on the version of bash (harder to read)
> c) replace the code without associative arrays (will be much more
> complicated and ugly)
> d) add a check for the bash version to the top of the test in t/
>
> I'm leaning towards d), followed by b).
>
> If only there was a clean way to do this, so c) would not be so ugly.
>
> After investigating different optins this seems to be the best:
>
> join -e empty -o '0 1.2 2.2' -a 2 <(echo "$before") <(echo "$after")
> | while read e a b; do
> test "$a" == "$b" && continue
> echo "changed $e"
> done
>
> But to me seems a bit harder to grasp. Not sure.
>
> Cheers.
>
Hi again,
I managed to have a working solution for
"d) add a check for the bash version to the top of the test in t/"
Please see diff below.
This unbreaks the test suite here.
Is this a good way forward?
Filipe, does the code line you mention above work for you?
If yes: I can test it here, if you send it as a patch.
/Torsten
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
old mode 100644
new mode 100755
index 6e4e078..ea3d0f3
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -13,6 +13,15 @@ compare_refs() {
test_cmp expect actual
}
+cat >"testbashArray" <<-EOF
+ declare -A assa
+EOF
+
+/bin/bash testbashArray || {
+ skip_all='t5801. /bin/bash does not handle associative arrays'
+ test_done
+}
+
test_expect_success 'setup repository' '
git init server &&
(cd server &&
^ permalink raw reply related
* Re: [PATCH] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
From: Junio C Hamano @ 2012-11-18 7:46 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Torsten Bögershausen, git
In-Reply-To: <50A7A161.9020805@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
>> ...
>> - V15_MINGW_HEADERS = YesPlease
>> + CYGWIN_OLD_WINSOCK_HEADERS = YesPlease
>>
> WINSOCK is certainly a part of the win32 api implementation, but it is
> is the entire win32api that changed, not just the tiny bit dealing
> with sockets.
> Basically, WINDOWS.h, and everything it includes, and all of the dlls
> it touches, and the .o files, changed.
> ... So my suggestion in the bike shedding
> category is to
>
> s/V15_MINGW_HEADERS/CYGWIN_V15_WIN32API/
Sounds sensible.
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Felipe Contreras @ 2012-11-18 8:23 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
In-Reply-To: <50A87718.4030806@web.de>
Hi,
On Sun, Nov 18, 2012 at 6:50 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> I managed to have a working solution for
> "d) add a check for the bash version to the top of the test in t/"
> Please see diff below.
>
> This unbreaks the test suite here.
> Is this a good way forward?
>
> Filipe, does the code line you mention above work for you?
>
> If yes: I can test it here, if you send it as a patch.
It's already sent:
http://article.gmane.org/gmane.comp.version-control.git/209364
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: Felipe Contreras @ 2012-11-18 8:34 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-5-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> @@ -155,6 +156,12 @@ test_expect_success '__gitcomp - suffix' '
> test_cmp expected out
> '
>
> +test_expect_failure '__gitcomp - doesnt fail because of invalid variable name' '
> + (
> + __gitcomp "$invalid_variable_name"
> + )
> +'
Why in a subshell?
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: Felipe Contreras @ 2012-11-18 8:34 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: SZEDER Gábor, git, Jeff King, Junio C Hamano
In-Reply-To: <20121117234059.GD3815@elie.Belkin>
On Sun, Nov 18, 2012 at 12:40 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> SZEDER Gábor wrote:
>
>> The breakage can
>> be simply bogus possible completion words, but it can also be a
>> failure:
>>
>> $ git branch '${foo.bar}'
>> $ git checkout <TAB>
>> bash: ${foo.bar}: bad substitution
>
> Or arbitrary code execution:
>
> $ git branch '$(>kilroy-was-here)'
> $ git checkout <TAB>
> $ ls -l kilroy-was-here
> -rw-rw-r-- 1 jrn jrn 0 nov 17 15:40 kilroy-was-here
>
> The final version of this patch should go to maint. Thanks for
> catching it.
Shouldn't this go to the commit message?
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/7] completion: fix args of run_completion() test helper
From: Felipe Contreras @ 2012-11-18 8:48 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-3-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> test_expect_success 'basic' '
> - run_completion "git \"\"" &&
> + run_completion git "" &&
I don't like this approach. I prefer
run_completion "git "
So that it's similar to how test_completion is called:
run_completion "git f"
test_completion "git f"
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
From: Felipe Contreras @ 2012-11-18 8:53 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-4-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> Test __gitcomp_nl()'s basic functionality, i.e. that trailing space,
> prefix, and suffix are added correctly.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
> t/t9902-completion.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
Too much code that can be simplified:
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -85,6 +85,22 @@ test_gitcomp ()
test_cmp expected out
}
+# Test __gitcomp_nl
+# Arguments are:
+# 1: typed text so far (cur)
+# -: the rest are passed to __gitcomp_nl
+test_gitcomp_nl ()
+{
+ local -a COMPREPLY &&
+ sed -e 's/Z$//' > expected &&
+ cur="$1" &&
+ shift &&
+ __gitcomp_nl "$@" &&
+ print_comp &&
+ cp expected out /tmp
+ test_cmp expected out
+}
+
invalid_variable_name="${foo.bar}"
test_expect_success '__gitcomp - trailing space - options' '
@@ -134,88 +150,39 @@ test_expect_success '__gitcomp - doesnt fail
because of invalid variable name' '
__gitcomp "$invalid_variable_name"
'
+read -r -d "" refs <<-\EOF
+maint
+master
+next
+pu
+EOF
+
test_expect_success '__gitcomp_nl - trailing space' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp_nl "m" "$refs" <<-EOF
maint Z
master Z
EOF
- (
- declare -a COMPREPLY &&
- refs="$(cat <<-\EOF
- maint
- master
- next
- pu
- EOF
- )" &&
- cur="m" &&
- __gitcomp_nl "$refs" &&
- print_comp
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp_nl - prefix' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp_nl "--fixup=m" "$refs" "--fixup=" "m" <<-EOF
--fixup=maint Z
--fixup=master Z
EOF
- (
- declare -a COMPREPLY &&
- refs="$(cat <<-\EOF
- maint
- master
- next
- pu
- EOF
- )" &&
- cur="--fixup=m" &&
- __gitcomp_nl "$refs" "--fixup=" "m" &&
- print_comp
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp_nl - suffix' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp_nl "branch.ma" "$refs" "branch." "ma" "." <<-\EOF
branch.maint.Z
branch.master.Z
EOF
- (
- declare -a COMPREPLY &&
- refs="$(cat <<-\EOF
- maint
- master
- next
- pu
- EOF
- )" &&
- cur="branch.ma" &&
- __gitcomp_nl "$refs" "branch." "ma" "." &&
- print_comp
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp_nl - no suffix' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp_nl "ma" "$refs" "" "ma" "" <<-\EOF
maintZ
masterZ
EOF
- (
- declare -a COMPREPLY &&
- refs="$(cat <<-\EOF
- maint
- master
- next
- pu
- EOF
- )" &&
- cur="ma" &&
- __gitcomp_nl "$refs" "" "ma" "" &&
- print_comp
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp_nl - doesnt fail because of invalid
variable name' '
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [PATCH 0/4] Some pathspec wildcard optimization
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This ports the "*.c" optimization from .gitignore to pathspec code.
Nguyễn Thái Ngọc Duy (4):
pathspec: save the non-wildcard length part
pathspec: do exact comparison on the leading non-wildcard part
pathspec: apply "*.c" optimization from exclude
tree_entry_interesting: do basedir compare on wildcard patterns when
possible
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 2 +-
cache.h | 5 +++-
dir.c | 35 ++++++++++++++++++++++---
dir.h | 9 +++++++
tree-walk.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++----
6 files changed, 117 insertions(+), 11 deletions(-)
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply
* [PATCH 1/4] pathspec: save the non-wildcard length part
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
We marks pathspec with wildcards with the field use_wildcard. We could
do better by saving the length of the non-wildcard part, which can be
for optimizations such as f9f6e2c (exclude: do strcmp as much as
possible before fnmatch - 2012-06-07)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 2 +-
cache.h | 2 +-
dir.c | 6 +++---
tree-walk.c | 4 ++--
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b5434af..4a9ee69 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -337,7 +337,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
matchbuf[0] = prefix;
matchbuf[1] = NULL;
init_pathspec(&pathspec, matchbuf);
- pathspec.items[0].use_wildcard = 0;
+ pathspec.items[0].nowildcard_len = pathspec.items[0].len;
} else
init_pathspec(&pathspec, NULL);
if (read_tree(tree, 1, &pathspec))
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 235c17c..fb76e38 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -168,7 +168,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
for (i = 0; i < pathspec.nr; i++)
- pathspec.items[i].use_wildcard = 0;
+ pathspec.items[i].nowildcard_len = pathspec.items[i].len;
pathspec.has_wildcard = 0;
tree = parse_tree_indirect(sha1);
if (!tree)
diff --git a/cache.h b/cache.h
index dbd8018..bf031f1 100644
--- a/cache.h
+++ b/cache.h
@@ -482,7 +482,7 @@ struct pathspec {
struct pathspec_item {
const char *match;
int len;
- unsigned int use_wildcard:1;
+ int nowildcard_len;
} *items;
};
diff --git a/dir.c b/dir.c
index 5a83aa7..c391d46 100644
--- a/dir.c
+++ b/dir.c
@@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
return MATCHED_RECURSIVELY;
}
- if (item->use_wildcard && !fnmatch(match, name, 0))
+ if (item->nowildcard_len < item->len && !fnmatch(match, name, 0))
return MATCHED_FNMATCH;
return 0;
@@ -1429,8 +1429,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
- if (item->use_wildcard)
+ item->nowildcard_len = simple_length(path);
+ if (item->nowildcard_len < item->len)
pathspec->has_wildcard = 1;
}
diff --git a/tree-walk.c b/tree-walk.c
index 3f54c02..af871c5 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -626,7 +626,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
&never_interesting))
return entry_interesting;
- if (item->use_wildcard) {
+ if (item->nowildcard_len < item->len) {
if (!fnmatch(match + baselen, entry->path, 0))
return entry_interesting;
@@ -642,7 +642,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
}
match_wildcards:
- if (!item->use_wildcard)
+ if (item->nowildcard_len == item->len)
continue;
/*
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
-O2 build on linux-2.6, without the patch:
$ time git rev-list --quiet HEAD -- '*.c'
real 0m40.770s
user 0m40.290s
sys 0m0.256s
With the patch
$ time ~/w/git/git rev-list --quiet HEAD -- '*.c'
real 0m34.288s
user 0m33.997s
sys 0m0.205s
The above command is not supposed to be widely popular. It's chosen
because it exercises pathspec matching a lot. The point is it cuts
down matching time for popular patterns like *.c, which could be used
as pathspec in other places.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 3 +++
dir.c | 17 +++++++++++++++--
dir.h | 1 +
tree-walk.c | 6 ++++--
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index bf031f1..d18f584 100644
--- a/cache.h
+++ b/cache.h
@@ -473,6 +473,8 @@ 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);
+#define PSF_ONESTAR 1
+
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int nr;
@@ -483,6 +485,7 @@ struct pathspec {
const char *match;
int len;
int nowildcard_len;
+ int flags;
} *items;
};
diff --git a/dir.c b/dir.c
index e4e6ca1..d00f240 100644
--- a/dir.c
+++ b/dir.c
@@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char *string,
pattern += prefix;
string += prefix;
}
+ if (flags & GF_ONESTAR) {
+ int pattern_len = strlen(++pattern);
+ int string_len = strlen(string);
+ return strcmp(pattern,
+ string + string_len - pattern_len);
+ }
return fnmatch(pattern, string, fnm_flags);
}
@@ -246,7 +252,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
}
if (item->nowildcard_len < item->len &&
- !git_fnmatch(match, name, 0, item->nowildcard_len - prefix))
+ !git_fnmatch(match, name,
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
return 0;
@@ -1446,8 +1454,13 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
item->nowildcard_len = simple_length(path);
- if (item->nowildcard_len < item->len)
+ item->flags = 0;
+ if (item->nowildcard_len < item->len) {
pathspec->has_wildcard = 1;
+ if (path[item->nowildcard_len] == '*' &&
+ no_wildcard(path + item->nowildcard_len + 1))
+ item->flags |= PSF_ONESTAR;
+ }
}
qsort(pathspec->items, pathspec->nr,
diff --git a/dir.h b/dir.h
index 4cd5074..590532b 100644
--- a/dir.h
+++ b/dir.h
@@ -143,6 +143,7 @@ extern int fnmatch_icase(const char *pattern, const char *string, int flags);
* The prefix part of pattern must not contains wildcards.
*/
#define GF_PATHNAME 1
+#define GF_ONESTAR 2
extern int git_fnmatch(const char *pattern, const char *string,
int flags, int prefix);
diff --git a/tree-walk.c b/tree-walk.c
index 2fcf3c0..42fe610 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -628,7 +628,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
if (item->nowildcard_len < item->len) {
if (!git_fnmatch(match + baselen, entry->path,
- 0, item->nowildcard_len - baselen))
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len - baselen))
return entry_interesting;
/*
@@ -654,7 +655,8 @@ match_wildcards:
strbuf_add(base, entry->path, pathlen);
if (!git_fnmatch(match, base->buf + base_offset,
- 0, item->nowildcard_len)) {
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len)) {
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 2/4] pathspec: do exact comparison on the leading non-wildcard part
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 18 +++++++++++++++++-
dir.h | 8 ++++++++
tree-walk.c | 6 ++++--
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index c391d46..e4e6ca1 100644
--- a/dir.c
+++ b/dir.c
@@ -34,6 +34,21 @@ int fnmatch_icase(const char *pattern, const char *string, int flags)
return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
}
+inline int git_fnmatch(const char *pattern, const char *string,
+ int flags, int prefix)
+{
+ int fnm_flags = 0;
+ if (flags & GF_PATHNAME)
+ fnm_flags |= FNM_PATHNAME;
+ if (prefix > 0) {
+ if (strncmp(pattern, string, prefix))
+ return FNM_NOMATCH;
+ pattern += prefix;
+ string += prefix;
+ }
+ return fnmatch(pattern, string, fnm_flags);
+}
+
static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
@@ -230,7 +245,8 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
return MATCHED_RECURSIVELY;
}
- if (item->nowildcard_len < item->len && !fnmatch(match, name, 0))
+ if (item->nowildcard_len < item->len &&
+ !git_fnmatch(match, name, 0, item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
return 0;
diff --git a/dir.h b/dir.h
index f5c89e3..4cd5074 100644
--- a/dir.h
+++ b/dir.h
@@ -139,4 +139,12 @@ extern int strcmp_icase(const char *a, const char *b);
extern int strncmp_icase(const char *a, const char *b, size_t count);
extern int fnmatch_icase(const char *pattern, const char *string, int flags);
+/*
+ * The prefix part of pattern must not contains wildcards.
+ */
+#define GF_PATHNAME 1
+
+extern int git_fnmatch(const char *pattern, const char *string,
+ int flags, int prefix);
+
#endif
diff --git a/tree-walk.c b/tree-walk.c
index af871c5..2fcf3c0 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -627,7 +627,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
return entry_interesting;
if (item->nowildcard_len < item->len) {
- if (!fnmatch(match + baselen, entry->path, 0))
+ if (!git_fnmatch(match + baselen, entry->path,
+ 0, item->nowildcard_len - baselen))
return entry_interesting;
/*
@@ -652,7 +653,8 @@ match_wildcards:
strbuf_add(base, entry->path, pathlen);
- if (!fnmatch(match, base->buf + base_offset, 0)) {
+ if (!git_fnmatch(match, base->buf + base_offset,
+ 0, item->nowildcard_len)) {
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 4/4] tree_entry_interesting: do basedir compare on wildcard patterns when possible
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
Currently we treat "*.c" and "path/to/*.c" the same way. Which means
we check all possible paths in repo against "path/to/*.c". One could
see that "path/elsewhere/foo.c" obviously cannot match "path/to/*.c"
and we only need to check all paths _inside_ "path/to/" against that
pattern.
This patch checks the leading fixed part of a pathspec against base
directory and exit early if possible. We could even optimize further
in "path/to/something*.c" case (i.e. check the fixed part against
name_entry as well) but that's more complicated and probably does not
gain us much.
-O2 build on linux-2.6, without and with this patch respectively:
$ time git rev-list --quiet HEAD -- 'drivers/*.c'
real 1m9.484s
user 1m9.128s
sys 0m0.181s
$ time ~/w/git/git rev-list --quiet HEAD -- 'drivers/*.c'
real 0m15.710s
user 0m15.564s
sys 0m0.107s
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
tree-walk.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/tree-walk.c b/tree-walk.c
index 42fe610..dcc1015 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -573,6 +573,52 @@ static int match_dir_prefix(const char *base,
}
/*
+ * Perform matching on the leading non-wildcard part of
+ * pathspec. item->nowildcard_len must be greater than zero. Return
+ * non-zero if base is matched.
+ */
+static int match_wildcard_base(const struct pathspec_item *item,
+ const char *base, int baselen,
+ int *matched)
+{
+ const char *match = item->match;
+ /* the wildcard part is not considered in this function */
+ int matchlen = item->nowildcard_len;
+
+ if (baselen) {
+ int dirlen;
+ /*
+ * Return early if base is longer than the
+ * non-wildcard part but it does not match.
+ */
+ if (baselen >= matchlen) {
+ *matched = matchlen;
+ return !strncmp(base, match, matchlen);
+ }
+
+ dirlen = matchlen;
+ while (dirlen && match[dirlen - 1] != '/')
+ dirlen--;
+
+ /* Return early if base is shorter than the
+ non-wildcard part but it does not match. Note that
+ base ends with '/' so we are sure it really matches
+ directory */
+ if (strncmp(base, match, baselen))
+ return 0;
+ *matched = baselen;
+ } else
+ *matched = 0;
+ /*
+ * we could have checked entry against the non-wildcard part
+ * that is not in base and does similar never_interesting
+ * optimization as in match_entry. For now just be happy with
+ * base comparison.
+ */
+ return entry_interesting;
+}
+
+/*
* Is a tree entry interesting given the pathspec we have?
*
* Pre-condition: either baselen == base_offset (i.e. empty path)
@@ -602,7 +648,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
const struct pathspec_item *item = ps->items+i;
const char *match = item->match;
const char *base_str = base->buf + base_offset;
- int matchlen = item->len;
+ int matchlen = item->len, matched = 0;
if (baselen >= matchlen) {
/* If it doesn't match, move along... */
@@ -647,9 +693,24 @@ match_wildcards:
if (item->nowildcard_len == item->len)
continue;
+ if (item->nowildcard_len &&
+ !match_wildcard_base(item, base_str, baselen, &matched))
+ return entry_not_interesting;
+
/*
* Concatenate base and entry->path into one and do
* fnmatch() on it.
+ *
+ * While we could avoid concatenation in certain cases
+ * [1], which saves a memcpy and potentially a
+ * realloc, it turns out not worth it. Measurement on
+ * linux-2.6 does not show any clear improvements,
+ * partly because of the nowildcard_len optimization
+ * in git_fnmatch(). Avoid micro-optimizations here.
+ *
+ * [1] if match_wildcard_base() says the base
+ * directory is already matched, we only need to match
+ * the rest, which is shorter so _in theory_ faster.
*/
strbuf_add(base, entry->path, pathlen);
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox