Calvin Wan <calvinwan@xxxxxxxxxx> writes:
> @@ -226,6 +242,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
> newmode = ce->ce_mode;
> } else {
> struct stat st;
> + unsigned ignore_untracked = 0;
> + int defer_submodule_status = !!revs->repo;
What is the reasoning behind this condition? I would expect revs->repo
to always be set, and we would always end up deferring.
> newmode = ce_mode_from_stat(ce, st.st_mode);
> + if (defer_submodule_status) {
> + struct submodule_status_util tmp = {
> + .changed = changed,
> + .dirty_submodule = 0,
> + .ignore_untracked = ignore_untracked,
> + .newmode = newmode,
> + .ce = ce,
> + .path = ce->name,
> + };
> + struct string_list_item *item;
> +
> + item = string_list_append(&submodules, ce->name);
> + item->util = xmalloc(sizeof(tmp));
> + memcpy(item->util, &tmp, sizeof(tmp));
(Not a C expert) Since we don't return the string list, I wonder if we
can avoid the memcpy() by using &tmp like so:
struct string_list_item *item;
item = string_list_append(&submodules, ce->name);
item->util = &tmp;
And then when we call string_list_clear(), we wouldn't need to free the
util since we exit the stack frame.
> +test_expect_success 'diff in superproject with submodules respects parallel settings' '
> + test_when_finished "rm -f trace.out" &&
> + (
> + GIT_TRACE=$(pwd)/trace.out git diff &&
> + grep "1 tasks" trace.out &&
> + >trace.out &&
> +
> + git config submodule.diffJobs 8 &&
> + GIT_TRACE=$(pwd)/trace.out git diff &&
> + grep "8 tasks" trace.out &&
> + >trace.out &&
> +
> + GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 diff &&
> + grep "preparing to run up to [0-9]* tasks" trace.out &&
> + ! grep "up to 0 tasks" trace.out &&
> + >trace.out
> + )
> +'
> +
Could we get tests to check that the output of git diff isn't changed by
setting parallelism? This might not be feasible for submodule.diffJobs >
1 due to raciness, but it would be good to see for submodule.diffJobs =
1 at least.
> test_expect_success 'git diff --raw HEAD' '
> hexsz=$(test_oid hexsz) &&
> git diff --raw --abbrev=$hexsz HEAD >actual &&
> diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
> index d050091345..52a82b703f 100755
> --- a/t/t7506-status-submodule.sh
> +++ b/t/t7506-status-submodule.sh
> @@ -412,4 +412,23 @@ test_expect_success 'status with added file in nested submodule (short)' '
> EOF
> '
>
> +test_expect_success 'status in superproject with submodules respects parallel settings' '
> + test_when_finished "rm -f trace.out" &&
> + (
> + GIT_TRACE=$(pwd)/trace.out git status &&
> + grep "1 tasks" trace.out &&
> + >trace.out &&
> +
> + git config submodule.diffJobs 8 &&
> + GIT_TRACE=$(pwd)/trace.out git status &&
> + grep "8 tasks" trace.out &&
> + >trace.out &&
> +
> + GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 status &&
> + grep "preparing to run up to [0-9]* tasks" trace.out &&
> + ! grep "up to 0 tasks" trace.out &&
> + >trace.out
> + )
> +'
> +
Ditto for "status".
