Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Felipe Balbi
Hi guys,

for whatever reason my git has started acting up with
sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
adress from the string:

 Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6

Removing the comment at the end of the line makes things work again. I
do remember, however, seeing this working since few weeks back I sent a
mail to stable (in fact the same one I'm using to test), so this could
be related to some perl updates, who knows ?!?

Anyway, here's output of git-send-email:

 $ git send-email --to linux-...@vger.kernel.org 
 ../linux/0001-usb-dwc3-gadget-fix-endpoint-always-busy-bug.diff --dry-run
 ../linux/0001-usb-dwc3-gadget-fix-endpoint-always-busy-bug.diff
 (mbox) Adding cc: Felipe Balbi ba...@ti.com from line 'From: Felipe Balbi 
 ba...@ti.com'
 (body) Adding cc: sta...@vger.kernel.org #v3.4 v3.5 v3.6 from line 'Cc: 
 sta...@vger.kernel.org #v3.4 v3.5 v3.6'
 (body) Adding cc: Felipe Balbi ba...@ti.com from line 'Signed-off-by: 
 Felipe Balbi ba...@ti.com'
 Use of uninitialized value $cc in string eq at 
 /usr/libexec/git-core/git-send-email line 997.
 Use of uninitialized value $cc in quotemeta at 
 /usr/libexec/git-core/git-send-email line 997.
 W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 
 v3.5 v3.6
 W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 
 v3.5 v3.6
 Dry-OK. Log says:
 Sendmail: /usr/bin/msmtp -i linux-...@vger.kernel.org ba...@ti.com
 From: Felipe Balbi ba...@ti.com
 To: linux-...@vger.kernel.org
 Cc: Felipe Balbi ba...@ti.com
 Subject: [PATCH] usb: dwc3: gadget: fix 'endpoint always busy' bug
 Date: Mon, 19 Nov 2012 11:54:16 +0200
 Message-Id: 1353318856-14987-1-git-send-email-ba...@ti.com
 X-Mailer: git-send-email 1.8.0
 
 Result: OK

$ perl --version

This is perl 5, version 14, subversion 2 (v5.14.2) built for 
x86_64-linux-gnu-thread-multi
(with 72 registered patches, see perl -V for more detail)

Copyright 1987-2011, Larry Wall

Perl may be copied only under the terms of either the Artistic License or the
GNU General Public License, which may be found in the Perl 5 source kit.

Complete documentation for Perl, including FAQ lists, should be found on
this system using man perl or perldoc perl.  If you have access to the
Internet, point your browser at http://www.perl.org/, the Perl Home Page.

And attached you can find the patch file which I'm using

-- 
balbi
From 041d81f493d90c940ec41f0ec98bc7c4f2fba431 Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Thu, 4 Oct 2012 11:58:00 +0300
Subject: [PATCH] usb: dwc3: gadget: fix 'endpoint always busy' bug

If a USB transfer has already been started, meaning
we have already issued StartTransfer command to that
particular endpoint, DWC3_EP_BUSY flag has also
already been set.

When we try to cancel this transfer which is already
in controller's cache, we will not receive XferComplete
event and we must clear DWC3_EP_BUSY in order to allow
subsequent requests to be properly started.

The best place to clear that flag is right after issuing
DWC3_DEPCMD_ENDTRANSFER.

Cc: sta...@vger.kernel.org #v3.4 v3.5 v3.6
Reported-by: Moiz Sonasath m-sonas...@ti.com
Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c9e729a..7b7dedd 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1904,7 +1904,7 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
 	ret = dwc3_send_gadget_ep_cmd(dwc, dep-number, cmd, params);
 	WARN_ON_ONCE(ret);
 	dep-resource_index = 0;
-
+	dep-flags = ~DWC3_EP_BUSY;
 	udelay(100);
 }
 
-- 
1.8.0



signature.asc
Description: Digital signature


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Krzysztof Mazur
On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
 Hi guys,
 
 for whatever reason my git has started acting up with
 sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
 adress from the string:
 
  Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6
 
 Removing the comment at the end of the line makes things work again. I
 do remember, however, seeing this working since few weeks back I sent a
 mail to stable (in fact the same one I'm using to test), so this could
 be related to some perl updates, who knows ?!?

You probably just installed Email::Valid package.

The current git-send-email works a little better and just prints an error:

W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 v3.5 
v3.6


This patch should fix the problem, now after email any garbage is
removed while extracting address.

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..bb659da 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -828,7 +828,7 @@ sub extract_valid_address {
# check for a local address:
return $address if ($address =~ /^($local_part_regexp)$/);
 
-   $address =~ s/^\s*(.*)\s*$/$1/;
+   $address =~ s/^\s*(.*).*$/$1/;
if ($have_email_valid) {
return scalar Email::Valid-address($address);
} else {

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Felipe Balbi
Hi,

On Mon, Nov 19, 2012 at 04:18:45PM +0100, Krzysztof Mazur wrote:
 On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
  Hi guys,
  
  for whatever reason my git has started acting up with
  sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
  adress from the string:
  
   Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6
  
  Removing the comment at the end of the line makes things work again. I
  do remember, however, seeing this working since few weeks back I sent a
  mail to stable (in fact the same one I'm using to test), so this could
  be related to some perl updates, who knows ?!?
 
 You probably just installed Email::Valid package.
 
 The current git-send-email works a little better and just prints an error:
 
 W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 
 v3.5 v3.6
 
 
 This patch should fix the problem, now after email any garbage is
 removed while extracting address.

worked like a charm. When you send as a proper patch, you can add my:

Tested-by: Felipe Balbi ba...@ti.com

thanks

-- 
balbi


signature.asc
Description: Digital signature


Re: Should --pretty=format:%b respect i18n.logoutputencoding just like normal git log?

2012-11-19 Thread Pat Notz
On Mon, Nov 19, 2012 at 6:08 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 Changes around this code in history is a bit unclear. User format
 learns to get log encoding from a field in 177b29d (pretty.c: teach
 format_commit_message() to reencode the output - 2010-11-02), but this
 field is only set for --fixup and --squash (in a few commits later).
 This makes git log --pretty=format:%b always ignore the output
 encoding config key. I don't think %b output should be different from
 normal log output, which does respect output encoding. Pat, any
 reasons not to do it?

Nope, that looks like it was an oversight on my part. Nice find.

~ Pat

 --
 Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 0/2] New zsh wrapper

2012-11-19 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 The second patch is new, in order for users to get the same features when
 sourcing the bash script (they don't need to change anything). They'll get a
 warning suggesting to check the new script git-completion.zsh. Eventually, 
 that
 support would be dropped from the bash script.

 Some people were suggesting something like this, so here it is.

 Can we merge the zsh wrapper now?

 Felipe Contreras (2):
   completion: add new zsh completion
   completion: start moving to the new zsh completion

  contrib/completion/git-completion.bash | 104 
 +++--
  contrib/completion/git-completion.zsh  |  78 +
  2 files changed, 139 insertions(+), 43 deletions(-)
  create mode 100644 contrib/completion/git-completion.zsh

Thanks; I am a bit puzzled as to the progression of this series, as
it spanned many months.  I *think* the following are the previous
ones, but I may be mixing up v$n patches for other series, so just
to make sure (please correct if I am mistaken):

 * (v1) http://thread.gmane.org/gmane.comp.version-control.git/189310
   with only git-completion.zsh without any changes to the bash
   side;

 * (v2) http://thread.gmane.org/gmane.comp.version-control.git/189381
   without bash side changes;

 * (v3) http://thread.gmane.org/gmane.comp.version-control.git/196720
   without bash side changes;

 * (v6) http://thread.gmane.org/gmane.comp.version-control.git/208170
   with COMPREPLY changes;

 * This one, with removal of zsh specific workarounds from bash
   completion script.

I do not care too much about how v4 and v5 looked like; I primarily
am interested in knowing if I can discard 208170 from my inbox
safely ;-).

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Junio C Hamano
Krzysztof Mazur krzys...@podlesie.net writes:

 On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
 Hi guys,
 
 for whatever reason my git has started acting up with
 sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
 adress from the string:
 
  Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6
 
 Removing the comment at the end of the line makes things work again. I
 do remember, however, seeing this working since few weeks back I sent a
 mail to stable (in fact the same one I'm using to test), so this could
 be related to some perl updates, who knows ?!?

 You probably just installed Email::Valid package.

 The current git-send-email works a little better and just prints an error:

 W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 
 v3.5 v3.6


 This patch should fix the problem, now after email any garbage is
 removed while extracting address.

 diff --git a/git-send-email.perl b/git-send-email.perl
 index 5a7c29d..bb659da 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -828,7 +828,7 @@ sub extract_valid_address {
   # check for a local address:
   return $address if ($address =~ /^($local_part_regexp)$/);
  
 - $address =~ s/^\s*(.*)\s*$/$1/;
 + $address =~ s/^\s*(.*).*$/$1/;
   if ($have_email_valid) {
   return scalar Email::Valid-address($address);
   } else {

Given that the problematic line

Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y

is not even a valid e-mail address, doesn't this new logic belong to
sanitize_address() conceptually?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4.1 5/5] push: update remote tags only with force

2012-11-19 Thread Junio C Hamano
Chris Rorvick ch...@rorvick.com writes:

 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.

This, if it were implemented back when we first added git push,
would have been a nice safety, but added after 1.8.0 it would be a
regression, so we would need backward compatibility notes in the
release notes.

Not an objection; just making a mental note.

 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

I can already see the next person asking I can update refs/notes
the same way.  The doc says it applies only to the branches.  Is
this a bug?.

 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,

This is somewhat an unfortunate churn.  Perhaps is_a_tag could have
started its life under its final name in the series?

I am assuming that the struct members will be initialized to 0 (false),
so everything by default is now not forwardable if somebody forgets
to set this flag?

   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.
*/

I think this is unreadable.  Isn't this more like

(1.5) if the destination is inside refs/tags/ and already
  exists, you are not allowed to overwrite it without
  --force or +A:B notation.

early in the rule set?

 - 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.
 +  */

Huh?  prefixcmp() excludes refs/tags/ hierarchy. What is an
annotated tag doing there?  Is this comment outdated???

/*
 * Also please end the first line of a multi-line
 * comment with '/', '*', and nothing else.
 */

Regarding the other arm of this if (not in refs/tags/ hierarchy),
what happens when refs/tags/foo is a reference to a 

Re: [PATCH v3 0/6] completion: test consolidations

2012-11-19 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 These started from a discussion with SZEDER, but then I realized there were
 many improvements possible.

Thanks; I'd loop him in and wait for his acks/reviews.


 Changes since v2:

  * Updated comments and commit messages

 Changes since v1:

  * A lot more cleanups

 Felipe Contreras (6):
   completion: add comment for test_completion()
   completion: standardize final space marker in tests
   completion: simplify tests using test_completion_long()
   completion: consolidate test_completion*() tests
   completion: refactor __gitcomp related tests
   completion: simplify __gitcomp() test helper

  t/t9902-completion.sh | 133 
 +++---
  1 file changed, 51 insertions(+), 82 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-describe fails with --dirty is incompatible with committishes if passing HEAD as argument

2012-11-19 Thread Francis Moreau
On Mon, Nov 19, 2012 at 8:36 PM, Junio C Hamano gits...@pobox.com wrote:
 Francis Moreau francis.m...@gmail.com writes:

 Inside the kernel repository, I tried this:

 $ git describe --dirty --match 'v[0-9]*' --abbrev=4 HEAD
 fatal: --dirty is incompatible with committishes

 If 'HEAD' is removed then git-describe works as expected.

 Is that expected ?

 I would say so, at least in modern codebase.

 git describe without any commit object name used to mean describe
 the HEAD commit using the better known points before the --dirty
 option was introduced.

 But --dirty makes it describe the current checkout.  For example,
 output from git describe --dirty v1.8.0-211-gd8b4531-dirty means
 your working tree contains work-in-progress based on d8b4531, which
 is 211 commits ahead of the v1.8.0 tag.  So conceptually, it should
 not take any commit, even if it were spelled HEAD.

 git describe --dirty HEAD^^ would be an utter nonsense for the
 same reason.

Thanks for explaining, that makes sense.

--
Francis
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: `git mv` has ambiguous error message for non-existing target

2012-11-19 Thread Patrick Lehner

On Sa 17 Nov 2012 20:35:09 CET, Junio C Hamano wrote:

Patrick Lehner lehner.patr...@gmx.de writes:


But just because mv's error essage isnt very good, does that mean git
mv's error message mustn't be better?


Did I say the error message from 'mv' was not very good in the
message you are responding to (by the way, this is why you should
never top-post when you are responding to a message on this list)?

I meant to say that the message from 'mv' is good enough, so is the
one given by 'git mv'.

I wouldn't reject a patch that updates our message to something more
informative without looking at it, though.


I apologize for top-posting -- I don't usually use mailing lists and am 
not aware of the usual netiquette.


And yes, I did interpret a bit more into your reply than was there.

I wouldn't call the 'mv' error message good enough in this case, but 
very well, opinions may very well differ. Unfortunately, I have no time 
to get into the git code and contribution guidelines, so I cannot 
submit a patch myself. I would appreciate if someone else who shares my 
sentiment and knows their way around the git source a bit could find 
the time to add this :)


Regards,
Patrick
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] pathspec: save the non-wildcard length part

2012-11-19 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 We marks pathspec with wildcards with the field use_wildcard. We could

s/marks/mark;

 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

s/for /used /;

 possible before fnmatch - 2012-06-07)

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com

The code looks straightforward and correct.  Thanks.

 ---
  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;
  
   /*
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] pathspec: do exact comparison on the leading non-wildcard part

2012-11-19 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@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);

How would we protect this optimization from future breakages?

Once we start using FNM_PERIOD, this becomes unsafe, as the simple
part in foo/bar*baz would be foo/bar with remainder *baz.

The pattern foo/bar*baz should match foo/bar.baz with FNM_PERIOD
set.  But with this optimization, fnmatch is fed pattern=*baz,
string=.baz and they would not match.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] pathspec: apply *.c optimization from exclude

2012-11-19 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 -O2 build on linux-2.6, without the patch:

Before the result, can you briefly explain what '*.c optimization
from exclude' the title talks about is?

When a pattern contains only a single asterisk, e.g. foo*bar,
after literally comparing the leading part foo with the
string, we can compare the tail of the string and make sure it
matches bar, instead of running fnmatch() on *bar against
the remainder of the string.

The funny thing was that trying to explain what the logic should do
makes one realize potential issues in the implementation of that
logic ;-)  See below.

 $ time git rev-list --quiet HEAD -- '*.c'

 real0m40.770s
 user0m40.290s
 sys 0m0.256s

 With the patch

 $ time ~/w/git/git rev-list --quiet HEAD -- '*.c'

 real0m34.288s
 user0m33.997s
 sys 0m0.205s

 The above command is not supposed to be widely popular.

Hrm, perhaps.  I use git grep pattern -- \*.c quite a lot, but
haven't seen use case for \*.c in the context of the log family.

 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

Together with the GF_ prefix in the previous, PSF_ prefix needs a
bit of in-code explanation.  Is it just an RC3L (random combination
of 3 letters?)

 @@ -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);
 + }

What happens when pattern=foo*oob and string=foob?

The prefix match before this code determines that the literal prefix
in the pattern matches with the early part of the string, and makes
pattern=*oob and string=b.

When you come to strcmp(), you see that string_len is 1, pattern_len
is 3, and pattern is oob.  string+string_len-pattern_len = oob,
one past the beginning of the original string foob.  They match.

Oops?

return (string_len  pattern_len) ||
strcmp(pattern, string + string_len - pattern_len);

perhaps?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 0/2] New zsh wrapper

2012-11-19 Thread Felipe Contreras
On Mon, Nov 19, 2012 at 7:55 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 The second patch is new, in order for users to get the same features when
 sourcing the bash script (they don't need to change anything). They'll get a
 warning suggesting to check the new script git-completion.zsh. Eventually, 
 that
 support would be dropped from the bash script.

 Some people were suggesting something like this, so here it is.

 Can we merge the zsh wrapper now?

 Felipe Contreras (2):
   completion: add new zsh completion
   completion: start moving to the new zsh completion

  contrib/completion/git-completion.bash | 104 
 +++--
  contrib/completion/git-completion.zsh  |  78 +
  2 files changed, 139 insertions(+), 43 deletions(-)
  create mode 100644 contrib/completion/git-completion.zsh

 Thanks; I am a bit puzzled as to the progression of this series, as
 it spanned many months.  I *think* the following are the previous
 ones, but I may be mixing up v$n patches for other series, so just
 to make sure (please correct if I am mistaken):

  * (v1) http://thread.gmane.org/gmane.comp.version-control.git/189310
with only git-completion.zsh without any changes to the bash
side;

Yes, and with a lot of code that is not strictly needed.

  * (v2) http://thread.gmane.org/gmane.comp.version-control.git/189381
without bash side changes;

Yes, also with more code.

  * (v3) http://thread.gmane.org/gmane.comp.version-control.git/196720
without bash side changes;

Yes, now it's simpler due to changes in bash side already in.

  * (v6) http://thread.gmane.org/gmane.comp.version-control.git/208170
with COMPREPLY changes;

Yeap.

  * This one, with removal of zsh specific workarounds from bash
completion script.

Yes, although that is a separate patch.

 I do not care too much about how v4 and v5 looked like; I primarily
 am interested in knowing if I can discard 208170 from my inbox
 safely ;-).

Yes you can. The rest of the patches I sent in a different series.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Felipe Contreras
On Mon, Nov 19, 2012 at 8:27 PM, Junio C Hamano gits...@pobox.com wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:

 On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
 Hi guys,

 for whatever reason my git has started acting up with
 sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
 adress from the string:

  Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6

 Removing the comment at the end of the line makes things work again. I
 do remember, however, seeing this working since few weeks back I sent a
 mail to stable (in fact the same one I'm using to test), so this could
 be related to some perl updates, who knows ?!?

 You probably just installed Email::Valid package.

 The current git-send-email works a little better and just prints an error:

 W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 
 v3.5 v3.6


 This patch should fix the problem, now after email any garbage is
 removed while extracting address.

 diff --git a/git-send-email.perl b/git-send-email.perl
 index 5a7c29d..bb659da 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -828,7 +828,7 @@ sub extract_valid_address {
   # check for a local address:
   return $address if ($address =~ /^($local_part_regexp)$/);

 - $address =~ s/^\s*(.*)\s*$/$1/;
 + $address =~ s/^\s*(.*).*$/$1/;
   if ($have_email_valid) {
   return scalar Email::Valid-address($address);
   } else {

 Given that the problematic line

 Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y

 is not even a valid e-mail address, doesn't this new logic belong to
 sanitize_address() conceptually?

That would be great, it would also help the cc-cmd stuff. The
get_maintainer.pl patch from the Linux kernel outputs something like:

David Airlie airl...@linux.ie (maintainer:DRM DRIVERS)
Ben Skeggs bske...@redhat.com
(commit_signer:17/19=89%,commit_signer:43/46=93%)
Maxim Levitsky maximlevit...@gmail.com (commit_signer:3/19=16%)
Greg Kroah-Hartman gre...@linuxfoundation.org (commit_signer:2/19=11%)
Dave Airlie airl...@redhat.com (commit_signer:2/19=11%,commit_signer:3/46=7%)
Alex Deucher alexander.deuc...@amd.com (commit_signer:1/19=5%)
dri-de...@lists.freedesktop.org (open list:DRM DRIVERS)
linux-ker...@vger.kernel.org (open list)

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Auto-repo-repair

2012-11-19 Thread Enrico Weigelt

  How would the broken repository be sure of what it is missing to
  request it from the other side?
 
  fsck will find missing objects.
 
 And what about the objects referred to by objects that are missing?

Will be fetched after multiple iterations.
We could even introduce some 'fsck --autorepair' mode, which triggers
it to fetch any missing object from its remotes.

Maybe even introduce a concept of peer object stores, which (a bit like
alternates) are asked for objects that arent locally availabe - that
could be even a plain venti store.


cu
-- 
Mit freundlichen Grüßen / Kind regards 

Enrico Weigelt 
VNC - Virtual Network Consult GmbH 
Head Of Development 

Pariser Platz 4a, D-10117 Berlin
Tel.: +49 (30) 3464615-20
Fax: +49 (30) 3464615-59

enrico.weig...@vnc.biz; www.vnc.de 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] completion: test consolidations

2012-11-19 Thread Felipe Contreras
On Mon, Nov 19, 2012 at 9:34 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 These started from a discussion with SZEDER, but then I realized there were
 many improvements possible.

 Thanks; I'd loop him in and wait for his acks/reviews.

I thought my series-cc-cmd would add him. Maybe I'm using a version of
'git send-email' that doesn't have that.

Either way, we already know what he will say, to the previous series
he said he was against consolidation of test scripts. So I don't see
how the situation would change.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Krzysztof Mazur
On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
 Given that the problematic line
 
   Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y
 
 is not even a valid e-mail address, doesn't this new logic belong to
 sanitize_address() conceptually?

Yes, it's much better to do it in the sanitize_address().

Felipe, may you check it?

Krzysiek
-- 8 --
Subject: [PATCH] git-send-email: remove garbage after email address

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

Cc: Stable kernel sta...@vger.kernel.org #v3.4 v3.5 v3.6

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 4 
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..9840d0a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
my ($recipient) = @_;
+
+   # remove garbage after email address
+   $recipient =~ s/(.*).*$/$1/;
+
my ($recipient_name, $recipient_addr) = ($recipient =~ 
/^(.*?)\s*(.*)/);
 
if (not $recipient_name) {
-- 
1.8.0.283.gc57d856

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git config --git-all can return non-zero error code

2012-11-19 Thread Timur Tabi
According to the man page for git-config, the --git-all option is not
supposed to return an error code:

--get-all
Like get, but does not fail if the number of values for the key is
not exactly one.

IMHO, zero is also not exactly one, but I still get a 1 exit code if the
key does not exist.

My .git/config says this:

[user cq.branch]
mastr = upstream/master

git-config --get-all does this:

$ git config --get-all user.cq.branch.master
$ echo $?
1
$ git config --get-all user.cq.branch.mastr
upstream/master
$ echo $?
0

I just want git to return nothing if the key doesn't exist.  I don't want
it to return an exit code.  Is there a way to do this?  I think either the
code is broken or the documentation needs to be changed.

I'm running git version 1.7.3.4

-- 
Timur Tabi
Linux kernel developer at Freescale

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur krzys...@podlesie.net 
 wrote:

 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -924,6 +924,10 @@ sub quote_subject {
  # use the simplest quoting being able to handle the recipient
  sub sanitize_address {
 my ($recipient) = @_;
 +
 +   # remove garbage after email address
 +   $recipient =~ s/(.*).*$/$1/;
 +

 Looks fine, but I would do s/(.*?)(.*)$/$1/, so that 'test
 f...@bar.com #comment' gets the second comment removed.

Yeah, but do you need to capture the second group?  IOW, like
s/(.*?).*$/$1/ perhaps?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Junio C Hamano
Krzysztof Mazur krzys...@podlesie.net writes:

 On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
 Given that the problematic line
 
  Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y
 
 is not even a valid e-mail address, doesn't this new logic belong to
 sanitize_address() conceptually?

 Yes, it's much better to do it in the sanitize_address().

Note that I did not check that all the addresses that are handled by
extract-valid-address came through sanitize-address function, so
unlike your original patch, this change alone may still pass some
garbage to Email::Valid-address().  I tend to think that is a
progress; we should make sure all the addresses are sanitized before
using them for sending messages out.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git config --git-all can return non-zero error code

2012-11-19 Thread Junio C Hamano
Timur Tabi ti...@freescale.com writes:

 According to the man page for git-config, the --git-all option is not
 supposed to return an error code:

 --get-all
   Like get, but does not fail if the number of values for the key is
   not exactly one.

 IMHO, zero is also not exactly one,...

It should have stated like get, but unlike get, it does not fail
when there are multiple values for the key, but the documentation
was written with that specific knowledge that --get has a logic to
make it fail when there are ambiguities, and wanted to stress that
difference.  It forgot to mention their similarity explicitly and
relied on Like get part to mean (1) shows the value(s) given to
the key, and (2) it is an error if there is no such key defined.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] config --get-all: it is an error not to see any value

2012-11-19 Thread Junio C Hamano
The wording ... is not exactly one. incorrectly hinted as if
having no value is not an error, but that was not what was
intended.  Clarify what similarity it wants to mention by
elaborating the Like get part of the description.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-config.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git i/Documentation/git-config.txt w/Documentation/git-config.txt
index eaea079..273239d 100644
--- i/Documentation/git-config.txt
+++ w/Documentation/git-config.txt
@@ -85,8 +85,9 @@ OPTIONS
found and error code 2 if multiple key values were found.
 
 --get-all::
-   Like get, but does not fail if the number of values for the key
-   is not exactly one.
+   Like `--get`, shows the value(s) given to the key, and signals an
+   error if there is no such key.  Unlike `--get`, does not
+   fail if there are multiple values defined for the key.
 
 --get-regexp::
Like --get-all, but interprets the name as a regular expression and
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] pickaxe: use textconv for -S counting

2012-11-19 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Nov 13, 2012 at 03:13:19PM -0800, Junio C Hamano wrote:

   static int has_changes(struct diff_filepair *p, struct diff_options *o,
regex_t *regexp, kwset_t kws)
   {
  +  struct userdiff_driver *textconv_one = get_textconv(p-one);
  +  struct userdiff_driver *textconv_two = get_textconv(p-two);
  +  mmfile_t mf1, mf2;
  +  int ret;
  +
 if (!o-pickaxe[0])
 return 0;
   
  -  if (!DIFF_FILE_VALID(p-one)) {
  -  if (!DIFF_FILE_VALID(p-two))
  -  return 0; /* ignore unmerged */
 
 What happened to this part that avoids showing nonsense for unmerged
 paths?

 It's moved down. fill_one will return an empty mmfile if
 !DIFF_FILE_VALID, so we end up here:

 fill_one(p-one, mf1, textconv_one);
 fill_one(p-two, mf2, textconv_two);

 if (!mf1.ptr) {
 if (!mf2.ptr)
 ret = 0; /* ignore unmerged */

 Prior to this change, we didn't use fill_one, so we had to check manually.

  +  /*
  +   * If we have an unmodified pair, we know that the count will be the
  +   * same and don't even have to load the blobs. Unless textconv is in
  +   * play, _and_ we are using two different textconv filters (e.g.,
  +   * because a pair is an exact rename with different textconv attributes
  +   * for each side, which might generate different content).
  +   */
  +  if (textconv_one == textconv_two  diff_unmodified_pair(p))
  +  return 0;
 
 I am not sure about this part that cares about the textconv.
 
 Wouldn't the normal git diff A B skip the filepair that are
 unmodified in the first place at the object name level without even
 looking at the contents (see e.g. diff_flush_patch())?

 Hmph. The point was to find the case when the paths are different (e.g.,
 in a rename), and therefore the textconvs might be different. But I
 think I missed the fact that diff_unmodified_pair will note the
 difference in paths. So just calling diff_unmodified_pair would be
 sufficient, as the code prior to my patch does.

 I thought the point was an optimization to avoid comparing contains() on
 the same data (which we can know will match without looking at it).

Yes.

 Exact renames are the obvious one, but they are not handled here.

That is half true.  Before this change, we will find the same number
of needles and this function would have said no differences in a
very inefficient way.  After this change, we may apply different
textconv filters and this function will say there is a difference,
even though we wouldn't see such a difference at the content level
if there wasn't any rename.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/4] diff: introduce diff.submodule configuration variable

2012-11-19 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 diff --git a/t/t4041-diff-submodule-option.sh 
 b/t/t4041-diff-submodule-option.sh
 index 6c01d0c..e401814 100755
 --- a/t/t4041-diff-submodule-option.sh
 +++ b/t/t4041-diff-submodule-option.sh
 @@ -33,6 +33,7 @@ test_create_repo sm1 
  add_file . foo /dev/null
  
  head1=$(add_file sm1 foo1 foo2)
 +fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)

That looks like quite a roundabout way to say

$(cd sm1; git rev-parse --verify HEAD)

doesn't it?  I know this is just moved code from the original, so I
am not saying this should be fixed in this commit.

Existing code in t7401 may want to be cleaned up, perhaps after this
topic settles.  The add_file() function, for example, has at least
these points:

 - do we know 7 hexdigits is always the right precision?
 - what happens when it fails to create a commit in one of the steps
   while looping over $@ in its loop?
 - the function uses the cd there and then come back, not
   go there in a subshell and do whatever it needs to pattern.

 +test_expect_success 'added submodule, set diff.submodule' 

s/added/add/;

Shouldn't it test the base case where no diff.submodule is set?  For
those people, the patch should not change the output when they do or
do not pass --submodule option, right?

 + git config diff.submodule log 
 + git add sm1 
 + git diff --cached actual 
 + cat expected -EOF 
 +Submodule sm1 000...$head1 (new submodule)
 +EOF
 + git config --unset diff.submodule 
 + test_cmp expected actual
 +
 +
 +test_expect_success '--submodule=short overrides diff.submodule' 
 + git config diff.submodule log 
 + git add sm1 
 + git diff --submodule=short --cached actual 
 + cat expected -EOF 
 +diff --git a/sm1 b/sm1
 +new file mode 16
 +index 000..a2c4dab
 +--- /dev/null
  b/sm1
 +@@ -0,0 +1 @@
 ++Subproject commit $fullhead1
 +EOF
 + git config --unset diff.submodule 
 + test_cmp expected actual
 +
 +
  commit_file sm1 
  head2=$(add_file sm1 foo3)
  
 @@ -73,7 +102,6 @@ EOF
   test_cmp expected actual
  
  
 -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
  fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
  test_expect_success 'modified submodule(forward) --submodule=short' 
   git diff --submodule=short actual 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option

2012-11-19 Thread Junio C Hamano
W. Trevor King wk...@tremily.us writes:

 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 am not quite sure I agree.  When the project says Use the tip of
'bar' branch for the submodule 'foo' at the top-level, does an
individual user who is not working on the submodule 'foo' but merely
is using it have any clue as to why the submodule's 'foo' branch
'foo' moved, or does he necessarily even care?

For such a user working at the top-level superproject, or working on
one part of the project, possibly on a submodule other than 'foo',
wouldn't the natural thing to do would be to run git pull at the
top-level, maybe with --recursive to update the top-level and all
the submodules to start the day.

Now, since somebody created the top-level commit you have just
pulled and checked out, other people may have worked on submodule
'foo' [*1*].  What should happen on git submodule update foo?  It
would notice that the submodule 'foo' is set to float, and would
check out the tip of the branch 'bar', not the commit recorded in
the top-level superproject, in the working tree for 'foo', no?

What should appear in git diff?  The working tree taken as a whole
is different from what the superproject's commit describes (which is
the state the person who created the superproject wanted to record)
even though this user does not have anything to do with the change
at 'foo' from the recorded commit to the current tip of 'bar'.  What
would his description for the reason why the branch was updated?

I think I would agree that git diff should not hide such changes
(after all, when this user records his change to the overall project
in the top-level supermodule, he will be recording the state with
the commit at the tip of 'bar' checked out in the working tree of
the submodule 'foo'), but I am not sure if the user can say anything
sensible, other than tip of 'bar' branch in submodule 'foo' was
changed by others, in the resulting commit.


[Footnote]

*1* This may look like a non-issue if you assume that the person who
updates the 'bar' branch of submodule 'foo' always updates the
gitlink in the superproject's commit to point at that updated
commit, but that assumption is flawed; the submodule project is a
project on its own and can be worked on without what other projects
bind it as their submodules.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option

2012-11-19 Thread W. Trevor King
On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
 W. Trevor King wk...@tremily.us writes:
 
  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 am not quite sure I agree.  When the project says Use the tip of
 'bar' branch for the submodule 'foo' at the top-level, does an
 individual user who is not working on the submodule 'foo' but merely
 is using it have any clue as to why the submodule's 'foo' branch
 'foo' moved, or does he necessarily even care?

If he doesn't care, why is he updating the submodule gitlink?

 Now, since somebody created the top-level commit you have just
 pulled and checked out, other people may have worked on submodule
 'foo' [*1*].  What should happen on git submodule update foo?

If the 'foo' checkout is not the one listed in the superproject's
.gitmodules, the update should bail with an appropriate error message,
and let the user sort things out.

  $ git submodule update --pull foo
  error: Your local changes to the following submodule would be
  overwritten by update:…

This is similar to how Git currently bails on dirty-tree branch
switches:

  $ git checkout my-branch
  error: Your local changes to the following files would be
  overwritten by checkout:…

Without --pull, the update command is intended to checkout the hash
specified in .gitmodules.  If you've committed some local work in foo
and then explicitly ask for an update, I suppose you get clobbered.

 What should appear in git diff?  The working tree taken as a whole
 is different from what the superproject's commit describes (which is
 the state the person who created the superproject wanted to record)
 even though this user does not have anything to do with the change
 at 'foo' from the recorded commit to the current tip of 'bar'.  What
 would his description for the reason why the branch was updated?

The submodule content is not part of the superproject.  All the
superproject has is a gitlink.  If the gitlink hasn't changed, git
diff in the superproject shouldn't say anything.

I'll probably have time to write up v4 over the weekend.  Maybe having
a more explicit example will clear things up.

-- 
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


signature.asc
Description: OpenPGP digital signature


Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)

2012-11-19 Thread Felipe Contreras
On Tue, Nov 20, 2012 at 12:55 AM, Junio C Hamano gits...@pobox.com wrote:

 * fc/remote-testgit-feature-done (2012-10-29) 1 commit
  - remote-testgit: properly check for errors

Pinging Sverre again.

I now think that we need a better solution with waitpid() to check the
status of the process, so that both import and export get proper error
checks. I found that out the hard way with a buggy remote helper.

I still think this patch is good regardless, but not so big of a
priority. I'm not going to purse it more, if it gets in, good, if not,
a waitpid() patch would take care of it.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4.1 5/5] push: update remote tags only with force

2012-11-19 Thread Chris Rorvick
On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano gits...@pobox.com wrote:
 Chris Rorvick ch...@rorvick.com writes:
  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

 I can already see the next person asking I can update refs/notes
 the same way.  The doc says it applies only to the branches.  Is
 this a bug?.

Sure, will fix.

 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,

 This is somewhat an unfortunate churn.  Perhaps is_a_tag could have
 started its life under its final name in the series?

 I am assuming that the struct members will be initialized to 0 (false),
 so everything by default is now not forwardable if somebody forgets
 to set this flag?

Yeah, this was one of a few stupid mistakes.  Previously I used
'forwardable' throughout, but that is awkward except in the last
commit since until then everything is allowed to fast-forward and the
flag is only used to output tag-specific advice.  But inverting the
meaning of the flag is dumb, and I didn't even do it right.

But, as I think you're suggesting, it probably makes more sense to use
a flag that prevents fast-forwarding when set.  So maybe
not_forwardable, or is_a_tag = not_forwardable if you think the
renaming is a good idea.  Or maybe just is_a_tag?  I think the
rename is good because its meaning is more general in the last commit.

 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.
*/

 I think this is unreadable.  Isn't this more like

 (1.5) if the destination is inside refs/tags/ and already
   exists, you are not allowed to overwrite it without
   --force or +A:B notation.

 early in the rule set?

Yes, something like that is much better.

 - 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.
 +  */

 Huh?  prefixcmp() excludes refs/tags/ hierarchy. What is an
 annotated tag doing there?  Is this comment outdated???

I think the comment is accurate, although inverting the meaning of the
flag probably doesn't help the clarity of this change.

What do you mean by there?  Annotated tags normally are referenced
via something under refs/tags/, but they could be elsewhere, right?
So what I meant to say was: if the reference is not under refs/tags/
then the starting point has to be a commit for it to be forwardable.

I interpreted parse_object(ref-old_sha1) == NULL as the old thing not
existing (rule 1) but I'm pretty sure that was a mistake.  I should
first test it with is_null_sha1, if true then it doesn't exist.
Otherwise parse_object() returning NULL means we just don't have the
object and therefore should follow rule 2.

 Regarding the other arm of this if (not in refs/tags/ hierarchy),
 what happens when refs/tags/foo is a reference to a blob or a tree?
 This series declares that the point of tag is not to let people to
 move it willy-nilly, and I think it is in line with its spirit if
 you just rejected non-creation events.

Nothing under refs/tags/ is updateable and only commits are updateable
outside of that due to the new logic.  The ref_newer() call will
reject updating to blobs and trees, although that probably isn't the
right thing to do.  Previously I was ensuring both old and new objects
were commits if the ref was not 

Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option

2012-11-19 Thread Junio C Hamano
W. Trevor King wk...@tremily.us writes:

 On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
 W. Trevor King wk...@tremily.us writes:
 ...
  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 am not quite sure I agree.  When the project says Use the tip of
 'bar' branch for the submodule 'foo' at the top-level, does an
 individual user who is not working on the submodule 'foo' but merely
 is using it have any clue as to why the submodule's 'foo' branch
 'foo' moved, or does he necessarily even care?

 If he doesn't care, why is he updating the submodule gitlink?

He may not be updating the gitlink with git add foo at the
top-level superproject level.  He is just using that submodule as
part of the larger whole as he is working on either the top-level or
some other submodule.  And checkout of 'foo' is necessary in the
working tree for him to work in the larger context of the project,
and 'foo' is set to float at the tip of its 'bar' branch.  And that
checkout results in a commit that is different from the commit the
gitlink suggests, perhaps because somebody worked in 'foo' submodule
and advanced the tip of branch 'bar'.

So:

 - at the top-level superproject level, entry 'foo' in the HEAD tree
   points at an older commit;

 - 'foo/.git/HEAD' points at refs/heads/bar, which matches the
   working tree of 'foo' and the index foo/.git/index..

I am not sure what should happen to the entry 'foo' in the index of
the top-level superproject after such a 'submodule floats at the
tip' checkout, but I imagine that it must match the contents of
foo/.git/HEAD's tree.  Otherwise, git diff at the top-level would
report local changes.

When committing his work at the top-level, he will see that 'foo'
gitlink is updated in that commit; after all that combination is the
context in which his work was done.

Or are you envisioning that such a check-out will and should show a
local difference at the submodule 'foo' by leaving the index of the
top-level superproject unchanged, and the user should refrain from
using git commit -a to avoid having to describe the changes made
on the 'bar' branch in the meantime in his top-level commit?  That
is certainly fine by me (I am no a heavy submodule user to begin
with), but I am not sure if that is useful and helpful to the
submodule users.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4.1 5/5] push: update remote tags only with force

2012-11-19 Thread Junio C Hamano
Chris Rorvick ch...@rorvick.com writes:

 On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano gits...@pobox.com wrote:

 Yeah, this was one of a few stupid mistakes.  Previously I used
 'forwardable' throughout, but that is awkward except in the last
 commit since until then everything is allowed to fast-forward and the
 flag is only used to output tag-specific advice.  But inverting the
 meaning of the flag is dumb, and I didn't even do it right.

 But, as I think you're suggesting, it probably makes more sense to use
 a flag that prevents fast-forwarding when set.  So maybe
 not_forwardable, or is_a_tag = not_forwardable if you think the
 renaming is a good idea.

Yeah, calling it not-forwardable from the beginning would be a
sensible approach, I would think.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 14/13] test-wildmatch: avoid Windows path mangling

2012-11-19 Thread Johannes Sixt
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

The MSYS bash mangles arguments that begin with a forward slash
when they are passed to test-wildmatch. This causes tests to fail.
Avoid mangling by prepending XXX, which is removed by
test-wildmatch before further processing.

[J6t: reworded commit message]

Reported-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Johannes Sixt j...@kdbg.org
---
 Works well, and I'm fine with this work-around.

 -- Hannes

 t/t3070-wildmatch.sh | 10 +-
 test-wildmatch.c |  8 
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index e6ad6f4..3155eab 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -74,7 +74,7 @@ match 0 0 'foo/bar' 'foo[/]bar'
 match 0 0 'foo/bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 1 'foo-bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 0 'foo' '**/foo'
-match 1 x '/foo' '**/foo'
+match 1 x 'XXX/foo' '**/foo'
 match 1 0 'bar/baz/foo' '**/foo'
 match 0 0 'bar/baz/foo' '*/foo'
 match 0 0 'foo/bar/baz' '**/bar*'
@@ -95,8 +95,8 @@ match 0 0 ']' '[!]-]'
 match 1 x 'a' '[!]-]'
 match 0 0 '' '\'
 match 0 x '\' '\'
-match 0 x '/\' '*/\'
-match 1 x '/\' '*/\\'
+match 0 x 'XXX/\' '*/\'
+match 1 x 'XXX/\' '*/\\'
 match 1 1 'foo' 'foo'
 match 1 1 '@foo' '@foo'
 match 0 0 'foo' '@foo'
@@ -187,8 +187,8 @@ match 0 0 '-' '[[-\]]'
 match 1 1 '-adobe-courier-bold-o-normal--12-120-75-75-m-70-iso8859-1' 
'-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-X-70-iso8859-1' 
'-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-/-70-iso8859-1' 
'-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
-match 1 1 '/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' 
'/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
-match 0 0 '/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 
'/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 1 1 'XXX/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' 
'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 0 0 'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 
'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
 match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
 match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
 diff --git a/test-wildmatch.c b/test-wildmatch.c
index 74c0864..e384c8e 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -3,6 +3,14 @@
  int main(int argc, char **argv)
 {
+   int i;
+   for (i = 2; i  argc; i++) {
+   if (argv[i][0] == '/')
+   die(Forward slash is not allowed at the beginning of 
the\n
+   pattern because Windows does not like it. Use 
`XXX/' instead.);
+   else if (!strncmp(argv[i], XXX/, 4))
+   argv[i] += 3;
+   }
if (!strcmp(argv[1], wildmatch))
return !!wildmatch(argv[3], argv[2], 0);
else if (!strcmp(argv[1], iwildmatch))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/13] compat/fnmatch: fix off-by-one character class's length check

2012-11-19 Thread Johannes Sixt
Am 11/11/2012 11:13, schrieb Nguyễn Thái Ngọc Duy:
 Character class xdigit is the only one that hits 6 character limit
 defined by CHAR_CLASS_MAX_LENGTH. All other character classes are 5
 character long and therefore never caught by this.
 
 This should make xdigit tests in t3070 pass on Windows.
 
 Reported-by: Johannes Sixt j...@kdbg.org
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  I tested with Linux (removing the ifdef __LIBC in fnmatch.c) but I
  think this should get an ACK from someone who actually uses it on
  Windows.

Works well here on Windows.

This does not affect Windows alone, but all platforms that fall back to
compat/fnmatch. It's perhaps worth its own topic branch.

  We may want to tell upstream (who?) about this if they haven't fixed
  it already.
 
  compat/fnmatch/fnmatch.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
 index 9473aed..0ff1d27 100644
 --- a/compat/fnmatch/fnmatch.c
 +++ b/compat/fnmatch/fnmatch.c
 @@ -345,7 +345,7 @@ internal_fnmatch (pattern, string, no_leading_period, 
 flags)
  
   for (;;)
 {
 - if (c1 == CHAR_CLASS_MAX_LENGTH)
 + if (c1  CHAR_CLASS_MAX_LENGTH)
 /* The name is too long and therefore the pattern
is ill-formed.  */
 return FNM_NOMATCH;
 

-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Krzysztof Mazur
On Mon, Nov 19, 2012 at 04:00:09PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
  Given that the problematic line
  
 Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y
  
  is not even a valid e-mail address, doesn't this new logic belong to
  sanitize_address() conceptually?
 
  Yes, it's much better to do it in the sanitize_address().
 
 Note that I did not check that all the addresses that are handled by
 extract-valid-address came through sanitize-address function, so

Before sending that patch, I checked that and tested with and without
Email::Valid.

 unlike your original patch, this change alone may still pass some
 garbage to Email::Valid-address().  I tend to think that is a
 progress; we should make sure all the addresses are sanitized before
 using them for sending messages out.

I will try to check that.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Failure to extra sta...@vger.kernel.org addresses

2012-11-19 Thread Krzysztof Mazur
On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur krzys...@podlesie.net 
  wrote:
 
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -924,6 +924,10 @@ sub quote_subject {
   # use the simplest quoting being able to handle the recipient
   sub sanitize_address {
  my ($recipient) = @_;
  +
  +   # remove garbage after email address
  +   $recipient =~ s/(.*).*$/$1/;
  +
 
  Looks fine, but I would do s/(.*?)(.*)$/$1/, so that 'test
  f...@bar.com #comment' gets the second comment removed.
 
 Yeah, but do you need to capture the second group?  IOW, like
 s/(.*?).*$/$1/ perhaps?

I also thought about removing everything after first , but I will
not work for addresses like:

Cc: foo  sta...@vger.kernel.org #v3.4 v3.5 v3.6

What about:

$recipient =~ s/(.*[^@]*@[^]]*).*$/$1/;

or even

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..b988c57 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
my ($recipient) = @_;
 
+   my $local_part_regexp = qr/[^\s@]+/;
+   my $domain_regexp = qr/[^.\s@]+(?:\.[^.\s@]+)+/;
+
# remove garbage after email address
-   $recipient =~ s/(.*).*$/$1/;
+   $recipient =~ s/(.*$local_part_regexp\@$domain_regexp).*$/$1/;
 
my ($recipient_name, $recipient_addr) = ($recipient =~ 
/^(.*?)\s*(.*)/);

which uses regex used by 99% accurate version of extract_valid_address().

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git-svn: What is --follow-parent / --no-follow-parent for?

2012-11-19 Thread Sebastian Leske
Hi,

on reading the docs of git-svn, I stumbled across this paragraph:

 --follow-parent
 This is especially helpful when we’re tracking a directory that has been
 moved around within the repository, or if we started tracking a branch
 and never tracked the trunk it was descended from.  This feature is
 enabled by default, use --no-follow-parent to disable it.

However, this does not make sense to me: This sounds like there is no
good reason *not* to enable this option.  So why is it there? And in
what situation might I want to use --no-follow-parent?

As a matter of fact, I'm not even sure what --no-follow-parent does
(and the docs don't really say). 

I tried it out with a small test repo with a single branch (produced by
copying the trunk, then later deleted). With --follow-parent git-svn
correctly detected the branch point, and modeled the branch deletion as
a merge. With --no-follow-parent it just acted as if branch and trunk
were completely unrelated.

Commit graph of git-svn result:

--follow-parent:   --no-follow-parent:


   |   |
  /| | |
 / | | |
 | | | |
 | | | |
 | | | |
 \ | | |
  \| | |
   |   | 


(please excuse cheap ASCII art)

Is that the only effect of --no-follow-parent? And again, why would I
want that?

I'd be grateful for any clarifications. If I manage to understand the
explanation, I'll volunteer to summarize it into doc patch (if there are
no objections).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html