Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only
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 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 expect \EOF * initial EOF +cat only_merges EOF - please no space after the - please add the at the end of the line: cat only_merges EOF (And the same further down) +test_expect_success 'log with config log.merges=show' ' +git config log.merges show Indent with TAB is good +git log --pretty=tformat:%s actual but indent with 4 spaces not ideal, please use a TAB as well. +test_cmp both_commits_merges actual +git config --unset log.merges Do we need the unset here? The log.merges is nicely set up before each test case, so can we drop the unset lines ? (Or do I miss something ?) Good point; we can drop only those unset lines whose next test sets the log.merges. However, if the next test does not set it, we must unset it as it affects the default behavior of git-log. -- Koosha -- 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 4/5] Add tests for git-log --merges=show|hide|only
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: 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 +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +Merge branch 'side' +EOF Current custom is to place this sort of setup code in its own test rather than having it at top-level. So, the above and below 'cat's should all go in a test named setup --merges= (or something better). Prefixing EOF with '-' allows you to indent the here-doc content and EOF so that it reads nicely inside a test. Also prefix EOF with '\' to indicate that you do not expect interpolation within the here-doc. Torsten already mentioned to drop the space following ''. Finally, within the setup test, chain them all together with . So: cat only_merges -\EOF ... EOF More below. +cat only_commits EOF +seventh +octopus-b +octopus-a +reach +tangle-a +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF + +cat both_commits_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +seventh +octopus-b +octopus-a +reach +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +tangle-a +Merge branch 'side' +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF t4202 already does this sort of fragile hard-coding of expected output, so this isn't a particularly strong objection, but it would be more robust if you could create these expected lists dynamically by issuing some git command other than the one you're testing. (That could also be considered fodder for a follow-on patch if you don't consider such a change worthwhile at this time.) + +test_expect_success 'log with config log.merges=show' ' + git config log.merges show +git log --pretty=tformat:%s actual Torsten already mentioned botched indentation. Use (8-character wide) tab for indentation everywhere. + test_cmp both_commits_merges actual +git config --unset log.merges If the test_cmp fails (because the expected and actual output differed), then the git-config --unset will never be invoked. To ensure that the config setting is undone when the test finishes (whether successful or not), use test_config(). The other option is to ensure that each test sets or clears log.merges as appropriate at the start of the test, and then not bother about resetting it. The shortcoming of this approach, however, is that any tests added following these new tests won't necessarily know that log.merges is set or that they may need to clear it. Consequently, test_config() at the start of each of these tests is probably the better approach. More below. +' + +test_expect_success 'log with config log.merges=only' ' + git config log.merges only +git log --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide' ' + git config log.merges hide +git log --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=show with git log --no-merges' ' + git config log.merges show +git log --no-merges --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide with git log ---merges' ' + git config log.merges hide +git log --merges --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=show' ' For these tests which expect log.merges to be unset, it would be more robust, and the intent clearer, if you actually ensured that it was unset. test_unconfig() at the start of the test would be the appropriate way to unset the config. It would also make the tests more self-contained, in case they are ever re-ordered. More below. +git log --merges=show --pretty=tformat:%s actual + test_cmp both_commits_merges actual +' + +test_expect_success 'log --merges=only' ' +git log --merges=only --pretty=tformat:%s actual + test_cmp only_merges actual +' + +test_expect_success 'log --merges=hide' ' +git log --merges=hide --pretty=tformat:%s actual + test_cmp only_commits actual
[PATCH 5/7] submodule: use strbuf_read_cmd
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 setup and cleanup). Signed-off-by: Jeff King p...@peff.net --- submodule.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/submodule.c b/submodule.c index d37d400..d85f1bb 100644 --- a/submodule.c +++ b/submodule.c @@ -576,12 +576,10 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20]) cp.env = local_repo_env; cp.git_cmd = 1; cp.no_stdin = 1; - cp.out = -1; cp.dir = path; - if (!run_command(cp) !strbuf_read(buf, cp.out, 1024)) + if (!strbuf_read_cmd(buf, cp, 1024) !buf.len) is_present = 1; - close(cp.out); strbuf_release(buf); } return is_present; -- 2.3.3.618.ga041503 -- 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 1
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: https://github.com/git/git.github.io/issues/17 You can also reply to this email. I tried to cc the persons who appear in the edition but maybe I missed some, sorry about that. Thomas and myself plan to publish this edition on Wednesday the 25th of March. We call it an edition instead of an issue to avoid confusion with GitHub issues. 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: Draft of Git Rev News edition 1
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 source of income, please consider contributing to his remuneration if you find this kind of improvements useful. Thank you very much for this heads-up. However, I'd replace previously with as of version 2.1.0. That's where the big difference is, so if people actually are impacted they'll know whether and what to benchmark and/or upgrade. Ok, there is now as of version 2.1.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
[PATCH] status: read submodule process output before calling wait()
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 if this is specific to the Ansible repo, or
whether the length of the summary is crossing some threshold that
triggers the bug to manifest. If I run the forked commands manually
from an interactive shell, they complete just fine.
It's the length of the summary. The fix is below.
-- 8 --
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, 1024);
Besides being a violation of the run-command API (which
makes no promises about the state of the struct after
run_command returns), this can easily lead to deadlock. The
submodule status process may fill up the pipe buffer and
block on write(). Meanwhile, the reading side in the parent
process is blocked in wait(), waiting for the child to
finish.
Instead, we should start the process, read everything it
produces, and only then call wait() to finish it off.
Signed-off-by: Jeff King p...@peff.net
---
I notice that we also don't detect when the sub-command fails. I don't
know what we would do in that case (abort the status? print a message?)
and it's orthogonal to this issue, so I left it for somebody more
clueful in the area to think about.
wt-status.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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);
sm_summary.out = -1;
- run_command(sm_summary);
-
+ start_command(sm_summary);
len = strbuf_read(cmd_stdout, sm_summary.out, 1024);
+ finish_command(sm_summary);
/* prepend header, only if there's an actual output */
if (len) {
--
2.3.3.618.ga041503
--
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/7] wt_status: fix signedness mismatch in strbuf_read call
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 output. Instead, we can just
check whether our buffer has anything in it (which is what
we care about anyway, and is the same thing since we know
the buffer was empty to begin with).
Note that the len variable actually has two roles in this
function. Now that we've eliminated the first, we can push the
declaration closer to the point of use for the second one.
Signed-off-by: Jeff King p...@peff.net
---
wt-status.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 1712762..b47f6d9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -729,7 +729,6 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
struct strbuf cmd_stdout = STRBUF_INIT;
struct strbuf summary = STRBUF_INIT;
char *summary_content;
- size_t len;
argv_array_pushf(sm_summary.env_array, GIT_INDEX_FILE=%s,
s-index_file);
@@ -749,10 +748,10 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
run_command(sm_summary);
- len = strbuf_read(cmd_stdout, sm_summary.out, 1024);
+ strbuf_read(cmd_stdout, sm_summary.out, 1024);
/* prepend header, only if there's an actual output */
- if (len) {
+ if (cmd_stdout.len) {
if (uncommitted)
strbuf_addstr(summary, _(Submodules changed but not
updated:));
else
@@ -763,6 +762,7 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
strbuf_release(cmd_stdout);
if (s-display_comment_prefix) {
+ size_t len;
summary_content = strbuf_detach(summary, len);
strbuf_add_commented_lines(summary, summary_content, len);
free(summary_content);
--
2.3.3.618.ga041503
--
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/7] wt-status: don't flush before running submodule status
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: ignore --for-status option, 2013-09-06), however, we pipe the sub-process output back to ourselves. So there's no longer any need to flush (it does not hurt, but it may leave readers wondering why we do it). Signed-off-by: Jeff King p...@peff.net --- wt-status.c | 1 - 1 file changed, 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index 7036fa2..1712762 100644 --- a/wt-status.c +++ b/wt-status.c @@ -745,7 +745,6 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt sm_summary.git_cmd = 1; sm_summary.no_stdin = 1; - fflush(s-fp); sm_summary.out = -1; run_command(sm_summary); -- 2.3.3.618.ga041503 -- 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/7] introduce strbuf_read_cmd to avoid deadlocks
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); sm_summary.out = -1; - run_command(sm_summary); - + start_command(sm_summary); len = strbuf_read(cmd_stdout, sm_summary.out, 1024); + finish_command(sm_summary); This isn't quite right. In both the original and the new one, if start_command fails, we may call strbuf_read on whatever it happens to have left in sm_summary.out. Worse, in the new one, we would call finish_command on something that was never started. And in both old and new, we leak the sm_summary.out descriptor. Furthermore, I found two other potential deadlocks (and one more descriptor leak). I tried at first to fix them all up like I did with the above patch, but I found that I was introducing similar problems in each case. So instead, I pulled this pattern out into its own strbuf helper. So here's a replacement series. [1/7]: wt-status: don't flush before running submodule status [2/7]: wt_status: fix signedness mismatch in strbuf_read call [3/7]: strbuf: introduce strbuf_read_cmd helper [4/7]: wt-status: use strbuf_read_cmd [5/7]: submodule: use strbuf_read_cmd [6/7]: trailer: use strbuf_read_cmd [7/7]: run-command: forbid using run_command with piped output -Peff -- 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: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git
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 has a
file D1/P in it and D2 has a directory D2/P under which there is a
file D2/P/X, and ask git diff --no-index D1 D2 to show their
differences, we simply get a refusal file/directory conflict.
The diff --no-index implementation has an underlying machinery
that can make it more in line with the normal Git if it wanted to,
but somehow it is not being exercised. The only thing we need to
do, when we see a file P and a directory P/ (or the other way
around) is to show the removal of a file P and then pretend as if we
are comparing nothing with a whole directory P/, as the code is
fully prepared to express a creation of everything in a directory
(and if the comparison is between a directory P/ and a file P, then
show the creation of the file and then let the existing code remove
everything in P/).
Signed-off-by: Junio C Hamano gits...@pobox.com
---
diff-no-index.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..52e9546 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o,
if (get_mode(name1, mode1) || get_mode(name2, mode2))
return -1;
- if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
- return error(file/directory conflict: %s, %s, name1, name2);
+ if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+ struct diff_filespec *d1, *d2;
+
+ if (S_ISDIR(mode1)) {
+ /* 2 is file that is created */
+ d1 = noindex_filespec(NULL, 0);
+ d2 = noindex_filespec(name2, mode2);
+ name2 = NULL;
+ mode2 = 0;
+ } else {
+ /* 1 is file that is deleted */
+ d1 = noindex_filespec(name1, mode2);
-^
I have not been following the discussion (or even really
studied this patch), but the asymmetry here caught my eye
as I was skimming the email.
[So, if this is actually correct, sorry for the noise!]
ATB,
Ramsay Jones
+ d2 = noindex_filespec(NULL, 0);
+ name1 = NULL;
+ mode1 = 0;
+ }
+ /* emit that file */
+ diff_queue(diff_queued_diff, d1, d2);
+
+ /* and then let the entire directory created or deleted */
+ }
if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
struct strbuf buffer1 = STRBUF_INIT;
--
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] status: read submodule process output before calling wait()
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, 1024);
By the way, I spotted this code as bogus immediately upon seeing it
(though certainly it helped to know there was a deadlock in the area,
which had me thinking about such things). So I wondered if it could have
been easy to catch in review, but its introduction was a little bit
subtle.
The original run_command invocation came in ac8d5af (builtin-status:
submodule summary support, 2008-04-12), which just let the sub-process
dump its stdout to the same descriptor that the rest of the status
output was going to. So the use of run_command there was fine. It was
later, in 3ba7407 (submodule summary: ignore --for-status option,
2013-09-06), that we started post-processing the output and it became
buggy. But that's harder to see in review.
Besides being a violation of the run-command API (which
makes no promises about the state of the struct after
run_command returns),
This may be overly harsh of me. Certainly we make no guarantees (and
things like the dynamic args and env_array are cleaned up
automatically after finish_command returns), but I would not be
surprised if there are other spots that treat struct child_process as
transparent rather than as a black box.
It's really the run_command + pipe construct that is really the danger
here. I wonder if we should do something like this:
diff --git a/run-command.c b/run-command.c
index 3afb124..78807de 100644
--- a/run-command.c
+++ b/run-command.c
@@ -557,7 +557,12 @@ int finish_command(struct child_process *cmd)
int run_command(struct child_process *cmd)
{
- int code = start_command(cmd);
+ int code;
+
+ if (cmd-out 0)
+ die(BUG: run_command with a pipe can cause deadlock);
+
+ code = start_command(cmd);
if (code)
return code;
return finish_command(cmd);
It seems to catch at least one other dubious construct.
-Peff
--
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] clone: Warn if LICENSE or COPYING file lacking and !clone.skiplicensecheck
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 may not be expecting that. Many projects accidentally omit a license, so this is common enough to note. You can disable this warning by setting clone.skiplicensecheck to true. For more info on the issue, feel free to see: http://choosealicense.com/no-license/ http://www.wired.com/2013/07/github-licenses/ https://twitter.com/stephenrwalli/status/247597785069789184 Signed-off-by: David A. Wheeler dwhee...@dwheeler.com The opt-out works only when placed in the system-wide or user configuration. That places a maintenance burden on *ALL* existing installations that do not want the warning. If you really want to hard-code such a policy decision into Git, then make it an opt-in so that members of the license squad can enable it. I don't need it. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] trailer: use strbuf_read_cmd
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-command.
Instead, let's use strbuf_read_cmd, which makes this simpler
(and we can get rid of our custom helper).
Signed-off-by: Jeff King p...@peff.net
---
trailer.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/trailer.c b/trailer.c
index 05b3859..b47bc0e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -214,16 +214,6 @@ static struct trailer_item *remove_first(struct
trailer_item **first)
return item;
}
-static int read_from_command(struct child_process *cp, struct strbuf *buf)
-{
- if (run_command(cp))
- return error(running trailer command '%s' failed,
cp-argv[0]);
- if (strbuf_read(buf, cp-out, 1024) 1)
- return error(reading from trailer command '%s' failed,
cp-argv[0]);
- strbuf_trim(buf);
- return 0;
-}
-
static const char *apply_command(const char *command, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
@@ -240,14 +230,16 @@ static const char *apply_command(const char *command,
const char *arg)
cp.argv = argv;
cp.env = local_repo_env;
cp.no_stdin = 1;
- cp.out = -1;
cp.use_shell = 1;
- if (read_from_command(cp, buf)) {
+ if (strbuf_read_cmd(buf, cp, 1024)) {
+ error(running trailer command '%s' failed, cmd.buf);
strbuf_release(buf);
result = xstrdup();
- } else
+ } else {
+ strbuf_trim(buf);
result = strbuf_detach(buf, NULL);
+ }
strbuf_release(cmd);
return result;
--
2.3.3.618.ga041503
--
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/7] strbuf: introduce strbuf_read_cmd helper
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 child with its stdout connected
to a pipe, of which the parent is the sole reader.
2. The parent calls wait(), blocking until the child exits.
3. The child writes to stdout. If it writes more data than
the OS pipe buffer can hold, the write() call will
block.
This is a deadlock; the parent is waiting for the child to
exit, and the child is waiting for the parent to call
read().
So we should do instead:
if (!start_command(cmd)) {
strbuf_read(buf, cmd.out, 0);
finish_command(cmd);
}
But note that this leaks cmd.out (which must be closed). And
there's no error handling for strbuf_read. We probably want
to know whether the operation succeeded, but we must make
sure to always run finish_command even if the read failed
(or else we leave a zombie child process).
Let's introduce a strbuf helper that can make this a bit
simpler for callers to do right.
Signed-off-by: Jeff King p...@peff.net
---
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.
strbuf.c | 17 +
strbuf.h | 10 ++
2 files changed, 27 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 88cafd4..9d1d48f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,6 +1,7 @@
#include cache.h
#include refs.h
#include utf8.h
+#include run-command.h
int starts_with(const char *str, const char *prefix)
{
@@ -414,6 +415,22 @@ int strbuf_readlink(struct strbuf *sb, const char *path,
size_t hint)
return -1;
}
+int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint)
+{
+ cmd-out = -1;
+ if (start_command(cmd) 0)
+ return -1;
+
+ if (strbuf_read(sb, cmd-out, hint) 0) {
+ close(cmd-out);
+ finish_command(cmd); /* throw away exit code */
+ return -1;
+ }
+
+ close(cmd-out);
+ return finish_command(cmd);
+}
+
int strbuf_getcwd(struct strbuf *sb)
{
size_t oldalloc = sb-alloc;
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
* APIs. Given that the length of the buffer is known, it's often better to
@@ -373,6 +375,14 @@ extern int strbuf_read_file(struct strbuf *sb, const char
*path, size_t hint);
extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
/**
+ * Execute the given command, capturing its stdout in the given strbuf.
+ * Returns -1 if starting the command fails or reading fails, and otherwise
+ * returns the exit code of the command. The output collected in the
+ * buffer is kept even if the command returns a non-zero exit.
+ */
+int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint);
+
+/**
* Read a line from a FILE *, overwriting the existing contents
* of the strbuf. The second argument specifies the line
* terminator character, typically `'\n'`.
--
2.3.3.618.ga041503
--
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/7] wt-status: use strbuf_read_cmd
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 descriptor we
just closed, but it is a bad idea for us to make assumptions
about how start_command implements its error handling). And
if start_command succeeds, we leak the file descriptor for
the pipe to the child.
All of these can be solved by using the strbuf_read_cmd
helper.
Signed-off-by: Jeff King p...@peff.net
---
wt-status.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index b47f6d9..fd85d61 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,11 +744,8 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
sm_summary.git_cmd = 1;
sm_summary.no_stdin = 1;
- sm_summary.out = -1;
- run_command(sm_summary);
-
- strbuf_read(cmd_stdout, sm_summary.out, 1024);
+ strbuf_read_cmd(cmd_stdout, sm_summary, 1024);
/* prepend header, only if there's an actual output */
if (cmd_stdout.len) {
--
2.3.3.618.ga041503
--
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 bash completion for git-branch --set-upstream-to on OSX
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 --set-upstream-to=origin/master
However, the completion for --set-upstream-to= completes the ref
correctly, but completely wipes the --set-upstream option; resulting
in:
$ git branch origin/master
Strange.
The code in question
case $cur in
--set-upstream-to=*)
__gitcomp $(__git_refs) ${cur##--set-upstream-to=}
;;
looks unsuspicious, this is the standard way to complete
'--option=TAB' (except that we should use __gitcomp_nl() for
completing refs, but that should have no effect on this issue). Does
the same issue affect other 'git cmd --option=TAB' completions as
well, e.g. 'git log --date=TAB' or 'git commit --reuse-message=TAB'?
We had a similar issue with ':' on the command line a looong while
ago, see v1.5.6.4~9^2 (bash completion: Resolve git show ref:pathtab
losing ref: portion, 2008-07-15). Maybe we are dealing with the same
issue here, just a different character?
What does
printf $COMP_WORDBREAKS |tr ' \t\n' STN
print out? Is the '=' missing, perhaps? If that's indeed the case,
does adding it back (COMP_WORDBREAKS=$COMP_WORDBREAKS=) fix the issue?
Gábor
--
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
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 a pull request, or by commenting on this GitHub issue: https://github.com/git/git.github.io/issues/17 You can also reply to this email. 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 source of income, please consider contributing to his remuneration if you find this kind of improvements useful. Thank you very much for this heads-up. However, I'd replace previously with as of version 2.1.0. That's where the big difference is, so if people actually are impacted they'll know whether and what to benchmark and/or upgrade. -- David Kastrup -- 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] completion: use __gitcomp_nl() for completing refs
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
---
contrib/completion/git-completion.bash | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 731c289..be85438 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -977,7 +977,7 @@ _git_branch ()
case $cur in
--set-upstream-to=*)
- __gitcomp $(__git_refs) ${cur##--set-upstream-to=}
+ __gitcomp_nl $(__git_refs) ${cur##--set-upstream-to=}
;;
--*)
__gitcomp
@@ -1045,7 +1045,7 @@ _git_checkout ()
_git_cherry ()
{
- __gitcomp $(__git_refs)
+ __gitcomp_nl $(__git_refs)
}
_git_cherry_pick ()
--
1.9.5.msysgit.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
[PATCH v2/GSoC/RFC] fetch: git fetch --deepen
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 excuting git fetch --depth=1 --deepen, (you) get one more tip and it becomes (you) o---A---B '--deepen' is designed to be a boolean option in this patch, which is a little different from [1]. It's designed in this way, because it can reuse '--depth' in the program, and just costs one more bit in some data structure, such as fetch_pack_args, git_transport_options. Of course, as a patch for discussion, it remains a long way to go before being complete. 1) Documents should be completed. 2) More test cases, expecially corner cases, should be added. 3) No need to get remote refs when it comes to '--deepen' option. 4) Validity on options combination should be checked. 5) smart-http protocol remains to be supported. [2] [1] http://article.gmane.org/gmane.comp.version-control.git/212950 [2] http://article.gmane.org/gmane.comp.version-control.git/265050 Helped-by: Duy Nguyen pclo...@gmail.com Helped-By: Eric Sunshine sunsh...@sunshineco.com Helped-By: Junio C Hamano gits...@pobox.com Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- -Background- At present the command git-fetch has one option depth which deepens or shortens the history of a shallow repository created by git clone with --depth=depth option (see git-clone[1]) to the specified number of commits from the tip of each remote branch history [2]. The drawback is that the starting point for deepening history is limited to the tip of each remote branch history. However, it's not able to satisfy the needs of users in some cases: 1) If the user wants to know the process of how a function was created and developed. What he expects is that he goes to the point of latest modification about the function and deepens the history further back from this current cut-off point by N commit directly, rather than having to guess what the depth is from tip of remote branch. 2) What's more, if the user only cares about the revisions of this function, the other commits after the latest modification are redundant for him, but he has to fetch them concomitantly, which wastes unnecessary time and space. For the convenience of users, a new option should be added as deepen to allow users to just get history commits deepened by N commits from current cut-off point, irrespective of location where the tips are [3]. -Goals- 1) Supports for command git fetch --deepen; 2) Conflict Detection for this option to guarantee robustness; 3) Sufficient tests; 4) Guarantee for compatibility; 5) Sufficient documents; More Details are shown in next section. -Details- --1. Current Workflow of git-fetch-- +-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+-+ | git-upload-pack | | git-unpack-objects | +-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+-+ | ^ | ^ v | v | fetch +-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+ +-+-+-+-+-+-+ --- | get remote refs | --- | fetch refs | --- | get_pack | -+ +-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+ +-+-+-+-+-+-+ | | ^| v |v +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+ | git-upload-pack | | update| + +-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+ + | local refs | | | generate wanted objs | --- | create_pack_file | | +-+-+-+-+-+-+-+ + |update shallows| +-+-+-+-+-+-+-+-+-+-+ + | +-+-+-+-+-+-+-+-+-+-+-+-+| ^ | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | | v | +-+-+-+-+-+-+-+-+-+-+ | git-pack-objects | +-+-+-+-+-+-+-+-+-+-+ Figure 1. Workflow of git-fetch (Please view it in fixed-width fonts) Figure 1 illustrates the workflow of git-fetch. When it comes to git-fetch, a series of operations are performed in cooperation with the server side via `transport` module. At first, it gets remote refs from git-upload-pack, and then it fetches refs, i.e. generating wanted objs, updating shallows and creating pack file. Finally, it unpacks objects from the pack file and updates the local refs. Actually, there are several protocols running the workflow above, which is showed in Table 1. It is determined in
Re: [PATCH] clone: Warn if clone lacks LICENSE or COPYING file
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 of service apply - and the cloner may not be expecting that. Many projects accidentally omit a license, so this is common enough to note. For more info on the issue, feel free to see: http://choosealicense.com/no-license/ http://www.wired.com/2013/07/github-licenses/ https://twitter.com/stephenrwalli/status/247597785069789184 As others have indicated here this feature is really specific to a single lint-like use-case and doesn't belong in clone as a built-in feature. However perhaps an interesting generalization of this would be something like a post-clone hook, obviously you couldn't store that in .git/hooks/ like other githooks(5) since there's no repo yet, but having it configured via the user/system config might be an interesting feature. If you're still interested in getting this functionality perhaps a patch to have some general post-clone hook mechanism would be accepted, then you could check license files or anything else you cared about. You could also just have a shell alias that wrapped git-clone... -- 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: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git
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 = noindex_filespec(name2, mode2);
+ name2 = NULL;
+ mode2 = 0;
+ } else {
+ /* 1 is file that is deleted */
+ d1 = noindex_filespec(name1, mode2);
I have not been following the discussion (or even really
studied this patch), but the asymmetry here caught my eye
as I was skimming the email.
Yes, the assymetry is a stupid cut-and-paste error. Thanks for
catching.
There was actually no discussion on this point other than
a tangential mention of the problem to be solved, so you
are up to date as long as you read the proposed log message
in the message you are responding to ;-)
Another thing I noticed while I was playing with this is that
when comparing D1 and D2, a path that was modified is
shown as a patch between a/D1/path and b/D2/path, but
a path that was created or removed shows D1 or D2 on
both sides of the comparison, which we may also want to
fix. This problem appears in the current codebase without
the patch under discussion.
--
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/RFC/GSOC] make git-pull a builtin
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 the converted builtin must be covered by tests. This should help to eliminate most sources of breakages. Anything further than that would require an experienced understanding of all the possible important inputs to be tested, which I personally feel would make the project quite tedious. I see git already has gcov support. For shell scripts, maybe kcov[1] could be used. With some slight code changes, I managed to generate a report for the git-pull tests[2] which should at least provide a good starting point for how the tests can be improved. While it is often a tempting idea to make test suites as thorough as possible, there lies a true danger herein. True war story: in one of the projects I was involved in, the test suite grew to a size that one complete run lasted two weeks. Yes, that is fourteen days. Needless to say: this test suite was run rarely. How useful is a test suite that is run rarely? More useful than a non-existent one, to be sure, but it is still more of a burden than a boon. Now, on Windows the test suite takes almost three hours to run. This really, really slows down development. So while we are not yet at the too large to be useful state, I would caution against trying to get there. Instead, I would really like to focus on the *usage*. Calling `git grep git pull t/` should give you an idea what usage of `git pull` is already tested. It should be pretty easy to come up with a list of *common* use cases, and if any of them are not covered, adding tests for them is simple and straight-forward, too. The code coverage tools can help here as well. The kcov output clearly shows which options of git-pull are currently not being tested. But yes, I agree that the test suite shouldn't be relied too much on compared to code inspection and review. On another important topic, though, along with git-pull.sh, I'm looking for another script to convert in parallel with git-pull.sh so that there will be no blocks due to patch review. Generally, I think rewriting scripts that are called frequently by users, or spawn a lot of processes due to loops, would be most desirable because the runtime gains would be much higher. A quick review of the scripts shows that git-am.sh, git-rebase--interactive.sh and git-quiltimport.sh have pretty heavy loops with lots of process spawning that grows with input. I'm currently leaning with git-am because not only is it a frequently used command, git-rebase--am.sh (for non-interactive rebase) calls it as well. In fact, quick tests show that it takes up 98% of git-rebase's execution time on Windows, so if git-am's performance improves it would be a huge win on many fronts. git-am's code also seems to be manageable for a 3-month project. Anyway, I would like to know if you (or anyone else) have any scripts in mind. (I also think that just 2 scripts would be enough to fill the 3 months, but that might be me just being too conservative) Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.
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 rest of the commit message can flesh out
the description if necessary. The subject on this patch is far too
long. Also, drop capitalization, mention the area you're touching, and
drop the final period. You might say instead:
Documentation: mention git-log --merges= and log.merges
In this case, it's probably not necessary to write anything else in
the commit message. The summary says enough, despite its conciseness.
More below.
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 1f7bc67..506125a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -190,6 +190,9 @@ log.showroot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
+log.merges::
+Default for `--merges` option. Defaults to `show`.
To disambiguate this from the --merges option, you should probably
spell it out explicitly as `--merges=` rather than just `--merges`.
mailmap.*::
See linkgit:git-shortlog[1].
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 4ed8587..398e657 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -99,6 +99,12 @@ if it is part of the log message.
--merges::
Print only merge commits. This is exactly the same as
`--min-parents=2`.
+--merges=show|hide|only::
+ If show is specified, merge commits will be shown in conjunction with
+ other commits. If hide is specified, it will work like `--no-merges`.
+ If only is specified, it will work like `--merges`. The default option
+ is show.
By The default option is show, do you mean when --merges= is not
specified? Perhaps it would be better to phrase it as:
Default is `show` if `--merges=` is not specified.
+
--no-merges::
Do not print commits with more than one parent. This is
exactly the same as `--max-parents=1`.
It might make more sense to rewrite the --merges and --no-merges
options in terms of the new --merges= option. For instance, something
like this:
--merges::
Shorthand for `--merges=only`.
--merges={show|hide|only}::
`show`: show merge and non-merge commits
`hide`: omit merge commits; same as `--max-parents=1`
`only`: show only merge commits; same as `--min-parents=2`
Default is `show` if `--merges=` is not specified.
--no-merges::
Shorthand for `--merges=hide`.
Alternately, --merges and --merges= could be combined like this
(taking inspiration from --decorate[=]):
--merges[=show|hide|only]::
But then you need additional explanation about what --merges means
without the '=' and following keyword.
--
2.3.3.263.g095251d.dirty
--
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
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 be more burden than enjoyment to you, it won't last). And I hope the answer is a resounding yes ;-) A few comments: - Some might be a bit too detailed. Because each header is a pointer to the list archive, picking only the points that you found are the most thought-provoking may be a good way to shorten it (and readers interested in the topic can follow the link). Another would be to drop the mention like Junio also reviewed... that does not say what was said in the review. If a review did not have much thought-provoking value to deserve a summary, perhaps it is enough only to leave it to be discovered by readers who are so interested to follow the link to find the full discussion. - You do not list your own contribution to the discussions, but you should. Of course it would take some discipline to prevent the newsletter from appearing to have a self- promoting agenda, but I think you two are adult enough to be capable of handling that ;-) - As a periodical, you would want to have This edition covers period between these two dates at the beginning of each and every edition. Publication date may serve as the upper bound of the range, but for an inaugural one, it is essential to have the date the coverage begins. - As an inaugural edition, we may want to have a word on the purpose of the publication. Perhaps a sentence or two to declare what the publication is about in the Welcome to section is good. I would imagine that the primary purpose is to cover the discussions on the list (but don't call that the list in this paragraph, but spell it out to help readers, as the Git mailing list) that is not visible in the git log output from my tree. - As an inaugural edition, we may want to have a word on how it came in existence by covering the discussion that led to its birth. Perhaps the discussion that led to the publication should be made into as an item on its own, next to make git-pull a builtin, Forbid log --graph... etc. Because it is neither a review nor a support discussion, Reviews Support heading may want to become Discussions. I think that is a better title for the section anyway, if its purpose is what happened on the list that are not visible from git log, as I expect future editions to cover design discussions that advanced the shared understanding of a problem but not quite solidified to become a patch series. Thomas and myself plan to publish this edition on Wednesday the 25th of March. We call it an edition instead of an issue to avoid confusion with GitHub issues. Good thinking. Thanks again. -- 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: What's cooking in git.git (Mar 2015, #07; Fri, 20)
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 futures) and didn't see serious reviews of the code that we will actually be using. If people have suggestions on how to improve it, I'm eager to hear them and submit a reroll or follow-up patches, as appropriate. Making changes now would be much better than having to do it down the line. Yeah, agreed on the last point, and that is why I kept it out of 'next' before people have enough time to think about it. 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 3/7] strbuf: introduce strbuf_read_cmd helper
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 large amount of output. [...] Let's introduce a strbuf helper that can make this a bit simpler for callers to do right. Signed-off-by: Jeff King p...@peff.net --- 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 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_output() capture_output() 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 * APIs. Given that the length of the buffer is known, it's often better to @@ -373,6 +375,14 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint); /** + * Execute the given command, capturing its stdout in the given strbuf. + * Returns -1 if starting the command fails or reading fails, and otherwise + * returns the exit code of the command. The output collected in the + * buffer is kept even if the command returns a non-zero exit. + */ +int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint); + +/** * Read a line from a FILE *, overwriting the existing contents * of the strbuf. The second argument specifies the line * terminator character, typically `'\n'`. -- 2.3.3.618.ga041503 -- 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/5] Add a new option 'merges' to revision.c
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 --merges={show|only|hide} option
revision.c: add a new option 'merges' with
No need to mention revision.c here since the patch itself shows this clearly.
Considering that there is already a --merges option, it is somewhat
misleading and not terribly clear to say only that you're adding a new
option 'merges'. Better would be to spell out --merges= explicitly.
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 list commit
items.
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
Considering that Junio actually wrote this patch[1], it would be more
correct and considerate to attribute it to him. You can do so by
ensuring that there is a From: header at the very top of the commit
message before mailing out the patch:
From: Junio C Hamano gits...@pobox.com
The customary way to indicate that you wrote the commit message and
made a few small adjustments to Junio's patch would be with a
bracketed comment (starting with your initials) just before your
sign-off. Something like this:
[kk: wrote commit message; added a couple missing
{min|max}_parents assignments]
[1]: http://article.gmane.org/gmane.comp.version-control.git/265597
---
diff --git a/revision.c b/revision.c
index 66520c6..edb7bed 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs,
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
}
+int parse_merges_opt(struct rev_info *revs, const char *param)
+{
+ if (!strcmp(param, show)) {
+ revs-min_parents = 0;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, only)) {
+ revs-min_parents = 2;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, hide)) {
+ revs-min_parents = 0;
+ revs-max_parents = 1;
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
int *unkc, const char **unkv)
{
@@ -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 (starts_with(arg, --merges=)) {
+ if (parse_merges_opt(revs, arg + 9))
+ die(unknown option: %s, arg);
} else if (!strcmp(arg, --merges)) {
+ revs-max_parents = -1;
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
+ revs-min_parents = 0;
revs-max_parents = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
diff --git a/revision.h b/revision.h
index 0ea8b4e..f9df58c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv,
struct rev_info *revs,
extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t
*ctx,
const struct option *options,
const char * const usagestr[]);
+extern int parse_merges_opt(struct rev_info *, const char *);
#define REVARG_CANNOT_BE_FILENAME 01
#define REVARG_COMMITTISH 02
extern int handle_revision_arg(const char *arg, struct rev_info *revs,
--
2.3.3.263.g095251d.dirty
--
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 5/5] Update Bash completion script to include git log --merges option
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
option, which it already knows about. Spell it out explicitly as
--merges=. Also, drop capitalization, mention area you're touching,
followed by colon, followed by short summary:
completion: teach about git-log --merges= and log.merges
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 731c289..b63bb95 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1406,7 +1406,7 @@ _git_ls_tree ()
__git_log_common_options=
--not --all
--branches --tags --remotes
- --first-parent --merges --no-merges
+ --first-parent --merges --merges= --no-merges
--max-count=
--max-age= --since= --after=
--min-age= --until= --before=
@@ -1451,6 +1451,10 @@ _git_log ()
__gitcomp long short ${cur##--decorate=}
return
;;
+--merges=*)
+__gitcomp show hide only ${cur##--merges=}
+return
+;;
--*)
__gitcomp
$__git_log_common_options
@@ -1861,6 +1865,10 @@ _git_config ()
__gitcomp $__git_log_date_formats
return
;;
+ log.merges)
+ __gitcomp show hide only
+ return
+ ;;
sendemail.aliasesfiletype)
__gitcomp mutt mailrc pine elm gnus
return
@@ -2150,6 +2158,7 @@ _git_config ()
interactive.singlekey
log.date
log.decorate
+ log.merges
log.showroot
mailmap.file
man.
--
2.3.3.263.g095251d.dirty
--
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 4/5] Add tests for git-log --merges=show|hide|only
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 +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF - please no space after the - please add the at the end of the line: cat only_merges EOF This code is outside any test, so has no impact (and would be slightly confusing). Better would be to move this setup code into a setup --merges= test, in which case the -chain would be appropriate. -- 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/5] Make git-log honor log.merges option
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 koo...@posteo.de
As discussed in my review of patch 1/5, since Junio actually wrote
this patch[1], it would be more correct and considerate to attribute
it to him by ensuring that a From: header is at the very top of the
commit message before mailing out the patch:
From: Junio C Hamano gits...@pobox.com
(If Junio had also signed-off on his patch, you would also include his
sign-off just above yours. In this case, he didn't sign off[1], so
your sign-off is all that is needed; and Junio will add his own if he
picks up this series.)
[1]: http://article.gmane.org/gmane.comp.version-control.git/265597
---
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;
static const char *fmt_patch_subject_prefix = PATCH;
static const char *fmt_pretty;
+static const char *log_merges;
static const char * const builtin_log_usage[] = {
N_(git log [options] [revision range] [[--] path...]),
@@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char
*value, void *cb)
decoration_style = 0; /* maybe warn? */
return 0;
}
+ if (!strcmp(var, log.merges)) {
+ return git_config_string(log_merges, var, value);
+ }
if (!strcmp(var, log.showroot)) {
default_show_root = git_config_bool(var, value);
return 0;
@@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char
*prefix)
init_revisions(rev, prefix);
rev.always_show_header = 1;
+ if (log_merges parse_merges_opt(rev, log_merges))
+ die(unknown config value for log.merges: %s, log_merges);
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
opt.revarg_opt = REVARG_COMMITTISH;
--
2.3.3.263.g095251d.dirty
--
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/GSoC/RFC] fetch: git fetch --deepen
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] Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d78f320..3b960c8 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -708,4 +708,16 @@ test_expect_success 'fetching a one-level ref works' ' ) ' +test_expect_success 'fetching deepen' ' + git clone . deepen --depth=1 ( Quoting Junio[1]: ...make sure tests serve as good examples, tests should stick to 'options first and then arguments'... + cd deepen + git fetch .. foo --depth=1 See [1]. + git show foo + test_must_fail git show foo~ + git fetch .. foo --depth=1 --deepen See [1]. + git show foo~ + test_must_fail git show foo~2 Mentioned previously[2]: Broken -chain throughout this test. + ) +' + test_done [1]: http://article.gmane.org/gmane.comp.version-control.git/265248 [2]: http://article.gmane.org/gmane.comp.version-control.git/265419 -- 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 4/5] Add tests for git-log --merges=show|hide|only
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 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF - please no space after the - please add the at the end of the line: cat only_merges EOF (And the same further down) +test_expect_success 'log with config log.merges=show' ' + git config log.merges show Indent with TAB is good +git log --pretty=tformat:%s actual but indent with 4 spaces not ideal, please use a TAB as well. + test_cmp both_commits_merges actual +git config --unset log.merges Do we need the unset here? The log.merges is nicely set up before each test case, so can we drop the unset lines ? (Or do I miss something ?) -- 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
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 working on free software is his sole source of income, please consider contributing to his remuneration if you find this kind of improvements useful. Thank you very much for this heads-up. Do you have a link to where people can go to donate/contribute? I searched around a bit but couldn't find anything. My Email address is linked at PayPal. However, it's the more affordable option in the Euro zone (which most definitely does not include GB) to ask me for my bank account data: SEPA-region transfers are by EU law required not to differentiate between in-country or cross-country payments. I don't maintain a personal home page or a blog or similar, so there is really not much to point people to than my Email address. -- David Kastrup -- 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
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 income, please consider contributing to his remuneration if you find this kind of improvements useful. Thank you very much for this heads-up. Do you have a link to where people can go to donate/contribute? I searched around a bit but couldn't find anything. -- 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
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 activity I mostly do regardless, either on Twitter or at Google+. Gathering the links in Git Rev News just means I collect them in a central place instead of sporadically posting on social media. So I think I can keep it up for an extended period, and if I ever get fed up, there are hopefully others who can keep that part going. Refining list activity into headlines, like Christian did, is a bigger challenge in my eyes. I think this depends on having someone active on the list, who also has time for producing this reader's digest. I guess the long term success depends, as with any volunteer effort, on how many others join in the fun, and how popular it gets outside the list. - As a periodical, you would want to have This edition covers period between these two dates at the beginning of each and every edition. Publication date may serve as the upper bound of the range, but for an inaugural one, it is essential to have the date the coverage begins. 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 Christian may change as he sees fit. - As an inaugural edition, we may want to have a word on the purpose of the publication. Perhaps a sentence or two to declare what the publication is about in the Welcome to section is good. I would imagine that the primary purpose is to cover the discussions on the list (but don't call that the list in this paragraph, but spell it out to help readers, as the Git mailing list) that is not visible in the git log output from my tree. Noted. I'l try working this in as well. - As an inaugural edition, we may want to have a word on how it came in existence by covering the discussion that led to its birth. Perhaps the discussion that led to the publication should be made into as an item on its own, next to make git-pull a builtin, Forbid log --graph... etc. Because it is neither a review nor a support discussion, Reviews Support heading may want to become Discussions. I think that is a better title for the section anyway, if its purpose is what happened on the list that are not visible from git log, as I expect future editions to cover design discussions that advanced the shared understanding of a problem but not quite solidified to become a patch series. I hope it's OK that I leave this bit to Christian. Thanks for the feedback! -- 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/5] Update Bash completion script to include git log --merges option
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
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1406,7 +1406,7 @@ _git_ls_tree ()
__git_log_common_options=
--not --all
--branches --tags --remotes
- --first-parent --merges --no-merges
+ --first-parent --merges --merges= --no-merges
--max-count=
--max-age= --since= --after=
--min-age= --until= --before=
@@ -1451,6 +1451,10 @@ _git_log ()
__gitcomp long short ${cur##--decorate=}
return
;;
+--merges=*)
+__gitcomp show hide only ${cur##--merges=}
+return
+;;
--*)
__gitcomp
$__git_log_common_options
@@ -1861,6 +1865,10 @@ _git_config ()
__gitcomp $__git_log_date_formats
return
;;
+ log.merges)
+ __gitcomp show hide only
+ return
+ ;;
sendemail.aliasesfiletype)
__gitcomp mutt mailrc pine elm gnus
return
@@ -2150,6 +2158,7 @@ _git_config ()
interactive.singlekey
log.date
log.decorate
+ log.merges
log.showroot
mailmap.file
man.
--
2.3.3.263.g095251d.dirty
--
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/5] Add a new option 'merges' to revision.c
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 list commit
items.
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
revision.c | 22 ++
revision.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/revision.c b/revision.c
index 66520c6..edb7bed 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs,
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
}
+int parse_merges_opt(struct rev_info *revs, const char *param)
+{
+ if (!strcmp(param, show)) {
+ revs-min_parents = 0;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, only)) {
+ revs-min_parents = 2;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, hide)) {
+ revs-min_parents = 0;
+ revs-max_parents = 1;
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
int *unkc, const char **unkv)
{
@@ -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 (starts_with(arg, --merges=)) {
+ if (parse_merges_opt(revs, arg + 9))
+ die(unknown option: %s, arg);
} else if (!strcmp(arg, --merges)) {
+ revs-max_parents = -1;
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
+ revs-min_parents = 0;
revs-max_parents = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
diff --git a/revision.h b/revision.h
index 0ea8b4e..f9df58c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv,
struct rev_info *revs,
extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t
*ctx,
const struct option *options,
const char * const usagestr[]);
+extern int parse_merges_opt(struct rev_info *, const char *);
#define REVARG_CANNOT_BE_FILENAME 01
#define REVARG_COMMITTISH 02
extern int handle_revision_arg(const char *arg, struct rev_info *revs,
--
2.3.3.263.g095251d.dirty
--
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/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.
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 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -190,6 +190,9 @@ log.showroot:: `git log -p` output would be shown without a diff attached. The default is `true`. +log.merges:: +Default for `--merges` option. Defaults to `show`. + mailmap.*:: See linkgit:git-shortlog[1]. diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 4ed8587..398e657 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -99,6 +99,12 @@ if it is part of the log message. --merges:: Print only merge commits. This is exactly the same as `--min-parents=2`. +--merges=show|hide|only:: + If show is specified, merge commits will be shown in conjunction with + other commits. If hide is specified, it will work like `--no-merges`. + If only is specified, it will work like `--merges`. The default option + is show. + --no-merges:: Do not print commits with more than one parent. This is exactly the same as `--max-parents=1`. -- 2.3.3.263.g095251d.dirty -- 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/5] Make git-log honor log.merges option
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 use_mailmap_config;
static const char *fmt_patch_subject_prefix = PATCH;
static const char *fmt_pretty;
+static const char *log_merges;
static const char * const builtin_log_usage[] = {
N_(git log [options] [revision range] [[--] path...]),
@@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char
*value, void *cb)
decoration_style = 0; /* maybe warn? */
return 0;
}
+ if (!strcmp(var, log.merges)) {
+ return git_config_string(log_merges, var, value);
+ }
if (!strcmp(var, log.showroot)) {
default_show_root = git_config_bool(var, value);
return 0;
@@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
init_revisions(rev, prefix);
rev.always_show_header = 1;
+ if (log_merges parse_merges_opt(rev, log_merges))
+ die(unknown config value for log.merges: %s, log_merges);
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
opt.revarg_opt = REVARG_COMMITTISH;
--
2.3.3.263.g095251d.dirty
--
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/5] Add tests for git-log --merges=show|hide|only
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 @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +Merge branch 'side' +EOF + +cat only_commits EOF +seventh +octopus-b +octopus-a +reach +tangle-a +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF + +cat both_commits_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +seventh +octopus-b +octopus-a +reach +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +tangle-a +Merge branch 'side' +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF + +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 +' + +test_expect_success 'log with config log.merges=only' ' + git config log.merges only +git log --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide' ' + git config log.merges hide +git log --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=show with git log --no-merges' ' + git config log.merges show +git log --no-merges --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide with git log ---merges' ' + git config log.merges hide +git log --merges --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=show' ' +git log --merges=show --pretty=tformat:%s actual + test_cmp both_commits_merges actual +' + +test_expect_success 'log --merges=only' ' +git log --merges=only --pretty=tformat:%s actual + test_cmp only_merges actual +' + +test_expect_success 'log --merges=hide' ' +git log --merges=hide --pretty=tformat:%s actual + test_cmp only_commits actual +' + +test_expect_success 'log --merges=show with config log.merges=hide' ' + git config log.merges hide +git log --merges=show --pretty=tformat:%s actual + test_cmp both_commits_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=show with config log.merges=only' ' + git config log.merges only +git log --merges=show --pretty=tformat:%s actual + test_cmp both_commits_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=only with config log.merges=show' ' + git config log.merges show +git log --merges=only --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=only with config log.merges=hide' ' + git config log.merges hide +git log --merges=only --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=hide with config log.merges=show' ' + git config log.merges show +git log --merges=hide --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log --merges=hide with config log.merges=only' ' + git config log.merges only +git log --merges=hide --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + test_expect_success 'log --graph with merge' ' git log --graph --date-order --pretty=tformat:%s | sed s/ *\$// actual -- 2.3.3.263.g095251d.dirty -- 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 03/15] read-cache: free cache entry in add_to_index in case of early return
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 it---otherwise we will leak it. Good. Additionally a free is moved towards the return. I am on the fence on this one between two schools and do not have a strong preference. One school is to free as soon as you know you do not need it, which is a valid stance to take. Another is, as you did, not to care about the minimum necessary lifetime of the storage and free them all at the end, which is also valid. Technically, the former could be more performant while the latter is easier on the eyes. I only recall to have seen the latter school so far, which is why I made the change in the first place assuming the school of freeing ASAP has no strong supporters inside the git community. I can resend the patch dropping the reordering, if you prefer. I already said that I do not have a preference ;-) Will queue 3/15 as-is, drop 2/15 and wait on 1/15. -- 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 02/15] read-cache: Improve readability
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
if (!pretend do_it_and_report_error())
error(...);
differently. I would explain that what the whole thing is trying to
achieve as 'do it' part is the primary thing we want to do, but it
only can be done when we are not pretending, and we show an error
when the 'do it' part fails. and would suggest structuring it more
like this:
if (pretend)
; /* nothing */
else if (do_it_and_report_error())
error(...);
or
if (!pretend) {
if (do_it_and_report_error())
error(...);
}
But you could say The final objective of the whole thing is to show
an error message, but if we are pretending or if our attempt to 'do
it' succeeds, we do not have to show the error, and such a view may
make sense depending on what that 'do it' is. The original may be
justified under such an interpretation.
We may be tempted to write (note: each boolean term may be a call
with many complex arguments)
if (A B C D E)
...
when it is in fact logically is more like this:
/* does not make sense to attempt C when A B does not hold */
if (A B) {
if (C D E)
...
}
But it becomes a judgement call if splitting that into nested two if
statements is better for overall readability when the top-level if
statement has an else clause. You cannot turn it into
/* does not make sense to attempt C when A B does not hold */
if (A B) {
if (C D E)
...
} else {
... /* this cannot be what the original's else clause did */
}
blindly. It would need further restructuring. I think the code
inside the refs.c is a result of making such judgement calls.
By the way, aren't we leaking ce when we are merely pretending?
Yes we are, that's how I found this spot. (coverity pointed out ce was
leaking, so I was refactoring to actually make it easier to fix it, and then
heavily reordered the patch series afterwards. That spot was forgotten
apparently.
I dropped 2/15 and expect the real fix in the future; no rush,
though.
--
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 09/15] http: release the memory of a http pack request as well
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 100644 --- a/http.c +++ b/http.c @@ -1462,6 +1462,7 @@ void release_http_pack_request(struct http_pack_request *preq) } preq-slot = NULL; free(preq-url); + free(preq); } int finish_http_pack_request(struct http_pack_request *preq) Freeing of preq in all the callers of this one looks sensible, except for the one in finish_request() of http-push.c that pulls an preq instance out of request-userData. Can somebody help me follow the dataflow to convince me that this is not leading to double-free in start_fetch_packed()? 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
rebase with commit.gpgsign = true
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 to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clone: Warn if LICENSE or COPYING file lacking and !clone.skiplicensecheck
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 of service apply - and the cloner may not be expecting that. Many projects accidentally omit a license, so this is common enough to note. You can disable this warning by setting clone.skiplicensecheck to true. Perhaps make this a hook and maybe install by default on new clones, e.g. templates/hooks--post-checkout.sample? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only
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 --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 expect \EOF * initial EOF +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 Do we need the unset here? The log.merges is nicely set up before each test case, so can we drop the unset lines ? (Or do I miss something ?) Good point; we can drop only those unset lines whose next test sets the log.merges. However, if the next test does not set it, we must unset it as it affects the default behavior of git-log. Such an approach would be too fragile. Tests may be re-ordered, added, or removed. Better is to make each test as self-contained as possible. See my review[1] of this patch for alternate suggestions. [1]: http://article.gmane.org/gmane.comp.version-control.git/266099 That's why I wrote them this way actually. Also, thanks for your review. I will refactor my code to consider your suggestions. Thanks. -- Koosha -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
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 in deadlock if the child process produces a large amount of output. [...] Let's introduce a strbuf helper that can make this a bit simpler for callers to do right. Signed-off-by: Jeff King p...@peff.net --- 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 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_output() capture_output() Sound like a good suggestion (but I haven't read the users of the proposed function, after doing which I might change my mind---I'll see). 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 3/7] strbuf: introduce strbuf_read_cmd helper
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() command_capture() run_command_with_output() capture_output() Sound like a good suggestion (but I haven't read the users of the proposed function, after doing which I might change my mind---I'll see). 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 agree with the phrase layering violation to call its current placement. 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: [RFC/PATCH] align D/F handling of diff --no-index with that of normal Git
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 commit.
However, if we compare two directories, D1 and D2, where D1 has a
file D1/P in it and D2 has a directory D2/P under which there is a
file D2/P/X, and ask git diff --no-index D1 D2 to show their
differences, we simply get a refusal file/directory conflict.
The diff --no-index implementation has an underlying machinery
that can make it more in line with the normal Git if it wanted to,
but somehow it is not being exercised. The only thing we need to
do, when we see a file P and a directory P/ (or the other way
around) is to show the removal of a file P and then pretend as if we
are comparing nothing with a whole directory P/, as the code is
fully prepared to express a creation of everything in a directory
(and if the comparison is between a directory P/ and a file P, then
show the creation of the file and then let the existing code remove
everything in P/).
Signed-off-by: Junio C Hamano gits...@pobox.com
---
diff-no-index.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..52e9546 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o,
if (get_mode(name1, mode1) || get_mode(name2, mode2))
return -1;
- if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
- return error(file/directory conflict: %s, %s, name1, name2);
+ if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+ struct diff_filespec *d1, *d2;
+
+ if (S_ISDIR(mode1)) {
+ /* 2 is file that is created */
+ d1 = noindex_filespec(NULL, 0);
+ d2 = noindex_filespec(name2, mode2);
+ name2 = NULL;
+ mode2 = 0;
+ } else {
+ /* 1 is file that is deleted */
+ d1 = noindex_filespec(name1, mode2);
+ d2 = noindex_filespec(NULL, 0);
+ name1 = NULL;
+ mode1 = 0;
+ }
+ /* emit that file */
+ diff_queue(diff_queued_diff, d1, d2);
+
+ /* and then let the entire directory created or deleted */
s/created/be created/
+ }
if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
struct strbuf buffer1 = STRBUF_INIT;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
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 * APIs. Given that the length of the buffer is known, it's often better to @@ -373,6 +375,14 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint); /** + * Execute the given command, capturing its stdout in the given strbuf. + * Returns -1 if starting the command fails or reading fails, and otherwise + * returns the exit code of the command. The output collected in the + * buffer is kept even if the command returns a non-zero exit. + */ +int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint); + +/** * 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 that may want to go to 'maint' and older, but our earlier jk/strbuf-doc-to-header topic introduces an unnecessary merge conflicts. I've wiggled this part and moved the doc elsewhere, only to remove that in the merge, which may not be optimal from the point of view of what I have to do when merging this topic down from pu to next to master to maint, but I do not see a good way around it. Thanks. The whole series looks very sensible. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
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 that may want to go to 'maint' and older, but our earlier jk/strbuf-doc-to-header topic introduces an unnecessary merge conflicts. Yeah, that is the worst part of refactoring and cleanup. Even when you make sure you are not hurting any topics in flight, you cannot know when a new topic will take off in your general area. I've wiggled this part and moved the doc elsewhere, only to remove that in the merge, which may not be optimal from the point of view of what I have to do when merging this topic down from pu to next to master to maint, but I do not see a good way around it. I'd suggest just dropping the documentation in the maint version (i.e., make it a moral cherry-pick of the function declaration only). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git-send-email.perl should check the version of Perl modules it uses
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 verify failed at /usr/libexec/git-core/git-send-email line 1294 Apparently, that is because the script is calling a subroutine (start_SSL()) that is missing in older versions. I guess the script should check the minimum required version before proceeding with the execution. -- 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 4/5] Add tests for git-log --merges=show|hide|only
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 5f2b290..ab6f371 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +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 Do we need the unset here? The log.merges is nicely set up before each test case, so can we drop the unset lines ? (Or do I miss something ?) Good point; we can drop only those unset lines whose next test sets the log.merges. However, if the next test does not set it, we must unset it as it affects the default behavior of git-log. Such an approach would be too fragile. Tests may be re-ordered, added, or removed. Better is to make each test as self-contained as possible. See my review[1] of this patch for alternate suggestions. [1]: http://article.gmane.org/gmane.comp.version-control.git/266099 -- 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/GSoC/RFC] fetch: git fetch --deepen
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] e.g. (upstream) ---o---o---o---A---B (you) A---B After excuting git fetch --depth=1 --deepen, (you) get one more tip and it becomes (you) o---A---B '--deepen' is designed to be a boolean option in this patch, which is a little different from [1]. It's designed in this way, because it can reuse '--depth' in the program, and just costs one more bit in some data structure, such as fetch_pack_args, git_transport_options. Of course, as a patch for discussion, it remains a long way to go before being complete. 1) Documents should be completed. 2) More test cases, expecially corner cases, should be added. 3) No need to get remote refs when it comes to '--deepen' option. 4) Validity on options combination should be checked. 5) smart-http protocol remains to be supported. [2] Quick notes before $DAYJOB starts. Cool pictures, perhaps they could be part of the commit message too. Personally i still don't think not moving the refs is worth the effort (and it's a waste if we have to send then drop objects for these updated refs, but I didn't check carefully). So if you we don't needs ref updates, we probably don't need to send want lines and sort of simplify processing at upload-pack side. And it makes me realise, we're loosing security a bit here. We normally don't send anything that's not reachable from the visible ref set. But we now would accept any shallow sha-1 and send some objects regardless if these sha-1 are connected to any refs. We may need some more checking in place to avoid this. See check_non_sha1_tip() for a way to do it. Pack bitmaps may help as well, but I think that's behind the scene (i.e. behind rev-list and we already can take advantage of it). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] Add a new option 'merges' to revision.c
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 (starts_with(arg, --merges=)) {
+ if (parse_merges_opt(revs, arg + 9))
+ die(unknown option: %s, arg);
This one makes sense to me (so does what the parse_merges_opt()
does).
} 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
that --merges does not affect the value of max_parents and she can
say log --max-parents=2 --merges to see only the two-way merges,
for example. This change just broke her, and I do not see why it is
a good thing.
} else if (!strcmp(arg, --no-merges)) {
+ revs-min_parents = 0;
revs-max_parents = 1;
Likewise.
--
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 5/5] Update Bash completion script to include git log --merges option
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 a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 731c289..b63bb95 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1406,7 +1406,7 @@ _git_ls_tree ()
__git_log_common_options=
--not --all
--branches --tags --remotes
- --first-parent --merges --no-merges
+ --first-parent --merges --merges= --no-merges
--max-count=
--max-age= --since= --after=
--min-age= --until= --before=
@@ -1451,6 +1451,10 @@ _git_log ()
__gitcomp long short ${cur##--decorate=}
return
;;
+--merges=*)
+__gitcomp show hide only ${cur##--merges=}
+return
+;;
Funny indentation, please use tabs instead of spaces.
Otherwise this patch looks good.
--*)
__gitcomp
$__git_log_common_options
@@ -1861,6 +1865,10 @@ _git_config ()
__gitcomp $__git_log_date_formats
return
;;
+ log.merges)
+ __gitcomp show hide only
+ return
+ ;;
sendemail.aliasesfiletype)
__gitcomp mutt mailrc pine elm gnus
return
@@ -2150,6 +2158,7 @@ _git_config ()
interactive.singlekey
log.date
log.decorate
+ log.merges
log.showroot
mailmap.file
man.
--
2.3.3.263.g095251d.dirty
--
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/5] Add a new option 'merges' to revision.c
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
that --merges does not affect the value of max_parents and she can
say log --max-parents=2 --merges to see only the two-way merges,
for example. This change just broke her, and I do not see why it is
a good thing.
The point is that if you have your log.merges conf option set to 'hide'
and you use git log --merges (two mutually conflicting options),
git will silently exit without anything to show.
That is not an excuse to break --merges and --no-merges for
existing users who do not care about setting log.merges option to
anything.
The whole point of introducing a new --merges=show option was so
that people can sanely countermand log.merges configuration from the
command line without breaking --merges and --no-merges, wasn't it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
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 f61b40c..5b0a666 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,115 @@ test_description='credential-store tests' +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' ' These test descriptions are quite long, often mirroring in prose what the test body already says more concisely. I generally try to keep descriptions short while still being descriptive enough to give a general idea about what is being tested. I've rewritten a few test descriptions (below) to be very short, in fact probably too terse; but perhaps better descriptions would lie somewhere between the two extremes. First example, for this test: !xdg: .git-credentials !xdg I will make the test descriptions shorter. However, I don't think the test descriptions need to be so terse such that a separate key is required. e.g. I will shorten the above to when xdg file does not exist, it's not created., or even terser: when xdg file not exists, it's not created.. I don't think symbols should be used, as many other test descriptions do not use them. Your updated test descriptions all sound fine. +XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store +unset XDG_CONFIG_HOME It's hard to spot the helper_test store at the end of line. I'd place it on a line by itself so that it is easy to see that it is wrapped by the setting and unsetting of the environment variable. Thanks, will fix. Although now it looks weird that the export is the only one with a continuation on a single line, so I split all of them so that they each have their own line. An -chain outside of a test is not meaningful in this case, so I meant either this: XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store unset XDG_CONFIG_HOME or, slightly more compact (using just to combine the assignment and export on one line): XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store unset XDG_CONFIG_HOME -- 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
Proofreading/Editing of Research Papers
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 requesting authors to send their research works for thorough grammatical editing. If interested, kindly submit papers or direct your colleague(s) to any of the following email addresses: submitpap...@global-proofreading.com submitartic...@global-proofreading.com call4pap...@globalproofreading.com call4manuscri...@globalproofreading.com gproofread...@gmail.com gproofedit...@gmail.com The research paper should be in the format Arial, font size 12 and 1.5 line spacing. Upon receipt of your work, an acknowledgement letter and the amount charged will be sent to you. Our rate is categorized below: $40 (USD) for research papers in the range of 1-10 pages $60 (USD) for research papers in the range of 11-15 pages $80 (USD) for research papers in the range of 16-20 pages $100 (USD) for research papers in the range of 21-25 pages $120(USD) for research papers in the range of 26-30 pages $140 (USD) for research papers in the range of 31-35 pages $ 3 (USD) is charged per page for lengthy research works e.g monographs dissertations and theses. Payment for proofread research paper is via any of the following ways below: Bank Transfer PayPal Western Union Money Transfer Ria Money Transfer MoneyGram Our effective editing board makes sure that each paper you send to us gets back to you within 96 hours (4 days) upon receipt of payment confirmation. Our services include: Proofreading Editing grammatical errors Proper punctuation Paraphrasing and editing of sentences, aligning of articles and any other errors Two copies of your work will be sent back to the author after proofreading: (1) Edited copy of research paper with track changes (2) Edited copy of research paper without track changes Finally, we wish you the best. Kind Regards, Miss Blessing Udeji Editorial Assistant (3) Global Proofreading gprcall4pap...@gmail.com editorialassista...@global-proofreading.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
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 layering violation. If moved to the run-command API, it could given one of the following names or something better: A layering violation implies there is an ordering to the APIs. Certainly we call APIs from other APIs all the time. I guess you could argue that these are the same layer, and should be next to each, and not building on each other (i.e., that strbuf has dependencies only on system APIs like stdio.h, and run-command only on system APIs like unistd.h, etc). But then reversing the order of the dependency does not really solve that. You would have to introduce a new higher-level API that combines them. But that seems silly for a single function (and I do not foresee any other similar functions). That being said, I'm not opposed to one of the reverse names if people feel strongly (I also considered making it an option flag to run_command_v_opt, but it ended up tangling things quite a bit more). -Peff -- 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/5] Add a new option 'merges' to revision.c
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)) {
revs-remove_empty_trees = 1;
+} else if (starts_with(arg, --merges=)) {
+if (parse_merges_opt(revs, arg + 9))
+die(unknown option: %s, arg);
This one makes sense to me (so does what the parse_merges_opt()
does).
In fact this change was written by you in your previous kind review :-)
I will add a 'From: Junio C Hamano gits...@pobox.com' header to my next patch.
} 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
that --merges does not affect the value of max_parents and she can
say log --max-parents=2 --merges to see only the two-way merges,
for example. This change just broke her, and I do not see why it is
a good thing.
The point is that if you have your log.merges conf option set to 'hide'
and you use git log --merges (two mutually conflicting options),
git will silently exit without anything to show. We need to clear the
max_parents set by parse_merges_opt() as the user should be able to continue
to use --merges without problem. But, your point is also valid.
} else if (!strcmp(arg, --no-merges)) {
+revs-min_parents = 0;
revs-max_parents = 1;
Likewise.
Similarly, without this, if log.merges is set to 'only' and you use git log
--no-merges,
you will still see the merges in the output.
--
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 0/7] introduce capture_command to avoid deadlocks
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 agree with the phrase layering violation to call its current placement. I was on the fence, and you both seem to prefer it in run-command, so here is a re-roll in that direction (no other changes). [1/7]: wt-status: don't flush before running submodule status [2/7]: wt_status: fix signedness mismatch in strbuf_read call [3/7]: run-command: introduce capture_command helper [4/7]: wt-status: use capture_command [5/7]: submodule: use capture_command [6/7]: trailer: use capture_command [7/7]: run-command: forbid using run_command with piped output -Peff -- 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 1/7] wt-status: don't flush before running submodule status
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: ignore --for-status option, 2013-09-06), however, we pipe the sub-process output back to ourselves. So there's no longer any need to flush (it does not hurt, but it may leave readers wondering why we do it). Signed-off-by: Jeff King p...@peff.net --- wt-status.c | 1 - 1 file changed, 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index 7036fa2..1712762 100644 --- a/wt-status.c +++ b/wt-status.c @@ -745,7 +745,6 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt sm_summary.git_cmd = 1; sm_summary.no_stdin = 1; - fflush(s-fp); sm_summary.out = -1; run_command(sm_summary); -- 2.3.3.618.ga041503 -- 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 3/7] run-command: introduce capture_command helper
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 child with its stdout connected
to a pipe, of which the parent is the sole reader.
2. The parent calls wait(), blocking until the child exits.
3. The child writes to stdout. If it writes more data than
the OS pipe buffer can hold, the write() call will
block.
This is a deadlock; the parent is waiting for the child to
exit, and the child is waiting for the parent to call
read().
So we might try instead:
start_command(cmd);
strbuf_read(buf, cmd.out, 0);
finish_command(cmd);
But that is not quite right either. We are examining cmd.out
and running finish_command whether start_command succeeded
or not, which is wrong. Moreover, these snippets do not do
any error handling. If our read() fails, we must make sure
to still call finish_command (to reap the child process).
And both snippets failed to close the cmd.out descriptor,
which they must do (provided start_command succeeded).
Let's introduce a run-command helper that can make this a
bit simpler for callers to get right.
Signed-off-by: Jeff King p...@peff.net
---
run-command.c | 16
run-command.h | 13 +
2 files changed, 29 insertions(+)
diff --git a/run-command.c b/run-command.c
index 3afb124..e591d2c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -829,3 +829,19 @@ int run_hook_le(const char *const *env, const char *name,
...)
return ret;
}
+
+int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
+{
+ cmd-out = -1;
+ if (start_command(cmd) 0)
+ return -1;
+
+ if (strbuf_read(buf, cmd-out, hint) 0) {
+ close(cmd-out);
+ finish_command(cmd); /* throw away exit code */
+ return -1;
+ }
+
+ close(cmd-out);
+ return finish_command(cmd);
+}
diff --git a/run-command.h b/run-command.h
index d6868dc..263b966 100644
--- a/run-command.h
+++ b/run-command.h
@@ -71,6 +71,19 @@ int run_command_v_opt(const char **argv, int opt);
*/
int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir,
const char *const *env);
+/**
+ * Execute the given command, capturing its stdout in the given strbuf.
+ * Returns -1 if starting the command fails or reading fails, and otherwise
+ * returns the exit code of the command. The output collected in the
+ * buffer is kept even if the command returns a non-zero exit. The hint field
+ * gives a starting size for the strbuf allocation.
+ *
+ * The fields of cmd should be set up as they would for a normal run_command
+ * invocation. But note that there is no need to set cmd-out; the function
+ * sets it up for the caller.
+ */
+int capture_command(struct child_process *cmd, struct strbuf *buf, size_t
hint);
+
/*
* The purpose of the following functions is to feed a pipe by running
* a function asynchronously and providing output that the caller reads.
--
2.3.3.618.ga041503
--
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 2/7] wt_status: fix signedness mismatch in strbuf_read call
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 output. Instead, we can just
check whether our buffer has anything in it (which is what
we care about anyway, and is the same thing since we know
the buffer was empty to begin with).
Note that the len variable actually has two roles in this
function. Now that we've eliminated the first, we can push the
declaration closer to the point of use for the second one.
Signed-off-by: Jeff King p...@peff.net
---
wt-status.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 1712762..b47f6d9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -729,7 +729,6 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
struct strbuf cmd_stdout = STRBUF_INIT;
struct strbuf summary = STRBUF_INIT;
char *summary_content;
- size_t len;
argv_array_pushf(sm_summary.env_array, GIT_INDEX_FILE=%s,
s-index_file);
@@ -749,10 +748,10 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
run_command(sm_summary);
- len = strbuf_read(cmd_stdout, sm_summary.out, 1024);
+ strbuf_read(cmd_stdout, sm_summary.out, 1024);
/* prepend header, only if there's an actual output */
- if (len) {
+ if (cmd_stdout.len) {
if (uncommitted)
strbuf_addstr(summary, _(Submodules changed but not
updated:));
else
@@ -763,6 +762,7 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
strbuf_release(cmd_stdout);
if (s-display_comment_prefix) {
+ size_t len;
summary_content = strbuf_detach(summary, len);
strbuf_add_commented_lines(summary, summary_content, len);
free(summary_content);
--
2.3.3.618.ga041503
--
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 4/7] wt-status: use capture_command
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 or a descriptor we
just closed, but it is a bad idea for us to make assumptions
about how start_command implements its error handling). And
if start_command succeeds, we leak the file descriptor for
the pipe to the child.
All of these can be solved by using the capture_command
helper.
Signed-off-by: Jeff King p...@peff.net
---
wt-status.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index b47f6d9..853419f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,11 +744,8 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
sm_summary.git_cmd = 1;
sm_summary.no_stdin = 1;
- sm_summary.out = -1;
- run_command(sm_summary);
-
- strbuf_read(cmd_stdout, sm_summary.out, 1024);
+ capture_command(sm_summary, cmd_stdout, 1024);
/* prepend header, only if there's an actual output */
if (cmd_stdout.len) {
--
2.3.3.618.ga041503
--
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
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 draft, and Christian may change as he sees fit. I agree weekly would be too much for any hobbist, given how high-volume our list has, but I probably shouldn't have said periodical. Surely, aiming for consistent update is a very good thing to gain reader trust if anything else, but it is OK if it were we will see a new release when enough interesting things happen, too. The primary reason I suggested to explicitly state the beginning of coverage is to set and manage the expectation of the readers. I think the current draft roughly covers 1/4 - 1/3 of discussions that happened in the month of March 2015 and nothing earlier than that, so This issue covers what happened in March or something would be appropriate. I'll throw a pull-request. - As an inaugural edition, we may want to have a word on how it came in existence by covering the discussion that led to its birth. Perhaps the discussion that led to the publication should be made into as an item on its own, next to make git-pull a builtin, Forbid log --graph... etc. Because it is neither a review nor a support discussion, Reviews Support heading may want to become Discussions. I think that is a better title for the section anyway, if its purpose is what happened on the list that are not visible from git log, as I expect future editions to cover design discussions that advanced the shared understanding of a problem but not quite solidified to become a patch series. I hope it's OK that I leave this bit to Christian. I took a stab at this myself, and threw another pull-request. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] run-command: forbid using run_command with piped output
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.
Worse, it only happens when the child fills the pipe
buffer, which means that the problem may come and go
depending on the platform and the size of the output
produced by the child.
Let's detect and flag this dangerous construct so that we
can catch potential bugs early in the test suite rather than
having them happen in the field.
Signed-off-by: Jeff King p...@peff.net
---
run-command.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index e591d2c..aad03ab 100644
--- a/run-command.c
+++ b/run-command.c
@@ -557,7 +557,12 @@ int finish_command(struct child_process *cmd)
int run_command(struct child_process *cmd)
{
- int code = start_command(cmd);
+ int code;
+
+ if (cmd-out 0 || cmd-err 0)
+ die(BUG: run_command with a pipe can cause deadlock);
+
+ code = start_command(cmd);
if (code)
return code;
return finish_command(cmd);
--
2.3.3.618.ga041503
--
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 5/7] submodule: use capture_command
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 setup and cleanup). Signed-off-by: Jeff King p...@peff.net --- submodule.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/submodule.c b/submodule.c index d37d400..c0e6c81 100644 --- a/submodule.c +++ b/submodule.c @@ -576,12 +576,10 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20]) cp.env = local_repo_env; cp.git_cmd = 1; cp.no_stdin = 1; - cp.out = -1; cp.dir = path; - if (!run_command(cp) !strbuf_read(buf, cp.out, 1024)) + if (!capture_command(cp, buf, 1024) !buf.len) is_present = 1; - close(cp.out); strbuf_release(buf); } return is_present; -- 2.3.3.618.ga041503 -- 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 6/7] trailer: use capture_command
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-command.
Instead, let's use capture_command, which makes this simpler
(and we can get rid of our custom helper).
Signed-off-by: Jeff King p...@peff.net
---
trailer.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/trailer.c b/trailer.c
index 05b3859..4b14a56 100644
--- a/trailer.c
+++ b/trailer.c
@@ -214,16 +214,6 @@ static struct trailer_item *remove_first(struct
trailer_item **first)
return item;
}
-static int read_from_command(struct child_process *cp, struct strbuf *buf)
-{
- if (run_command(cp))
- return error(running trailer command '%s' failed,
cp-argv[0]);
- if (strbuf_read(buf, cp-out, 1024) 1)
- return error(reading from trailer command '%s' failed,
cp-argv[0]);
- strbuf_trim(buf);
- return 0;
-}
-
static const char *apply_command(const char *command, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
@@ -240,14 +230,16 @@ static const char *apply_command(const char *command,
const char *arg)
cp.argv = argv;
cp.env = local_repo_env;
cp.no_stdin = 1;
- cp.out = -1;
cp.use_shell = 1;
- if (read_from_command(cp, buf)) {
+ if (capture_command(cp, buf, 1024)) {
+ error(running trailer command '%s' failed, cmd.buf);
strbuf_release(buf);
result = xstrdup();
- } else
+ } else {
+ strbuf_trim(buf);
result = strbuf_detach(buf, NULL);
+ }
strbuf_release(cmd);
return result;
--
2.3.3.618.ga041503
--
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 0/7] introduce capture_command to avoid deadlocks
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
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 aim for a monthly consistent release. I'll try working this into the draft, and Christian may change as he sees fit. I agree weekly would be too much for any hobbist, given how high-volume our list has, but I probably shouldn't have said periodical. Surely, aiming for consistent update is a very good thing to gain reader trust if anything else, but it is OK if it were we will see a new release when enough interesting things happen, too. Yeah, I prefer not to commit to a specific frequency... The primary reason I suggested to explicitly state the beginning of coverage is to set and manage the expectation of the readers. I think the current draft roughly covers 1/4 - 1/3 of discussions that happened in the month of March 2015 and nothing earlier than that, so This issue covers what happened in March or something would be appropriate. I'll throw a pull-request. ... but I agree that we should say what we cover. - As an inaugural edition, we may want to have a word on how it came in existence by covering the discussion that led to its birth. Perhaps the discussion that led to the publication should be made into as an item on its own, next to make git-pull a builtin, Forbid log --graph... etc. Because it is neither a review nor a support discussion, Reviews Support heading may want to become Discussions. I think that is a better title for the section anyway, if its purpose is what happened on the list that are not visible from git log, as I expect future editions to cover design discussions that advanced the shared understanding of a problem but not quite solidified to become a patch series. I hope it's OK that I leave this bit to Christian. I took a stab at this myself, and threw another pull-request. Thanks. Thank you for your pull requests. They are all merged and your name is in the Credits section at the end. 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: [PATCH 4/5] Add tests for git-log --merges=show|hide|only
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 review time and the execution time of the test and increase the clean-ness with 50% by simply writing +test_expect_success 'log with config log.merges=show' ' + git -c log.merges=show log --pretty=tformat:%s actual + test_cmp both_commits_merges actual + ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
