Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
(replying from webmail interface)
On Fri, Sep 5, 2014 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Redoing what e3f67d30 (am: fix patch format detection for
Thunderbird Save As emails, 2010-01-25) tried to do without
wasting a fork and a pipe may be a workable improvement.
I see Stephen who wrote the original Thunderbird save-as is
already on the Cc list. How about doing it this way instead?
It was so long ago I can't even remember writing that patch. But I
googled the thread from 4.5 years ago and I see that you suggested we
use tr because \r is not portable[1].
Not that way, but more like this.
git-am.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..32e3039 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with
the
# header field names...
- tr -d '\015' $1 |
- sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
+ sed -n -e 's/\r$//' -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
fi
[1]
http://git.661346.n2.nabble.com/PATCH-am-fix-patch-format-detection-for-Thunderbird-quot-Save-As-quot-emails-td4184273.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rev-list | git cherry-pick --stdin is leaky
On 05/09/13 08:10, René Scharfe wrote: Am 06.05.2013 22:16, schrieb Stephen Boyd: Ok. I tested it and it definitely helps. ==10728== LEAK SUMMARY: ==10728==definitely lost: 316,355,458 bytes in 8,652 blocks ==10728==indirectly lost: 1,327,251,588 bytes in 16,180,628 blocks ==10728== possibly lost: 677,049,918 bytes in 7,381,801 blocks ==10728==still reachable: 9,238,039 bytes in 63,947 blocks ==10728== suppressed: 0 bytes in 0 blocks vs. ==27614== LEAK SUMMARY: ==27614==definitely lost: 2,369,692,222 bytes in 20,005,707 blocks ==27614==indirectly lost: 829,151,786 bytes in 9,594,715 blocks ==27614== possibly lost: 658,069,373 bytes in 6,345,172 blocks ==27614==still reachable: 8,806,386 bytes in 50,199 blocks ==27614== suppressed: 0 bytes in 0 blocks Thanks, Stephen. I'm going to prepare a series around that patch which will (hopefully) show that freeing these entries is safe by passing only const pointers down to the callbacks. It's too late for 1.8.3, of course, but it shouldn't take another year as most of that series is done already. :) Yes I started trying to throw const around everywhere but then I just sent out the email in hopes someone had already solved this problem. We still have an impressive amount of leakage here. I wonder why indirectly lost increased so much. Do you perhaps still have the full output of valgrind for the run with the patch applied? I think it increased because the first one ran to completion while the second one failed half way through so it's not an apples to apples comparison. I can re-run with a certain range that is known not to fail if you're interested, but I think you got the idea that the patch helps. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rev-list | git cherry-pick --stdin is leaky
On 04/30/13 15:47, René Scharfe wrote: Am 30.04.2013 02:11, schrieb Stephen Boyd: (resending since the attachment seems to make vger sad) Hi, I'm running git rev-list | git cherry-pick --stdin on a range of about 300 commits. Eventually the chery-pick dies with: error: cannot fork() for commit: Cannot allocate memory Running valgrind shows me that the tree traversal code is leaking gigabytes of memory (particularly unpack_callback). Since cherry-pick is a very long running process all these allocations are never freed and eventually I run out of memory. The worst offender and summary is: ==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in 7,765,439 blocks are definitely lost in loss record 257 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4F5281: unpack_callback (unpack-trees.c:539) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986== ==7986== LEAK SUMMARY: ==7986==definitely lost: 2,514,117,692 bytes in 21,210,861 blocks ==7986==indirectly lost: 885,481,947 bytes in 10,165,801 blocks ==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks ==7986==still reachable: 7,734,870 bytes in 47,794 blocks ==7986== suppressed: 0 bytes in 0 blocks I looked at that particular leak a year ago but couldn't convince myself to submit the patch below. If the callback function we call through call_unpack_fn does something strange like free()ing entries itself or adding them to some list without duplication then the added free() can cause trouble. Looking at it again today I don't understand that concern any more. The current callback functions don't do something like that, in any case. Maybe I'm missing something. Anyway, could you please check if the patch helps with your use case? Ok. I tested it and it definitely helps. ==10728== LEAK SUMMARY: ==10728==definitely lost: 316,355,458 bytes in 8,652 blocks ==10728==indirectly lost: 1,327,251,588 bytes in 16,180,628 blocks ==10728== possibly lost: 677,049,918 bytes in 7,381,801 blocks ==10728==still reachable: 9,238,039 bytes in 63,947 blocks ==10728== suppressed: 0 bytes in 0 blocks vs. ==27614== LEAK SUMMARY: ==27614==definitely lost: 2,369,692,222 bytes in 20,005,707 blocks ==27614==indirectly lost: 829,151,786 bytes in 9,594,715 blocks ==27614== possibly lost: 658,069,373 bytes in 6,345,172 blocks ==27614==still reachable: 8,806,386 bytes in 50,199 blocks ==27614== suppressed: 0 bytes in 0 blocks -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rev-list | git cherry-pick --stdin is leaky
On 04/30/13 15:47, René Scharfe wrote: Am 30.04.2013 02:11, schrieb Stephen Boyd: (resending since the attachment seems to make vger sad) Hi, I'm running git rev-list | git cherry-pick --stdin on a range of about 300 commits. Eventually the chery-pick dies with: error: cannot fork() for commit: Cannot allocate memory Running valgrind shows me that the tree traversal code is leaking gigabytes of memory (particularly unpack_callback). Since cherry-pick is a very long running process all these allocations are never freed and eventually I run out of memory. The worst offender and summary is: ==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in 7,765,439 blocks are definitely lost in loss record 257 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4F5281: unpack_callback (unpack-trees.c:539) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986== ==7986== LEAK SUMMARY: ==7986==definitely lost: 2,514,117,692 bytes in 21,210,861 blocks ==7986==indirectly lost: 885,481,947 bytes in 10,165,801 blocks ==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks ==7986==still reachable: 7,734,870 bytes in 47,794 blocks ==7986== suppressed: 0 bytes in 0 blocks I looked at that particular leak a year ago but couldn't convince myself to submit the patch below. If the callback function we call through call_unpack_fn does something strange like free()ing entries itself or adding them to some list without duplication then the added free() can cause trouble. Looking at it again today I don't understand that concern any more. The current callback functions don't do something like that, in any case. Maybe I'm missing something. Anyway, could you please check if the patch helps with your use case? Ok I think I will make a copy of my .git first before I try out your patch. In case you're curious here are the next big leaks. ==7986== 433,116,790 (432,950,308 direct, 166,482 indirect) bytes in 4,146,402 blocks are definitely lost in loss record 253 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4F5281: unpack_callback (unpack-trees.c:539) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F6FAE: unpack_trees (unpack-trees.c:1074) ==7986==by 0x4AD095: git_merge_trees (merge-recursive.c:241) ==7986==by 0x4AF3D5: merge_trees (merge-recursive.c:1811) ==7986==by 0x4D9A7A: do_pick_commit (sequencer.c:311) ==7986== ==7986== 482,083,201 (46,465,928 direct, 435,617,273 indirect) bytes in 93 blocks are definitely lost in loss record 255 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4C4AE4: read_index_from (read-cache.c:1452) ==7986==by 0x4D99BC: do_pick_commit (sequencer.c:297) ==7986==by 0x4DA750: pick_commits (sequencer.c:995) ==7986==by 0x4DAFD6: sequencer_pick_revisions (sequencer.c:1124) ==7986==by 0x463E7C: cmd_cherry_pick (revert.c:236) ==7986==by 0x404C86: handle_internal_command (git.c:284) ==7986==by 0x40541C: main (git.c:492) ==7986== ==7986== 557,706,880 (548,062,684 direct, 9,644,196 indirect) bytes in 4,931,819 blocks are definitely lost in loss record 256 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4F5281: unpack_callback (unpack-trees.c:539) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F6FAE: unpack_trees (unpack-trees.c:1074) ==7986==by 0x4AD095: git_merge_trees (merge-recursive.c:241) ==7986== Full report can be found at https://gist.github.com/bebarino/5497181 -- Qualcomm Innovation Center, Inc. is a member of Code
git rev-list | git cherry-pick --stdin is leaky
(resending since the attachment seems to make vger sad) Hi, I'm running git rev-list | git cherry-pick --stdin on a range of about 300 commits. Eventually the chery-pick dies with: error: cannot fork() for commit: Cannot allocate memory Running valgrind shows me that the tree traversal code is leaking gigabytes of memory (particularly unpack_callback). Since cherry-pick is a very long running process all these allocations are never freed and eventually I run out of memory. The worst offender and summary is: ==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in 7,765,439 blocks are definitely lost in loss record 257 of 257 ==7986==at 0x4C267CC: calloc (vg_replace_malloc.c:467) ==7986==by 0x4FAF57: xcalloc (wrapper.c:119) ==7986==by 0x4F5281: unpack_callback (unpack-trees.c:539) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986==by 0x4F586C: unpack_callback (unpack-trees.c:467) ==7986==by 0x4F40E5: traverse_trees (tree-walk.c:407) ==7986== ==7986== LEAK SUMMARY: ==7986==definitely lost: 2,514,117,692 bytes in 21,210,861 blocks ==7986==indirectly lost: 885,481,947 bytes in 10,165,801 blocks ==7986== possibly lost: 650,712,395 bytes in 6,014,309 blocks ==7986==still reachable: 7,734,870 bytes in 47,794 blocks ==7986== suppressed: 0 bytes in 0 blocks This is against recent git.git (89740333e8d398f1da701e9023675321bbb9a85b). A workaround is to limit the amount of commits per cherry-pick invocation, but can we somehow fix the leaks? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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/TEST 0/2] bugs with cherry-pick renames
On 02/11/13 20:27, Stephen Boyd wrote: I ran into these bugs the other day and didn't have time to investigate further. So I wrote test cases for them instead. Stephen Boyd (2): t3501: Expose bug with cherry-pick into dirty trees w/ renames t3501: Expose addinfo_cache error message in cherry-pick t/t3501-revert-cherry-pick.sh | 60 +++ 1 file changed, 60 insertions(+) Any comments? Anyone else running into these bugs? -- 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?] am --abort on null cache entries fails
Hi, I was trying git am -3 with a patch that touched files that didn't exist in the branch I was on. Obviously it failed badly, so I wanted to abort out of the git am state with git am --abort. Unfortunately, it seems that git am --abort in this scenario fails with this error: error: cache entry has null sha1: non-existant-file and then leaves the file in my working tree untracked. This didn't used to happen, so I bisected it down to this commit commit 4337b5856f88f18da47c176e3cbc95a35627044c Author: Jeff King p...@peff.net Date: Sat Jul 28 11:05:24 2012 -0400 do not write null sha1s to on-disk index Which definitely introduced that error message. How do we fix this? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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/TEST 0/2] bugs with cherry-pick renames
I ran into these bugs the other day and didn't have time to investigate further. So I wrote test cases for them instead. Stephen Boyd (2): t3501: Expose bug with cherry-pick into dirty trees w/ renames t3501: Expose addinfo_cache error message in cherry-pick t/t3501-revert-cherry-pick.sh | 60 +++ 1 file changed, 60 insertions(+) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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/TEST 2/2] t3501: Expose addinfo_cache error message in cherry-pick
I encountered a mysterious error message while doing some cherry-picking into a dirty tree. In this case, the working tree was dirty with changes to two files that had been renamed, we'll call them 'file and 'otherfile'. The change I wanted to cherry-pick was made along a branch before the rename to 'file'. When I cherry-picked the change from the other branch without the rename to my current branch with the rename, the change applied cleanly and the dirty bits were committed but a mysterious error message was printed indicating something went wrong with the file that was still dirty after the cherry-pick. error: addinfo_cache failed for path 'otherfile' I suspect this error message shouldn't be printed, so recreate the problem in t3501 so that it can be fixed. Signed-off-by: Stephen Boyd sb...@codeaurora.org --- t/t3501-revert-cherry-pick.sh | 32 1 file changed, 32 insertions(+) diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index eef4d8c..522a9fd 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -137,4 +137,36 @@ test_expect_success 'cherry-pick on dirty rename should stay dirty' ' ! git diff --quiet ' +test_expect_success 'cherry-pick on dirty rename should not complain' ' + git checkout initial + + test_commit file1 + + for l in b c d e f g h i j k l m n o + do + echo $l$l$l$l$l$l$l$l$l + done oops + + test_tick + git add oops + git commit -m drop2 + git tag drop2 + + git checkout file1 + test_tick + git mv oops spoo + git mv file1.t file2.t + git commit -m rename4 + git tag rename4 + + echo file2 file2.t + for l in b c d e f g h i j k m n o + do + echo $l$l$l$l$l$l$l$l$l + done spoo + + git cherry-pick drop2 2 errors + ! test -s errors +' + test_done -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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/TEST 1/2] t3501: Expose bug with cherry-pick into dirty trees w/ renames
I encountered this bug while doing some cherry-picking into a dirty tree. In this case, the working tree was dirty with some changes to a file that had been renamed. The change I wanted to cherry-pick was made along another branch before the rename and it matched a subset of my working tree modulo the file rename. When I cherry-picked the change from the other branch without the rename to my current branch with the rename, the change applied cleanly and the dirty bits were committed but the other dirty bits in the file were lost. Make this into a test to expose this bug. Signed-off-by: Stephen Boyd sb...@codeaurora.org --- t/t3501-revert-cherry-pick.sh | 28 1 file changed, 28 insertions(+) diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index 6f489e2..eef4d8c 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -109,4 +109,32 @@ test_expect_success 'chery-pick on unborn branch' ' ! test_cmp_rev initial HEAD ' +test_expect_success 'cherry-pick on dirty rename should stay dirty' ' + git checkout initial + + for l in b c d e f g h i j k l m n o + do + echo $l$l$l$l$l$l$l$l$l + done oops + + test_tick + git add oops + git commit -m drop + git tag drop + + git checkout initial + test_tick + git mv oops spoo + git commit -m rename3 + git tag rename3 + + for l in b c d e f g h i j k m n o + do + echo $l$l$l$l$l$l$l$l$l + done spoo + + git cherry-pick drop + ! git diff --quiet +' + test_done -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] run-command: Be more informative about what failed
On 01/31/13 08:24, Junio C Hamano wrote: Stephen Boyd sb...@codeaurora.org writes: While debugging an error with verify_signed_buffer() the error messages from run-command weren't very useful: error: cannot create pipe for gpg: Too many open files error: could not run gpg. because they didn't indicate *which* pipe couldn't be created. For the message emitted here with your update (or without for that matter) to be useful, it has to hold that there is a single leaker, that leaker fails in this codepath, and that there is nobody else involved. Otherwise, you may be able to tell that one caller could not create its stdin, but the reason it couldn't may be because somebody else consumed all the available file descriptors. I am not opposed to this change per-se, but I am not sure that saying stdin etc. makes the message more useful for the purpose of debugging. It helped me avoid firing up gdb, but if you don't see much use feel free to ignore this patch. For example, the above error now prints: error: cannot create stderr pipe for gpg: Too many open files error: could not run gpg. I'd prefer to see these names spelled out (e.g. standard error) in any case. Sure, I can do that. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gpg: Close stderr once finished with it in verify_signed_buffer()
On 01/30/13 21:50, Jeff King wrote:
The strbuf_read above will read to EOF, so it should be equivalent (and
IMHO slightly more readable) to do:
diff --git a/gpg-interface.c b/gpg-interface.c
index 0863c61..5f142f6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -130,8 +130,10 @@ int verify_signed_buffer(const char *payload, size_t
payload_size,
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
- if (gpg_output)
+ if (gpg_output) {
strbuf_read(gpg_output, gpg.err, 0);
+ close(gpg.err);
+ }
ret = finish_command(gpg);
unlink_or_warn(path);
But that is a minor nit; either way, the patch looks good to me.
Looks better. I'll resend with this.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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
[PATCHv2 1/3] gpg: Close stderr once finished with it in verify_signed_buffer()
Failing to close the stderr pipe in verify_signed_buffer() causes
git to run out of file descriptors if there are many calls to
verify_signed_buffer(). An easy way to trigger this is to run
git log --show-signature --merges | grep key
on the linux kernel git repo. Eventually it will fail with
error: cannot create pipe for gpg: Too many open files
error: could not run gpg.
Close the stderr pipe so that this can't happen.
Suggested-by: Jeff King p...@peff.net
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
gpg-interface.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 0863c61..5f142f6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -130,8 +130,10 @@ int verify_signed_buffer(const char *payload, size_t
payload_size,
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
- if (gpg_output)
+ if (gpg_output) {
strbuf_read(gpg_output, gpg.err, 0);
+ close(gpg.err);
+ }
ret = finish_command(gpg);
unlink_or_warn(path);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
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/3] GPG running out of pipes fixes
While running --show-signatures on the linux kernel I noticed that after a while git failed with an error message indicating it had run out of file descriptors. The first patch fixes this problem, and the next two are randmom bits since I was in the area. Stephen Boyd (3): gpg: Close stderr once finished with it in verify_signed_buffer() run-command: Be more informative about what failed gpg: Allow translation of more error messages gpg-interface.c | 8 +--- run-command.c | 8 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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/3] gpg: Close stderr once finished with it in verify_signed_buffer()
Failing to close the stderr pipe in verify_signed_buffer() causes git to run out of file descriptors if there are many calls to verify_signed_buffer(). An easy way to trigger this is to run git log --show-signature --merges | grep key on the linux kernel git repo. Eventually it will fail with error: cannot create pipe for gpg: Too many open files error: could not run gpg. Close the stderr pipe so that this can't happen. Signed-off-by: Stephen Boyd sb...@codeaurora.org --- gpg-interface.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gpg-interface.c b/gpg-interface.c index 0863c61..2c0bed3 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -133,6 +133,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size, if (gpg_output) strbuf_read(gpg_output, gpg.err, 0); ret = finish_command(gpg); + if (gpg_output) + close(gpg.err); unlink_or_warn(path); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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/3] gpg: Allow translation of more error messages
Mark these strings for translation so that error messages are
printed in the user's language of choice.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
gpg-interface.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 2c0bed3..474c037 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -109,10 +109,10 @@ int verify_signed_buffer(const char *payload, size_t
payload_size,
args_gpg[0] = gpg_program;
fd = git_mkstemp(path, PATH_MAX, .git_vtag_tmpXX);
if (fd 0)
- return error(could not create temporary file '%s': %s,
+ return error(_(could not create temporary file '%s': %s),
path, strerror(errno));
if (write_in_full(fd, signature, signature_size) 0)
- return error(failed writing detached signature to '%s': %s,
+ return error(_(failed writing detached signature to '%s': %s),
path, strerror(errno));
close(fd);
@@ -124,7 +124,7 @@ int verify_signed_buffer(const char *payload, size_t
payload_size,
args_gpg[2] = path;
if (start_command(gpg)) {
unlink(path);
- return error(could not run gpg.);
+ return error(_(could not run gpg.));
}
write_in_full(gpg.in, payload, payload_size);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
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/3] run-command: Be more informative about what failed
While debugging an error with verify_signed_buffer() the error messages from run-command weren't very useful: error: cannot create pipe for gpg: Too many open files error: could not run gpg. because they didn't indicate *which* pipe couldn't be created. Print which pipe failed to be created in the error message so we can more easily debug similar problems in the future. For example, the above error now prints: error: cannot create stderr pipe for gpg: Too many open files error: could not run gpg. Signed-off-by: Stephen Boyd sb...@codeaurora.org --- run-command.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index 12d4ddb..016dd05 100644 --- a/run-command.c +++ b/run-command.c @@ -274,6 +274,7 @@ int start_command(struct child_process *cmd) int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; int failed_errno = failed_errno; + char *str; /* * In case of errors we must keep the promise to close FDs @@ -286,6 +287,7 @@ int start_command(struct child_process *cmd) failed_errno = errno; if (cmd-out 0) close(cmd-out); + str = stdin; goto fail_pipe; } cmd-in = fdin[1]; @@ -301,6 +303,7 @@ int start_command(struct child_process *cmd) close_pair(fdin); else if (cmd-in) close(cmd-in); + str = stdout; goto fail_pipe; } cmd-out = fdout[0]; @@ -318,9 +321,10 @@ int start_command(struct child_process *cmd) close_pair(fdout); else if (cmd-out) close(cmd-out); + str = stderr; fail_pipe: - error(cannot create pipe for %s: %s, - cmd-argv[0], strerror(failed_errno)); + error(cannot create %s pipe for %s: %s, + str, cmd-argv[0], strerror(failed_errno)); errno = failed_errno; return -1; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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] send-email: validate reconfirm interactive responses
(Sorry sending this from web interface)
On Wed, Sep 5, 2012 at 8:29 PM, Junio C Hamano gits...@pobox.com wrote:
Stephen Boyd bebar...@gmail.com writes:
This is now bugging me if I just hit enter and don't want to specify
anything for
these headers (I want the defaults or what's in the files already).
Can we allow
the empty string to be valid as well so I don't have to go through
these prompts?
That indeed was the intention, and if it is not behaving, you found
a bug.
The relevant code in sub ask does this:
...
$resp = $term-readline($prompt);
if (!defined $resp) { # EOF
print \n;
return defined $default ? $default : undef;
}
if ($resp eq '' and defined $default) {
return $default;
}
if (!defined $valid_re or $resp =~ /$valid_re/) {
return $resp;
}
I am scratching my head wondering why your just hit enter does not
trigger the if response is empty and we have default, just return it
codepath we can see above. It shouldn't even trigger the regexp
based validation codepath in the first place.
It works fine for Who should the emails appear to be from? but
beyond that we have Who should the emails be sent to? and
Message-ID to be used as In-Reply-To for the first email? which I
typically just hit enter to. It seems that they have no default
argument so that second if fails. I suppose we can add a default =
to these two asks?
8-
diff --git a/git-send-email.perl b/git-send-email.perl
index 607137b..13d813e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -760,6 +760,7 @@ if (!defined $sender) {
if (!@initial_to !defined $to_cmd) {
my $to = ask(Who should the emails be sent to? ,
+default = ,
valid_re = qr/\@.*\./, confirm_only = 1);
push @initial_to, parse_address_line($to) if defined $to; #
sanitized/validated later
$prompting++;
@@ -787,6 +788,7 @@ sub expand_one_alias {
if ($thread !defined $initial_reply_to $prompting) {
$initial_reply_to = ask(
Message-ID to be used as In-Reply-To for the first email? ,
+ default = ,
valid_re = qr/\@.*\./, confirm_only = 1);
}
if (defined $initial_reply_to) {
--
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] send-email: validate reconfirm interactive responses
On Thu, Sep 6, 2012 at 1:03 PM, Junio C Hamano gits...@pobox.com wrote: If you let $to to go empty with the first hunk of your patch, where does the mail eventually go? Does anybody later in the code decide to add some recipient? If there is a reason why an empty input is a valid here, I think there is a stronger need (that is, stronger than the above ase for $initial_reply_to) to explain when the user wants to leave this empty. I almost never type anything and just use the To header in the patch I want to send. -- 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] send-email: initial_to and initial_reply_to are both optional
On Thu, Sep 6, 2012 at 2:21 PM, Junio C Hamano gits...@pobox.com wrote: We may pick up additional recipients from the format-patch output files we are sending, in which case it is perfectly valid to leave the @initial_to empty when the prompt asks. We may want to start a new discussion thread without replying to anything, and it is valid to leave $initial_reply_to empty. An earlier update to avoid y...@example.com stuffed in address fields did not take these two cases into account. Noticed and fix suggested by Stephen Boyd. Signed-off-by: Junio C Hamano gits...@pobox.com --- * I am tempted to queue this, after asking you to eyeball it, and then update the author to pass the blame to you before merging it to 'next'. 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: [PATCH] send-email: validate reconfirm interactive responses
On Tue, Aug 14, 2012 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
@@ -745,13 +752,15 @@ sub file_declares_8bit_cte {
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
$sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender);
+ default = $sender,
+ valid_re = qr/\@.*\./, confirm_only = 1);
This is now bugging me if I just hit enter and don't want to specify
anything for
these headers (I want the defaults or what's in the files already).
Can we allow
the empty string to be valid as well so I don't have to go through
these prompts?
--
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
send-email and in-reply-to = n
Can we throw up a big warning or just outright fail if someone types 'n' or 'y' and hits enter for the in-reply-to question in git-send-email? I saw a git-send-email sent patch with an In-Reply-To header containing n on lkml today and it makes threading in my mail client get confused. https://lkml.org/lkml/headers/2012/8/13/503 -- 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
