Re: [PATCHv2 5/8] fetching submodules: Respect `submodule.jobs` config option
On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller wrote:
> This allows to configure fetching and updating in parallel
> without having the command line option.
>
> This moved the responsibility to determine how many parallel processes
> to start from builtin/fetch to submodule.c as we need a way to communicate
> "The user did not specify the number of parallel processes in the command
> line options" in the builtin fetch. The submodule code takes care of
> the precedence (CLI > config > default)
>
> Signed-off-by: Stefan Beller
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 391a0c3..785721a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2643,6 +2643,13 @@ submodule..ignore::
> "--ignore-submodules" option. The 'git submodule' commands are not
> affected by this setting.
>
> +submodule.jobs::
> + This is used to determine how many submodules can be operated on in
> + parallel. Specifying a positive integer allows up to that number
> + of submodules being fetched in parallel. This is used in fetch
> + and clone operations only. A value of 0 will give some reasonable
> + default. The defaults may change with different versions of Git.
I'm not sure that "default" is the correct word here. When you talk
about a "default", you're normally explaining what happens when the
configuration is not provided. (In fact, the default number of jobs is
1, which you may want to document here).
> tag.sort::
> This variable controls the sort ordering of tags when displayed by
> linkgit:git-tag[1]. Without the "--sort=" option provided, the
> diff --git a/submodule-config.c b/submodule-config.c
> index 1cea404..07bdcdf 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -32,6 +32,7 @@ enum lookup_type {
>
> static struct submodule_cache cache;
> static int is_cache_init;
> +static int parallel_jobs = -1;
>
> static int config_path_cmp(const struct submodule_entry *a,
>const struct submodule_entry *b,
> @@ -235,6 +236,9 @@ static int parse_generic_submodule_config(const char *var,
> const char *key,
> const char *value)
> {
> + if (!strcmp(key, "jobs")) {
> + parallel_jobs = strtol(value, NULL, 10);
> + }
Style: unnecessary braces
Why does this allow a negative value? The documentation doesn't
mention anything about it.
> return 0;
> }
>
> diff --git a/submodule.c b/submodule.c
> index 0257ea3..188ba02 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -752,6 +752,11 @@ int fetch_populated_submodules(const struct argv_array
> *options,
> argv_array_push(&spf.args, "--recurse-submodules-default");
> /* default value, "--submodule-prefix" and its value are added later
> */
>
> + if (max_parallel_jobs < 0)
> + max_parallel_jobs = config_parallel_submodules();
> + if (max_parallel_jobs < 0)
> + max_parallel_jobs = 1;
run_process_parallel() itself specially handles max_parallel_jobs==0,
so you don't need to consider it here. Okay.
> +
> calculate_changed_submodule_paths();
> run_processes_parallel(max_parallel_jobs,
>get_next_submodule,
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 1b4ce69..5c3579c 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -470,4 +470,18 @@ test_expect_success "don't fetch submodule when newly
> recorded commits are alrea
> test_i18ncmp expect.err actual.err
> '
>
> +test_expect_success 'fetching submodules respects parallel settings' '
> + git config fetch.recurseSubmodules true &&
> + (
> + cd downstream &&
> + GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
> + grep "7 children" trace.out &&
> + git config submodule.jobs 8 &&
> + GIT_TRACE=$(pwd)/trace.out git fetch &&
> + grep "8 children" trace.out &&
> + GIT_TRACE=$(pwd)/trace.out git fetch --jobs 9 &&
> + grep "9 children" trace.out
> + )
> +'
Not specifically related to this test, but maybe add tests to check
cases when --jobs is not specified, and --jobs=1?
> +
> test_done
> --
> 2.5.0.281.g4ed9cdb
>
--
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
[PATCHv2 5/8] fetching submodules: Respect `submodule.jobs` config option
This allows to configure fetching and updating in parallel
without having the command line option.
This moved the responsibility to determine how many parallel processes
to start from builtin/fetch to submodule.c as we need a way to communicate
"The user did not specify the number of parallel processes in the command
line options" in the builtin fetch. The submodule code takes care of
the precedence (CLI > config > default)
Signed-off-by: Stefan Beller
---
Documentation/config.txt| 7 +++
builtin/fetch.c | 2 +-
submodule-config.c | 9 +
submodule-config.h | 2 ++
submodule.c | 5 +
t/t5526-fetch-submodules.sh | 14 ++
6 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..785721a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2643,6 +2643,13 @@ submodule..ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
+submodule.jobs::
+ This is used to determine how many submodules can be operated on in
+ parallel. Specifying a positive integer allows up to that number
+ of submodules being fetched in parallel. This is used in fetch
+ and clone operations only. A value of 0 will give some reasonable
+ default. The defaults may change with different versions of Git.
+
tag.sort::
This variable controls the sort ordering of tags when displayed by
linkgit:git-tag[1]. Without the "--sort=" option provided, the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9cc1c9d..60e6797 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */
static int all, append, dry_run, force, keep, multiple, update_head_ok,
verbosity;
static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int tags = TAGS_DEFAULT, unshallow, update_shallow;
-static int max_children = 1;
+static int max_children = -1;
static const char *depth;
static const char *upload_pack;
static struct strbuf default_rla = STRBUF_INIT;
diff --git a/submodule-config.c b/submodule-config.c
index 1cea404..07bdcdf 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -32,6 +32,7 @@ enum lookup_type {
static struct submodule_cache cache;
static int is_cache_init;
+static int parallel_jobs = -1;
static int config_path_cmp(const struct submodule_entry *a,
const struct submodule_entry *b,
@@ -235,6 +236,9 @@ static int parse_generic_submodule_config(const char *var,
const char *key,
const char *value)
{
+ if (!strcmp(key, "jobs")) {
+ parallel_jobs = strtol(value, NULL, 10);
+ }
return 0;
}
@@ -483,3 +487,8 @@ void submodule_free(void)
cache_free(&cache);
is_cache_init = 0;
}
+
+int config_parallel_submodules(void)
+{
+ return parallel_jobs;
+}
diff --git a/submodule-config.h b/submodule-config.h
index f9e2a29..d9bbf9a 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -27,4 +27,6 @@ const struct submodule *submodule_from_path(const unsigned
char *commit_sha1,
const char *path);
void submodule_free(void);
+int config_parallel_submodules(void);
+
#endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index 0257ea3..188ba02 100644
--- a/submodule.c
+++ b/submodule.c
@@ -752,6 +752,11 @@ int fetch_populated_submodules(const struct argv_array
*options,
argv_array_push(&spf.args, "--recurse-submodules-default");
/* default value, "--submodule-prefix" and its value are added later */
+ if (max_parallel_jobs < 0)
+ max_parallel_jobs = config_parallel_submodules();
+ if (max_parallel_jobs < 0)
+ max_parallel_jobs = 1;
+
calculate_changed_submodule_paths();
run_processes_parallel(max_parallel_jobs,
get_next_submodule,
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 1b4ce69..5c3579c 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -470,4 +470,18 @@ test_expect_success "don't fetch submodule when newly
recorded commits are alrea
test_i18ncmp expect.err actual.err
'
+test_expect_success 'fetching submodules respects parallel settings' '
+ git config fetch.recurseSubmodules true &&
+ (
+ cd downstream &&
+ GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
+ grep "7 children" trace.out &&
+ git config submodule.jobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git fetch &&
+ grep "8 children" trace.out &&
+ GIT_TRACE=$(pwd)/trace.out git fetch --jobs 9 &&
+ grep "9 children" trace.out
+ )
+'
+
t
