Re: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
Karthik Nayak writes: > On 06/01/2015 02:16 AM, Matthieu Moy wrote: >> Karthik Nayak writes: >> >> > That is kind of a problem, If I need to swap those commits also, I'd >> > have to add the part where ref-filter is added to the Makefile with >> > the code movement from for-each-ref to ref-filter. This again will not >> > just be Code movement. >> >> You can have a preparatory patch that adds ref-filter.c containing just >> "#include ref-filter.h" and ref-filter.h with proper content. After this >> preparatory patch, you're in a rather silly situation because you have >> an almost empty .c file, but it's still passing all tests and >> compileable. >> >> Then, the next patch can be just code movement. >> > Would it be okay, If I just include the Makefile addition along with the code > movement > from 'for-each-ref' to 'ref-filter.c' like Eric suggested? I prefer pure code movement, but as Eric said it's also OK to have small other changes as long as they are related (so it makes sense to include them in the same commit) and separate (in your case, they touch different files so it's easy to notice the Makefile change in the patch). In any case, because I prefer one way doesn't mean it's the only way or even that it's the right way. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
Christian Couder writes: > sort_refs() -> ref_array_sort() > struct ref_sort -> struct ref_sort_criteria > default_sort() -> ref_default_sort_criteria() > opt_parse_sort() -> opt_parse_ref_sort_criteria() BTW, having such summary in the log message would help review: we could think and discuss the naming by looking just at the summary, and then check that you did it correctly (easy). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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] Doc, git-svn, added mention of config key: svn-remote..include-paths
Signed-off-by: Brett Randall --- Documentation/git-svn.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 11d1e2f..0c0f60b 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -174,6 +174,9 @@ Skip "branches" and "tags" of first level directories;; (including automatic fetches due to 'clone', 'dcommit', 'rebase', etc) on a given repository. '--ignore-paths' takes precedence over '--include-paths'. ++ +[verse] +config key: svn-remote..include-paths --log-window-size=;; Fetch log entries per request when scanning Subversion history. -- 2.3.5 -- 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 6/9] send-email: simplify sendmail aliases comment and blank line recognizer
On Sun, May 31, 2015 at 6:29 PM, Eric Sunshine wrote:
> Replace unnecessarily complex regular expression for recognizing comment
> and blanks lines in sendmail aliases with idiomatic expressions which
s/blanks/blank/
> can be easily understood at a glance.
>
> Signed-off-by: Eric Sunshine
> ---
> git-send-email.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 76bb499..e777bd3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -505,7 +505,7 @@ sub parse_sendmail_alias {
> sub parse_sendmail_aliases {
> my $fh = shift;
> while (<$fh>) {
> - if (/^\s*(?:#.*)?$/) { next; }
> + next if /^\s*$/ || /^\s*#/;
> parse_sendmail_alias($_);
> }
> }
> --
> 2.4.2.538.g5f4350e
--
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: Staging commits with visual diff tools?
On Sun, May 31, 2015 at 10:36:52AM +0100, John Lee wrote: > On Sat, 30 May 2015, David Aguilar wrote: > > >On Tue, May 26, 2015 at 09:50:49PM +0100, John Lee wrote: > >>Hi > >> > >>Does anybody have code to stage commits using a the visual > >>diff/merge tools supported by git-difftool? Is there support in git > ... > >I'm a g/vim user, so git-cola is finely tuned for keyboard > >usage. If we implement these feature in Git, we should consider > >providing the same workflows/hotkeys as cola. > > Just to be clear I'm not planning on contributing my script back to > git, it will just be a standalone script in a separate repo. > > I'll give git-cola a try, thanks. I just ran it and see it supports > e.g. right click to stage and launching difftools -- does it also > support launching a difftool to edit the index? If you have stuff staged and use ctrl-d (or right-click->difftool), git cola will run, "git difftool --staged -- ". difftool will already point back to your worktree whenever possible through the GIT_EXTERNAL_DIFF machinery that it uses. After changes are made in the worktree you select files, lines, or hunks and stage them. Doing it in one step could be as simple as having a script to automatically call "git add" after difftool, or "ctrl-s" in cola to stage the whole file. A not as simple (but potentially quite helpful) use case is a tool that does a 3-way merge between HEAD, index, and worktree, and writes the result back to the worktree (or index?). The mergetool--lib backend could be leveraged for that feature. Were you thinking of something like this, or something else? Can you describe your use case a bit more? It seems like we have most of the use cases covered with stock git, but I'm not sure if I'm missing something. cheers, -- David -- 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: Bug in 'git am' when applying a broken patch
On Mon, Jun 01, 2015 at 09:17:59AM +0900, Greg KH wrote: > Hi all, > > I received the patch attached below as part of a submission against the > Linux kernel tree. The patch seems to have been hand-edited, and is not > correct, and patch verifies this as being a problem: > > $ patch -p1 --dry-run < bad_patch.mbox > checking file drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c > patch: malformed patch at line 133:skb_put(skb, > sizeof(struct ieee80211_authentication)); > > But git will actually apply it: > $ git am -s bad_patch.mbox > Applying: staging: rtl8192u: ieee80211: Fix sparse endianness warnings > > But, there's nothing in the patch at all except the commit message: > > $ git show HEAD > commit f6643dfef5b701db86f23be9ce6fb5b3bafe76b6 > Author: Gaston Gonzalez > Date: Sun May 31 12:17:48 2015 -0300 > > staging: rtl8192u: ieee80211: Fix sparse endianness warnings > > Fix the following sparse warnings: > > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32: warning: > incorrect type in assignment (different base types) > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32: > expected restricted __le16 [usertype] frame_ctl > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32:got int > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50: warning: > invalid assignment: |= > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:left > side has type restricted __le16 > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:right > side has type int > > Signed-off-by: Gaston Gonzalez > Signed-off-by: Greg Kroah-Hartman > > $ git diff HEAD^ > $ > > Any ideas what is going on here? Shouldn't 'git am' have failed? > > Oh, I'm using git version 2.4.2 right now. > > I've asked Gaston for the original patch to verify before he hand-edited > it, to verify that git wasn't creating something wrong here, as well. Gaston sent me his original patch, before he edited it, and it was correct, so git is correctly creating the patch, which is good. So it's just a 'git am' issue with a broken patch file. thanks, greg k-h -- 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] format-patch: dereference tags with --ignore-if-in-upstream
format-patch would segfault if provided a tag as one of the range
endpoints in conjunction with --ignore-if-in-upstream, as it assumed the
object was a commit and attempted to cast it to struct commit.
Dereference the tag as soon as possible to prevent this, but not until
after copying the necessary flags.
Reported-by: Bruce Korb
Diagnosed-by: Christian Couder
Signed-off-by: brian m. carlson
---
builtin/log.c | 6 ++
t/t4014-format-patch.sh | 10 ++
2 files changed, 16 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..e0465ba 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -807,6 +807,12 @@ static void get_patch_ids(struct rev_info *rev, struct
patch_ids *ids)
o2 = rev->pending.objects[1].item;
flags2 = o2->flags;
+ o1 = deref_tag(o1, NULL, 0);
+ o2 = deref_tag(o2, NULL, 0);
+
+ if (!o1 || !o2)
+ die(_("Invalid tag."));
+
if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
die(_("Not a range."));
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c39e500..60b9875 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -57,6 +57,16 @@ test_expect_success "format-patch --ignore-if-in-upstream" '
'
+test_expect_success "format-patch --ignore-if-in-upstream handles tags" '
+
+ git tag -a v1 -m tag side &&
+ git format-patch --stdout \
+ --ignore-if-in-upstream master..v1 >patch1 &&
+ cnt=$(grep "^From " patch1 | wc -l) &&
+ test $cnt = 2
+
+'
+
test_expect_success "format-patch doesn't consider merge commits" '
git checkout -b slave master &&
--
2.4.0
--
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
Bug in 'git am' when applying a broken patch
Hi all, I received the patch attached below as part of a submission against the Linux kernel tree. The patch seems to have been hand-edited, and is not correct, and patch verifies this as being a problem: $ patch -p1 --dry-run < bad_patch.mbox checking file drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c patch: malformed patch at line 133:skb_put(skb, sizeof(struct ieee80211_authentication)); But git will actually apply it: $ git am -s bad_patch.mbox Applying: staging: rtl8192u: ieee80211: Fix sparse endianness warnings But, there's nothing in the patch at all except the commit message: $ git show HEAD commit f6643dfef5b701db86f23be9ce6fb5b3bafe76b6 Author: Gaston Gonzalez Date: Sun May 31 12:17:48 2015 -0300 staging: rtl8192u: ieee80211: Fix sparse endianness warnings Fix the following sparse warnings: drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32: warning: incorrect type in assignment (different base types) drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32:expected restricted __le16 [usertype] frame_ctl drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:663:32:got int drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50: warning: invalid assignment: |= drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:left side has type restricted __le16 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:664:50:right side has type int Signed-off-by: Gaston Gonzalez Signed-off-by: Greg Kroah-Hartman $ git diff HEAD^ $ Any ideas what is going on here? Shouldn't 'git am' have failed? Oh, I'm using git version 2.4.2 right now. I've asked Gaston for the original patch to verify before he hand-edited it, to verify that git wasn't creating something wrong here, as well. thanks, greg k-h bad_patch.mbox Description: application/mbox
Re: seg fault in "git format-patch"
On Mon, Jun 1, 2015 at 1:53 AM, Christian Couder wrote: > On Mon, Jun 1, 2015 at 1:14 AM, Christian Couder > wrote: >> On Sun, May 31, 2015 at 10:45 PM, Bruce Korb wrote: >>> Oh, you can also clone the gnu-pw-mgr and likely get the same result: >> >> Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git >> I get the following backtrace: >> >> Program received signal SIGSEGV, Segmentation fault. >> 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, >> commit=0x84e8d0, mark=139) at commit.c:528 >> 528 while ((parents = parents->next)) >> (gdb) bt >> #0 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, >> commit=0x84e8d0, mark=139) at commit.c:528 >> #1 0x004b2743 in clear_commit_marks_many (nr=-1, >> commit=0x7fffbfa0, mark=139) at commit.c:544 >> #2 0x004b2771 in clear_commit_marks (commit=0x84e8d0, >> mark=139) at commit.c:549 >> #3 0x004537cc in get_patch_ids (rev=0x7fffd190, >> ids=0x7fffc910) at builtin/log.c:832 >> #4 0x00455580 in cmd_format_patch (argc=1, >> argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425 >> #5 0x00405807 in run_builtin (p=0x80cac8 , >> argc=5, argv=0x7fffdc20) at git.c:350 >> #6 0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20) >> at git.c:532 >> #7 0x00405b31 in run_argv (argcp=0x7fffdafc, >> argv=0x7fffdb10) at git.c:578 >> #8 0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686 >> >> (Please don't top post if you reply to this email as it is frown upon >> on this list.) > > When running the command that gives the above segfault: > > $ git format-patch -o patches --ignore-if-in-upstream > 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d > > It is interesting to note that the last sha1 refers to a tag: > > $ git cat-file tag 2de9eef391259dfc8748dbaf76a5d55427f37b0d > object 524ccbdbe319068ab18a3950119b9e9a5d135783 > type commit > tag v1.4 > tagger Bruce Korb 1428847577 -0700 > > Release 1.4 > > * sort-pw-cfg: a sort/merge program for combining and organizing > configurations. > > * --delete: a new option to remove any entries for a password id > > It works when the tag is replaced by the commit it points to, and the > segfault happens because the we try to access the "parents" field of > the tag object as if it was a commit. Yeah, in builtin/log.c we are doing: o2 = rev->pending.objects[1].item; and then we are casting the object into a commit when passing it to clear_commit_marks(): clear_commit_marks((struct commit *)o2, SEEN | UNINTERESTING | SHOWN | ADDED); but I don't know where we should have peeled the tag to get a commit, and it's late here so I will leave it someone else to find a fix. Best, Christian. -- 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: seg fault in "git format-patch"
On Mon, Jun 1, 2015 at 1:14 AM, Christian Couder wrote: > On Sun, May 31, 2015 at 10:45 PM, Bruce Korb wrote: >> Oh, you can also clone the gnu-pw-mgr and likely get the same result: > > Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git > I get the following backtrace: > > Program received signal SIGSEGV, Segmentation fault. > 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, > commit=0x84e8d0, mark=139) at commit.c:528 > 528 while ((parents = parents->next)) > (gdb) bt > #0 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, > commit=0x84e8d0, mark=139) at commit.c:528 > #1 0x004b2743 in clear_commit_marks_many (nr=-1, > commit=0x7fffbfa0, mark=139) at commit.c:544 > #2 0x004b2771 in clear_commit_marks (commit=0x84e8d0, > mark=139) at commit.c:549 > #3 0x004537cc in get_patch_ids (rev=0x7fffd190, > ids=0x7fffc910) at builtin/log.c:832 > #4 0x00455580 in cmd_format_patch (argc=1, > argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425 > #5 0x00405807 in run_builtin (p=0x80cac8 , > argc=5, argv=0x7fffdc20) at git.c:350 > #6 0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20) > at git.c:532 > #7 0x00405b31 in run_argv (argcp=0x7fffdafc, > argv=0x7fffdb10) at git.c:578 > #8 0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686 > > (Please don't top post if you reply to this email as it is frown upon > on this list.) When running the command that gives the above segfault: $ git format-patch -o patches --ignore-if-in-upstream 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d It is interesting to note that the last sha1 refers to a tag: $ git cat-file tag 2de9eef391259dfc8748dbaf76a5d55427f37b0d object 524ccbdbe319068ab18a3950119b9e9a5d135783 type commit tag v1.4 tagger Bruce Korb 1428847577 -0700 Release 1.4 * sort-pw-cfg: a sort/merge program for combining and organizing configurations. * --delete: a new option to remove any entries for a password id It works when the tag is replaced by the commit it points to, and the segfault happens because the we try to access the "parents" field of the tag object as if it was a commit. -- 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] checkout: don't check worktrees when not necessary
When --patch or pathspecs are passed to git checkout, the working tree
will not be switching branch, so there's no need to check if the branch
that we are running checkout on is already checked out.
Signed-off-by: Spencer Baugh
---
builtin/checkout.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..7039c5c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char
**argv,
char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
+ !(opts->patch_mode || opts->pathspec.nr) &&
!opts->ignore_other_worktrees)
check_linked_checkouts(new);
free(head_ref);
--
2.4.2.339.g77bd3ea
--
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: seg fault in "git format-patch"
On Sun, May 31, 2015 at 10:45 PM, Bruce Korb wrote: > Oh, you can also clone the gnu-pw-mgr and likely get the same result: Yeah, after cloning from http://git.savannah.gnu.org/r/gnu-pw-mgr.git I get the following backtrace: Program received signal SIGSEGV, Segmentation fault. 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, commit=0x84e8d0, mark=139) at commit.c:528 528 while ((parents = parents->next)) (gdb) bt #0 0x004b26b1 in clear_commit_marks_1 (plist=0x7fffbf78, commit=0x84e8d0, mark=139) at commit.c:528 #1 0x004b2743 in clear_commit_marks_many (nr=-1, commit=0x7fffbfa0, mark=139) at commit.c:544 #2 0x004b2771 in clear_commit_marks (commit=0x84e8d0, mark=139) at commit.c:549 #3 0x004537cc in get_patch_ids (rev=0x7fffd190, ids=0x7fffc910) at builtin/log.c:832 #4 0x00455580 in cmd_format_patch (argc=1, argv=0x7fffdc20, prefix=0x0) at builtin/log.c:1425 #5 0x00405807 in run_builtin (p=0x80cac8 , argc=5, argv=0x7fffdc20) at git.c:350 #6 0x00405a15 in handle_builtin (argc=5, argv=0x7fffdc20) at git.c:532 #7 0x00405b31 in run_argv (argcp=0x7fffdafc, argv=0x7fffdb10) at git.c:578 #8 0x00405d29 in main (argc=5, av=0x7fffdc18) at git.c:686 (Please don't top post if you reply to this email as it is frown upon on this list.) -- 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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On Sun, May 31, 2015 at 10:50 PM, Karthik Nayak wrote: > On 06/01/2015 02:16 AM, Matthieu Moy wrote: >> >> You can have a preparatory patch that adds ref-filter.c containing just >> "#include ref-filter.h" and ref-filter.h with proper content. After this >> preparatory patch, you're in a rather silly situation because you have >> an almost empty .c file, but it's still passing all tests and >> compileable. >> >> Then, the next patch can be just code movement. >> > Would it be okay, If I just include the Makefile addition along with the > code movement > from 'for-each-ref' to 'ref-filter.c' like Eric suggested? Yeah, I think it is ok as well. -- 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 4/9] send-email: fix style: cuddle 'elsif' and 'else' with closing brace
Signed-off-by: Eric Sunshine
---
git-send-email.perl | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 0b18682..1380e6e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -517,22 +517,15 @@ my %parse_alias = (
} },
sendmail => sub { my $fh = shift; while (<$fh>) {
- if (/^\s*(?:#.*)?$/) { }
-
- elsif (/"/) {
+ if (/^\s*(?:#.*)?$/) {
+ } elsif (/"/) {
print STDERR "warning: sendmail alias with quotes is
not supported: $_\n";
- }
-
- elsif (/^\s|\\$/) {
+ } elsif (/^\s|\\$/) {
print STDERR "warning: sendmail continuation line is
not supported: $_\n";
- }
-
- elsif (/^(\S+?)\s*:\s*(.+)$/) {
+ } elsif (/^(\S+?)\s*:\s*(.+)$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
- }
-
- else {
+ } else {
print STDERR "warning: sendmail line is not recognized:
$_\n";
}}},
--
2.4.2.538.g5f4350e
--
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 5/9] send-email: refactor sendmail aliases parser
The sendmail aliases parser inlined into %parse_alias is already
uncomfortably large and is expected to grow as additional functionality
is implemented, so extract it to improve manageability.
Signed-off-by: Eric Sunshine
---
git-send-email.perl | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 1380e6e..76bb499 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -487,6 +487,29 @@ sub split_addrs {
}
my %aliases;
+
+sub parse_sendmail_alias {
+ local $_ = shift;
+ if (/"/) {
+ print STDERR "warning: sendmail alias with quotes is not
supported: $_\n";
+ } elsif (/^\s|\\$/) {
+ print STDERR "warning: sendmail continuation line is not
supported: $_\n";
+ } elsif (/^(\S+?)\s*:\s*(.+)$/) {
+ my ($alias, $addr) = ($1, $2);
+ $aliases{$alias} = [ split_addrs($addr) ];
+ } else {
+ print STDERR "warning: sendmail line is not recognized: $_\n";
+ }
+}
+
+sub parse_sendmail_aliases {
+ my $fh = shift;
+ while (<$fh>) {
+ if (/^\s*(?:#.*)?$/) { next; }
+ parse_sendmail_alias($_);
+ }
+}
+
my %parse_alias = (
# multiline formats can be supported in the future
mutt => sub { my $fh = shift; while (<$fh>) {
@@ -515,20 +538,7 @@ my %parse_alias = (
$aliases{$alias} = [ split_addrs($addr) ];
}
} },
-
- sendmail => sub { my $fh = shift; while (<$fh>) {
- if (/^\s*(?:#.*)?$/) {
- } elsif (/"/) {
- print STDERR "warning: sendmail alias with quotes is
not supported: $_\n";
- } elsif (/^\s|\\$/) {
- print STDERR "warning: sendmail continuation line is
not supported: $_\n";
- } elsif (/^(\S+?)\s*:\s*(.+)$/) {
- my ($alias, $addr) = ($1, $2);
- $aliases{$alias} = [ split_addrs($addr) ];
- } else {
- print STDERR "warning: sendmail line is not recognized:
$_\n";
- }}},
-
+ sendmail => \&parse_sendmail_aliases,
gnus => sub { my $fh = shift; while (<$fh>) {
if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
$aliases{$1} = [ $2 ];
--
2.4.2.538.g5f4350e
--
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 9/9] t9001: add sendmail aliases line continuation tests
A line beginning with whitespace is folded into the preceding line.
A line ending with '\' consumes the following line.
While here, also test an empty sendmail aliases file.
Signed-off-by: Eric Sunshine
---
t/t9001-send-email.sh | 34 ++
1 file changed, 34 insertions(+)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1012fa3..db2f45e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1587,6 +1587,40 @@ test_sendmail_aliases 'sendemail.aliasfiletype=sendmail'
\
bcgrp: bob, chloe, Other
EOF
+test_sendmail_aliases 'sendmail aliases line folding' \
+ alice1 \
+ bob1 bob2 \
+ chuck1 chuck2 \
+ darla1 darla2 darla3 \
+ elton1 elton2 elton3 \
+ fred1 fred2 \
+ greg1 <<-\EOF
+ alice: alice1
+ bob: bob1,\
+ bob2
+ chuck: chuck1,
+ chuck2
+ darla: darla1,\
+ darla2,
+ darla3
+ elton: elton1,
+ elton2,\
+ elton3
+ fred: fred1,\
+ fred2
+ greg: greg1
+ bcgrp: bob, chuck, darla, elton, fred, greg
+ EOF
+
+test_sendmail_aliases 'sendmail aliases tolerate bogus line folding' \
+ alice1 bob1 <<-\EOF
+ alice: alice1
+ bcgrp: bob1\
+ EOF
+
+test_sendmail_aliases 'sendmail aliases empty' alice bcgrp <<-\EOF
+ EOF
+
do_xmailer_test () {
expected=$1 params=$2 &&
git format-patch -1 &&
--
2.4.2.538.g5f4350e
--
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 8/9] t9001: refactor sendmail aliases test infrastructure
Several new tests of sendmail aliases parsing will be added in a
subsequent patch, so factor out functionality common to all of them
into a new helper function.
Signed-off-by: Eric Sunshine
---
t/t9001-send-email.sh | 47 +--
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index a3663da..1012fa3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1549,10 +1549,35 @@ test_expect_success $PREREQ
'sendemail.aliasfile=~/.mailrc' '
grep "^!someone@example\.org!$" commandline1
'
-test_expect_success $PREREQ 'sendemail.aliasfiletype=sendmail' '
- clean_fake_sendmail && rm -fr outdir &&
- git format-patch -1 -o outdir &&
- cat >>.tmp-email-aliases <<-\EOF &&
+test_sendmail_aliases () {
+ msg="$1" && shift &&
+ expect="$@" &&
+ cat >.tmp-email-aliases &&
+
+ test_expect_success $PREREQ "$msg" '
+ clean_fake_sendmail && rm -fr outdir &&
+ git format-patch -1 -o outdir &&
+ git config --replace-all sendemail.aliasesfile \
+ "$(pwd)/.tmp-email-aliases" &&
+ git config sendemail.aliasfiletype sendmail &&
+ git send-email \
+ --from="Example " \
+ --to=alice --to=bcgrp \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ outdir/0001-*.patch \
+ 2>errors >out &&
+ for i in $expect
+ do
+ grep "^!$i!$" commandline1 || return 1
+ done
+ '
+}
+
+test_sendmail_aliases 'sendemail.aliasfiletype=sendmail' \
+ 'awol@example\.com' \
+ 'bob@example\.com' \
+ 'chloe@example\.com' \
+ 'o@example\.com' <<-\EOF
alice: Alice W Land
bob: Robert Bobbyton
# this is a comment
@@ -1561,20 +1586,6 @@ test_expect_success $PREREQ
'sendemail.aliasfiletype=sendmail' '
abgroup: alice, bob
bcgrp: bob, chloe, Other
EOF
- git config --replace-all sendemail.aliasesfile \
- "$(pwd)/.tmp-email-aliases" &&
- git config sendemail.aliasfiletype sendmail &&
- git send-email \
- --from="Example " \
- --to=alice --to=bcgrp \
- --smtp-server="$(pwd)/fake.sendmail" \
- outdir/0001-*.patch \
- 2>errors >out &&
- grep "^!awol@example\.com!$" commandline1 &&
- grep "^!bob@example\.com!$" commandline1 &&
- grep "^!chloe@example\.com!$" commandline1 &&
- grep "^!o@example\.com!$" commandline1
-'
do_xmailer_test () {
expected=$1 params=$2 &&
--
2.4.2.538.g5f4350e
--
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 6/9] send-email: simplify sendmail aliases comment and blank line recognizer
Replace unnecessarily complex regular expression for recognizing comment
and blanks lines in sendmail aliases with idiomatic expressions which
can be easily understood at a glance.
Signed-off-by: Eric Sunshine
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 76bb499..e777bd3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -505,7 +505,7 @@ sub parse_sendmail_alias {
sub parse_sendmail_aliases {
my $fh = shift;
while (<$fh>) {
- if (/^\s*(?:#.*)?$/) { next; }
+ next if /^\s*$/ || /^\s*#/;
parse_sendmail_alias($_);
}
}
--
2.4.2.538.g5f4350e
--
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 7/9] send-email: implement sendmail aliases line continuation support
Logical lines in sendmail aliases files can be spread over multiple
physical lines[1]. A line beginning with whitespace is folded into the
preceding line. A line ending with '\' consumes the following line.
[1]: https://www.freebsd.org/cgi/man.cgi?query=aliases&sektion=5
Signed-off-by: Eric Sunshine
---
This implementation silently and "sanely" tolerates continuation line
scenarios for which behavior is not defined by [1]. In particular, an
indented line which is the first (non-comment) line in the file is
treated as a single logical line. Ditto for a line ending with '\' which
is the last (non-comment) line in the file.
An earlier iteration emitted warnings for such cases, but it wasn't
clear if warning about undefined behavior was useful; and it made the
implementation much more noisy, so this version silently tolerates such
anomalies.
Documentation/git-send-email.txt | 2 --
git-send-email.perl | 10 +++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index e6d466e..7ae467b 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -394,8 +394,6 @@ described below:
sendmail;;
* Quoted aliases and quoted addresses are not supported: lines that
contain a `"` symbol are ignored.
-* Line continuations are not supported: lines that start with
- whitespace characters, or end with a `\` symbol are ignored.
* Redirection to a file (`/path/name`) or pipe (`|command`) is not
supported.
* File inclusion (`:include: /path/name`) is not supported.
diff --git a/git-send-email.perl b/git-send-email.perl
index e777bd3..eb1d678 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -492,8 +492,6 @@ sub parse_sendmail_alias {
local $_ = shift;
if (/"/) {
print STDERR "warning: sendmail alias with quotes is not
supported: $_\n";
- } elsif (/^\s|\\$/) {
- print STDERR "warning: sendmail continuation line is not
supported: $_\n";
} elsif (/^(\S+?)\s*:\s*(.+)$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
@@ -504,10 +502,16 @@ sub parse_sendmail_alias {
sub parse_sendmail_aliases {
my $fh = shift;
+ my $s = '';
while (<$fh>) {
+ chomp;
next if /^\s*$/ || /^\s*#/;
- parse_sendmail_alias($_);
+ $s .= $_, next if $s =~ s/\\$// || s/^\s+//;
+ parse_sendmail_alias($s) if $s;
+ $s = $_;
}
+ $s =~ s/\\$//; # silently tolerate stray '\' on last line
+ parse_sendmail_alias($s) if $s;
}
my %parse_alias = (
--
2.4.2.538.g5f4350e
--
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 3/9] send-email: drop noise comments which merely repeat what code says
Signed-off-by: Eric Sunshine
---
git-send-email.perl | 5 -
1 file changed, 5 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 819f87e..0b18682 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -517,26 +517,21 @@ my %parse_alias = (
} },
sendmail => sub { my $fh = shift; while (<$fh>) {
- # ignore blank lines and comment lines
if (/^\s*(?:#.*)?$/) { }
- # warn on lines that contain quotes
elsif (/"/) {
print STDERR "warning: sendmail alias with quotes is
not supported: $_\n";
}
- # warn on lines that continue
elsif (/^\s|\\$/) {
print STDERR "warning: sendmail continuation line is
not supported: $_\n";
}
- # recognize lines that look like an alias
elsif (/^(\S+?)\s*:\s*(.+)$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
}
- # warn on lines that are not recognized
else {
print STDERR "warning: sendmail line is not recognized:
$_\n";
}}},
--
2.4.2.538.g5f4350e
--
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 0/9] send-email: add sendmail aliases line continuation support
This series adds line continuation support for sendmail aliases. It extends basic sendmail aliases functionality implemented by ah/send-email-sendmail-alias (currently d1205b07 in 'pu') Eric Sunshine (9): send-email: further document missing sendmail aliases functionality send-email: visually distinguish sendmail aliases parser warnings send-email: drop noise comments which merely repeat what code says send-email: fix style: cuddle 'elsif' and 'else' with closing brace send-email: refactor sendmail aliases parser send-email: simplify sendmail aliases comment and blank line recognizer send-email: implement sendmail aliases line continuation support t9001: refactor sendmail aliases test infrastructure t9001: add sendmail aliases line continuation tests Documentation/git-send-email.txt | 5 ++- git-send-email.perl | 54 ++- t/t9001-send-email.sh| 81 +++- 3 files changed, 94 insertions(+), 46 deletions(-) -- 2.4.2.538.g5f4350e -- 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 1/9] send-email: further document missing sendmail aliases functionality
Sendmail aliases[1] supports expansion to a file ("/path/name") or
pipe ("|command"), as well as file inclusion (":include: /path/name"),
however, our implementation does not support such functionality.
[1]: https://www.freebsd.org/cgi/man.cgi?query=aliases&sektion=5
Signed-off-by: Eric Sunshine
---
Documentation/git-send-email.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b48a764..e6d466e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -396,6 +396,9 @@ sendmail;;
contain a `"` symbol are ignored.
* Line continuations are not supported: lines that start with
whitespace characters, or end with a `\` symbol are ignored.
+* Redirection to a file (`/path/name`) or pipe (`|command`) is not
+ supported.
+* File inclusion (`:include: /path/name`) is not supported.
* Warnings are printed on the standard error output for any
explicitly unsupported constructs, and any other lines that are not
recognized by the parser.
--
2.4.2.538.g5f4350e
--
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 2/9] send-email: visually distinguish sendmail aliases parser warnings
Although emitted to stderr, warnings from the sendmail aliases parser
are not visually distinguished as such, and thus can easily be
overlooked in the normal noisy send-email output.
Signed-off-by: Eric Sunshine
---
This prepends lowercase "warning:" rather than uppercase since lowercase
is used elsewhere in git-send-email.perl for diagnostic message
prefixes.
git-send-email.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 6bedf74..819f87e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -522,12 +522,12 @@ my %parse_alias = (
# warn on lines that contain quotes
elsif (/"/) {
- print STDERR "sendmail alias with quotes is not
supported: $_\n";
+ print STDERR "warning: sendmail alias with quotes is
not supported: $_\n";
}
# warn on lines that continue
elsif (/^\s|\\$/) {
- print STDERR "sendmail continuation line is not
supported: $_\n";
+ print STDERR "warning: sendmail continuation line is
not supported: $_\n";
}
# recognize lines that look like an alias
@@ -538,7 +538,7 @@ my %parse_alias = (
# warn on lines that are not recognized
else {
- print STDERR "sendmail line is not recognized: $_\n";
+ print STDERR "warning: sendmail line is not recognized:
$_\n";
}}},
gnus => sub { my $fh = shift; while (<$fh>) {
--
2.4.2.538.g5f4350e
--
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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On 06/01/2015 02:16 AM, Matthieu Moy wrote: Karthik Nayak writes: > That is kind of a problem, If I need to swap those commits also, I'd > have to add the part where ref-filter is added to the Makefile with > the code movement from for-each-ref to ref-filter. This again will not > just be Code movement. You can have a preparatory patch that adds ref-filter.c containing just "#include ref-filter.h" and ref-filter.h with proper content. After this preparatory patch, you're in a rather silly situation because you have an almost empty .c file, but it's still passing all tests and compileable. Then, the next patch can be just code movement. Would it be okay, If I just include the Makefile addition along with the code movement from 'for-each-ref' to 'ref-filter.c' like Eric suggested? -- Regards, Karthik -- 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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
Karthik Nayak writes: > That is kind of a problem, If I need to swap those commits also, I'd > have to add the part where ref-filter is added to the Makefile with > the code movement from for-each-ref to ref-filter. This again will not > just be Code movement. You can have a preparatory patch that adds ref-filter.c containing just "#include ref-filter.h" and ref-filter.h with proper content. After this preparatory patch, you're in a rather silly situation because you have an almost empty .c file, but it's still passing all tests and compileable. Then, the next patch can be just code movement. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: seg fault in "git format-patch"
Oh, you can also clone the gnu-pw-mgr and likely get the same result: $ cat .git/config [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true [remote "origin"] fetch = +refs/heads/*:refs/remotes/origin/* url = ssh://git.sv.gnu.org/srv/git/gnu-pw-mgr.git [branch "master"] remote = origin merge = refs/heads/master On Sun, May 31, 2015 at 1:41 PM, Bruce Korb wrote: > bt won't help much: > > Program received signal SIGSEGV, Segmentation fault. > 0x0047e62f in ?? () > (gdb) bt > #0 0x0047e62f in ?? () > #1 0x0047e6ba in ?? () > #2 0x0043cb9a in ?? () > #3 0x0043e9cc in ?? () > #4 0x0040647d in ?? () > #5 0x00405863 in ?? () > #6 0x76fc3be5 in __libc_start_main () from /lib64/libc.so.6 > #7 0x00405cd5 in ?? () > > $ git --version > git version 1.8.4.5 > $ rpm -q -a|grep '^git' > git-email-1.8.4.5-3.8.4.x86_64 > gitg-0.2.7-3.1.4.x86_64 > git-cvs-1.8.4.5-3.8.4.x86_64 > git-svn-1.8.4.5-3.8.4.x86_64 > git-web-1.8.4.5-3.8.4.x86_64 > git-gui-1.8.4.5-3.8.4.x86_64 > gitg-lang-0.2.7-3.1.4.noarch > git-1.8.4.5-3.8.4.x86_64 > git-core-1.8.4.5-3.8.4.x86_64 > gitk-1.8.4.5-3.8.4.x86_64 > > $ head -n 300 /etc/*eleas* > ==> /etc/SuSE-release <== > openSUSE 13.1 (x86_64) > VERSION = 13.1 > CODENAME = Bottle > # /etc/SuSE-release is deprecated and will be removed in the future, > use /etc/os-release instead > > ==> /etc/lsb-release <== > LSB_VERSION="core-2.0-noarch:core-3.2-noarch:core-4.0-noarch:core-2.0-x86_64:core-3.2-x86_64:core-4.0-x86_64" > > ==> /etc/lsb-release.d <== > head: error reading '/etc/lsb-release.d': Is a directory > > ==> /etc/os-release <== > NAME=openSUSE > VERSION="13.1 (Bottle)" > VERSION_ID="13.1" > PRETTY_NAME="openSUSE 13.1 (Bottle) (x86_64)" > ID=opensuse > ANSI_COLOR="0;32" > CPE_NAME="cpe:/o:opensuse:opensuse:13.1" > BUG_REPORT_URL="https://bugs.opensuse.org"; > HOME_URL="https://opensuse.org/"; > ID_LIKE="suse" > > > On Sun, May 31, 2015 at 1:26 PM, Christian Couder > wrote: >> On Sun, May 31, 2015 at 9:13 PM, Bruce Korb wrote: >>> $ git format-patch -o patches --ignore-if-in-upstream >>> 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d >>> Segmentation fault >>> /u/gnu/proj/gnu-pw-mgr >>> $ git format-patch -o patches >>> 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d >>> patches/0001-remove-dead-code.patch >>> patches/0002-dead-code-removal.patch >>> patches/0003-add-sort-pw-cfg-program.patch >>> patches/0004-add-doc-for-sort-pw-cfg.patch >>> patches/0005-clean-up-doc-makefile.patch >>> patches/0006-clean-up-doc-makefile.patch >>> patches/0007-happy-2015-and-add-delete-option.patch >>> patches/0008-fix-doc-Makefile.am.patch >>> patches/0009-re-fix-copyright.patch >>> patches/0010-finish-debugging-remove_pwid.patch >>> patches/0011-only-update-file-if-something-was-removed.patch >>> patches/0012-update-NEWS.patch >>> patches/0013-bootstrap-cleanup.patch >> >> Could you tell us which git version you are using? You can use "git >> --version". >> The operating system you are using could also be useful. >> And maybe you could also run git under gdb and give us the output of >> the "bt" (backtrace) gdb command when it crashes? >> >> Thanks, >> Christian. -- 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: seg fault in "git format-patch"
bt won't help much: Program received signal SIGSEGV, Segmentation fault. 0x0047e62f in ?? () (gdb) bt #0 0x0047e62f in ?? () #1 0x0047e6ba in ?? () #2 0x0043cb9a in ?? () #3 0x0043e9cc in ?? () #4 0x0040647d in ?? () #5 0x00405863 in ?? () #6 0x76fc3be5 in __libc_start_main () from /lib64/libc.so.6 #7 0x00405cd5 in ?? () $ git --version git version 1.8.4.5 $ rpm -q -a|grep '^git' git-email-1.8.4.5-3.8.4.x86_64 gitg-0.2.7-3.1.4.x86_64 git-cvs-1.8.4.5-3.8.4.x86_64 git-svn-1.8.4.5-3.8.4.x86_64 git-web-1.8.4.5-3.8.4.x86_64 git-gui-1.8.4.5-3.8.4.x86_64 gitg-lang-0.2.7-3.1.4.noarch git-1.8.4.5-3.8.4.x86_64 git-core-1.8.4.5-3.8.4.x86_64 gitk-1.8.4.5-3.8.4.x86_64 $ head -n 300 /etc/*eleas* ==> /etc/SuSE-release <== openSUSE 13.1 (x86_64) VERSION = 13.1 CODENAME = Bottle # /etc/SuSE-release is deprecated and will be removed in the future, use /etc/os-release instead ==> /etc/lsb-release <== LSB_VERSION="core-2.0-noarch:core-3.2-noarch:core-4.0-noarch:core-2.0-x86_64:core-3.2-x86_64:core-4.0-x86_64" ==> /etc/lsb-release.d <== head: error reading '/etc/lsb-release.d': Is a directory ==> /etc/os-release <== NAME=openSUSE VERSION="13.1 (Bottle)" VERSION_ID="13.1" PRETTY_NAME="openSUSE 13.1 (Bottle) (x86_64)" ID=opensuse ANSI_COLOR="0;32" CPE_NAME="cpe:/o:opensuse:opensuse:13.1" BUG_REPORT_URL="https://bugs.opensuse.org"; HOME_URL="https://opensuse.org/"; ID_LIKE="suse" On Sun, May 31, 2015 at 1:26 PM, Christian Couder wrote: > On Sun, May 31, 2015 at 9:13 PM, Bruce Korb wrote: >> $ git format-patch -o patches --ignore-if-in-upstream >> 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d >> Segmentation fault >> /u/gnu/proj/gnu-pw-mgr >> $ git format-patch -o patches >> 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d >> patches/0001-remove-dead-code.patch >> patches/0002-dead-code-removal.patch >> patches/0003-add-sort-pw-cfg-program.patch >> patches/0004-add-doc-for-sort-pw-cfg.patch >> patches/0005-clean-up-doc-makefile.patch >> patches/0006-clean-up-doc-makefile.patch >> patches/0007-happy-2015-and-add-delete-option.patch >> patches/0008-fix-doc-Makefile.am.patch >> patches/0009-re-fix-copyright.patch >> patches/0010-finish-debugging-remove_pwid.patch >> patches/0011-only-update-file-if-something-was-removed.patch >> patches/0012-update-NEWS.patch >> patches/0013-bootstrap-cleanup.patch > > Could you tell us which git version you are using? You can use "git > --version". > The operating system you are using could also be useful. > And maybe you could also run git under gdb and give us the output of > the "bt" (backtrace) gdb command when it crashes? > > Thanks, > Christian. -- 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: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
Junio C Hamano writes:
> Karthik Nayak writes:
>
>> /*
>> + * Given a refname, return 1 if the refname matches with one of the patterns
>> + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
>> + * and so on, else return 0. Supports use of wild characters.
>> + */
>> +static int match_name_as_path(const char **pattern, const char *refname)
>> +{
>
> I wonder why this is not "match_refname", in other words, what the
> significance of "as path" part of the name?
Just match_refname may not carry all the semantics of the function,
which matches a prefix up to the end of string, or up to a / (but you
can't just be a prefix stopping in the middle of a word). To me, the
"_as_path" helped understanding the overall behavior of the function.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: seg fault in "git format-patch"
On Sun, May 31, 2015 at 9:13 PM, Bruce Korb wrote: > $ git format-patch -o patches --ignore-if-in-upstream > 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d > Segmentation fault > /u/gnu/proj/gnu-pw-mgr > $ git format-patch -o patches > 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d > patches/0001-remove-dead-code.patch > patches/0002-dead-code-removal.patch > patches/0003-add-sort-pw-cfg-program.patch > patches/0004-add-doc-for-sort-pw-cfg.patch > patches/0005-clean-up-doc-makefile.patch > patches/0006-clean-up-doc-makefile.patch > patches/0007-happy-2015-and-add-delete-option.patch > patches/0008-fix-doc-Makefile.am.patch > patches/0009-re-fix-copyright.patch > patches/0010-finish-debugging-remove_pwid.patch > patches/0011-only-update-file-if-something-was-removed.patch > patches/0012-update-NEWS.patch > patches/0013-bootstrap-cleanup.patch Could you tell us which git version you are using? You can use "git --version". The operating system you are using could also be useful. And maybe you could also run git under gdb and give us the output of the "bt" (backtrace) gdb command when it crashes? Thanks, Christian. -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On 05/31/2015 11:18 PM, Junio C Hamano wrote:
Karthik Nayak writes:
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
I am not sure if this is a good external interface, i.e. to expect
callers of ref-filter API to do this:
prepare cbdata;
for_each_ref(ref_filter_handler, &cbdata);
It might be better to expect callers to do this instead:
prepare cbdata;
filter_refs(for_each_ref, &cbdata);
i.e. introducing a new "filter_refs()" function as the entry point
to the ref-filter API. The filter_refs() entry point may internally
use ref_filter_handler() that will be file-scope static to ref-filter.c
and at that point the overly generic "-handler" name would not bother
anybody ;-) but more importantly, then you can extend the function
signature of filter_refs() not to be so tied to for_each_ref() API.
It could be that the internals of cbdata may not be something the
callers of filter-refs API does not even have to know about, in
which case the call might even become something like:
struct ref_array refs = REF_ARRAY_INIT;
const char **ref_patterns = { "refs/heads/*", "refs/tags/*", NULL};
filter_refs(&refs, for_each_rawref, ref_patterns);
/* now "refs" has the result, the caller uses them */
for (i = 0; i < refs.nr; i++)
refs.item[i];
Just a thought.
Thats brilliant, How about I introduce something of this sort
int filter_refs(int (*for_each_ref_fn)(each_ref_fn, void *),
ref_filter_cbdata *cbdata)
{
return for_each_ref_fn(ref_filter_handler, cbdata);
}
where its the most basic form, and things like
>
>struct ref_array refs = REF_ARRAY_INIT;
> const char **ref_patterns = { "refs/heads/*", "refs/tags/*",
NULL};
>
>filter_refs(&refs, for_each_rawref, ref_patterns);
>
> /* now "refs" has the result, the caller uses them */
>for (i = 0; i < refs.nr; i++)
>refs.item[i];
>
Could be achieved using a simple wrapper around 'filter_refs()'
something like this perhaps.
int filter_refs_with_pattern(struct ref_array *ref, int
(*for_each_ref_fn)(each_ref_fn, void *), char **patterns)
{
int i;
struct ref_filter_cbdata data;
data.filter.name_patterns = patterns;
filter_refs(for_each_ref_fn, &data);
refs->nr = data.array.nr;
for(i = 0; i < refs->nr; i++) {
/* copy over the refs */
}
return 0;
}
Is this on the lines of what you had in mind? If it is, than I could
just create a new patch which would make ref_filter_handler() private
and introduce filter_refs() as shown.
--
Regards,
Karthik
--
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 v3] blame: add blame.showEmail configuration
Complement existing --show-email option with fallback
configuration variable, with tests.
Signed-off-by: Quentin Neill
---
Documentation/git-blame.txt | 2 ++
builtin/blame.c | 10 +++-
t/t8002-blame.sh| 62 +
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 9f23a86..e6e947c 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -76,6 +76,8 @@ include::blame-options.txt[]
-e::
--show-email::
Show the author email instead of author name (Default: off).
+ This can also be controlled via the `blame.showEmail` config
+ option.
-w::
Ignore whitespace when comparing the parent's version and
diff --git a/builtin/blame.c b/builtin/blame.c
index 8d70623..60039d3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2185,6 +2185,14 @@ static int git_blame_config(const char *var, const char
*value, void *cb)
blank_boundary = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "blame.showemail")) {
+ int *output_option = cb;
+ if (git_config_bool(var, value))
+ *output_option |= OUTPUT_SHOW_EMAIL;
+ else
+ *output_option &= ~OUTPUT_SHOW_EMAIL;
+ return 0;
+ }
if (!strcmp(var, "blame.date")) {
if (!value)
return config_error_nonbool(var);
@@ -2529,7 +2537,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
unsigned int range_i;
long anchor;
- git_config(git_blame_config, NULL);
+ git_config(git_blame_config, &output_option);
init_revisions(&revs, NULL);
revs.date_mode = blame_date_mode;
DIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index 5cdf3f1..ff09ace 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -19,4 +19,66 @@ test_expect_success 'blame --show-email' '
"" 1
'
+test_expect_success 'setup showEmail tests' '
+ echo "bin: test number 1" >one &&
+ git add one &&
+ GIT_AUTHOR_NAME=name1 \
+ GIT_AUTHOR_EMAIL=ema...@test.git \
+ git commit -m First --date="2010-01-01 01:00:00" &&
+ cat >expected_n <<-\EOF &&
+ (name1 2010-01-01 01:00:00 + 1) bin: test number 1
+ EOF
+ cat >expected_e <<-\EOF
+ ( 2010-01-01 01:00:00 + 1) bin: test number 1
+ EOF
+'
+
+find_blame () {
+ sed -e 's/^[^(]*//'
+}
+
+test_expect_success 'blame with no options and no config' '
+ git blame one >blame &&
+ find_blame result &&
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showemail options' '
+ git blame --show-email one >blame1 &&
+ find_blame result &&
+ test_cmp expected_e result &&
+ git blame -e one >blame2 &&
+ find_blame result &&
+ test_cmp expected_e result &&
+ git blame --no-show-email one >blame3 &&
+ find_blame result &&
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showEmail config false' '
+ git config blame.showEmail false &&
+ git blame one >blame1 &&
+ find_blame result &&
+ test_cmp expected_n result &&
+ git blame --show-email one >blame2 &&
+ find_blame result &&
+ test_cmp expected_e result &&
+ git blame -e one >blame3 &&
+ find_blame result &&
+ test_cmp expected_e result &&
+ git blame --no-show-email one >blame4 &&
+ find_blame result &&
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showEmail config true' '
+ git config blame.showEmail true &&
+ git blame one >blame1 &&
+ find_blame result &&
+ test_cmp expected_e result &&
+ git blame --no-show-email one >blame2 &&
+ find_blame result &&
+ test_cmp expected_n result
+'
+
test_done
--
2.4.2.340.g5ecd853
--
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 v2] blame: add blame.showEmail configuration
On Sun, May 31, 2015 at 1:13 PM, Junio C Hamano wrote:
> Quentin Neill writes:
>
>> From: Quentin Neill
>>
>> Complement existing --show-email option with fallback
>> configuration variable, with tests.
>> ---
>
> The patch itself looks very reasonable. Thanks for getting back to
> us ;-)
>
> A few minor nits:
>
> - Your in-body "From:" is redundant and unnecessary, as your
> e-mail is coming from the same address.
I tried using 'git send-email' directly on the commit, not sure how or
why that occurred.
I will fall back to 'git format-patch' and 'git send-email' as I did
in my first post.
> - You need "Signed-off-by: Quentin Neill "
> after your log message (separate it with a blank line before
> the sign-off, and place the sign-off before the three-dash
> lines).
>
>> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
>> index 5cdf3f1..faf1660 100755
>> --- a/t/t8002-blame.sh
>> +++ b/t/t8002-blame.sh
>> @@ -19,4 +19,66 @@ test_expect_success 'blame --show-email' '
>> "" 1
>> '
>>
>> +test_expect_success 'setup showEmail tests' '
>> + echo "bin: test number 1" >one &&
>> + git add . &&
>> + GIT_AUTHOR_NAME=name1 GIT_AUTHOR_EMAIL=ema...@test.git git commit -a
>> -m First --date="2010-01-01 01:00:00"
>> +'
>> +
>> +cat >expected_n <<\EOF &&
>> +(name1 2010-01-01 01:00:00 + 1) bin: test number 1
>> +EOF
>> +
>> +cat >expected_e <<\EOF &&
>> +( 2010-01-01 01:00:00 + 1) bin: test number 1
>> +EOF
>
> These two commands outside test_expect_success are part of setup, so
>
> test_expect_success 'setup showEmail tests' '
> echo "bin: test number 1" >one &&
> git add one &&
> GIT_AUTHOR_NAME=name1 \
> GIT_AUTHOR_EMAIL=ema...@test.git \
> git commit -m First --date="2010-01-01 01:00:00" &&
> cat >expected_n <<-\EOF &&
> (name1 ...
> EOF
> cat >expected_e <<-\EOF
> ( EOF
> '
>
> Also do not hesitate to break overlong lines with "\".
>
>> +find_blame() {
>
> style: "find_blame () {"
>
> Other than that, the patch looks good.
>
> Thanks.
Thanks for the help, one more version on the way.
--
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
seg fault in "git format-patch"
$ git format-patch -o patches --ignore-if-in-upstream 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d Segmentation fault /u/gnu/proj/gnu-pw-mgr $ git format-patch -o patches 14949fa8f39d29e44b43f4332ffaf35f11546502..2de9eef391259dfc8748dbaf76a5d55427f37b0d patches/0001-remove-dead-code.patch patches/0002-dead-code-removal.patch patches/0003-add-sort-pw-cfg-program.patch patches/0004-add-doc-for-sort-pw-cfg.patch patches/0005-clean-up-doc-makefile.patch patches/0006-clean-up-doc-makefile.patch patches/0007-happy-2015-and-add-delete-option.patch patches/0008-fix-doc-Makefile.am.patch patches/0009-re-fix-copyright.patch patches/0010-finish-debugging-remove_pwid.patch patches/0011-only-update-file-if-something-was-removed.patch patches/0012-update-NEWS.patch patches/0013-bootstrap-cleanup.patch -- 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] strbuf_read: skip unnecessary strbuf_grow at eof
Make strbuf_read not try to do read_in_full's job too. If xread returns
less than was requested it can be either eof or an interrupted read. If
read_in_full returns less than was requested, it's eof. Use read_in_full
to detect eof and not iterate when eof has been seen.
Signed-off-by: Jim Hill
---
strbuf.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 88cafd4..c70733e 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -364,19 +364,19 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t
hint)
strbuf_grow(sb, hint ? hint : 8192);
for (;;) {
- ssize_t cnt;
+ ssize_t want = sb->alloc - sb->len - 1;
+ ssize_t got = read_in_full(fd, sb->buf + sb->len, want);
- cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
- if (cnt < 0) {
+ if (got < 0) {
if (oldalloc == 0)
strbuf_release(sb);
else
strbuf_setlen(sb, oldlen);
return -1;
}
- if (!cnt)
+ sb->len += got;
+ if (got < want)
break;
- sb->len += cnt;
strbuf_grow(sb, 8192);
}
--
2.4.1.4.gfc728c2
--
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 0/2] don't add duplicate paths to info/alternates
These patches address http://thread.gmane.org/gmane.comp.version-control.git/269050/focus=269415 linked from the git blame page, avoiding adding duplicates to info/alternates and removing hold_lock_file_for_append which is too heavyweight for logging and too limited for anything else. There's an argument to be made that since a-t-a-f is only used by clone, it shouldn't even bother taking a lock -- but then it should be moved to builtin/clone.c and reduced to a single write of the pre-deduped list, followed by a single read_info_alternates call. One thing at a time. Taking out the locking in the incremental version here doesn't really simplify the code much anyway. -- 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 2/2] remove hold_lock_file_for_append
No uses of hold_lock_file_for_append remain, so remove it.
hold_lock_file_for_append copies its target file internally. This makes
it too heavyweight for logging and too limited for anything else. It
shouldn't be used.
Signed-off-by: Jim Hill
---
lockfile.c | 38 --
1 file changed, 38 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 9889277..1467778 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -187,44 +187,6 @@ int hold_lock_file_for_update(struct lock_file *lk, const
char *path, int flags)
return fd;
}
-int hold_lock_file_for_append(struct lock_file *lk, const char *path, int
flags)
-{
- int fd, orig_fd;
-
- fd = lock_file(lk, path, flags);
- if (fd < 0) {
- if (flags & LOCK_DIE_ON_ERROR)
- unable_to_lock_die(path, errno);
- return fd;
- }
-
- orig_fd = open(path, O_RDONLY);
- if (orig_fd < 0) {
- if (errno != ENOENT) {
- int save_errno = errno;
-
- if (flags & LOCK_DIE_ON_ERROR)
- die("cannot open '%s' for copying", path);
- rollback_lock_file(lk);
- error("cannot open '%s' for copying", path);
- errno = save_errno;
- return -1;
- }
- } else if (copy_fd(orig_fd, fd)) {
- int save_errno = errno;
-
- if (flags & LOCK_DIE_ON_ERROR)
- exit(128);
- close(orig_fd);
- rollback_lock_file(lk);
- errno = save_errno;
- return -1;
- } else {
- close(orig_fd);
- }
- return fd;
-}
-
FILE *fdopen_lock_file(struct lock_file *lk, const char *mode)
{
if (!lk->active)
--
2.4.1.4.gfc728c2
--
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 1/2] add_to_alternates_file: don't add duplicate paths
Check for an existing match before appending a path to the alternates
file. Beyond making git look smart to anyone checking the alternates
file, this removes the last use of hold_lock_file_for_append.
Signed-off-by: Jim Hill
---
sha1_file.c| 29 +
t/t5700-clone-reference.sh | 4 ++--
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 47f56f2..43d9530 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -403,14 +403,35 @@ void read_info_alternates(const char * relative_base, int
depth)
void add_to_alternates_file(const char *reference)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_lock_file_for_append(lock,
git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
+ static struct lock_file lock = {0};
char *alt = mkpath("%s\n", reference);
+ char *alts = git_path("objects/info/alternates");
+ int fd = hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
+ struct strbuf altdata = STRBUF_INIT;
+ struct string_list lines = STRING_LIST_INIT_NODUP;
+
+ if (strbuf_read_file(&altdata, alts, 0) < 0)
+ if (errno != ENOENT)
+ die("alternates file unreadable");
+ strbuf_complete_line(&altdata);
+ write_or_die(fd, altdata.buf, altdata.len);
+
+ string_list_split_in_place(&lines, altdata.buf, '\n', -1);
+ lines.cmp = strcmp_icase;
+ if (unsorted_string_list_has_string(&lines, reference)) {
+ rollback_lock_file(&lock);
+ goto cleanup;
+ }
+
write_or_die(fd, alt, strlen(alt));
- if (commit_lock_file(lock))
- die("could not close alternates file");
+ if (commit_lock_file(&lock))
+ die("could not update alternates file");
if (alt_odb_tail)
link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0);
+
+cleanup:
+ strbuf_reset(&altdata);
+ string_list_clear(&lines,0);
}
int foreach_alt_odb(alt_odb_fn fn, void *cb)
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 3e783fc..cd9fa34 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -29,11 +29,11 @@ git prune'
cd "$base_dir"
test_expect_success 'cloning with reference (-l -s)' \
-'git clone -l -s --reference B A C'
+'git clone -l -s --reference B --reference A --reference B A C'
cd "$base_dir"
-test_expect_success 'existence of info/alternates' \
+test_expect_success 'existence of info/alternates, no duplicates' \
'test_line_count = 2 C/.git/objects/info/alternates'
cd "$base_dir"
--
2.4.1.4.gfc728c2
--
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 v2] blame: add blame.showEmail configuration
Quentin Neill writes:
> From: Quentin Neill
>
> Complement existing --show-email option with fallback
> configuration variable, with tests.
> ---
The patch itself looks very reasonable. Thanks for getting back to
us ;-)
A few minor nits:
- Your in-body "From:" is redundant and unnecessary, as your
e-mail is coming from the same address.
- You need "Signed-off-by: Quentin Neill "
after your log message (separate it with a blank line before
the sign-off, and place the sign-off before the three-dash
lines).
> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> index 5cdf3f1..faf1660 100755
> --- a/t/t8002-blame.sh
> +++ b/t/t8002-blame.sh
> @@ -19,4 +19,66 @@ test_expect_success 'blame --show-email' '
> "" 1
> '
>
> +test_expect_success 'setup showEmail tests' '
> + echo "bin: test number 1" >one &&
> + git add . &&
> + GIT_AUTHOR_NAME=name1 GIT_AUTHOR_EMAIL=ema...@test.git git commit -a -m
> First --date="2010-01-01 01:00:00"
> +'
> +
> +cat >expected_n <<\EOF &&
> +(name1 2010-01-01 01:00:00 + 1) bin: test number 1
> +EOF
> +
> +cat >expected_e <<\EOF &&
> +( 2010-01-01 01:00:00 + 1) bin: test number 1
> +EOF
These two commands outside test_expect_success are part of setup, so
test_expect_success 'setup showEmail tests' '
echo "bin: test number 1" >one &&
git add one &&
GIT_AUTHOR_NAME=name1 \
GIT_AUTHOR_EMAIL=ema...@test.git \
git commit -m First --date="2010-01-01 01:00:00" &&
cat >expected_n <<-\EOF &&
(name1 ...
EOF
cat >expected_e <<-\EOF
( +find_blame() {
style: "find_blame () {"
Other than that, the patch looks good.
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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
Christian Couder writes: > On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote: >> >> -static void sort_refs(struct ref_sort *sort, struct ref_array *array) >> +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) > > It is probably better to call the above function ref_array_sort()... Care to explain the reasoning behind that suggestion? The function "sorts" "ref_array", so verb-object sounds like a valid construction of a name. All the functions externalized by the patch under discussion follows that pattern, I think (e.g. parse-ref-filter-atom, verify-ref-format, show-ref-array-item). Making the API function share a common prefix is another valid school of design; while that may justify ref-array-sort(), but if you are going in that direction, all the others need to be renamed along that line consistently, I would think. -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
Karthik Nayak writes:
> /*
> - * A call-back given to for_each_ref(). Filter refs and keep them for
> + * A call-back given to for_each_ref(). Filter refs and keep them for
> * later object processing.
> */
> -static int grab_single_ref(const char *refname, const unsigned char *sha1,
> int flag, void *cb_data)
> +int ref_filter_handler(const char *refname, const unsigned char *sha1, int
> flag, void *cb_data)
> {
I am not sure if this is a good external interface, i.e. to expect
callers of ref-filter API to do this:
prepare cbdata;
for_each_ref(ref_filter_handler, &cbdata);
It might be better to expect callers to do this instead:
prepare cbdata;
filter_refs(for_each_ref, &cbdata);
i.e. introducing a new "filter_refs()" function as the entry point
to the ref-filter API. The filter_refs() entry point may internally
use ref_filter_handler() that will be file-scope static to ref-filter.c
and at that point the overly generic "-handler" name would not bother
anybody ;-) but more importantly, then you can extend the function
signature of filter_refs() not to be so tied to for_each_ref() API.
It could be that the internals of cbdata may not be something the
callers of filter-refs API does not even have to know about, in
which case the call might even become something like:
struct ref_array refs = REF_ARRAY_INIT;
const char **ref_patterns = { "refs/heads/*", "refs/tags/*", NULL};
filter_refs(&refs, for_each_rawref, ref_patterns);
/* now "refs" has the result, the caller uses them */
for (i = 0; i < refs.nr; i++)
refs.item[i];
Just a thought.
--
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: [WIP/PATCH v4 3/8] for-each-ref: rename 'refinfo' to 'ref_array_item'
On 05/31/2015 11:07 PM, Junio C Hamano wrote: Karthik Nayak writes: Rename 'refinfo' to 'ref_array_item' as a preparatory step for introduction of new structures in the forthcoming patch. Re-order the fields in 'ref_array_item' so that refname can be eventually converted to a FLEX_ARRAY. Not a big enough deal to require a reroll, but I think it would be easier to read if this reordering is done in the same patch that makes refname[] into a flex array. Yes I was thinking the same, I'll add that in the re-roll. -- Regards, Karthik -- 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: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
On 05/31/2015 11:04 PM, Junio C Hamano wrote:
Karthik Nayak writes:
/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
I wonder why this is not "match_refname", in other words, what the
significance of "as path" part of the name? If you later are going
to introuce match_name_as_something_else() and that name may not be
a refname, then this naming is perfectly sane. If such a function
you will later introduce will still deal with names that are only
refnames, then match_refname_as_path() would be sensible. Otherwise
this name seems overly long (i.e. "as_path" may not be adding value)
while not being desctiptive enough (i.e. is meant to be limited to
refnames but just says "name").
Just being curious.
Good Question, This is because in the future I'll eventually add pattern
matching for 'tag -l' and 'branch -l' which wont match as path, but
would be a regular string match (with wild character support). Hence
keeping that in mind I included 'as_path()' in the function name.
The comment still is a good reminder for those who will later touch
this grab_single_ref() function to make them think twice.
Thanks.
Yes! It was removed it by mistake.
--
Regards,
Karthik
--
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: [WIP/PATCH v4 3/8] for-each-ref: rename 'refinfo' to 'ref_array_item'
Karthik Nayak writes: > Rename 'refinfo' to 'ref_array_item' as a preparatory step for introduction > of new structures in the forthcoming patch. > > Re-order the fields in 'ref_array_item' so that refname can be > eventually converted to a FLEX_ARRAY. Not a big enough deal to require a reroll, but I think it would be easier to read if this reordering is done in the same patch that makes refname[] into a flex array. -- 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: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
Karthik Nayak writes:
> /*
> + * Given a refname, return 1 if the refname matches with one of the patterns
> + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
> + * and so on, else return 0. Supports use of wild characters.
> + */
> +static int match_name_as_path(const char **pattern, const char *refname)
> +{
I wonder why this is not "match_refname", in other words, what the
significance of "as path" part of the name? If you later are going
to introuce match_name_as_something_else() and that name may not be
a refname, then this naming is perfectly sane. If such a function
you will later introduce will still deal with names that are only
refnames, then match_refname_as_path() would be sensible. Otherwise
this name seems overly long (i.e. "as_path" may not be adding value)
while not being desctiptive enough (i.e. is meant to be limited to
refnames but just says "name").
Just being curious.
> @@ -851,40 +888,16 @@ static int grab_single_ref(const char *refname, const
> unsigned char *sha1, int f
> return 0;
> }
>
> - if (*cb->grab_pattern) {
> -...
> - }
> + if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
> + return 0;
>
> - /*
> - * We do not open the object yet; sort may only need refname
> - * to do its job and the resulting list may yet to be pruned
> - * by maxcount logic.
> - */
> - ref = xcalloc(1, sizeof(*ref));
The comment still is a good reminder for those who will later touch
this grab_single_ref() function to make them think twice.
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: [PATCH/WIP 5/8] am: detect mbox patches
On Wed, May 27, 2015 at 9:33 AM, Paul Tan wrote:
> Since 15ced75 (git-am foreign patch support: autodetect some patch
> formats, 2009-05-27), git-am.sh is able to autodetect mbox, stgit and
> mercurial patches through heuristics.
>
> Re-implement support for autodetecting mbox/maildir files.
A few minor comments below...
> Signed-off-by: Paul Tan
> ---
> diff --git a/builtin/am.c b/builtin/am.c
> index 9c7b058..d589ec5 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -108,6 +108,97 @@ static void am_destroy(const struct am_state *state)
> strbuf_release(&sb);
> }
>
> +/*
> + * Returns 1 if the file looks like a piece of email a-la RFC2822, 0
> otherwise.
> + * We check this by grabbing all the non-indented lines and seeing if they
> look
> + * like they begin with valid header field names.
> + */
> +static int is_email(const char *filename)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + FILE *fp = xfopen(filename, "r");
> + int ret = 1;
> +
> + while (!strbuf_getline(&sb, fp, '\n')) {
> + const char *x;
> +
> + strbuf_rtrim(&sb);
> +
> + if (!sb.len)
> + break; /* End of header */
> +
> + /* Ignore indented folded lines */
> + if (*sb.buf == '\t' || *sb.buf == ' ')
> + continue;
> +
> + /* It's a header if it matches the regexp "^[!-9;-~]+:" */
> + for (x = sb.buf; *x; x++) {
> + if (('!' <= *x && *x <= '9') || (';' <= *x && *x <=
> '~'))
> + continue;
> + if (*x == ':' && x != sb.buf)
> + break;
> + ret = 0;
> + goto fail;
> + }
> + }
> +
> +fail:
It's a bit odd seeing a label named "fail" through which both the
successful and failing cases flow. Upon seeing it, I had expected to
find a "return 1" somewhere above for the successful case. Perhaps a
name such as "done" would be clearer.
> + fclose(fp);
> + strbuf_release(&sb);
> + return ret;
> +}
> +
> +/**
> + * Attempts to detect the patch_format of the patches contained in `paths`,
> + * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if
> + * detection fails.
> + */
> +static int detect_patch_format(struct string_list *paths)
> +{
> + enum patch_format ret = PATCH_FORMAT_UNKNOWN;
> + struct strbuf l1 = STRBUF_INIT;
> + struct strbuf l2 = STRBUF_INIT;
> + struct strbuf l3 = STRBUF_INIT;
> + FILE *fp;
> +
> + /*
> +* We default to mbox format if input is from stdin and for
> directories
> +*/
> + if (!paths->nr || !strcmp(paths->items->string, "-") ||
> + is_directory(paths->items->string)) {
> + strbuf_release(&l1);
> + strbuf_release(&l2);
> + strbuf_release(&l3);
> + return PATCH_FORMAT_MBOX;
A couple observations:
Some people will argue that these strbuf_release() calls are pointless
since the strbufs haven't been used yet, thus should be dropped. (I
don't care strongly.)
Perhaps this case could be a bit less noisy if coded like this:
if (... || ...) {
ret = PATCH_FORMAT_MBOX;
goto done;
}
with a corresponding "done" label added before the strbuf_release()
calls near the bottom of the function.
> + }
> +
> + /*
> +* Otherwise, check the first few 3 lines of the first patch, starting
Either: s/few 3/few/ or s/few 3/3/
> +* from the first non-blank line, to try to detect its format.
> +*/
> + fp = xfopen(paths->items->string, "r");
> + while (!strbuf_getline(&l1, fp, '\n')) {
> + strbuf_trim(&l1);
> + if (l1.len)
> + break;
> + }
> + strbuf_getline(&l2, fp, '\n');
> + strbuf_trim(&l2);
> + strbuf_getline(&l3, fp, '\n');
> + strbuf_trim(&l3);
> + fclose(fp);
> +
> + if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: "))
> + ret = PATCH_FORMAT_MBOX;
> + else if (l1.len && l2.len && l3.len && is_email(paths->items->string))
> + ret = PATCH_FORMAT_MBOX;
> +
> + strbuf_release(&l1);
> + strbuf_release(&l2);
> + strbuf_release(&l3);
> + return ret;
> +}
> +
> /**
> * Splits out individual patches from `paths`, where each path is either a
> mbox
> * file or a Maildir. Return 0 on success, -1 on failure.
> @@ -162,6 +253,14 @@ static int split_patches(struct am_state *state, enum
> patch_format patch_format,
> static void am_setup(struct am_state *state, enum patch_format patch_format,
> struct string_list *paths)
> {
> + if (!patch_format)
> + patch_format = detect_patch_format(paths);
> +
> + if (!patch_format) {
> + fprintf_ln(stderr, _("Patch fo
Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On 05/31/2015 07:33 PM, Christian Couder wrote: On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak wrote: > On 05/31/2015 01:41 PM, Christian Couder wrote: >> >> On Sun, May 31, 2015 at 10:04 AM, Christian Couder >> wrote: >>> >>> On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak >>> wrote: -static void sort_refs(struct ref_sort *sort, struct ref_array *array) +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) >>> >>> >>> It is probably better to call the above function ref_array_sort()... >>> >>> [...] >>> -static struct ref_sort *default_sort(void) +/* If no sorting option is given, use refname to sort as default */ +struct ref_sort *ref_default_sort(void) >>> >>> >>> ... especially since you call the above ref_default_sort()... >>> -static int opt_parse_sort(const struct option *opt, const char *arg, int unset) +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset) >>> >>> >>> ... and the above opt_parse_sort(). >> >> >> After saying that I realize that these two other functions are not >> doing the same thing. >> This might suggest that they are not named very well as well. >> > > What do you mean by "not doing the same thing." sort_ref_array() is actually sorting a ref_array, while ref_default_sort() and opt_parse_ref_sort() are not sorting anything. Maybe it would all be clearer with a renaming like the following: sort_refs() -> ref_array_sort() struct ref_sort -> struct ref_sort_criteria default_sort() -> ref_default_sort_criteria() opt_parse_sort() -> opt_parse_ref_sort_criteria() Thanks will follow this, but will change opt_parse_ref_sort_criteria() to parse_opt_ref_sort_criteria() as this is what is commonly followed. -- Regards, Karthik -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On Sun, May 31, 2015 at 11:17 AM, Karthik Nayak wrote: > On 05/31/2015 01:41 PM, Christian Couder wrote: >> >> On Sun, May 31, 2015 at 10:04 AM, Christian Couder >> wrote: >>> >>> On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak >>> wrote: -static void sort_refs(struct ref_sort *sort, struct ref_array *array) +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) >>> >>> >>> It is probably better to call the above function ref_array_sort()... >>> >>> [...] >>> -static struct ref_sort *default_sort(void) +/* If no sorting option is given, use refname to sort as default */ +struct ref_sort *ref_default_sort(void) >>> >>> >>> ... especially since you call the above ref_default_sort()... >>> -static int opt_parse_sort(const struct option *opt, const char *arg, int unset) +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset) >>> >>> >>> ... and the above opt_parse_sort(). >> >> >> After saying that I realize that these two other functions are not >> doing the same thing. >> This might suggest that they are not named very well as well. >> > > What do you mean by "not doing the same thing." sort_ref_array() is actually sorting a ref_array, while ref_default_sort() and opt_parse_ref_sort() are not sorting anything. Maybe it would all be clearer with a renaming like the following: sort_refs() -> ref_array_sort() struct ref_sort -> struct ref_sort_criteria default_sort() -> ref_default_sort_criteria() opt_parse_sort() -> opt_parse_ref_sort_criteria() -- 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: Staging commits with visual diff tools?
On Sat, 30 May 2015, David Aguilar wrote: On Tue, May 26, 2015 at 09:50:49PM +0100, John Lee wrote: Hi Does anybody have code to stage commits using a the visual diff/merge tools supported by git-difftool? Is there support in git ... I'm a g/vim user, so git-cola is finely tuned for keyboard usage. If we implement these feature in Git, we should consider providing the same workflows/hotkeys as cola. Just to be clear I'm not planning on contributing my script back to git, it will just be a standalone script in a separate repo. I'll give git-cola a try, thanks. I just ran it and see it supports e.g. right click to stage and launching difftools -- does it also support launching a difftool to edit the index? John -- 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
Draft of Git Rev News edition 4
Hi, A draft of Git Rev News edition 4 is available here: https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-4.md Everyone is welcome to contribute in any section, like Junio and Matthieu already did, either by editing the above page on GitHub and sending a pull request, or by commenting on this GitHub issue: https://github.com/git/git.github.io/issues/63 You can also reply to this email. I tried to cc everyone who appears in this edition but maybe I missed some people, sorry about that. Thomas, Nicola and myself plan to publish this edition on Wednesday the 3rd of June. Thanks, Christian. -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On 05/31/2015 01:41 PM, Christian Couder wrote: On Sun, May 31, 2015 at 10:04 AM, Christian Couder wrote: On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote: -static void sort_refs(struct ref_sort *sort, struct ref_array *array) +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) It is probably better to call the above function ref_array_sort()... [...] -static struct ref_sort *default_sort(void) +/* If no sorting option is given, use refname to sort as default */ +struct ref_sort *ref_default_sort(void) ... especially since you call the above ref_default_sort()... -static int opt_parse_sort(const struct option *opt, const char *arg, int unset) +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset) ... and the above opt_parse_sort(). After saying that I realize that these two other functions are not doing the same thing. This might suggest that they are not named very well as well. What do you mean by "not doing the same thing." -- Regards, Karthik -- 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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On 05/31/2015 01:50 PM, Christian Couder wrote:
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote:
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
Where is this used_atom array?
I searched but couldn't find it in the same file.
- unsigned reverse : 1;
-};
It has been carried over since 9f613ddd, after which used_atom was
removed, I will remove that comment, just causes confusion
--
Regards,
Karthik
--
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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On 05/31/2015 01:59 PM, Eric Sunshine wrote: On Sun, May 31, 2015 at 4:19 AM, Karthik Nayak wrote: > On 05/31/2015 09:13 AM, Eric Sunshine wrote: >> On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak >> wrote: >>> >>> Create 'ref-filter.h', also add ref-filter to the Makefile. >>> This completes movement of creation of 'ref-filter' from >>> 'for-each-ref'. >> >> It's important that the project can be built successfully and function >> correctly at each stage of a patch series. Unfortunately, the way this >> series is organized, the build breaks after application of patch 7/8 >> since for-each-ref.c is trying to access functionality which was moved >> to ref-filter.c, but there is no header at that stage which advertises >> the functionality. Fixing this may be as simple as swapping patches >> 7/8 and 8/8, along with whatever minor adjustments they might need to >> keep them sane. (The alternative would be to combine patches 7/8 and >> 8/8, however, Matthieu didn't seem to favor that approach[1].) > > That is kind of a problem, If I need to swap those commits also, I'd have to > add the part where ref-filter is added to the Makefile with the code > movement from for-each-ref to ref-filter. This again will not just be Code > movement. > But I guess thats a fair trade-off, will change it. Thanks Don't take the "code movement-only" recommendation too literally. What people mean is that you shouldn't make changes to the lines you're moving from one location to another (other than absolutely mandatory changes to support the movement) since it's difficult to spot and review changes in lines which are being moved. The recommendation is not saying that the patch itself can't include any other changes (though, it's often best to minimize other changes in the same patch). In this case, the Makefile change is logically related to that particular code movement, so it's correct to include it in the patch. (Also, the Makefile change is so minor that it's not a burden upon reviewers.) Thanks for clearing it up. -- Regards, Karthik -- 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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On Sun, May 31, 2015 at 4:19 AM, Karthik Nayak wrote: > On 05/31/2015 09:13 AM, Eric Sunshine wrote: >> On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak >> wrote: >>> >>> Create 'ref-filter.h', also add ref-filter to the Makefile. >>> This completes movement of creation of 'ref-filter' from >>> 'for-each-ref'. >> >> It's important that the project can be built successfully and function >> correctly at each stage of a patch series. Unfortunately, the way this >> series is organized, the build breaks after application of patch 7/8 >> since for-each-ref.c is trying to access functionality which was moved >> to ref-filter.c, but there is no header at that stage which advertises >> the functionality. Fixing this may be as simple as swapping patches >> 7/8 and 8/8, along with whatever minor adjustments they might need to >> keep them sane. (The alternative would be to combine patches 7/8 and >> 8/8, however, Matthieu didn't seem to favor that approach[1].) > > That is kind of a problem, If I need to swap those commits also, I'd have to > add the part where ref-filter is added to the Makefile with the code > movement from for-each-ref to ref-filter. This again will not just be Code > movement. > But I guess thats a fair trade-off, will change it. Thanks Don't take the "code movement-only" recommendation too literally. What people mean is that you shouldn't make changes to the lines you're moving from one location to another (other than absolutely mandatory changes to support the movement) since it's difficult to spot and review changes in lines which are being moved. The recommendation is not saying that the patch itself can't include any other changes (though, it's often best to minimize other changes in the same patch). In this case, the Makefile change is logically related to that particular code movement, so it's correct to include it in the patch. (Also, the Makefile change is so minor that it's not a burden upon reviewers.) -- 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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote:
>
> -struct ref_sort {
> - struct ref_sort *next;
> - int atom; /* index into used_atom array */
Where is this used_atom array?
I searched but couldn't find it in the same file.
> - unsigned reverse : 1;
> -};
--
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: [WIP/PATCH v4 5/8] for-each-ref: introduce 'ref_filter_clear_data()'
As this is clearing the array only, it would be better to have it in a
ref_array_clear() function.
There are already argv_array_clear() and sha1_array_clear() by the way.
And maybe if many such ref_array functions are created and start being
used elsewhere we can easily move everything into new ref-array.{c,h}
files.
Makes sense. Will change.
--
Regards,
Karthik
--
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: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On 05/31/2015 09:13 AM, Eric Sunshine wrote: On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak wrote: Create 'ref-filter.h', also add ref-filter to the Makefile. This completes movement of creation of 'ref-filter' from 'for-each-ref'. It's important that the project can be built successfully and function correctly at each stage of a patch series. Unfortunately, the way this series is organized, the build breaks after application of patch 7/8 since for-each-ref.c is trying to access functionality which was moved to ref-filter.c, but there is no header at that stage which advertises the functionality. Fixing this may be as simple as swapping patches 7/8 and 8/8, along with whatever minor adjustments they might need to keep them sane. (The alternative would be to combine patches 7/8 and 8/8, however, Matthieu didn't seem to favor that approach[1].) Overall, this round has been a more pleasant read than previous rounds. [1]: http://article.gmane.org/gmane.comp.version-control.git/270192 That is kind of a problem, If I need to swap those commits also, I'd have to add the part where ref-filter is added to the Makefile with the code movement from for-each-ref to ref-filter. This again will not just be Code movement. But I guess thats a fair trade-off, will change it. Thanks -- Regards, Karthik -- 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 11/14] pull: teach git pull about --rebase
Hi Johannes,
On Tue, May 19, 2015 at 9:04 PM, Johannes Schindelin
wrote:
> On 2015-05-18 17:06, Paul Tan wrote:
>> +/**
>> + * Given a refspec, returns the merge branch. Returns NULL if the refspec
>> src
>> + * does not refer to a branch.
>> + *
>> + * FIXME: It should return the tracking branch. Currently only works with
>> the
>> + * default mapping.
>> + */
>> +static char *get_merge_branch_2(const char *repo, const char *refspec)
>> +{
>> + struct refspec *spec;
>> + const char *remote;
>> + char *merge_branch;
>> +
>> + spec = parse_fetch_refspec(1, &refspec);
>> + remote = spec->src;
>> + if (!*remote || !strcmp(remote, "HEAD"))
>> + remote = "HEAD";
>> + else if (skip_prefix(remote, "heads/", &remote))
>> + ;
>> + else if (skip_prefix(remote, "refs/heads/", &remote))
>> + ;
>> + else if (starts_with(remote, "refs/") ||
>> + starts_with(remote, "tags/") ||
>> + starts_with(remote, "remotes/"))
>> + remote = "";
>> +
>> + if (*remote) {
>> + if (!strcmp(repo, "."))
>> + merge_branch = mkpathdup("refs/heads/%s", remote);
>> + else
>> + merge_branch = mkpathdup("refs/remotes/%s/%s", repo,
>> remote);
>> + } else
>> + merge_branch = NULL;
>> +
>> + free_refspec(1, spec);
>> + return merge_branch;
>> +}
>
> I have to admit that it took me a substantial amount of time to deduce from
> the code what `get_merge_branch_2()` really does (judging from the
> description, I thought that it would be essentially the same as
> `get_merge_branch_1()` without the hard-coded "HEAD"). If the comment above
> the function would have said something like:
>
> /**
> * Given a refspec, returns the name of the local tracking ref.
> */
>
> I would have had an easier time. Also, I wonder if something like this would
> do the job:
Yeah whoops, this came from a confusion over the difference over
"merge branch" and "remote tracking branch". A merge branch would be a
remote tracking branch, but a remote tracking branch is not
necessarily a merge branch.
> spec = parse_fetch_refspec(1, &refspec);
> if (spec->dst)
> return spec->dst;
Hmm, I notice that get_remote_merge_branch() only looks at the src
part of the refspec. However, I guess it is true that if the dst part
is provided, the user may be wishing for that to be interpreted as the
"remote tracking branch", so we should be looking at it to calculate
the fork point.
> if (!(remote = get_remote(remote_name)))
> return NULL;
> if (remote_find_tracking(remote, spec))
> return spec->dst;
... and if the dst part of the refspec is not provided, we fall back
to see if there is any remote tracking branch in the repo for the src
part of the ref, which matches the intention of
get_remote_merge_branch() I think, while being better because
remote_find_tracking() takes into account the actual configured fetch
refspecs for the remote.
However, we also need to consider if the user provided a wildcard
refspec, as it will not make sense in this case. From my reading,
remote_find_tracking(), which calls query_refspecs(), would just match
the src part literally, so I guess we should explicitly detect and
error out in this case.
> return NULL;
>
> (I guess we'd have to `xstrdup()` the return values because the return value
> of `get_merge_branch_1()` needs to be `free()`d, but maybe we can avoid so
> much `malloc()/free()`ing.) Again, the function name should probably be
> changed to something clearer, maybe to `get_tracking_branch()`.
Yeah, it should be changed to get_tracking_branch(), since it is
different from get_merge_branch().
> One thing that is not clear at all to me is whether
>
> git pull --rebase origin master next
>
> would error out as expected, or simply rebase to `origin/master`.
In git-pull.sh, for the rebase fork point calculation it just used the
first refspec. However, when running git-rebase it checked to see if
there was only one merge base, which is replicated here:
@@ -573,6 +792,10 @@ int cmd_pull(int argc, const char **argv,
const char *prefix)
if (merge_heads.nr > 1)
die(_("Cannot merge multiple branches into
empty head."));
return pull_into_void(*merge_heads.sha1, curr_head);
+ } else if (opt_rebase) {
+ if (merge_heads.nr > 1)
+ die(_("Cannot rebase onto multiple branches."));
+ return run_rebase(curr_head, *merge_heads.sha1,
rebase_fork_point);
} else
return run_merge();
}
Since this is just a 1:1 rewrite I just tried to keep as close to the
original as possible. However, thinking about it, since we *are* just
using the first refspec for fork point calculation, I do agree th
Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On 05/31/2015 08:51 AM, Eric Sunshine wrote:
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak wrote:
Rename some of the functions and make them publically available.
s/publically/publicly/
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f896e1c..8fed04b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
}
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
Sneaking in whitespace change?
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = &ref_cbdata->filter;
Noted. Will fix!
--
Regards,
Karthik
--
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: [WIP/PATCH v4 4/8] for-each-ref: introduce new structures for better organisation
On 05/31/2015 08:44 AM, Eric Sunshine wrote:
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak wrote:
Intoduce 'ref_filter_cbdata' which will hold 'ref_filter'
s/Intoduce/Introduce/
(Conditions to filter the refs on) and 'ref_array' (The array
s/Conditions/conditions/
s/The/the/
of ref_array_items). Modify the code to use these new structures.
This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and making it publically available.
s/publically/publicly/
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e634fd2..ef54c90 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -85,7 +99,7 @@ static struct {
* a "*" to denote deref_tag().
*
* We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. ref_array_item
+ * of properties that we need to extract out of objects. ref_array_item
Sneaking in whitespace change?
* structure will hold an array of values extracted that can be
* indexed with the "atom number", which is an index into this
* array.
@@ -1076,12 +1085,12 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
- int i, num_refs;
+ int i;
const char *format = "%(objectname) %(objecttype)\t%(refname)";
struct ref_sort *sort = NULL, **sort_tail = &sort;
int maxcount = 0, quote_style = 0;
- struct ref_array_item **refs;
- struct grab_ref_cbdata cbdata;
+ struct ref_filter_cbdata ref_cbdata;
+ memset(&ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
Declaration (struct option opts[]) after statement (memset). I think
you want to leave the memset() where it was below.
OPT_BIT('s', "shell", "e_style,
@@ -1119,17 +1128,14 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(&cbdata, 0, sizeof(cbdata));
- cbdata.grab_pattern = argv;
- for_each_rawref(grab_single_ref, &cbdata);
- refs = cbdata.grab_array;
- num_refs = cbdata.grab_cnt;
+ ref_cbdata.filter.name_patterns = argv;
+ for_each_rawref(grab_single_ref, &ref_cbdata);
- sort_refs(sort, refs, num_refs);
+ sort_refs(sort, &ref_cbdata.array);
- if (!maxcount || num_refs < maxcount)
- maxcount = num_refs;
+ if (!maxcount || ref_cbdata.array.nr < maxcount)
+ maxcount = ref_cbdata.array.nr;
for (i = 0; i < maxcount; i++)
- show_ref(refs[i], format, quote_style);
+ show_ref(ref_cbdata.array.items[i], format, quote_style);
return 0;
}
--
2.4.2
Will change these, thanks!
--
Regards,
Karthik
--
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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On Sun, May 31, 2015 at 10:04 AM, Christian Couder wrote: > On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote: >> >> -static void sort_refs(struct ref_sort *sort, struct ref_array *array) >> +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) > > It is probably better to call the above function ref_array_sort()... > > [...] > >> -static struct ref_sort *default_sort(void) >> +/* If no sorting option is given, use refname to sort as default */ >> +struct ref_sort *ref_default_sort(void) > > ... especially since you call the above ref_default_sort()... > >> -static int opt_parse_sort(const struct option *opt, const char *arg, int >> unset) >> +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset) > > ... and the above opt_parse_sort(). After saying that I realize that these two other functions are not doing the same thing. This might suggest that they are not named very well as well. -- 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: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
Why did this comment get removed? Isn't it still meaningful to the overall logic of this function? Wasn't intentional, will put that back. Sneaking in whitespace change? Noted. thanks -- Regards, Karthik -- 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: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote: > > -static void sort_refs(struct ref_sort *sort, struct ref_array *array) > +void sort_ref_array(struct ref_sort *sort, struct ref_array *array) It is probably better to call the above function ref_array_sort()... [...] > -static struct ref_sort *default_sort(void) > +/* If no sorting option is given, use refname to sort as default */ > +struct ref_sort *ref_default_sort(void) ... especially since you call the above ref_default_sort()... > -static int opt_parse_sort(const struct option *opt, const char *arg, int > unset) > +int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset) ... and the above opt_parse_sort(). -- 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: [WIP/PATCH v4 5/8] for-each-ref: introduce 'ref_filter_clear_data()'
On Sat, May 30, 2015 at 7:53 PM, Karthik Nayak wrote:
> Introduce and implement 'ref_filter_clear_data()' which will free
> all allocated memory for 'ref_filter_cbdata' and its underlying array
> of 'ref_array_item'.
>
> Mentored-by: Christian Couder
> Mentored-by: Matthieu Moy
> Signed-off-by: Karthik Nayak
> ---
> builtin/for-each-ref.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index ef54c90..f896e1c 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -907,6 +907,18 @@ static int grab_single_ref(const char *refname, const
> unsigned char *sha1, int f
> return 0;
> }
>
> +/* Free all memory allocated for ref_filter_cbdata */
> +void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
> +{
> + int i;
> +
> + for (i = 0; i < ref_cbdata->array.nr; i++)
> + free(ref_cbdata->array.items[i]);
> + free(ref_cbdata->array.items);
> + ref_cbdata->array.items = NULL;
> + ref_cbdata->array.nr = ref_cbdata->array.alloc = 0;
> +}
As this is clearing the array only, it would be better to have it in a
ref_array_clear() function.
There are already argv_array_clear() and sha1_array_clear() by the way.
And maybe if many such ref_array functions are created and start being
used elsewhere we can easily move everything into new ref-array.{c,h}
files.
Thanks,
Christian.
--
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
