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

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 wrote: > Thomas Ferris Nicolaisen 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 consisten

Re: Draft of Git Rev News edition 1

2015-03-22 Thread Junio C Hamano
Thomas Ferris Nicolaisen 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 draft, and Christi

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

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

[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 sub-co

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

[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 summary:

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

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

[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

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 wrote: > On Thu, Mar 19, 2015 at 3:26 AM, Eric Sunshine > wrote: >> On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan wrote: >>> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh >>> index f61b40c..5b0a666 100755 >>> --- a/t/t0302-credentia

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

2015-03-22 Thread Junio C Hamano
Koosha Khajehmoogahi 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 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 : Signed-off-by: Koosha Khajehmoogahi --- contrib/completion/git-completion.bash | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/con

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 ver

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 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")) { >>

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

2015-03-22 Thread Junio C Hamano
Junio C Hamano writes: > Eric Sunshine 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() >> command_capture() >> run_command_with

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

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

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

2015-03-22 Thread Junio C Hamano
Koosha Khajehmoogahi 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 (start

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 requesti

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

2015-03-22 Thread Junio C Hamano
Jeff King 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 v2/GSoC/RFC] fetch: git fetch --deepen

2015-03-22 Thread Duy Nguyen
On Sun, Mar 22, 2015 at 10:24 PM, Dongcan Jiang 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] > > e.g. > >> (upstr

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

2015-03-22 Thread Junio C Hamano
Eric Sunshine writes: > On Sun, Mar 22, 2015 at 6:07 AM, Jeff King 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 c

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 > 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 --- diff --git a/t/t4202-lo

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 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 >>> --- >>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh >>> index 5f2b290..ab6f371 100755 >

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 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: Koosha Khajehmo

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 >> --- >> t/t4202-log.sh | 141 >> + >> 1 file changed, 141 insertions(+) >> >> diff --git a/t/t42

Re: Draft of Git Rev News edition 1

2015-03-22 Thread David Kastrup
Thomas Ferris Nicolaisen writes: > On Sun, Mar 22, 2015 at 12:21 PM, David Kastrup 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 softwa

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 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 option, which it

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 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 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 rest of the com

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 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 income, p

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 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 activity I mostly d

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 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 As discussed in

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 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 --merges={show|

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 wrote: > On 22.03.15 19:28, Koosha Khajehmoogahi wrote: >> Signed-off-by: Koosha Khajehmoogahi >> --- >> 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 >> @@ -428,6

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

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

2015-03-22 Thread Junio C Hamano
Stefan Beller 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 > --- > http.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/http.c b/http.c > index 9c825af..4b179f6 100644 > --- a/http.c > ++

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

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

2015-03-22 Thread Junio C Hamano
Stefan Beller 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 if (!pretend

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 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] > Signed-off-by: Dong

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 writes: > On Fri, Mar 20, 2015 at 8:31 PM, Junio C Hamano wrote: >> Stefan Beller 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 it---otherwise we will leak it. Good. >>

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

2015-03-22 Thread Junio C Hamano
"brian m. carlson" 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 futures) and didn't s

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 to

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

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi --- 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 use_mailmap_config; stati

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

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi --- 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 @@ -428,6 +428,147 @@ cat

[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 --- 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 --- a/Documentation/git-log.txt +++

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

2015-03-22 Thread Koosha Khajehmoogahi
Signed-off-by: Koosha Khajehmoogahi --- 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 --- a/contrib/completion/gi

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

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 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 of service apply -

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 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 = noindex_filespec(na

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 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 the >> converted builti

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

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

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

[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 --- contrib/completion/git-completion.bash

Re: Draft of Git Rev News edition 1

2015-03-22 Thread David Kastrup
Christian Couder 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 a pull request, or

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

2015-03-22 Thread SZEDER Gábor
Hi, Quoting Jason Karns : 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/mast I would expect it to complete to: $ git branch --set-upstream-to=origin/mast

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

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

[PATCH 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 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 sub-co

[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" o

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

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

[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 summary:

[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: [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 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 of service apply -

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

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_summa

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

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 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 commit. > > However