* Re: diff.ignoreSubmoudles config setting broken?
From: Stefan Beller @ 2017-03-09 1:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sebastian Schuberth, Jacob Keller, Jeff King, Git Mailing List,
Jens Lehmann
In-Reply-To: <xmqqa88v1i5f.fsf@gitster.mtv.corp.google.com>
On Wed, Mar 8, 2017 at 3:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Yeah the .gitmodules file is a good hint.
And by that I meant that I am not sure if we're going
down the right rabbit hole here. So before we take action
maybe Sebastian can tell us more about his project (and all
configurations and settings involved)
>>
>> Here is my understanding of the precedence:
>>
>> command line options > .git/config (in various forms) > .gitmodules
>>
>> where in the .git config we have precedence levels for different files
>>
>> .git/config > ~/.gitconfig
>>
>> as well as different settings:
>>
>> submodule.<name>.ignore > diff.ignoreSubmodules
>
> I've never understood why people thought it a good idea to let
> .gitmodules supplied by the upstream override the configuration
> setting the end user has like this. This is quite bad.
Apart from from the name <-> path mapping, the .gitmodules
file is a collection of suggestions, some more severe than
others.
I think the issue here is to define the correct
and clear order of precedence, specifically between along these
2 different dimensions (different configuration settings vs different
files with configuration), such that the .gitmodules file is only ever
consulted when the user has obviously nothing configured that
would contradict the .gitmodules file.
>
> Perhaps this is a good starting point?
>
> diff.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index a628ac3a95..75b7140c63 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -273,8 +273,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
> if (!strcmp(var, "diff.orderfile"))
> return git_config_pathname(&diff_order_file_cfg, var, value);
>
> - if (!strcmp(var, "diff.ignoresubmodules"))
> + if (!strcmp(var, "diff.ignoresubmodules")) {
> handle_ignore_submodules_arg(&default_diff_options, value);
> + DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
s/options/&default_diff_options/ makes it compile. (I did not think
whether that is
correct though.)
In other occurrences of handle_ignore_submodules_arg, the DIFF_OPT_SET
is set before the handle_ignore_submodules_arg, though.
When trying these suggestions, ./t4027-diff-submodule.sh breaks.
log on that file yields e.g. 302ad7a9930 (Submodules: Use "ignore" settings
from .gitmodules too for diff and status), which tells us that in 2010 people
were not as concerned by this, but the user had to use the exact option to
override the upstream default-suggestion.
Thanks,
Stefan
^ permalink raw reply
* [PATCH v3 2/2] submodule--helper.c: remove duplicate code
From: Valery Tolstov @ 2017-03-09 1:27 UTC (permalink / raw)
To: git; +Cc: bmwill, sbeller, me, gitster
In-Reply-To: <20170309012734.21541-1-me@vtolstov.org>
Remove code fragment from module_clone that duplicates functionality
of connect_work_tree_and_git_dir in dir.c
Signed-off-by: Valery Tolstov <me@vtolstov.org>
---
builtin/submodule--helper.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e..86bafe166 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -579,9 +579,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
int progress = 0;
- FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
- struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
char *sm_alternate = NULL, *error_strategy = NULL;
@@ -653,27 +651,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)
strbuf_reset(&sb);
}
- /* Write a .git file in the submodule to redirect to the superproject. */
- strbuf_addf(&sb, "%s/.git", path);
- if (safe_create_leading_directories_const(sb.buf) < 0)
- die(_("could not create leading directories of '%s'"), sb.buf);
- submodule_dot_git = fopen(sb.buf, "w");
- if (!submodule_dot_git)
- die_errno(_("cannot open file '%s'"), sb.buf);
-
- fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
- relative_path(sm_gitdir, path, &rel_path));
- if (fclose(submodule_dot_git))
- die(_("could not close file %s"), sb.buf);
- strbuf_reset(&sb);
- strbuf_reset(&rel_path);
+ /* Connect module worktree and git dir */
+ connect_work_tree_and_git_dir(path, sm_gitdir);
- /* Redirect the worktree of the submodule in the superproject's config */
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file(p, "core.worktree",
- relative_path(path, sm_gitdir, &rel_path));
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
git_config_get_string("submodule.alternateLocation", &sm_alternate);
@@ -689,7 +672,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
free(error_strategy);
strbuf_release(&sb);
- strbuf_release(&rel_path);
free(sm_gitdir);
free(path);
free(p);
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply related
* [PATCH v3 1/2] connect_work_tree_and_git_dir: safely create leading directories
From: Valery Tolstov @ 2017-03-09 1:27 UTC (permalink / raw)
To: git; +Cc: bmwill, sbeller, me, gitster
In-Reply-To: <20170309012734.21541-1-me@vtolstov.org>
From: Stefan Beller <sbeller@google.com>
In a later patch we'll use connect_work_tree_and_git_dir when the
directory for the gitlink file doesn't exist yet. This patch makes
connect_work_tree_and_git_dir safe to use for both cases of
either the git dir or the working dir missing.
To do so, we need to call safe_create_leading_directories[_const]
on both directories. However this has to happen before we construct
the absolute paths as real_pathdup assumes the directories to
be there already.
So for both the config file in the git dir as well as the .git link
file we need to
a) construct the name
b) call SCLD
c) get the absolute path
d) once a-c is done for both we can consume the absolute path
to compute the relative path to each other and store those
relative paths.
The implementation provided here puts a) and b) for both cases first,
and then performs c and d after.
One of the two users of 'connect_work_tree_and_git_dir' already checked
for the directory being there, so we can loose that check as
connect_work_tree_and_git_dir handles this functionality now.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 32 +++++++++++++++++++++-----------
submodule.c | 11 ++---------
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/dir.c b/dir.c
index 4541f9e14..6f52af7ab 100644
--- a/dir.c
+++ b/dir.c
@@ -2728,23 +2728,33 @@ void untracked_cache_add_to_index(struct index_state *istate,
/* Update gitfile and core.worktree setting to connect work tree and git dir */
void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
{
- struct strbuf file_name = STRBUF_INIT;
+ struct strbuf gitfile_sb = STRBUF_INIT;
+ struct strbuf cfg_sb = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- char *git_dir = real_pathdup(git_dir_);
- char *work_tree = real_pathdup(work_tree_);
+ char *git_dir, *work_tree;
- /* Update gitfile */
- strbuf_addf(&file_name, "%s/.git", work_tree);
- write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, work_tree, &rel_path));
+ /* Prepare .git file */
+ strbuf_addf(&gitfile_sb, "%s/.git", work_tree_);
+ if (safe_create_leading_directories_const(gitfile_sb.buf))
+ die(_("could not create directories for %s"), gitfile_sb.buf);
+
+ /* Prepare config file */
+ strbuf_addf(&cfg_sb, "%s/config", git_dir_);
+ if (safe_create_leading_directories_const(cfg_sb.buf))
+ die(_("could not create directories for %s"), cfg_sb.buf);
+ git_dir = real_pathdup(git_dir_);
+ work_tree = real_pathdup(work_tree_);
+
+ /* Write .git file */
+ write_file(gitfile_sb.buf, "gitdir: %s",
+ relative_path(git_dir, work_tree, &rel_path));
/* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
+ git_config_set_in_file(cfg_sb.buf, "core.worktree",
relative_path(work_tree, git_dir, &rel_path));
- strbuf_release(&file_name);
+ strbuf_release(&gitfile_sb);
+ strbuf_release(&cfg_sb);
strbuf_release(&rel_path);
free(work_tree);
free(git_dir);
diff --git a/submodule.c b/submodule.c
index 3b98766a6..45e93a1d5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1445,8 +1445,6 @@ void absorb_git_dir_into_superproject(const char *prefix,
/* Not populated? */
if (!sub_git_dir) {
- char *real_new_git_dir;
- const char *new_git_dir;
const struct submodule *sub;
if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
@@ -1469,13 +1467,8 @@ void absorb_git_dir_into_superproject(const char *prefix,
sub = submodule_from_path(null_sha1, path);
if (!sub)
die(_("could not lookup name for submodule '%s'"), path);
- new_git_dir = git_path("modules/%s", sub->name);
- if (safe_create_leading_directories_const(new_git_dir) < 0)
- die(_("could not create directory '%s'"), new_git_dir);
- real_new_git_dir = real_pathdup(new_git_dir);
- connect_work_tree_and_git_dir(path, real_new_git_dir);
-
- free(real_new_git_dir);
+ connect_work_tree_and_git_dir(path,
+ git_path("modules/%s", sub->name));
} else {
/* Is it already absorbed into the superprojects git dir? */
char *real_sub_git_dir = real_pathdup(sub_git_dir);
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply related
* [PATCH v3 0/2] Remove duplicate code from module_clone()
From: Valery Tolstov @ 2017-03-09 1:27 UTC (permalink / raw)
To: git; +Cc: bmwill, sbeller, me, gitster
In-Reply-To: <20170309003858.GB153031@google.com>
Stefan Beller (1):
connect_work_tree_and_git_dir: safely create leading directories
Valery Tolstov (1):
submodule--helper.c: remove duplicate code
builtin/submodule--helper.c | 22 ++--------------------
dir.c | 32 +++++++++++++++++++++-----------
submodule.c | 11 ++---------
3 files changed, 25 insertions(+), 40 deletions(-)
--
2.12.0.192.gbdb9d28a5
^ permalink raw reply
* [PATCH v2 05/11] submodule--helper clone: check for configured submodules using helper
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
Use the 'is_submodule_initialized()' helper to check for configured
submodules instead of manually checking for the submodule's URL in the
config.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/submodule--helper.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dac02604d..bceb62a20 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -760,7 +760,6 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
struct strbuf displaypath_sb = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
const char *displaypath = NULL;
- char *url = NULL;
int needs_cloning = 0;
if (ce_stage(ce)) {
@@ -794,15 +793,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
goto cleanup;
}
- /*
- * Looking up the url in .git/config.
- * We must not fall back to .gitmodules as we only want
- * to process configured submodules.
- */
- strbuf_reset(&sb);
- strbuf_addf(&sb, "submodule.%s.url", sub->name);
- git_config_get_string(sb.buf, &url);
- if (!url) {
+ /* Check if the submodule has been initialized. */
+ if (!is_submodule_initialized(ce->name)) {
next_submodule_warn_missing(suc, out, displaypath);
goto cleanup;
}
@@ -836,7 +828,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
argv_array_push(&child->args, "--depth=1");
argv_array_pushl(&child->args, "--path", sub->path, NULL);
argv_array_pushl(&child->args, "--name", sub->name, NULL);
- argv_array_pushl(&child->args, "--url", url, NULL);
+ argv_array_pushl(&child->args, "--url", sub->url, NULL);
if (suc->references.nr) {
struct string_list_item *item;
for_each_string_list_item(item, &suc->references)
@@ -846,7 +838,6 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
argv_array_push(&child->args, suc->depth);
cleanup:
- free(url);
strbuf_reset(&displaypath_sb);
strbuf_reset(&sb);
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 02/11] submodule status: use submodule--helper is-active
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
git-submodule.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 554bd1c49..19660b9c0 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1000,14 +1000,13 @@ cmd_status()
do
die_if_unmatched "$mode" "$sha1"
name=$(git submodule--helper name "$sm_path") || exit
- url=$(git config submodule."$name".url)
displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
if test "$stage" = U
then
say "U$sha1 $displaypath"
continue
fi
- if test -z "$url" ||
+ if ! git submodule--helper is-active "$sm_path" ||
{
! test -d "$sm_path"/.git &&
! test -f "$sm_path"/.git
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 04/11] submodule sync: use submodule--helper is-active
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
git-submodule.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 1c2064cc1..7ed1aaba3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1101,7 +1101,7 @@ cmd_sync()
;;
esac
- if git config "submodule.$name.url" >/dev/null 2>/dev/null
+ if git submodule--helper is-active "$sm_path"
then
displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 00/11] decoupling a submodule's existence and its url
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170223234728.164111-1-bmwill@google.com>
Differences from v1:
* Reordered some of the patches
* added submodule.<name>.active boolean config option which can be used to do
more fine grain selection of submodules while still decoupling the existence
from the URL.
* submodule.<name>.active has precedence over the pathspec stored in
submodule.active
Brandon Williams (11):
submodule--helper: add is_active command
submodule status: use submodule--helper is-active
submodule deinit: use most reliable url
submodule sync: use submodule--helper is-active
submodule--helper clone: check for configured submodules using helper
submodule: decouple url and submodule existence
submodule update: add `--init-active` switch
clone: add --submodule-spec=<pathspec> switch
completion: clone can initialize specific submodules
submodule--helper init: set submodule.<name>.active
submodule add: respect submodule.active and submodule.<name>.active
Documentation/config.txt | 15 ++++-
Documentation/git-clone.txt | 23 ++++---
Documentation/git-submodule.txt | 11 +++-
builtin/clone.c | 36 ++++++++++-
builtin/submodule--helper.c | 33 ++++++----
contrib/completion/git-completion.bash | 1 +
git-submodule.sh | 40 +++++++++---
submodule.c | 36 +++++++++--
t/t7400-submodule-basic.sh | 109 +++++++++++++++++++++++++++++++++
t/t7413-submodule-is-active.sh | 107 ++++++++++++++++++++++++++++++++
10 files changed, 376 insertions(+), 35 deletions(-)
create mode 100755 t/t7413-submodule-is-active.sh
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply
* [PATCH v2 01/11] submodule--helper: add is_active command
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
There are a lot of places where an explicit check for
submodule."<name>".url is done to see if a submodule exists. In order
to centralize this check introduce a helper which can be used to query
if a submodule is active or not.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/submodule--helper.c | 11 +++++++++++
t/t7413-submodule-is-active.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100755 t/t7413-submodule-is-active.sh
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index df0d9c166..dac02604d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1128,6 +1128,16 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
return 0;
}
+static int is_active(int argc, const char **argv, const char *prefix)
+{
+ if (argc != 2)
+ die("submodule--helper is-active takes exactly 1 arguments");
+
+ gitmodules_config();
+
+ return !is_submodule_initialized(argv[1]);
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1147,6 +1157,7 @@ static struct cmd_struct commands[] = {
{"init", module_init, 0},
{"remote-branch", resolve_remote_submodule_branch, 0},
{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
+ {"is-active", is_active, 0},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
new file mode 100755
index 000000000..f18e0c925
--- /dev/null
+++ b/t/t7413-submodule-is-active.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='Test submodule--helper is-active
+
+This test verifies that `git submodue--helper is-active` correclty identifies
+submodules which are "active" and interesting to the user.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ git init sub &&
+ test_commit -C sub initial &&
+ git init super &&
+ test_commit -C super initial &&
+ git -C super submodule add ../sub sub1 &&
+ git -C super submodule add ../sub sub2 &&
+ git -C super commit -a -m "add 2 submodules at sub{1,2}"
+'
+
+test_expect_success 'is-active works with urls' '
+ git -C super submodule--helper is-active sub1 &&
+ git -C super submodule--helper is-active sub2 &&
+
+ git -C super config --unset submodule.sub1.URL &&
+ test_must_fail git -C super submodule--helper is-active sub1 &&
+ git -C super config submodule.sub1.URL ../sub &&
+ git -C super submodule--helper is-active sub1
+'
+
+test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 11/11] submodule add: respect submodule.active and submodule.<name>.active
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
In addition to adding submodule.<name>.url to the config, set
submodule.<name>.active to true unless submodule.active is configured
and the submodule's path matches the configured pathspec.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
git-submodule.sh | 12 ++++++++++++
t/t7413-submodule-is-active.sh | 21 +++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/git-submodule.sh b/git-submodule.sh
index 43a393d7e..d90fde655 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -271,6 +271,18 @@ or you are unsure what this means choose another name with the '--name' option."
fi &&
git add --force .gitmodules ||
die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
+
+ if git config --get submodule.active >/dev/null
+ then
+ # If the submodule being adding isn't already covered by the
+ # current configured pathspec, set the submodule's active flag
+ if ! git submodule--helper is-active "$sm_path"
+ then
+ git config --add submodule."$sm_name".active "true"
+ fi
+ else
+ git config --add submodule."$sm_name".active "true"
+ fi
}
#
diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
index c41b899ab..865931978 100755
--- a/t/t7413-submodule-is-active.sh
+++ b/t/t7413-submodule-is-active.sh
@@ -15,6 +15,12 @@ test_expect_success 'setup' '
test_commit -C super initial &&
git -C super submodule add ../sub sub1 &&
git -C super submodule add ../sub sub2 &&
+
+ # Remove submodule.<name>.active entries in order to test in an
+ # environment where only URLs are present in the conifg
+ git -C super config --unset submodule.sub1.active &&
+ git -C super config --unset submodule.sub2.active &&
+
git -C super commit -a -m "add 2 submodules at sub{1,2}"
'
@@ -83,4 +89,19 @@ test_expect_success 'is-active with submodule.active and submodule.<name>.active
git -C super config --unset submodule.sub2.active
'
+test_expect_success 'is-active, submodule.active and submodule add' '
+ test_when_finished "rm -rf super2" &&
+ git init super2 &&
+ test_commit -C super2 initial &&
+ git -C super2 config --add submodule.active "sub*" &&
+
+ # submodule add should only add submodule.<name>.active
+ # to the config if not matched by the pathspec
+ git -C super2 submodule add ../sub sub1 &&
+ test_must_fail git -C super2 config --get submodule.sub1.active &&
+
+ git -C super2 submodule add ../sub mod &&
+ git -C super2 config --get submodule.mod.active
+'
+
test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 06/11] submodule: decouple url and submodule existence
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
Currently the submodule.<name>.url config option is used to determine
if a given submodule exists and is interesting to the user. This
however doesn't work very well because the URL is a config option for
the scope of a repository, whereas the existence of a submodule is an
option scoped to the working tree.
In a future with worktree support for submodules, there will be multiple
working trees, each of which may only need a subset of the submodules
checked out. The URL (which is where the submodule repository can be
obtained) should not differ between different working trees.
It may also be convenient for users to more easily specify groups of
submodules they are interested in as apposed to running "git submodule
init <path>" on each submodule they want checked out in their working
tree.
To this end two config options are introduced, submodule.active and
submodule.<name>.active. The submodule.active config holds a pathspec
that specifies which submodules should exist in the working tree. The
submodule.<name>.active config is a boolean flag used to indicate if
that particular submodule should exist in the working tree.
Given these multiple ways to check for a submodule's existence the more
fine-grained submodule.<name>.active option has the highest order of
precedence followed by the pathspec check against submodule.active. To
ensure backwards compatibility, if neither of these options are set git
falls back to checking the submodule.<name>.url option to determine a
submodule's existence.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/config.txt | 15 ++++++++++--
submodule.c | 36 ++++++++++++++++++++++++---
t/t7413-submodule-is-active.sh | 55 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..21037e8c5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2888,8 +2888,9 @@ submodule.<name>.url::
The URL for a submodule. This variable is copied from the .gitmodules
file to the git config via 'git submodule init'. The user can change
the configured URL before obtaining the submodule via 'git submodule
- update'. After obtaining the submodule, the presence of this variable
- is used as a sign whether the submodule is of interest to git commands.
+ update'. If neither submodule.<name>.active or submodule.active are
+ set, the presence of this variable is used as a fallback to indicate
+ whether the submodule is of interest to git commands.
See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
submodule.<name>.update::
@@ -2927,6 +2928,16 @@ submodule.<name>.ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
+submodule.<name>.active::
+ Boolean value indicating if the submodule is of interest to git
+ commands. This config option takes precedence over the
+ submodule.active config option.
+
+submodule.active::
+ A repeated field which contains a pathspec used to match against a
+ submodule's path to determine if the submodule is of interest to git
+ commands.
+
submodule.fetchJobs::
Specifies how many submodules are fetched/cloned at the same time.
A positive integer allows up to that number of submodules fetched
diff --git a/submodule.c b/submodule.c
index 4c4f033e8..6125736fc 100644
--- a/submodule.c
+++ b/submodule.c
@@ -217,13 +217,41 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)
int is_submodule_initialized(const char *path)
{
int ret = 0;
- const struct submodule *module = NULL;
+ char *key;
+ const struct string_list *sl;
+ const struct submodule *module = submodule_from_path(null_sha1, path);
- module = submodule_from_path(null_sha1, path);
+ /* early return if there isn't a path->module mapping */
+ if (!module)
+ return 0;
+
+ /* submodule.<name>.active is set */
+ key = xstrfmt("submodule.%s.active", module->name);
+ if (!git_config_get_bool(key, &ret)) {
+ free(key);
+ return ret;
+ }
+ free(key);
+
+ sl = git_config_get_value_multi("submodule.active");
- if (module) {
- char *key = xstrfmt("submodule.%s.url", module->name);
+ if (sl) {
+ struct pathspec ps;
+ struct argv_array args = ARGV_ARRAY_INIT;
+ const struct string_list_item *item;
+
+ for_each_string_list_item(item, sl) {
+ argv_array_push(&args, item->string);
+ }
+
+ parse_pathspec(&ps, 0, 0, 0, args.argv);
+ ret = match_pathspec(&ps, path, strlen(path), 0, NULL, 1);
+
+ argv_array_clear(&args);
+ clear_pathspec(&ps);
+ } else {
char *value = NULL;
+ key = xstrfmt("submodule.%s.url", module->name);
ret = !git_config_get_string(key, &value);
diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
index f18e0c925..c41b899ab 100755
--- a/t/t7413-submodule-is-active.sh
+++ b/t/t7413-submodule-is-active.sh
@@ -28,4 +28,59 @@ test_expect_success 'is-active works with urls' '
git -C super submodule--helper is-active sub1
'
+test_expect_success 'is-active works with submodule.<name>.active config' '
+ git -C super config --bool submodule.sub1.active "false" &&
+ test_must_fail git -C super submodule--helper is-active sub1 &&
+
+ git -C super config --bool submodule.sub1.active "true" &&
+ git -C super config --unset submodule.sub1.URL &&
+ git -C super submodule--helper is-active sub1 &&
+
+ git -C super config submodule.sub1.URL ../sub &&
+ git -C super config --unset submodule.sub1.active
+'
+
+test_expect_success 'is-active works with basic submodule.active config' '
+ git -C super config --add submodule.active "." &&
+ git -C super config --unset submodule.sub1.URL &&
+
+ git -C super submodule--helper is-active sub1 &&
+ git -C super submodule--helper is-active sub2 &&
+
+ git -C super config submodule.sub1.URL ../sub &&
+ git -C super config --unset-all submodule.active
+'
+
+test_expect_success 'is-active correctly works with paths that are not submodules' '
+ test_must_fail git -C super submodule--helper is-active not-a-submodule &&
+
+ git -C super config --add submodule.active "." &&
+ test_must_fail git -C super submodule--helper is-active not-a-submodule &&
+
+ git -C super config --unset-all submodule.active
+'
+
+test_expect_success 'is-active works with exclusions in submodule.active config' '
+ git -C super config --add submodule.active "." &&
+ git -C super config --add submodule.active ":(exclude)sub1" &&
+
+ test_must_fail git -C super submodule--helper is-active sub1 &&
+ git -C super submodule--helper is-active sub2 &&
+
+ git -C super config --unset-all submodule.active
+'
+
+test_expect_success 'is-active with submodule.active and submodule.<name>.active' '
+ git -C super config --add submodule.active "sub1" &&
+ git -C super config --bool submodule.sub1.active "false" &&
+ git -C super config --bool submodule.sub2.active "true" &&
+
+ test_must_fail git -C super submodule--helper is-active sub1 &&
+ git -C super submodule--helper is-active sub2 &&
+
+ git -C super config --unset-all submodule.active &&
+ git -C super config --unset submodule.sub1.active &&
+ git -C super config --unset submodule.sub2.active
+'
+
test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 09/11] completion: clone can initialize specific submodules
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff80f..4e473aa90 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1138,6 +1138,7 @@ _git_clone ()
--single-branch
--branch
--recurse-submodules
+ --submodule-spec
"
return
;;
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 07/11] submodule update: add `--init-active` switch
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
The new switch `--init-active` initializes the submodules which are
configured in `submodule.active` instead of those given as
command line arguments before updating. In the first implementation this
is made incompatible with further command line arguments as it is
unclear what the user means by
git submodule update --init --init-active <paths>
This new switch allows users to record more complex patterns as it saves
retyping them whenever you invoke update.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-submodule.txt | 11 ++++++++++-
git-submodule.sh | 21 ++++++++++++++++++---
t/t7400-submodule-basic.sh | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 918bd1d1b..626b9760a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,7 @@ SYNOPSIS
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
-'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
+'git submodule' [--quiet] update [--init[-active]] [--remote] [-N|--no-fetch]
[--[no-]recommend-shallow] [-f|--force] [--rebase|--merge]
[--reference <repository>] [--depth <depth>] [--recursive]
[--jobs <n>] [--] [<path>...]
@@ -195,6 +195,10 @@ If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
submodule with the `--init` option.
+You can configure a set of submodules using pathspec syntax in
+submodule.active you can use `--init-active` to initialize
+those before updating.
+
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
--
@@ -378,6 +382,11 @@ the submodule itself.
Initialize all submodules for which "git submodule init" has not been
called so far before updating.
+--init-active::
+ This option is only valid for the update command.
+ Initialize all submodules configured in "`submodule.active`"
+ that have not been updated before.
+
--name::
This option is only valid for the add command. It sets the submodule's
name to the given string instead of defaulting to its path. The name
diff --git a/git-submodule.sh b/git-submodule.sh
index 7ed1aaba3..43a393d7e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init[-active]] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
@@ -499,7 +499,12 @@ cmd_update()
progress="--progress"
;;
-i|--init)
- init=1
+ test -z $init || test $init = by_args || die "$(gettext "Only one of --init or --init-active may be used.")"
+ init=by_args
+ ;;
+ --init-active)
+ test -z $init || test $init = by_config || die "$(gettext "Only one of --init or --init-active may be used.")"
+ init=by_config
;;
--remote)
remote=1
@@ -568,7 +573,17 @@ cmd_update()
if test -n "$init"
then
- cmd_init "--" "$@" || return
+ if test "$init" = "by_config"
+ then
+ if test $# -gt 0
+ then
+ die "$(gettext "path arguments are incompatible with --init-active")"
+ fi
+ cmd_init "--" $(git config --get-all submodule.active) || return
+ else
+ cmd_init "--" "$@" || return
+ fi
+
fi
{
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b77cce8e4..f5df0a5b4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1116,5 +1116,44 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '
test_cmp expect actual
'
+test_expect_success 'setup superproject with submodules' '
+ git init sub1 &&
+ test_commit -C sub1 test &&
+ test_commit -C sub1 test2 &&
+ git init multisuper &&
+ git -C multisuper submodule add ../sub1 sub0 &&
+ git -C multisuper submodule add ../sub1 sub1 &&
+ git -C multisuper submodule add ../sub1 sub2 &&
+ git -C multisuper submodule add ../sub1 sub3 &&
+ git -C multisuper commit -m "add some submodules"
+'
+
+cat >expect <<-EOF
+-sub0
+ sub1 (test2)
+ sub2 (test2)
+ sub3 (test2)
+EOF
+
+test_expect_success 'submodule update --init with a specification' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/multisuper multisuper_clone &&
+ git -C multisuper_clone submodule update --init . ":(exclude)sub0" &&
+ git -C multisuper_clone submodule status |cut -c 1,43- >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'submodule update --init-active' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/multisuper multisuper_clone &&
+ git -C multisuper_clone config submodule.active "." &&
+ git -C multisuper_clone config --add submodule.active ":(exclude)sub0" &&
+ git -C multisuper_clone submodule update --init-active &&
+ git -C multisuper_clone submodule status |cut -c 1,43- >actual &&
+ test_must_fail git -C multisuper_clone submodule update --init-active sub0 &&
+ test_cmp expect actual
+'
test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 08/11] clone: add --submodule-spec=<pathspec> switch
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
The new switch passes the pathspec to `git submodule update
--init-active` which is called after the actual clone is done.
Additionally this configures the submodule.active option to
be the given pathspec, such that any future invocation of
`git submodule update --init-active` will keep up
with the pathspec.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-clone.txt | 23 ++++++++++-----
builtin/clone.c | 36 +++++++++++++++++++++--
t/t7400-submodule-basic.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 120 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 35cc34b2f..9692eab30 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -15,7 +15,8 @@ SYNOPSIS
[--dissociate] [--separate-git-dir <git dir>]
[--depth <depth>] [--[no-]single-branch]
[--recursive | --recurse-submodules] [--[no-]shallow-submodules]
- [--jobs <n>] [--] <repository> [<directory>]
+ [--submodule-spec <pathspec>] [--jobs <n>] [--]
+ <repository> [<directory>]
DESCRIPTION
-----------
@@ -217,12 +218,20 @@ objects from the source repository into a pack in the cloned repository.
--recursive::
--recurse-submodules::
- After the clone is created, initialize all submodules within,
- using their default settings. This is equivalent to running
- `git submodule update --init --recursive` immediately after
- the clone is finished. This option is ignored if the cloned
- repository does not have a worktree/checkout (i.e. if any of
- `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+ After the clone is created, initialize and clone all submodules
+ within, using their default settings. This is equivalent to
+ running `git submodule update --recursive --init` immediately
+ after the clone is finished. This option is ignored if the
+ cloned repository does not have a worktree/checkout (i.e. if
+ any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+
+--submodule-spec::
+ After the clone is created, initialize and clone specified
+ submodules within, using their default settings. It is possible
+ to give multiple specifications by giving this argument multiple
+ times. This is equivalent to configuring `submodule.active`
+ and then running `git submodule update --init-active`
+ immediately after the clone is finished.
--[no-]shallow-submodules::
All submodules which are cloned will be shallow with a depth of 1.
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ef81927a..15a8d3ef0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,6 +56,16 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
static int option_dissociate;
static int max_jobs = -1;
+static struct string_list submodule_spec;
+
+static int submodule_spec_cb(const struct option *opt, const char *arg, int unset)
+{
+ if (unset)
+ return -1;
+
+ string_list_append((struct string_list *)opt->value, arg);
+ return 0;
+}
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -112,6 +122,9 @@ static struct option builtin_clone_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+ OPT_CALLBACK(0, "submodule-spec", &submodule_spec, N_("<pathspec>"),
+ N_("clone specific submodules. Pass multiple times for complex pathspecs"),
+ submodule_spec_cb),
OPT_END()
};
@@ -733,13 +746,21 @@ static int checkout(int submodule_progress)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
sha1_to_hex(sha1), "1", NULL);
- if (!err && option_recursive) {
+ if (!err && (option_recursive || submodule_spec.nr > 0)) {
struct argv_array args = ARGV_ARRAY_INIT;
- argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
+ argv_array_pushl(&args, "submodule", "update", NULL);
+
+ if (submodule_spec.nr > 0)
+ argv_array_pushf(&args, "--init-active");
+ else
+ argv_array_pushf(&args, "--init");
if (option_shallow_submodules == 1)
argv_array_push(&args, "--depth=1");
+ if (option_recursive)
+ argv_array_pushf(&args, "--recursive");
+
if (max_jobs != -1)
argv_array_pushf(&args, "--jobs=%d", max_jobs);
@@ -887,6 +908,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_no_checkout = 1;
}
+ if (submodule_spec.nr > 0) {
+ struct string_list_item *item;
+ struct strbuf sb = STRBUF_INIT;
+ for_each_string_list_item(item, &submodule_spec) {
+ strbuf_addf(&sb, "submodule.active=%s",
+ item->string);
+ string_list_append(&option_config,
+ strbuf_detach(&sb, NULL));
+ }
+ }
+
if (!option_origin)
option_origin = "origin";
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index f5df0a5b4..39364ef68 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1156,4 +1156,74 @@ test_expect_success 'submodule update --init-active' '
test_cmp expect actual
'
+test_expect_success 'clone --submodule-spec works' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ cat >expected <<-\EOF &&
+ sub0 (test2)
+ -sub1
+ -sub2
+ -sub3
+ EOF
+
+ git clone --recurse-submodules --submodule-spec="sub0" multisuper multisuper_clone &&
+ git -C multisuper_clone submodule status |cut -c1,43- >actual &&
+ test_cmp actual expected
+'
+
+test_expect_success 'clone with multiple --submodule-spec options' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ cat >expect <<-\EOF &&
+ -sub0
+ sub1 (test2)
+ -sub2
+ sub3 (test2)
+ EOF
+
+ git clone --recurse-submodules \
+ --submodule-spec="." \
+ --submodule-spec ":(exclude)sub0" \
+ --submodule-spec ":(exclude)sub2" \
+ multisuper multisuper_clone &&
+ git -C multisuper_clone submodule status |cut -c1,43- >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone and subsequent updates correctly auto-initialize submodules' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ cat <<-\EOF >expect &&
+ -sub0
+ sub1 (test2)
+ -sub2
+ sub3 (test2)
+ EOF
+
+ cat <<-\EOF >expect2 &&
+ -sub0
+ sub1 (test2)
+ -sub2
+ sub3 (test2)
+ -sub4
+ sub5 (test2)
+ EOF
+
+ git clone --recurse-submodules \
+ --submodule-spec="." \
+ --submodule-spec ":(exclude)sub0" \
+ --submodule-spec ":(exclude)sub2" \
+ --submodule-spec ":(exclude)sub4" \
+ multisuper multisuper_clone &&
+
+ git -C multisuper_clone submodule status |cut -c1,43- >actual &&
+ test_cmp expect actual &&
+
+ git -C multisuper submodule add ../sub1 sub4 &&
+ git -C multisuper submodule add ../sub1 sub5 &&
+ git -C multisuper commit -m "add more submodules" &&
+ # obtain the new superproject
+ git -C multisuper_clone pull &&
+ git -C multisuper_clone submodule update --init-active &&
+ git -C multisuper_clone submodule status |cut -c1,43- >actual &&
+ test_cmp expect2 actual
+'
+
test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH v2 10/11] submodule--helper init: set submodule.<name>.active
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
When initializing a submodule set the submodule.<name>.active config to
true to indicate that the submodule is active.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/submodule--helper.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bceb62a20..44f2c02ba 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -329,6 +329,13 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
die(_("No url found for submodule path '%s' in .gitmodules"),
displaypath);
+ /* Set active flag for the submodule being initialized */
+ if (!is_submodule_initialized(path)) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "submodule.%s.active", sub->name);
+ git_config_set_gently(sb.buf, "true");
+ }
+
/*
* Copy url setting when it is not set yet.
* To look up the url in .git/config, we must not fall back to
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH] read-tree: mark a file-local symbol as static
From: Ramsay Jones @ 2017-03-09 1:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Stefan,
If you need to re-roll your 'sb/checkout-recurse-submodules' branch,
could you please squash this into the relevant patch (commit 97aadb19f1,
"builtin/read-tree: add --recurse-submodules switch", 06-03-2017).
ATB,
Ramsay Jones
builtin/read-tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 2dc5cc06d..23e212ee8 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -21,7 +21,7 @@
static int nr_trees;
static int read_empty;
static struct tree *trees[MAX_UNPACK_TREES];
-int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int list_tree(unsigned char *sha1)
{
--
2.12.0
^ permalink raw reply related
* Re: [PATCH v2 03/11] submodule deinit: use most reliable url
From: Stefan Beller @ 2017-03-09 1:56 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170309012345.180702-4-bmwill@google.com>
On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams <bmwill@google.com> wrote:
> The user could have configured the submodule to have a different URL
> from the one in the superproject's config. To account for this read
> what the submodule has configured for remote.origin.url and use that
> instead.
When reading this commit message, I first thought this is an unrelated
bug fix. However it is not unrelated. In a later patch you introduce a mode
where submodule.<name>.url may not exist in .git/config any more,
(there may be a .existence flag instead?) such that url="", which is
obviously a bad UI:
Submodule '$name' (<empty string>) unregistered for path '$displaypath'"
Like the first patch of this series, we could have a helper function
"git submodule--helper url <name>" that figures out how to get the URL:
1) Look into that GIT_DIR
2) if non-existent check .git/config for the URL or
3) fall back to .gitmodules?
Instead of having such a helper, we directly look into that first option
hoping it exists, however it doesn't have to:
git submodule init <ps>
# no command in between
git submodule deinit <ps>
# submodule was not cloned yet, but we still test positive for
> # Remove the .git/config entries (unless the user already did it)
> if test -n "$(git config --get-regexp submodule."$name\.")"
I am not sure if there is an easy way out here.
Thinking about the name for such a url helper lookup, I wonder if
we want to have
git submodule--helper show-url <name>
as potentially we end up having this issue for a lot
of different things (e.g. submodule.<name>.shallow = (true,false),
or in case the submodule is cloned you can give the actual depth
as an integral number), so maybe we'd want to introduce one
layer of indirection
git submodule--helper ask-property \
(is-active/URL/depth/size/..) <name>
So with that said, I wonder if we also want to ease up:
GIT_DIR="$(git rev-parse --git-path modules/$name
to be
GIT_DIR=$(git submodule--helper show-git-dir $name)
> then
> # Remove the whole section so we have a clean state when
> # the user later decides to init this submodule again
> - url=$(git config submodule."$name".url)
> + url=$(GIT_DIR="$(git rev-parse --git-path modules/$name)" git config remote.origin.url)
In the submodule helper we have get_default_remote(), which we do not
have in shell
(which we seemed to have in shell?), so maybe it's worth not using origin here,
although I guess it is rare that the original remote is named other than origin.
> git config --remove-section submodule."$name" 2>/dev/null &&
> say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
> fi
> --
> 2.12.0.246.ga2ecc84866-goog
>
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v2 07/11] submodule update: add `--init-active` switch
From: Stefan Beller @ 2017-03-09 2:16 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170309012345.180702-8-bmwill@google.com>
On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams <bmwill@google.com> wrote:
> The new switch `--init-active` initializes the submodules which are
> configured in `submodule.active` instead of those given as
> command line arguments before updating. In the first implementation this
> is made incompatible with further command line arguments as it is
> unclear what the user means by
>
> git submodule update --init --init-active <paths>
>
> This new switch allows users to record more complex patterns as it saves
> retyping them whenever you invoke update.
>
> Based on a patch by Stefan Beller <sbeller@google.com>
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> @@ -568,7 +573,17 @@ cmd_update()
>
> if test -n "$init"
> then
> - cmd_init "--" "$@" || return
> + if test "$init" = "by_config"
> + then
> + if test $# -gt 0
> + then
> + die "$(gettext "path arguments are incompatible with --init-active")"
> + fi
> + cmd_init "--" $(git config --get-all submodule.active) || return
What happens with submodule.<name>.actives here?
^ permalink raw reply
* Re: [PATCH v2 10/11] submodule--helper init: set submodule.<name>.active
From: Stefan Beller @ 2017-03-09 2:28 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170309012345.180702-11-bmwill@google.com>
On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams <bmwill@google.com> wrote:
> When initializing a submodule set the submodule.<name>.active config to
> true to indicate that the submodule is active.
So by this patch an init of a submodule performs both
a "regular init" (copy URL and the update setting)
as well as setting the new submodule.<name>.active.
And both happens in the same config file (i.e. no worktree
support here)
Later on we *could* remove the URL as the .active is the new
flag of existence.
But enough of my speculation, I am left wondering what this
patch accomplishes, as this states no agenda as why it is useful
on its own.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> builtin/submodule--helper.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index bceb62a20..44f2c02ba 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -329,6 +329,13 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
> die(_("No url found for submodule path '%s' in .gitmodules"),
> displaypath);
>
> + /* Set active flag for the submodule being initialized */
> + if (!is_submodule_initialized(path)) {
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "submodule.%s.active", sub->name);
> + git_config_set_gently(sb.buf, "true");
> + }
> +
> /*
> * Copy url setting when it is not set yet.
> * To look up the url in .git/config, we must not fall back to
> --
> 2.12.0.246.ga2ecc84866-goog
>
^ permalink raw reply
* [PATCH] Allow "-" as a short-hand for "@{-1}" in branch deletions
From: Shuyang Shi @ 2017-03-09 3:30 UTC (permalink / raw)
To: git
The "-" shorthand that stands for "the branch we were previously on",
like we did for "git merge -" sometime after we introduced "git checkout -".
Now I am introducing this shorthand to branch delete, i.e.
"git branch -d -".
More reference:
https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
And this has been tested:
Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
[00:21:26] ./t3200-branch.sh .. ok 12293 ms ( 0.04 usr 0.01 sys +
5.97 cusr 2.52 csys = 8.54 CPU)
[00:21:39]
All tests successful.
Files=1, Tests=113, 13 wallclock secs ( 0.07 usr 0.02 sys +
5.97 cusr 2.52 csys = 8.58 CPU)
Result: PASS
Signed-off-by: Shuyang Shi <shuyang790@gmail.com>
---
builtin/branch.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7f..1b72d80 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,8 +215,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
for (i = 0; i < argc; i++, strbuf_release(&bname)) {
char *target = NULL;
int flags = 0;
+ const char *arg = argv[i];
- strbuf_branchname(&bname, argv[i]);
+ if (!strcmp(arg, "-"))
+ arg = "@{-1}";
+
+ strbuf_branchname(&bname, arg);
free(name);
name = mkpathdup(fmt, bname.buf);
--
https://github.com/git/git/pull/337
^ permalink raw reply related
* [PATCH GSoC] Allow "-" as a short-hand for "@{-1}" in branch deletions
From: Shuyang Shi @ 2017-03-09 3:31 UTC (permalink / raw)
To: git
In-Reply-To: <0102015ab11e8237-01e52ffe-882f-4589-8886-2c0b231ac3c6-000000@eu-west-1.amazonses.com>
The "-" shorthand that stands for "the branch we were previously on",
like we did for "git merge -" sometime after we introduced "git checkout -".
Now I am introducing this shorthand to branch delete, i.e.
"git branch -d -".
More reference:
https://public-inbox.org/git/7vppuewl6h.fsf@alter.siamese.dyndns.org/
And this has been tested:
Ivan:git Ivan$ (cd t; prove --timer --jobs 1 ./t3200-branch.sh)
[00:21:26] ./t3200-branch.sh .. ok 12293 ms ( 0.04 usr 0.01 sys +
5.97 cusr 2.52 csys = 8.54 CPU)
[00:21:39]
All tests successful.
Files=1, Tests=113, 13 wallclock secs ( 0.07 usr 0.02 sys +
5.97 cusr 2.52 csys = 8.58 CPU)
Result: PASS
Signed-off-by: Shuyang Shi <shuyang790@gmail.com>
---
builtin/branch.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7f..1b72d80 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,8 +215,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
for (i = 0; i < argc; i++, strbuf_release(&bname)) {
char *target = NULL;
int flags = 0;
+ const char *arg = argv[i];
- strbuf_branchname(&bname, argv[i]);
+ if (!strcmp(arg, "-"))
+ arg = "@{-1}";
+
+ strbuf_branchname(&bname, arg);
free(name);
name = mkpathdup(fmt, bname.buf);
--
https://github.com/git/git/pull/337
^ permalink raw reply related
* Re: [PATCH] submodule--helper.c: remove duplicate code
From: Valery Tolstov @ 2017-03-08 23:05 UTC (permalink / raw)
To: sbeller; +Cc: git, me
In-Reply-To: <CAGZ79kbnpUtrKdjQdQ-r6rRuVvnawooLFk1bO8jOSgxNkx2Dbg@mail.gmail.com>
> Then the next step (as outlined by Documentation/SubmittingPatches)
> is to figure out how to best present this to the mailing list; I think the best
> way is to send out a patch series consisting of both of these 2 patches,
> the "connect_work_tree_and_git_dir: safely create leading directories,"
> first and then your deduplication patch.
Is there a handy way to forward your patch in new patch series? Also,
should I start new thread for new patch series?
Regards,
Valery Tolstov
^ permalink raw reply
* [PATCH v2 03/11] submodule deinit: use most reliable url
From: Brandon Williams @ 2017-03-09 1:23 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster
In-Reply-To: <20170309012345.180702-1-bmwill@google.com>
The user could have configured the submodule to have a different URL
from the one in the superproject's config. To account for this read
what the submodule has configured for remote.origin.url and use that
instead.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
git-submodule.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 19660b9c0..1c2064cc1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -455,7 +455,7 @@ Submodule work tree '\$displaypath' contains a .git directory
then
# Remove the whole section so we have a clean state when
# the user later decides to init this submodule again
- url=$(git config submodule."$name".url)
+ url=$(GIT_DIR="$(git rev-parse --git-path modules/$name)" git config remote.origin.url)
git config --remove-section submodule."$name" 2>/dev/null &&
say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
fi
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH][GSoc] Changed signed flags to unsigned type
From: Vedant Bassi @ 2017-03-09 4:24 UTC (permalink / raw)
To: git
As part of my microproject :
Use unsigned integral type for collection of bits:
Pick one field of a structure that (1) is of signed integral type and
(2) is used as a collection of multiple bits. Discuss if there is a
good reason why it has to be a signed integral field and change it to
an unsigned type otherwise.
More ref: https://public-inbox.org/git/xmqqsiebrlez.fsf@gitster.dls.corp.google.com
http://stackoverflow.com/questions/29795170/usage-of-signed-vs-unsigned-variables-for-flags-in-c
I have found several structures where a signed int was used on flags
for bitwise & to check various cases.
diff --git a/bisect.h b/bisect.h
index a979a7f..4b562a8 100644
--- a/bisect.h
+++ b/bisect.h
@@ -16,6 +16,8 @@ extern struct commit_list *filter_skipped(struct
commit_list *list,
struct rev_list_info {
struct rev_info *revs;
+
+ // int flags changed to unsigned int
unsigned int flags;
int show_timestamp;
int hdr_termination;
diff --git a/builtin/add.c b/builtin/add.c
index 9f53f02..1212eea 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,6 +26,8 @@ static int patch_interactive, add_interactive,
edit_interactive;
static int take_worktree_changes;
struct update_callback_data {
+
+ // flags supposed to be unsigned
int flags;
int add_errors;
};
diff --git a/parse-options.h b/parse-options.h
index dcd8a09..ad180c9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -107,7 +107,9 @@ struct option {
const char *argh;
const char *help;
- int flags;
+ // int flags changed to unsigned
+
+ unsigned int flags;
parse_opt_cb *callback;
intptr_t defval;
};
@@ -201,7 +203,10 @@ struct parse_opt_ctx_t {
const char **out;
int argc, cpidx, total;
const char *opt;
- int flags;
+
+ // int flags changed to unsigned
+
+ unsigned int flags;
const char *prefix;
};
-------
result : the changes were made in bisect.h , parse-options.h and builtin/add.c
I have not yet tested these changes.
^ permalink raw reply
* Re: [PATCH] t*: avoid using pipes
From: Prathamesh Chavan @ 2017-03-09 5:26 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Johannes Sixt, git@vger.kernel.org, Pranit Bauva
In-Reply-To: <CAME+mvUzR6--AeUff6yGZ69GN-hE6AyDP-CkdFxnFAwccpn2yg@mail.gmail.com>
I have created the required changes and submitted a single file patch.
Also I tried my best to include each of the suggestions in that patch.
https://public-inbox.org/git/0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com/
On Wed, Mar 8, 2017 at 7:02 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> On Wed, Mar 8, 2017 at 11:33 AM, Jeff King <peff@peff.net> wrote:
>> On Tue, Mar 07, 2017 at 12:52:49PM -0800, Stefan Beller wrote:
>>
>>> On Tue, Mar 7, 2017 at 12:39 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>>
>>> > Welcome to the Git community!
>>>
>>> >
>>> > Actually, being a *micro* project, it should stay so. Not doing all of the
>>> > changes would leave some tasks for other apprentices to get warm with our
>>> > review process.
>>>
>>> right, so just pick one file.
>>
>> I also wonder if we really want all invocations of git to be marked up
>> in this way. If the primary goal of the test is checking that a certain
>> git command runs successfully and generates the expected output, then I
>> think it is a good candidate for conversion.
>>
>> So in a hunk like this:
>>
>> test_expect_success 'git commit-tree records the correct tree in a commit' '
>> commit0=$(echo NO | git commit-tree $P) &&
>> - tree=$(git show --pretty=raw $commit0 |
>> - sed -n -e "s/^tree //p" -e "/^author /q") &&
>> + tree=$(git show --pretty=raw $commit0 >out &&
>> + sed -n -e "s/^tree //p" -e "/^author /q" <out) &&
>> test "z$tree" = "z$P"
>>
>> we are interested in testing commit-tree, not "git show". Is it worth
>> avoiding pipes there? I admit the cost to using the intermediate file is
>> not huge there, but it feels more awkward and un-shell-like to me as a
>> reader.
>>
>> -Peff
>
> Thank you everyone, for reviewing my changes. And as said in the
> reviews, I'll send a single patch file as my microproject, leaving the other
> files as low hanging fruit for the others to look at. Also, I try to include as
> many suggested improvements as possible and will also remember them for
> my future patches.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox