Skip to content

Commit

Permalink
Use is_submodule_active() instead of spawning a process
Browse files Browse the repository at this point in the history
Use is_submodule_active() instead of spawning a new process for every
submodule in prepare_submodule_summary(). Quote from Dscho:

> +		/* Also show added or modified modules which are checked out */
> +		cp_rev_parse.dir = p->sm_path;
> +		cp_rev_parse.git_cmd = 1;
> +		cp_rev_parse.no_stderr = 1;
> +		cp_rev_parse.no_stdout = 1;
> +
> +		argv_array_pushl(&cp_rev_parse.args, "rev-parse",
> +				 "--git-dir", NULL);
> +
> +		if (!run_command(&cp_rev_parse))

I wonder whether we really need to waste an entire spawned process on
figuring out whether `p->sm_path` refers to an active repository. Wouldn't
`is_submodule_active(r, p->sm_path)` fulfill the same purpose?
-----------------------------------------------------------

The shell version just tests for a gitdir.
--------------8<-----------
GIT_DIR="$sm_path/.git" git rev-parse --git-dir >/dev/null 2>&1 &&
printf '%s\n' "$sm_path"
-------------->8-----------
Then we should also do just that instead of spawning a CP ever.

The 'rev-parse' Documentation states
=========================
--git-dir
Show $GIT_DIR if defined. Otherwise show the path to the .git directory.
The path shown, when relative, is relative to the current working directory.

If $GIT_DIR is not defined and the current directory is not detected
to lie in a Git repository or work tree print a message to stderr and
exit with nonzero status.
=========================

Another way to do this is via the helper function
'is_nonbare_repository_dir()' (not using is_git_directory() because of
the failure it was causing before)
823a287

Therefore, instead of spawning a CP ever, do a check with the
aforementioned function. All tests pass.

https://lore.kernel.org/git/nycvar.QRO.7.76.6.2007031712160.50@tvgsbejvaqbjf.bet/
  • Loading branch information
periperidip committed Jul 18, 2020
1 parent f0f8ba3 commit 4765017
Showing 1 changed file with 4 additions and 10 deletions.
14 changes: 4 additions & 10 deletions builtin/submodule--helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ static void prepare_submodule_summary(struct summary_cb *info,
for (i = 0; i < list->nr; i++) {
const struct submodule *sub;
struct module_cb *p = list->entries[i];
struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
struct strbuf sm_gitdir = STRBUF_INIT;

if (p->status == 'D' || p->status == 'T') {
generate_submodule_summary(info, p);
Expand All @@ -1219,16 +1219,10 @@ static void prepare_submodule_summary(struct summary_cb *info,
}

/* Also show added or modified modules which are checked out */
cp_rev_parse.dir = p->sm_path;
cp_rev_parse.git_cmd = 1;
cp_rev_parse.no_stderr = 1;
cp_rev_parse.no_stdout = 1;

argv_array_pushl(&cp_rev_parse.args, "rev-parse",
"--git-dir", NULL);

if (!run_command(&cp_rev_parse))
strbuf_addstr(&sm_gitdir, p->sm_path);
if (is_nonbare_repository_dir(&sm_gitdir))
generate_submodule_summary(info, p);
strbuf_release(&sm_gitdir);
}
}

Expand Down

0 comments on commit 4765017

Please sign in to comment.