Failure to extra sta...@vger.kernel.org addresses
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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?
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