Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Koosha Khajehmoogahi
On 03/22/2015 08:57 PM, Torsten Bögershausen wrote: On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- t/t4202-log.sh | 141 + 1 file changed, 141 insertions(+) diff --git

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Add tests for git-log --merges=show|hide|only Drop capitalization, mention area you're touching, followed by colon, followed by short summary: t4202-log: add --merges= tests More below. Signed-off-by:

[PATCH 5/7] submodule: use strbuf_read_cmd

2015-03-22 Thread Jeff King
In is_submodule_commit_present, we call run_command followed by a pipe read, which is prone to deadlock. It is unlikely to happen in this case, as rev-list should never produce more than a single line of output, but it does not hurt to avoid an anti-pattern (and using the helper simplifies the

Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
Hi, A draft of Git Rev News edition 1 is available here: https://github.com/git/git.github.io/blob/master/rev_news/draft/edition-1.md Everyone is welcome to contribute in any section either by editing the above page on GitHub and sending a pull request, or by commenting on this GitHub issue:

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
On Sun, Mar 22, 2015 at 12:21 PM, David Kastrup d...@gnu.org wrote: I've seen David Kastrup (dak at gnu.org) previously reimplemented significant parts of git blame for a vast gain in performance with complex histories and large files. As working on free software is his sole

[PATCH] status: read submodule process output before calling wait()

2015-03-22 Thread Jeff King
On Sat, Mar 21, 2015 at 10:56:54PM -0700, Wincent Colaiuta wrote: I've never seen this hang before despite frequent use of submodules. Oddly, I was able to work around the hang by moving the submodule in two hops (one from Ansible v1.6.10 to v1.7.0, then from v1.7.0 to v1.8.4). I am not sure

[PATCH 2/7] wt_status: fix signedness mismatch in strbuf_read call

2015-03-22 Thread Jeff King
We call strbuf_read(), and want to know whether we got any output. To do so, we assign the result to a size_t, and check whether it is non-zero. But strbuf_read returns a signed ssize_t. If it encounters an error, it will return -1, and we'll end up treating this the same as if we had gotten

[PATCH 1/7] wt-status: don't flush before running submodule status

2015-03-22 Thread Jeff King
This is a holdover from the original implementation in ac8d5af (builtin-status: submodule summary support, 2008-04-12), which just had the sub-process output to our descriptor; we had to make sure we had flushed any data that we produced before it started writing. Since 3ba7407 (submodule

[PATCH 0/7] introduce strbuf_read_cmd to avoid deadlocks

2015-03-22 Thread Jeff King
On Sun, Mar 22, 2015 at 03:44:55AM -0400, Jeff King wrote: diff --git a/wt-status.c b/wt-status.c index 7036fa2..96f0033 100644 --- a/wt-status.c +++ b/wt-status.c @@ -748,9 +748,9 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt fflush(s-fp);

Re: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git

2015-03-22 Thread Ramsay Jones
On 22/03/15 05:11, Junio C Hamano wrote: When a commit changes a path P that used to be a file to a directory and create a new path P/X in it, git show would say that file P was removed and file P/X was created for such a commit. However, if we compare two directories, D1 and D2, where D1

Re: [PATCH] status: read submodule process output before calling wait()

2015-03-22 Thread Jeff King
On Sun, Mar 22, 2015 at 03:44:55AM -0400, Jeff King wrote: The status code tries to read the output of git submodule summary over a pipe by waiting for the program to finish and then reading its output, like this: run_command(sm_summary); len = strbuf_read(cmd_stdout, sm_summary.out,

Re: [PATCH] clone: Warn if LICENSE or COPYING file lacking and !clone.skiplicensecheck

2015-03-22 Thread Johannes Sixt
Am 22.03.2015 um 01:16 schrieb David A. Wheeler: Warn cloners if there is no LICENSE* or COPYING* file that makes the license clear. This is a useful warning, because if there is no license somewhere, then local copyright laws (which forbid many uses) and terms of service apply - and the cloner

[PATCH 6/7] trailer: use strbuf_read_cmd

2015-03-22 Thread Jeff King
When we read from a trailer.*.command sub-program, the current code uses run_command followed by a pipe read, which can result in deadlock (though in practice you would have to have a large trailer for this to be a problem). The current code also leaks the file descriptor for the pipe to the

[PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Jeff King
Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: if (!run_command(cmd)) strbuf_read(buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. What happens is: 1. The parent spawns the

[PATCH 4/7] wt-status: use strbuf_read_cmd

2015-03-22 Thread Jeff King
When we spawn git submodule status to read its output, we use run_command() followed by a strbuf_read() from a pipe. This can deadlock if the subprocess output is larger than the system pipe buffer. Furthermore, if start_command() fails, we'll try to read from a bogus descriptor (probably -1 or a

Re: bug in bash completion for git-branch --set-upstream-to on OSX

2015-03-22 Thread SZEDER Gábor
Hi, Quoting Jason Karns karns...@gmail.com: There appears to be a bug in the bash completion for git-branch when attempting to complete the remote ref argument for --set-upstream-to= When: $ git branch --set-upstream-to=origin/mastTAB I would expect it to complete to: $ git branch

Re: Draft of Git Rev News edition 1

2015-03-22 Thread David Kastrup
Christian Couder christian.cou...@gmail.com writes: Hi, A draft of Git Rev News edition 1 is available here: https://github.com/git/git.github.io/blob/master/rev_news/draft/edition-1.md Everyone is welcome to contribute in any section either by editing the above page on GitHub and sending

[PATCH] completion: use __gitcomp_nl() for completing refs

2015-03-22 Thread SZEDER Gábor
We do that almost everywhere, because it's faster for large number of refs, see a31e62629 (completion: optimize refs completion, 2011-10-15). These were the last two places where we still used __gitcomp() for completing refs. Signed-off-by: SZEDER Gábor sze...@ira.uka.de ---

[PATCH v2/GSoC/RFC] fetch: git fetch --deepen

2015-03-22 Thread Dongcan Jiang
This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] e.g. (upstream) ---o---o---o---A---B (you) A---B After

Re: [PATCH] clone: Warn if clone lacks LICENSE or COPYING file

2015-03-22 Thread Ævar Arnfjörð Bjarmason
On Sat, Mar 21, 2015 at 7:06 PM, David A. Wheeler dwhee...@dwheeler.com wrote: Warn cloners if there is no LICENSE* or COPYING* file that makes the license clear. This is a useful warning, because if there is no license somewhere, then local copyright laws (which forbid many uses) and terms

Re: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git

2015-03-22 Thread Junio C Hamano
On Sun, Mar 22, 2015 at 5:37 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: On 22/03/15 05:11, Junio C Hamano wrote: + if (S_ISDIR(mode1)) { + /* 2 is file that is created */ + d1 = noindex_filespec(NULL, 0); + d2 =

Re: [PATCH/RFC/GSOC] make git-pull a builtin

2015-03-22 Thread Paul Tan
Hi, On Sun, Mar 22, 2015 at 1:35 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Maybe code coverage tools could help here so we only need to focus on the code paths that are untested by the test suite. At the minimum, all of the non-trivial code paths in both the shell script and

Re: [PATCH 3/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Update documentations for git-log to include the new --merges option and also its corresponding config option. The Subject: should be a short summary of the change; ideally less than 70 or 72 characters. The

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Junio C Hamano
Thanks. The most important question I would ask you is this: Did you two enjoy writing it? That ends up counting the most, as it affects the quality of the end result (readers would enjoy reading it and feel the love you put into its production), and also its longer term relevance (if it gets

Re: What's cooking in git.git (Mar 2015, #07; Fri, 20)

2015-03-22 Thread Junio C Hamano
brian m. carlson sand...@crustytoothpaste.net writes: On Fri, Mar 20, 2015 at 08:20:58PM -0700, Junio C Hamano wrote: I had an impression that the series may see at least one reroll to polish it further before it gets ready for 'next', as I only saw discussions on the tangent (e.g. possible

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 6:07 AM, Jeff King p...@peff.net wrote: Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: if (!run_command(cmd)) strbuf_read(buf, cmd.out, 0); can result in deadlock if the child process produces a

Re: [PATCH 1/5] Add a new option 'merges' to revision.c

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Add a new option 'merges' to revision.c For the subject, mention the area you're touching, followed by a colon, followed by a short but meaningful summary of the change. Drop capitalization. revision: add

Re: [PATCH 5/5] Update Bash completion script to include git log --merges option

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Update Bash completion script to include git log --merges option Nice to see a patch covering this oft-overlooked corner of the project. It's misleading to say that you're updating it to include the --merges

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 3:57 PM, Torsten Bögershausen tbo...@web.de wrote: On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..ab6f371 100755 --- a/t/t4202-log.sh +++

Re: [PATCH 2/5] Make git-log honor log.merges option

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Make git-log honor log.merges option Drop capitalization, mention area you're touching, followed by colon, followed by short summary: log: honor log.merges option Signed-off-by: Koosha Khajehmoogahi

Re: [PATCH v2/GSoC/RFC] fetch: git fetch --deepen

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 11:24 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1]

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Torsten Bögershausen
On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- t/t4202-log.sh | 141 + 1 file changed, 141 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..ab6f371

Re: Draft of Git Rev News edition 1

2015-03-22 Thread David Kastrup
Thomas Ferris Nicolaisen tfn...@gmail.com writes: On Sun, Mar 22, 2015 at 12:21 PM, David Kastrup d...@gnu.org wrote: David Kastrup (dak at gnu.org) previously reimplemented significant parts of git blame for a vast gain in performance with complex histories and large files. As

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Thomas Ferris Nicolaisen
On Sun, Mar 22, 2015 at 12:21 PM, David Kastrup d...@gnu.org wrote: David Kastrup (dak at gnu.org) previously reimplemented significant parts of git blame for a vast gain in performance with complex histories and large files. As working on free software is his sole source of

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Thomas Ferris Nicolaisen
On Sun, Mar 22, 2015 at 7:47 PM, Junio C Hamano gits...@pobox.com wrote: Thanks. The most important question I would ask you is this: Did you two enjoy writing it? To be clear, apart from some minor wording and nitpicking, I only contributed the links from outside the list. This is an

[PATCH 5/5] Update Bash completion script to include git log --merges option

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- contrib/completion/git-completion.bash | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 731c289..b63bb95 100644 ---

[PATCH 1/5] Add a new option 'merges' to revision.c

2015-03-22 Thread Koosha Khajehmoogahi
revision.c: add a new option 'merges' with possible values of 'only', 'show' and 'hide'. The option is used when showing the list of commits. The value 'only' lists only merges. The value 'show' is the default behavior which shows the commits as well as merges and the value 'hide' makes it just

[PATCH 3/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- Documentation/git-log.txt | 3 +++ Documentation/rev-list-options.txt | 6 ++ 2 files changed, 9 insertions(+) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 1f7bc67..506125a 100644 ---

[PATCH 2/5] Make git-log honor log.merges option

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- builtin/log.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dd8f3fc..c7a7aad 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -36,6 +36,7 @@ static int decoration_given; static int

[PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- t/t4202-log.sh | 141 + 1 file changed, 141 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..ab6f371 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@

Re: [PATCH 03/15] read-cache: free cache entry in add_to_index in case of early return

2015-03-22 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes: On Fri, Mar 20, 2015 at 8:31 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: This frees `ce` would be leaking in the error path. At this point ce is not yet added to the index, so it is clear it is safe to free

Re: [PATCH 02/15] read-cache: Improve readability

2015-03-22 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes: Maybe I have read too much of the refs code lately as there we have these long chains which combine precondition with error checking. Of course, things are not so black-and-white in the real world. You can also take an extreme stance and view

Re: [PATCH 09/15] http: release the memory of a http pack request as well

2015-03-22 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes: The cleanup function is used in 4 places now and it's always safe to free up the memory as well. Signed-off-by: Stefan Beller sbel...@google.com --- http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/http.c b/http.c index 9c825af..4b179f6

rebase with commit.gpgsign = true

2015-03-22 Thread Johannes Schneider
Hey guys, I love the commit.gpgsign feature. When rebasing some commits, every commit is signed - even those of other authors. Is there a way to configure git to only sign my commits? Thanks, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message

Re: [PATCH] clone: Warn if LICENSE or COPYING file lacking and !clone.skiplicensecheck

2015-03-22 Thread Duy Nguyen
On Sun, Mar 22, 2015 at 7:16 AM, David A. Wheeler dwhee...@dwheeler.com wrote: Warn cloners if there is no LICENSE* or COPYING* file that makes the license clear. This is a useful warning, because if there is no license somewhere, then local copyright laws (which forbid many uses) and terms

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Koosha Khajehmoogahi
On 03/22/2015 11:40 PM, Eric Sunshine wrote: On Sun, Mar 22, 2015 at 6:07 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: On 03/22/2015 08:57 PM, Torsten Bögershausen wrote: On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- diff

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes: On Sun, Mar 22, 2015 at 6:07 AM, Jeff King p...@peff.net wrote: Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: if (!run_command(cmd)) strbuf_read(buf, cmd.out, 0); can result

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Eric Sunshine sunsh...@sunshineco.com writes: It does feel like a layering violation. If moved to the run-command API, it could given one of the following names or something better: run_command_capture() capture_command()

Re: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 1:11 AM, Junio C Hamano gits...@pobox.com wrote: When a commit changes a path P that used to be a file to a directory and create a new path P/X in it, git show would say that file P s/create/creates/ More below... was removed and file P/X was created for such a

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: diff --git a/strbuf.h b/strbuf.h index 1883494..93a50da 100644 --- a/strbuf.h +++ b/strbuf.h @@ -1,6 +1,8 @@ #ifndef STRBUF_H #define STRBUF_H +struct child_process; + /** * strbuf's are meant to be used with all the usual C string and memory

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Jeff King
On Sun, Mar 22, 2015 at 04:22:50PM -0700, Junio C Hamano wrote: +/** * Read a line from a FILE *, overwriting the existing contents * of the strbuf. The second argument specifies the line * terminator character, typically `'\n'`. It is an unfortunate tangent that this is a bugfix

git-send-email.perl should check the version of Perl modules it uses

2015-03-22 Thread Koosha Khajehmoogahi
On Debian Wheezy with its outdated packages, the version of Net::SMTP::SSL is 1.01. If you try to use send-email, the script will crash with the following error: STARTTLS failed! SSL connect attempt failed with unknown error error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Eric Sunshine
On Sun, Mar 22, 2015 at 6:07 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: On 03/22/2015 08:57 PM, Torsten Bögershausen wrote: On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index

Re: [PATCH v2/GSoC/RFC] fetch: git fetch --deepen

2015-03-22 Thread Duy Nguyen
On Sun, Mar 22, 2015 at 10:24 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1]

Re: [PATCH 1/5] Add a new option 'merges' to revision.c

2015-03-22 Thread Junio C Hamano
Koosha Khajehmoogahi koo...@posteo.de writes: @@ -1800,9 +1817,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs-show_all = 1; } else if (!strcmp(arg, --remove-empty)) { revs-remove_empty_trees = 1; + } else if

Re: [PATCH 5/5] Update Bash completion script to include git log --merges option

2015-03-22 Thread SZEDER Gábor
Hi, I agree with Eric's comment about the subject line. Quoting Koosha Khajehmoogahi koo...@posteo.de: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- contrib/completion/git-completion.bash | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git

Re: [PATCH 1/5] Add a new option 'merges' to revision.c

2015-03-22 Thread Junio C Hamano
Koosha Khajehmoogahi koo...@posteo.de writes: } else if (!strcmp(arg, --merges)) { + revs-max_parents = -1; revs-min_parents = 2; But is this change warranted? An existing user who is not at all interested in the new --merges= option may be relying on the fact

Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME

2015-03-22 Thread Eric Sunshine
On Sat, Mar 21, 2015 at 1:46 AM, Paul Tan pyoka...@gmail.com wrote: On Thu, Mar 19, 2015 at 3:26 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan pyoka...@gmail.com wrote: diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index

Proofreading/Editing of Research Papers

2015-03-22 Thread submitarticles
Dear Colleague, I wish to inform you that Global Proofreading is still accepting research papers, theses, commentaries, dissertations and write-ups for proofreading. Our major aim is to help researchers publish quality research works devoid of grammatical errors. We are therefore

Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper

2015-03-22 Thread Jeff King
On Sun, Mar 22, 2015 at 03:36:01PM -0400, Eric Sunshine wrote: This is really at the intersection of the strbuf and run-command APIs, so you could argue for it being part of either It is logically quite like the strbuf_read_file() function, so I put it there. It does feel like a

Re: [PATCH 1/5] Add a new option 'merges' to revision.c

2015-03-22 Thread Koosha Khajehmoogahi
On 03/23/2015 12:31 AM, Junio C Hamano wrote: Koosha Khajehmoogahi koo...@posteo.de writes: @@ -1800,9 +1817,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs-show_all = 1; } else if (!strcmp(arg, --remove-empty)) {

[PATCH v2 0/7] introduce capture_command to avoid deadlocks

2015-03-22 Thread Jeff King
On Sun, Mar 22, 2015 at 04:40:37PM -0700, Junio C Hamano wrote: Now I read the callers, it does look like this new function better fits in the run-command suite, essentially allowing us to do what we would do with $(cmd) or `cmd` in shell and Perl scripts, even though I do not particularly

[PATCH v2 1/7] wt-status: don't flush before running submodule status

2015-03-22 Thread Jeff King
This is a holdover from the original implementation in ac8d5af (builtin-status: submodule summary support, 2008-04-12), which just had the sub-process output to our descriptor; we had to make sure we had flushed any data that we produced before it started writing. Since 3ba7407 (submodule

[PATCH v2 3/7] run-command: introduce capture_command helper

2015-03-22 Thread Jeff King
Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: cmd.out = -1; run_command(cmd); strbuf_read(buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. What happens is: 1. The parent spawns the

[PATCH v2 2/7] wt_status: fix signedness mismatch in strbuf_read call

2015-03-22 Thread Jeff King
We call strbuf_read(), and want to know whether we got any output. To do so, we assign the result to a size_t, and check whether it is non-zero. But strbuf_read returns a signed ssize_t. If it encounters an error, it will return -1, and we'll end up treating this the same as if we had gotten

[PATCH v2 4/7] wt-status: use capture_command

2015-03-22 Thread Jeff King
When we spawn git submodule status to read its output, we use run_command() followed by strbuf_read() read from the pipe. This can deadlock if the subprocess output is larger than the system pipe buffer. Furthermore, if start_command() fails, we'll try to read from a bogus descriptor (probably -1

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Junio C Hamano
Thomas Ferris Nicolaisen tfn...@gmail.com writes: Good point. There hasn't been a decision on frequency. Weekly is a good rhythm for publications seeking readership, but that's a lot of work. My vote is we should first aim for a monthly consistent release. I'll try working this into the

[PATCH v2 7/7] run-command: forbid using run_command with piped output

2015-03-22 Thread Jeff King
Because run_command both spawns and wait()s for the command before returning control to the caller, any reads from the pipes we open must necessarily happen after wait() returns. This can lead to deadlock, as the child process may block on writing to us while we are blocked waiting for it to exit.

[PATCH v2 5/7] submodule: use capture_command

2015-03-22 Thread Jeff King
In is_submodule_commit_present, we call run_command followed by a pipe read, which is prone to deadlock. It is unlikely to happen in this case, as rev-list should never produce more than a single line of output, but it does not hurt to avoid an anti-pattern (and using the helper simplifies the

[PATCH v2 6/7] trailer: use capture_command

2015-03-22 Thread Jeff King
When we read from a trailer.*.command sub-program, the current code uses run_command followed by a pipe read, which can result in deadlock (though in practice you would have to have a large trailer for this to be a problem). The current code also leaks the file descriptor for the pipe to the

Re: [PATCH v2 0/7] introduce capture_command to avoid deadlocks

2015-03-22 Thread Junio C Hamano
Thanks; will queue. -- 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: Draft of Git Rev News edition 1

2015-03-22 Thread Christian Couder
On Mon, Mar 23, 2015 at 5:49 AM, Junio C Hamano gits...@pobox.com wrote: Thomas Ferris Nicolaisen tfn...@gmail.com writes: Good point. There hasn't been a decision on frequency. Weekly is a good rhythm for publications seeking readership, but that's a lot of work. My vote is we should first

Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only

2015-03-22 Thread Torsten Bögershausen
Back to the original discussion: +test_expect_success 'log with config log.merges=show' ' +git config log.merges show +git log --pretty=tformat:%s actual +test_cmp both_commits_merges actual +git config --unset log.merges These days I would probably shorten the code, the