* [PATCH 15/15] builtin/checkout: add --recurse-submodules switch
From: Stefan Beller @ 2017-02-23 22:57 UTC (permalink / raw)
To: sbeller; +Cc: git, sandals, jrnieder, bmwill, gitster, novalis
In-Reply-To: <20170223225735.10994-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-checkout.txt | 7 +++++++
builtin/checkout.c | 28 ++++++++++++++++++++++++++++
t/lib-submodule-update.sh | 33 ++++++++++++++++++++++++---------
t/t2013-checkout-submodule.sh | 5 +++++
4 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8e2c0662dd..d6399c0af8 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -256,6 +256,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
out anyway. In other words, the ref can be held by more than one
worktree.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject. If
+ local modifications in a submodule would be overwritten the checkout
+ will fail unless `-f` is used. If nothing (or --no-recurse-submodules)
+ is used, the work trees of submodules will not be updated.
+
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f174f50303..207ce09771 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,12 +21,31 @@
#include "submodule-config.h"
#include "submodule.h"
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
static const char * const checkout_usage[] = {
N_("git checkout [<options>] <branch>"),
N_("git checkout [<options>] [<branch>] -- <file>..."),
NULL,
};
+int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
struct checkout_opts {
int patch_mode;
int quiet;
@@ -1163,6 +1182,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
N_("second guess 'git checkout <no-such-branch>'")),
OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the given ref")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
OPT_END(),
};
@@ -1193,6 +1215,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
+ if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+ git_config(submodule_config, NULL);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+ set_config_update_recurse_submodules(recurse_submodules);
+ }
+
if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
die(_("-b, -B and --orphan are mutually exclusive"));
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 0b26f0e20f..54cd8a6366 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -756,6 +756,11 @@ test_submodule_forced_switch () {
test_submodule_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -865,7 +870,7 @@ test_submodule_switch_recursing () {
'
# Replacing a submodule with files in a directory must succeeds
# when the submodule is clean
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -877,7 +882,7 @@ test_submodule_switch_recursing () {
)
'
# ... absorbing a .git directory.
- test_expect_success "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
+ test_expect_$RESULT "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -905,7 +910,7 @@ test_submodule_switch_recursing () {
'
# ... must check its local work tree for untracked files
- test_expect_success "$command: replace submodule with a file must fail with untracked files" '
+ test_expect_$RESULT "$command: replace submodule with a file must fail with untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -961,16 +966,21 @@ test_submodule_switch_recursing () {
)
'
+ # This test fails, due to missing setup, we do not clone sub2 into
+ # submodule_update, because it doesn't exist in the 'add_sub1' version
+ #
test_expect_success "$command: modified submodule updates submodule recursively" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
cd submodule_update &&
git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
- $command modify_sub1_recursively &&
- test_superproject_content origin/modify_sub1_recursively &&
- test_submodule_content sub1 origin/modify_sub1_recursively
- test_submodule_content sub1/sub2
+ test_must_fail $command modify_sub1_recursively &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ # test_superproject_content origin/modify_sub1_recursively &&
+ # test_submodule_content sub1 origin/modify_sub1_recursively &&
+ # test_submodule_content sub1/sub2 no_submodule
)
'
}
@@ -980,6 +990,11 @@ test_submodule_switch_recursing () {
# the superproject as well as the submodule is allowed.
test_submodule_forced_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear creates empty dir ...
test_expect_success "$command: added submodule is checked out" '
@@ -1074,7 +1089,7 @@ test_submodule_forced_switch_recursing () {
)
'
# Replacing a submodule with files in a directory ...
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -1125,7 +1140,7 @@ test_submodule_forced_switch_recursing () {
'
# ... but stops for untracked files that would be lost
- test_expect_success "$command: replace submodule with a file" '
+ test_expect_$RESULT "$command: replace submodule with a file stops for untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 6847f75822..aa35223369 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -63,6 +63,11 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+test_submodule_switch_recursing "git checkout --recurse-submodules"
+
+test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
+
test_submodule_switch "git checkout"
test_submodule_forced_switch "git checkout -f"
--
2.12.0.rc1.16.ge4278d41a0.dirty
^ permalink raw reply related
* Re: [PATCH 1/3] add collision-detecting sha1 implementation
From: Stefan Beller @ 2017-02-23 23:15 UTC (permalink / raw)
To: Jeff King; +Cc: Linus Torvalds, Joey Hess, Git Mailing List
In-Reply-To: <20170223230536.tdmtsn46e4lnrimx@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 3:05 PM, Jeff King <peff@peff.net> wrote:
> +* Copyright 2017 Marc Stevens <marc@marc-stevens.nl>, Dan Shumow (danshu@microsoft.com)
> +* Distributed under the MIT Software License.
> +* See accompanying file LICENSE.txt or copy at
The accompanying LICENSE file did not make it into this patch,
that is more specialized/verbose than the one at
https://opensource.org/licenses/MIT
w.r.t. copyright notice requirement.
Apart from that MIT seems to be compatible with GPL
according to the FSF, though IANAL.
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Junio C Hamano @ 2017-02-23 23:19 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqo9xt8jcf.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> FWIW, the code looks OK here. It is a shame to duplicate the policy
>> found in git_config_parse_key(), though.
>>
>> I wonder if we could make a master version of that which canonicalizes
>> in-place, and then just wrap it for the git_config_parse_key()
>> interface. Actually, I guess the function you just wrote _is_ that inner
>> function, as long as it learned about the "quiet" flag.
>
> Hmm, I obviously missed an opportunity....
> ...
> But you are right. config-parse-key does have the simpler string
> that can just be given to the canonicalize thing and we should be
> able to reuse it.
Actually, I think we can just use the existing config_parse_key()
instead of adding the new function. It adds one allocation and
deallocation, but it's not like this is a performance-critical
codepath that we absolutely avoid extra allocations. After all, we
are still using the strbuf-split thing :-/.
The attached patch shows the updated fix. It needs a preparatory
code move (not shown here) to make git_config_parse_key() available
to git_config_parse_parameter(), though.
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 23 Feb 2017 15:04:40 -0800
Subject: [PATCH] config: reject invalid VAR in 'git -c VAR=VAL command' and
keep subsection intact
The parsing of one-shot assignments of configuration variables that
come from the command line historically was quite loose and allowed
anything to pass. It also downcased everything in the variable name,
even a three-level <section>.<subsection>.<variable> name in which
the <subsection> part must be treated in a case sensible manner.
Existing git_config_parse_key() helper is used to parse the variable
name that comes from the command line, i.e. "git config VAR VAL",
and handles these details correctly. Replace the strbuf_tolower()
call in git-config_parse_parameter() with a call to it to correct
both issues. git_config_parse_key() does a bit more things that are
not necessary for the purpose of this codepath (e.g. it allocates a
separate buffer to return the canonicalized variable name because it
takes a "const char *" input), but we are not in a performance-critical
codepath here.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
config.c | 14 ++++++++----
t/t1300-repo-config.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c
index b8cce1dffa..39f20dcd2c 100644
--- a/config.c
+++ b/config.c
@@ -295,7 +295,9 @@ int git_config_parse_parameter(const char *text,
config_fn_t fn, void *data)
{
const char *value;
+ char *canonical_name;
struct strbuf **pair;
+ int ret = 0;
pair = strbuf_split_str(text, '=', 2);
if (!pair[0])
@@ -313,13 +315,15 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error("bogus config parameter: %s", text);
}
- strbuf_tolower(pair[0]);
- if (fn(pair[0]->buf, value, data) < 0) {
- strbuf_list_free(pair);
+
+ if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL))
return -1;
- }
+
+ ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
+
+ free(canonical_name);
strbuf_list_free(pair);
- return 0;
+ return ret;
}
int git_config_from_parameters(config_fn_t fn, void *data)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a26..ea371020fa 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1097,6 +1097,68 @@ test_expect_success 'multiple git -c appends config' '
test_cmp expect actual
'
+test_expect_success 'last one wins: two level vars' '
+
+ # sec.var and sec.VAR are the same variable, as the first
+ # and the last level of a configuration variable name is
+ # case insensitive.
+
+ echo VAL >expect &&
+
+ git -c sec.var=val -c sec.VAR=VAL config --get sec.var >actual &&
+ test_cmp expect actual &&
+ git -c SEC.var=val -c sec.var=VAL config --get sec.var >actual &&
+ test_cmp expect actual &&
+
+ git -c sec.var=val -c sec.VAR=VAL config --get SEC.var >actual &&
+ test_cmp expect actual &&
+ git -c SEC.var=val -c sec.var=VAL config --get sec.VAR >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'last one wins: three level vars' '
+
+ # v.a.r and v.A.r are not the same variable, as the middle
+ # level of a three-level configuration variable name is
+ # case sensitive.
+
+ echo val >expect &&
+ git -c v.a.r=val -c v.A.r=VAL config --get v.a.r >actual &&
+ test_cmp expect actual &&
+ git -c v.a.r=val -c v.A.r=VAL config --get V.a.R >actual &&
+ test_cmp expect actual &&
+
+ # v.a.r and V.a.R are the same variable, as the first
+ # and the last level of a configuration variable name is
+ # case insensitive.
+
+ echo VAL >expect &&
+ git -c v.a.r=val -c v.a.R=VAL config --get v.a.r >actual &&
+ test_cmp expect actual &&
+ git -c v.a.r=val -c V.a.r=VAL config --get v.a.r >actual &&
+ test_cmp expect actual &&
+ git -c v.a.r=val -c v.a.R=VAL config --get V.a.R >actual &&
+ test_cmp expect actual &&
+ git -c v.a.r=val -c V.a.r=VAL config --get V.a.R >actual &&
+ test_cmp expect actual
+'
+
+for VAR in a .a a. a.0b a."b c". a."b c".0d
+do
+ test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
+ test_must_fail git -c "$VAR=VAL" config -l
+ '
+done
+
+for VAR in a.b a."b c".d
+do
+ test_expect_success "git -c $VAR=VAL works with valid '$VAR'" '
+ echo VAL >expect &&
+ git -c "$VAR=VAL" config --get "$VAR" >actual &&
+ test_cmp expect actual
+ '
+done
+
test_expect_success 'git -c is not confused by empty environment' '
GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
'
--
2.12.0-rc2-289-g9f26f1516a
^ permalink raw reply related
* Re: SHA1 collisions found
From: Linus Torvalds @ 2017-02-23 23:14 UTC (permalink / raw)
To: Jeff King; +Cc: Joey Hess, Git Mailing List
In-Reply-To: <20170223230507.kuxjqtg3ghcfskc6@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 3:05 PM, Jeff King <peff@peff.net> wrote:
>
> (By the way, I don't see your version on the list, Linus, which probably
> means it was eaten by the 100K filter).
Ahh. I didn't even think about a size filter.
Doesn't matter, your version looks fine.
Linus
^ permalink raw reply
* [PATCH 06/10] submodule add: respect submodule.active
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-1-bmwill@google.com>
When submodule.active is configured, in addition to adding
submodule."<name>".url to the config, add the path of the added
submodule if it isn't already covered by the current config values.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
git-submodule.sh | 11 +++++++++++
t/t7413-submodule-is-active.sh | 22 ++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/git-submodule.sh b/git-submodule.sh
index e5b9a8920..4633a4336 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -271,6 +271,17 @@ 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, add the submodule's path to
+ # 'submodule.active'
+ if ! git submodule--helper is-active "$sm_path"
+ then
+ git config --add submodule.active "$sm_path"
+ fi
+ fi
}
#
diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
index 683487020..8a519163b 100755
--- a/t/t7413-submodule-is-active.sh
+++ b/t/t7413-submodule-is-active.sh
@@ -60,4 +60,26 @@ test_expect_success 'is-active works with exclusions in submodule.active config'
git -C super config --unset-all submodule.active
'
+test_expect_success 'is-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*" &&
+
+ cat >expect <<-\EOF &&
+ sub*
+ EOF
+ git -C super2 submodule add ../sub sub1 &&
+ git -C super2 config --get-all submodule.active >actual &&
+ test_cmp actual expect &&
+
+ cat >expect <<-\EOF &&
+ sub*
+ mod
+ EOF
+ git -C super2 submodule add ../sub mod &&
+ git -C super2 config --get-all submodule.active >actual &&
+ test_cmp actual expect
+'
+
test_done
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 00/10] decoupling a submodule's existence and its url
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
There are two motivations for decoupling a submodule's existence from the url
that is stored in .git/config.
1. Worktrees can't really be used with submodules since the existence is
checked based on the shared .git/config. This means that two different
worktress have to have the same initialized submodules. By decoupling a
submodule's existence from the url, two different work trees can be
configured to have different submodules checked out.
2. Easily configure gorups of submodules that a user is interested in. In a
repository with hundreds of submodules it would be difficult to easily tell git
which modules to worry about without having to individually init all of
them. Instead, a pathspec can be used to more easily select or deselect
groups of submodules.
This patch series works to do this decoupling and instead allows a user to
configure submodule.active with a pathspec to use to check if a submodule is
initialized.
Brandon Williams (10):
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: add is_active command
submodule add: respect submodule.active
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
Documentation/git-clone.txt | 23 ++++---
Documentation/git-submodule.txt | 11 +++-
builtin/clone.c | 36 ++++++++++-
builtin/submodule--helper.c | 22 ++++---
contrib/completion/git-completion.bash | 1 +
git-submodule.sh | 39 +++++++++---
submodule.c | 25 +++++++-
t/t7400-submodule-basic.sh | 109 +++++++++++++++++++++++++++++++++
t/t7413-submodule-is-active.sh | 85 +++++++++++++++++++++++++
9 files changed, 323 insertions(+), 28 deletions(-)
create mode 100755 t/t7413-submodule-is-active.sh
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply
* [PATCH 09/10] submodule sync: use submodule--helper is-active
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 02b85dceb..f35345775 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1127,7 +1127,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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 04/10] completion: clone can initialize specific submodules
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 01/10] submodule: decouple url and submodule existence
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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, the config option submodule.active is introduced which
holds a pathspec that specifies which submodules should exist in the
working tree.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4c4f033e8..991066ddf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -217,11 +217,16 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)
int is_submodule_initialized(const char *path)
{
int ret = 0;
- const struct submodule *module = NULL;
+ 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;
+
+ sl = git_config_get_value_multi("submodule.active");
- if (module) {
+ if (!sl) {
char *key = xstrfmt("submodule.%s.url", module->name);
char *value = NULL;
@@ -229,6 +234,20 @@ int is_submodule_initialized(const char *path)
free(value);
free(key);
+ } else {
+ 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);
}
return ret;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 03/10] clone: add --submodule-spec=<pathspec> switch
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 07/10] submodule status: use submodule--helper is-active
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 4633a4336..f8adfb179 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1026,14 +1026,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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 08/10] submodule deinit: use most reliable url
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 f8adfb179..02b85dceb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -466,7 +466,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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 10/10] submodule--helper clone: check for configured submodules using helper
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dac02604d..68d21b600 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)) {
@@ -795,14 +794,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
}
/*
- * Looking up the url in .git/config.
+ * Check if the submodule has been initialized.
* 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) {
+ if (!is_submodule_initialized(ce->name)) {
next_submodule_warn_missing(suc, out, displaypath);
goto cleanup;
}
@@ -836,7 +832,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 +842,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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 02/10] submodule update: add `--init-active` switch
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 554bd1c49..e5b9a8920 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.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 05/10] submodule--helper: add is_active command
From: Brandon Williams @ 2017-02-23 23:47 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <20170223234728.164111-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 more easily facilitate the use of the submodule.active config option
to indicate active submodules, add 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 | 63 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 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..683487020
--- /dev/null
+++ b/t/t7413-submodule-is-active.sh
@@ -0,0 +1,63 @@
+#!/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_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 config --unset submodule.sub2.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 submodule.sub2.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_done
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: [BUG] allowtipsha1inwant serves unreachable blobs if you know its hash
From: Junio C Hamano @ 2017-02-23 23:50 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, peff, peartben, benpeart
In-Reply-To: <20170223230358.30050-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> Adding "--objects" works, and all existing tests pass, except for the
> potential performance issue and the side effect that even fetching a
> reachable blob no longer works. This is due to a possible bug where a
> call like "git rev-list --objects $tree ^master" (where $tree is the
> tree object corresponding to master) prints out objects ...
The "reachable from this, excluding what is reachable from that"
notation was originally designed to work only on commits, and I
wouldn't be surprised if "$tree ^master" did not work as you expect
in the current implementation.
I agree that ideally it shouldn't show anything, but I suspect that
it would make it very expensive if done naively---we'd end up having
to call mark_tree_uninteresting() for all uninteresting commits, not
just for the commits at the edge of the DAG as we do right now.
^ permalink raw reply
* Re: [PATCH 00/10] decoupling a submodule's existence and its url
From: Stefan Beller @ 2017-02-23 23:58 UTC (permalink / raw)
To: Brandon Williams, Duy Nguyen; +Cc: git@vger.kernel.org
In-Reply-To: <20170223234728.164111-1-bmwill@google.com>
On Thu, Feb 23, 2017 at 3:47 PM, Brandon Williams <bmwill@google.com> wrote:
> There are two motivations for decoupling a submodule's existence from the url
> that is stored in .git/config.
>
> 1. Worktrees can't really be used with submodules since the existence is
> checked based on the shared .git/config. This means that two different
> worktress have to have the same initialized submodules. By decoupling a
> submodule's existence from the url, two different work trees can be
> configured to have different submodules checked out.
cc Duy for this one:
Well once we have the per-worktree configuration, e.g. [1] we can *technically*
have different submodules in different worktrees by saying
workingtree0: submodule.<name>.url = git://example.org/real-submodule
workingtree1: submodule.<name>.url = bogus
workingtree2: submodule.<name>.url = more bogus
and once we used the URL in the first workingtree all urls are
degraded to a boolean flag,
so the different (possible bogus) content does not do harm, only user-confusion,
because the model of the URL being the flag indicating existence doesn't quite
fit multiple working trees.
[1] https://public-inbox.org/git/20170110112524.12870-3-pclouds@gmail.com/
> 2. Easily configure gorups of submodules that a user is interested in. In a
> repository with hundreds of submodules it would be difficult to easily tell git
> which modules to worry about without having to individually init all of
> them. Instead, a pathspec can be used to more easily select or deselect
> groups of submodules.
>
> This patch series works to do this decoupling and instead allows a user to
> configure submodule.active with a pathspec to use to check if a submodule is
> initialized.
Thanks for stating both intentions!
Stefan
^ permalink raw reply
* Re: [PATCH 1/3] add collision-detecting sha1 implementation
From: Jeff King @ 2017-02-24 0:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: Linus Torvalds, Joey Hess, Git Mailing List
In-Reply-To: <CAGZ79kZHPdBTKEqJeAa5xDcsC5v9x4DdUuDOiRNSgOV5aCx9Kw@mail.gmail.com>
On Thu, Feb 23, 2017 at 03:15:11PM -0800, Stefan Beller wrote:
> On Thu, Feb 23, 2017 at 3:05 PM, Jeff King <peff@peff.net> wrote:
>
> > +* Copyright 2017 Marc Stevens <marc@marc-stevens.nl>, Dan Shumow (danshu@microsoft.com)
> > +* Distributed under the MIT Software License.
> > +* See accompanying file LICENSE.txt or copy at
>
> The accompanying LICENSE file did not make it into this patch,
> that is more specialized/verbose than the one at
> https://opensource.org/licenses/MIT
> w.r.t. copyright notice requirement.
You know, I didn't even look at the LICENSE file, since it said MIT and
had a link here. It would be trivial to copy it over, too, of course.
> Apart from that MIT seems to be compatible with GPL
> according to the FSF, though IANAL.
Yeah, that's always been my understanding.
-Peff
^ permalink raw reply
* Re: [BUG] allowtipsha1inwant serves unreachable blobs if you know its hash
From: Jeff King @ 2017-02-24 0:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Tan, git, peartben, benpeart
In-Reply-To: <xmqqtw7k5uxj.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 03:50:00PM -0800, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
> > Adding "--objects" works, and all existing tests pass, except for the
> > potential performance issue and the side effect that even fetching a
> > reachable blob no longer works. This is due to a possible bug where a
> > call like "git rev-list --objects $tree ^master" (where $tree is the
> > tree object corresponding to master) prints out objects ...
>
> The "reachable from this, excluding what is reachable from that"
> notation was originally designed to work only on commits, and I
> wouldn't be surprised if "$tree ^master" did not work as you expect
> in the current implementation.
>
> I agree that ideally it shouldn't show anything, but I suspect that
> it would make it very expensive if done naively---we'd end up having
> to call mark_tree_uninteresting() for all uninteresting commits, not
> just for the commits at the edge of the DAG as we do right now.
Yes, it's super-expensive to do naively (like 40+ seconds of CPU on
torvalds/linux). Bitmaps should generally make it tolerable, though
there are corner cases (e.g., if a ref tip has to walk a bit to get to a
bitmap; we try to put bitmaps near the ref tips, but when you have
50,000 tags it's hard to do).
We've explored similar things at GitHub for doing reachability checks on
all 40-hex lookups (because right now I can point you to
.../git/git/commit/1234abcd and you see that object, even if it's only
in _my_ fork and not reachable from git/git).
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] add collision-detecting sha1 implementation
From: Linus Torvalds @ 2017-02-24 0:12 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Joey Hess, Git Mailing List
In-Reply-To: <20170224000143.cate5yncjq74hsys@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 4:01 PM, Jeff King <peff@peff.net> wrote:
>
> You know, I didn't even look at the LICENSE file, since it said MIT and
> had a link here. It would be trivial to copy it over, too, of course.
You should do it. It's just good to be careful and clear with
licenses, and the license text does require that the copyright notice
and permission file should be included in copies.
My patch did it. "Pats self on head".
Linus
PS. And just to be polite, we should probably also just cc at least
Marc Stevens and Dan Shumow if we take that patch further. Their email
addresses are in the that LICENSE.txt file.
^ permalink raw reply
* [PATCH] submodule init: warn about falling back to a local path
From: Stefan Beller @ 2017-02-24 0:17 UTC (permalink / raw)
To: philipoakley; +Cc: git, gitster, sop, Stefan Beller
In-Reply-To: <CAGZ79kYXyk=giuM6CnU=gnvkCw3aaVEn=WbJ6tjhn8kZumrmTA@mail.gmail.com>
When a submodule is initialized, the config variable 'submodule.<name>.url'
is set depending on the value of the same variable in the .gitmodules
file. When the URL indicates to be relative, then the url is computed
relative to its default remote. The default remote cannot be determined
accurately in all cases, such that it falls back to 'origin'.
The 'origin' remote may not exist, though. In that case we give up looking
for a suitable remote and we'll just assume it to be a local relative path.
This can be confusing to users as there is a lot of guessing involved,
which is not obvious to the user.
So in the corner case of assuming a local autoritative truth, warn the
user to lessen the confusion.
This behavior was introduced in 4d6893200 (submodule add: allow relative
repository path even when no url is set, 2011-06-06), which shared the
code with submodule-init and then ported to C in 3604242f080a (submodule:
port init from shell to C, 2016-04-15).
In case of submodule-add, this behavior makes sense in some use cases[1],
however for submodule-init there does not seem to be an immediate obvious
use case to fall back to a local submodule. However there might be, so
warn instead of die here.
While adding the warning, also clarify the behavior of relative URLs in
the documentation.
[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private
"store a secret locally in a submodule, with no intention to publish it"
Reported-by: Shawn Pearce <spearce@spearce.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 22 +++++++++++++++-------
builtin/submodule--helper.c | 8 +++-----
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8acc72ebb8..d23dee2a5b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.
+
<repository> is the URL of the new submodule's origin repository.
This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
+or ../), the location relative to the superproject's default remote
repository (Please note that to specify a repository 'foo.git'
which is located right next to a superproject 'bar.git', you'll
have to use '../foo.git' instead of './foo.git' - as one might expect
when following the rules for relative URLs - because the evaluation
of relative URLs in Git is identical to that of relative directories).
-If the superproject doesn't have an origin configured
++
+The default remote is the remote of the remote tracking branch
+of the current branch. If no such remote tracking branch exists or
+the in detached HEAD mode, "origin" is assumed to be the default remote.
+If the superproject doesn't have a default remote configured
the superproject is its own authoritative upstream and the current
working directory is used instead.
+
@@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).
init [--] [<path>...]::
Initialize the submodules recorded in the index (which were
- added and committed elsewhere) by copying submodule
- names and urls from .gitmodules to .git/config.
+ added and committed elsewhere) by copying `submodule.$name.url`
+ from .gitmodules to .git/config, resolving relative urls to be
+ relative to the default remote.
++
Optional <path> arguments limit which submodules will be initialized.
- It will also copy the value of `submodule.$name.update` into
- .git/config.
- The key used in .git/config is `submodule.$name.url`.
+ If no path is specified all submodules are initialized.
++
+ When present, it will also copy the value of `submodule.$name.update`.
This command does not alter existing information in .git/config.
You can then customize the submodule clone URLs in .git/config
for your local setup and proceed to `git submodule update`;
you can also just use `git submodule update --init` without
the explicit 'init' step if you do not intend to customize
any submodule locations.
++
+ See the add subcommand for the defintion of default remote.
deinit [-f|--force] (--all|[--] <path>...)::
Unregister the given submodules, i.e. remove the whole
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e3..44c11dd91e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
strbuf_addf(&remotesb, "remote.%s.url", remote);
free(remote);
- if (git_config_get_string(remotesb.buf, &remoteurl))
- /*
- * The repository is its own
- * authoritative upstream
- */
+ if (git_config_get_string(remotesb.buf, &remoteurl)) {
remoteurl = xgetcwd();
+ warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
+ }
relurl = relative_url(remoteurl, url, NULL);
strbuf_release(&remotesb);
free(remoteurl);
--
2.12.0.rc1.16.ge4278d41a0.dirty
^ permalink raw reply related
* Re: [PATCH 1/3] add collision-detecting sha1 implementation
From: Jeff King @ 2017-02-24 0:16 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Stefan Beller, Joey Hess, Git Mailing List
In-Reply-To: <CA+55aFx4gXEXqr-z12E4gz5fm2fye-vVio_Td7EbKxymGF2QUw@mail.gmail.com>
On Thu, Feb 23, 2017 at 04:12:01PM -0800, Linus Torvalds wrote:
> On Thu, Feb 23, 2017 at 4:01 PM, Jeff King <peff@peff.net> wrote:
> >
> > You know, I didn't even look at the LICENSE file, since it said MIT and
> > had a link here. It would be trivial to copy it over, too, of course.
>
> You should do it. It's just good to be careful and clear with
> licenses, and the license text does require that the copyright notice
> and permission file should be included in copies.
>
> My patch did it. "Pats self on head".
And that's why yours crossed the 100K barrier. :)
But yeah, I agree it is better to be safe (and that's we should contact
the authors). I'll point them out-of-band to this thread, and cc them if
it ends up being re-rolled.
-Peff
^ permalink raw reply
* Re: [BUG] allowtipsha1inwant serves unreachable blobs if you know its hash
From: Junio C Hamano @ 2017-02-23 23:59 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, peff, peartben, benpeart
In-Reply-To: <xmqqtw7k5uxj.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Adding "--objects" works, and all existing tests pass, except for the
>> potential performance issue and the side effect that even fetching a
>> reachable blob no longer works. This is due to a possible bug where a
>> call like "git rev-list --objects $tree ^master" (where $tree is the
>> tree object corresponding to master) prints out objects ...
>
> The "reachable from this, excluding what is reachable from that"
> notation was originally designed to work only on commits, and I
> wouldn't be surprised if "$tree ^master" did not work as you expect
> in the current implementation.
>
> I agree that ideally it shouldn't show anything, but I suspect that
> it would make it very expensive if done naively---we'd end up having
> to call mark_tree_uninteresting() for all uninteresting commits, not
> just for the commits at the edge of the DAG as we do right now.
BTW, by "it would be expensive" I didn't mean "hence it shouldn't be
done." Even though I do not know by how much expensive it would
become, I think this approach is a lot better way going forward than
punting and say "no, you cannot specify anything lower than commit."
Thanks for looking into this.
^ permalink raw reply
* Re: [BUG] allowtipsha1inwant serves unreachable blobs if you know its hash
From: Jeff King @ 2017-02-24 0:44 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, peartben, benpeart
In-Reply-To: <20170223230358.30050-1-jonathantanmy@google.com>
On Thu, Feb 23, 2017 at 03:03:58PM -0800, Jonathan Tan wrote:
> If a server sets allowtipsha1inwant (or allowreachablesha1inwant), a
> client can call "git fetch <server> <SHA-1>" where SHA-1 is the hash of
> a blob (reachable or unreachable) to obtain it. The test below (which
> passes) demonstrates that.
Thanks for a nice demonstration.
TBH, I think the whole "we will not give you unreachable things" is
overblown as a security measure. There are a lot of ways to leak
information out of the repo. E.g., claiming you _do_ have the
unreachable thing, at which point pack-objects may create a delta
against it. Done repeatedly, it works as an oracle by which you can
guess the contents of the unreachable thing.
> This happens most likely because the "rev-list" call in
> "do_reachable_revlist" (called through "has_unreachable") is invoked
> without the "--objects" argument. "has_unreachable" determines that an
> object is unreachable if nothing is printed to stdout, which normally
> works, except that "rev-list" prints nothing when asked which commits
> are reachable from a blob (which makes sense).
>
> Adding "--objects" works, and all existing tests pass, except for the
> potential performance issue and the side effect that even fetching a
> reachable blob no longer works. This is due to a possible bug where a
> call like "git rev-list --objects $tree ^master" (where $tree is the
> tree object corresponding to master) prints out objects even though all
> objects reachable from the tree are also reachable from master. (There
> should be no issue with "get_reachable_list", the other invoker of
> "do_reachable_revlist", because non-commits in the command's stdout are
> skipped.)
I think "--objects --use-bitmap-index" would be faster. If you have
bitmaps.
-Peff
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Jeff King @ 2017-02-24 0:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqy3ww5wbl.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 03:19:58PM -0800, Junio C Hamano wrote:
> > But you are right. config-parse-key does have the simpler string
> > that can just be given to the canonicalize thing and we should be
> > able to reuse it.
>
> Actually, I think we can just use the existing config_parse_key()
> instead of adding the new function. It adds one allocation and
> deallocation, but it's not like this is a performance-critical
> codepath that we absolutely avoid extra allocations. After all, we
> are still using the strbuf-split thing :-/.
Yeah, you're right. This is much nicer, and everything else was
premature optimization.
> -- >8 --
> From: Junio C Hamano <gitster@pobox.com>
> Date: Thu, 23 Feb 2017 15:04:40 -0800
> Subject: [PATCH] config: reject invalid VAR in 'git -c VAR=VAL command' and
> keep subsection intact
Long subject. :)
I'd have just said:
config: pass variables through git_config_parse_parameter()
That is "what", but the "why" can come in the next paragraph.
> The parsing of one-shot assignments of configuration variables that
> come from the command line historically was quite loose and allowed
> anything to pass. It also downcased everything in the variable name,
> even a three-level <section>.<subsection>.<variable> name in which
> the <subsection> part must be treated in a case sensible manner.
>
> Existing git_config_parse_key() helper is used to parse the variable
> name that comes from the command line, i.e. "git config VAR VAL",
> and handles these details correctly. Replace the strbuf_tolower()
> call in git-config_parse_parameter() with a call to it to correct
> both issues. git_config_parse_key() does a bit more things that are
> not necessary for the purpose of this codepath (e.g. it allocates a
> separate buffer to return the canonicalized variable name because it
> takes a "const char *" input), but we are not in a performance-critical
> codepath here.
Nicely explained.
> diff --git a/config.c b/config.c
> index b8cce1dffa..39f20dcd2c 100644
> --- a/config.c
> +++ b/config.c
> @@ -295,7 +295,9 @@ int git_config_parse_parameter(const char *text,
> config_fn_t fn, void *data)
> {
> const char *value;
> + char *canonical_name;
> struct strbuf **pair;
> + int ret = 0;
>
> pair = strbuf_split_str(text, '=', 2);
> if (!pair[0])
Hmm. I suspect one cannot do:
git -c 'section.subsection with an = in it.key=foo' ...
Definitely not a new problem, nor something that should block your
patch. But if we want to fix it, I suspect the problem will ultimately
involve parsing left-to-right to get the key first, then confirming it
has an =, and then the value.
> @@ -313,13 +315,15 @@ int git_config_parse_parameter(const char *text,
> strbuf_list_free(pair);
> return error("bogus config parameter: %s", text);
> }
> - strbuf_tolower(pair[0]);
> - if (fn(pair[0]->buf, value, data) < 0) {
> - strbuf_list_free(pair);
> +
> + if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL))
> return -1;
> - }
I think git_config_parse_key() will free canonical_name itself if it
returns failure. But do you need to strbuf_list_free(pair) here?
Or alternatively:
int ret = -1;
if (!parse(...))
ret = fn(...);
or use a "got out". Whatever. You don't need me to teach you about error
exits. :)
> + ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
> +
> + free(canonical_name);
> strbuf_list_free(pair);
> - return 0;
> + return ret;
Looks good.
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 923bfc5a26..ea371020fa 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
I just skimmed these, as they look like the previous ones.
So overall I like it, modulo the minor error-leak.
-Peff
^ 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