@matheusbsbs / matheus-bagliano / commits / af6865a7f1

submodule.c: convert is_submodule_modified to use strbuf_getwholeline

Instead of implementing line reading yet again, make use of our beautiful library function to read one line. By using strbuf_getwholeline instead of strbuf_read, we avoid having to allocate memory for the entire child process output at once. That is, we limit maximum memory usage. Also we can start processing the output as it comes in, no need to wait for all of it. Once we know all information that we care about, we can terminate the child early. In that case we do not care about its exit code as well. By just closing our side of the pipe the child process will get a SIGPIPE signal, which it will not report nor do we report it in finish_command, ac78663b0d (run-command: don't warn on SIGPIPE deaths, 2015-12-29). Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Mar 24, 2017 at 17:36 UTC af6865a7f1e1d915d3b63e998226028ca4abb6ee

2 files changed +31 -17

submodule.c

+14 -16

@@ -1041,12 +1041,12 @@ out:

1041

1042 unsigned is_submodule_modified(const char *path, int ignore_untracked)

1043 {

1044 - ssize_t len;

1044 struct child_process cp = CHILD_PROCESS_INIT;

1045 struct strbuf buf = STRBUF_INIT;

1046 + FILE *fp;

1047 unsigned dirty_submodule = 0;

1048 - const char *line, *next_line;

1048 const char *git_dir;

1049 + int ignore_cp_exit_code = 0;

1050

1051 strbuf_addf(&buf, "%s/.git", path);

1052 git_dir = read_gitfile(buf.buf);

@@ -1072,29 +1072,27 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)

1072 if (start_command(&cp))

1073 die("Could not run 'git status --porcelain' in submodule %s", path);

1074

1075 - len = strbuf_read(&buf, cp.out, 1024);

1076 - line = buf.buf;

1077 - while (len > 2) {

1078 - if ((line[0] == '?') && (line[1] == '?'))

1075 + fp = xfdopen(cp.out, "r");

1076 + while (strbuf_getwholeline(&buf, fp, '\n') != EOF) {

1077 + if ((buf.buf[0] == '?') && (buf.buf[1] == '?'))

1078 dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;

1079 else

1080 dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;

1081

1082 if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) &&

1083 ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ||

1085 - ignore_untracked))

1086 - break;

1087 -

1088 - next_line = strchr(line, '\n');

1089 - if (!next_line)

1084 + ignore_untracked)) {

1085 + /*

1086 + * We're not interested in any further information from

1087 + * the child any more, neither output nor its exit code.

1088 + */

1089 + ignore_cp_exit_code = 1;

1090 break;

1091 - next_line++;

1092 - len -= (next_line - line);

1093 - line = next_line;

1091 + }

1092 }

1095 - close(cp.out);

1093 + fclose(fp);

1094

1097 - if (finish_command(&cp))

1095 + if (finish_command(&cp) && !ignore_cp_exit_code)

1096 die("'git status --porcelain' failed in submodule %s", path);

1097

1098 strbuf_release(&buf);

t/t7506-status-submodule.sh

+17 -1

@@ -177,8 +177,24 @@ test_expect_success 'status with added file in modified submodule with .git file

177 test_i18ngrep "modified: sub (new commits, modified content)" output

178 '

179

180 +test_expect_success 'status with a lot of untracked files in the submodule' '

181 + (

182 + cd sub

183 + i=0 &&

184 + while test $i -lt 1024

185 + do

186 + >some-file-$i

187 + i=$(( $i + 1 ))

188 + done

189 + ) &&

190 + git status --porcelain sub 2>err.actual &&

191 + test_must_be_empty err.actual &&

192 + rm err.actual

193 +'

194 +

195 test_expect_success 'rm submodule contents' '

181 - rm -rf sub/* sub/.git

196 + rm -rf sub &&

197 + mkdir sub

198 '

199

200 test_expect_success 'status clean (empty submodule dir)' '