| Message ID | 20221011232604.839941-3-calvinwan@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | submodule: parallelize diff | expand |
On Tue, Oct 11 2022, Calvin Wan wrote: > A future patch requires the ability to parse the output of git > status --porcelain=2. Move parsing code from is_submodule_modified to > parse_status_porcelain. > > Signed-off-by: Calvin Wan <calvinwan@google.com> > --- > submodule.c | 71 +++++++++++++++++++++++++++++------------------------ > 1 file changed, 39 insertions(+), 32 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 72b295b87b..a3410ed8f0 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1864,6 +1864,43 @@ int fetch_submodules(struct repository *r, > return spf.result; > } > > +static int parse_status_porcelain(char *buf, unsigned *dirty_submodule, int ignore_untracked) > +{ > + /* regular untracked files */ > + if (buf[0] == '?') > + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; > + > + if (buf[0] == 'u' || > + buf[0] == '1' || > + buf[0] == '2') { > + /* T = line type, XY = status, SSSS = submodule state */ > + if (strlen(buf) < strlen("T XY SSSS")) > + BUG("invalid status --porcelain=2 line %s", > + buf); > + > + if (buf[5] == 'S' && buf[8] == 'U') > + /* nested untracked file */ > + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; > + > + if (buf[0] == 'u' || > + buf[0] == '2' || > + memcmp(buf + 5, "S..U", 4)) > + /* other change */ > + *dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; > + } > + > + if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && > + ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || > + ignore_untracked)) { > + /* > + * We're not interested in any further information from > + * the child any more, neither output nor its exit code. > + */ > + return 1; > + } > + return 0; > +} > + > unsigned is_submodule_modified(const char *path, int ignore_untracked) > { > struct child_process cp = CHILD_PROCESS_INIT; > @@ -1900,39 +1937,9 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) > > fp = xfdopen(cp.out, "r"); > while (strbuf_getwholeline(&buf, fp, '\n') != EOF) { > - /* regular untracked files */ > - if (buf.buf[0] == '?') > - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; > - > - if (buf.buf[0] == 'u' || > - buf.buf[0] == '1' || > - buf.buf[0] == '2') { > - /* T = line type, XY = status, SSSS = submodule state */ > - if (buf.len < strlen("T XY SSSS")) > - BUG("invalid status --porcelain=2 line %s", > - buf.buf); > - > - if (buf.buf[5] == 'S' && buf.buf[8] == 'U') > - /* nested untracked file */ > - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; > - > - if (buf.buf[0] == 'u' || > - buf.buf[0] == '2' || > - memcmp(buf.buf + 5, "S..U", 4)) > - /* other change */ > - dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; > - } > - > - if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && > - ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || > - ignore_untracked)) { > - /* > - * We're not interested in any further information from > - * the child any more, neither output nor its exit code. > - */ > - ignore_cp_exit_code = 1; > + ignore_cp_exit_code = parse_status_porcelain(buf.buf, &dirty_submodule, ignore_untracked); > + if (ignore_cp_exit_code) > break; > - } > } > fclose(fp); This isn't just a code move, but a rewrite of much of the code to accept a "char *buf" rather than a "struct strbuf buf". I can see in a later commit that you'd like to change this to accept a .string from a string_list. Without changing anything else, if you lead with a commit that does (in the initial loop): char *str = buf.buf; const size_t len = buf.len; And then make it use "buf" and "len" instead you could follow with the move commit, which at that ponit would benefit from the rename detection more more than it does now. Also: If we have a strbuf in this caller let's pass a "len", and not here make it need to do a strlen() on every line when we've computed it already, the other caller could just pass another strlen([...].string).
On Tue, Oct 11 2022, Calvin Wan wrote: Just commenting on one case, but this is in other parts of the series (e.g. your additions in submodule.c): > + BUG("invalid status --porcelain=2 line %s", > + buf); > [...] > - BUG("invalid status --porcelain=2 line %s", > - buf.buf); Your editor's indentation settings need fixing. Arguments should align with the opening "(", here you replaced 4 spaces with a "\t". A "\t" is == 8 spaces for the purposes of our identation, if you need 7 spaces to align with the "7" you insert 7 spaces, if it's 8 a "\t", then for one more a "\t" and one space etc.
diff --git a/submodule.c b/submodule.c index 72b295b87b..a3410ed8f0 100644 --- a/submodule.c +++ b/submodule.c @@ -1864,6 +1864,43 @@ int fetch_submodules(struct repository *r, return spf.result; } +static int parse_status_porcelain(char *buf, unsigned *dirty_submodule, int ignore_untracked) +{ + /* regular untracked files */ + if (buf[0] == '?') + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; + + if (buf[0] == 'u' || + buf[0] == '1' || + buf[0] == '2') { + /* T = line type, XY = status, SSSS = submodule state */ + if (strlen(buf) < strlen("T XY SSSS")) + BUG("invalid status --porcelain=2 line %s", + buf); + + if (buf[5] == 'S' && buf[8] == 'U') + /* nested untracked file */ + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; + + if (buf[0] == 'u' || + buf[0] == '2' || + memcmp(buf + 5, "S..U", 4)) + /* other change */ + *dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; + } + + if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && + ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || + ignore_untracked)) { + /* + * We're not interested in any further information from + * the child any more, neither output nor its exit code. + */ + return 1; + } + return 0; +} + unsigned is_submodule_modified(const char *path, int ignore_untracked) { struct child_process cp = CHILD_PROCESS_INIT; @@ -1900,39 +1937,9 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) fp = xfdopen(cp.out, "r"); while (strbuf_getwholeline(&buf, fp, '\n') != EOF) { - /* regular untracked files */ - if (buf.buf[0] == '?') - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - - if (buf.buf[0] == 'u' || - buf.buf[0] == '1' || - buf.buf[0] == '2') { - /* T = line type, XY = status, SSSS = submodule state */ - if (buf.len < strlen("T XY SSSS")) - BUG("invalid status --porcelain=2 line %s", - buf.buf); - - if (buf.buf[5] == 'S' && buf.buf[8] == 'U') - /* nested untracked file */ - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - - if (buf.buf[0] == 'u' || - buf.buf[0] == '2' || - memcmp(buf.buf + 5, "S..U", 4)) - /* other change */ - dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; - } - - if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && - ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || - ignore_untracked)) { - /* - * We're not interested in any further information from - * the child any more, neither output nor its exit code. - */ - ignore_cp_exit_code = 1; + ignore_cp_exit_code = parse_status_porcelain(buf.buf, &dirty_submodule, ignore_untracked); + if (ignore_cp_exit_code) break; - } } fclose(fp);
A future patch requires the ability to parse the output of git status --porcelain=2. Move parsing code from is_submodule_modified to parse_status_porcelain. Signed-off-by: Calvin Wan <calvinwan@google.com> --- submodule.c | 71 +++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 32 deletions(-)