* [PATCH v3 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master'
2019-02-07 10:18 ` [PATCH v3 0/3] Teach submodule " Denton Liu
@ 2019-02-07 10:18 ` Denton Liu
2019-02-07 10:18 ` [PATCH v3 2/3] submodule--helper: teach config subcommand --unset Denton Liu
` (3 subsequent siblings)
4 siblings, 0 replies; 25+ messages in thread
From: Denton Liu @ 2019-02-07 10:18 UTC (permalink / raw)
To: git; +Cc: gitster
This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.
Also, add the missed argument to the -b/--branch option.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Documentation/git-submodule.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..4150148fa3 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -255,13 +255,14 @@ OPTIONS
This option is only valid for the deinit command. Unregister all
submodules in the working tree.
--b::
---branch::
+-b <branch>::
+--branch <branch>::
Branch of repository to add as submodule.
The name of the branch is recorded as `submodule.<name>.branch` in
`.gitmodules` for `update --remote`. A special value of `.` is used to
indicate that the name of the branch in the submodule should be the
- same name as the current branch in the current repository.
+ same name as the current branch in the current repository. If the
+ option is not specified, it defaults to 'master'.
-f::
--force::
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 2/3] submodule--helper: teach config subcommand --unset
2019-02-07 10:18 ` [PATCH v3 0/3] Teach submodule " Denton Liu
2019-02-07 10:18 ` [PATCH v3 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master' Denton Liu
@ 2019-02-07 10:18 ` Denton Liu
2019-02-07 20:05 ` Junio C Hamano
2019-02-07 22:29 ` Junio C Hamano
2019-02-07 10:19 ` [PATCH v3 3/3] submodule: teach set-branch subcommand Denton Liu
` (2 subsequent siblings)
4 siblings, 2 replies; 25+ messages in thread
From: Denton Liu @ 2019-02-07 10:18 UTC (permalink / raw)
To: git; +Cc: gitster
This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
builtin/submodule--helper.c | 18 ++++++++++++------
t/t7411-submodule-config.sh | 9 +++++++++
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b80fc4ba3d..a86eacf167 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2148,17 +2148,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
static int module_config(int argc, const char **argv, const char *prefix)
{
enum {
- CHECK_WRITEABLE = 1
- } command = 0;
+ NONE,
+ CHECK_WRITEABLE,
+ DO_UNSET
+ } command = NONE;
struct option module_config_options[] = {
OPT_CMDMODE(0, "check-writeable", &command,
N_("check if it is safe to write to the .gitmodules file"),
CHECK_WRITEABLE),
+ OPT_CMDMODE(0, "unset", &command,
+ N_("unset the config in the .gitmodules file"),
+ DO_UNSET),
OPT_END()
};
const char *const git_submodule_helper_usage[] = {
- N_("git submodule--helper config name [value]"),
+ N_("git submodule--helper config name [--unset] [value]"),
N_("git submodule--helper config --check-writeable"),
NULL
};
@@ -2170,15 +2175,16 @@ static int module_config(int argc, const char **argv, const char *prefix)
return is_writing_gitmodules_ok() ? 0 : -1;
/* Equivalent to ACTION_GET in builtin/config.c */
- if (argc == 2)
+ if (argc == 2 && command != DO_UNSET)
return print_config_from_gitmodules(the_repository, argv[1]);
/* Equivalent to ACTION_SET in builtin/config.c */
- if (argc == 3) {
+ if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
if (!is_writing_gitmodules_ok())
die(_("please make sure that the .gitmodules file is in the working tree"));
- return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+ const char *value = (argc == 3) ? argv[2] : NULL;
+ return config_set_in_gitmodules_file_gently(argv[1], value);
}
usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
)
'
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+ (cd super &&
+ git submodule--helper config --unset submodule.submodule.url &&
+ git submodule--helper config submodule.submodule.url >actual &&
+ test_must_be_empty actual
+ )
+'
+
+
test_expect_success 'writing submodules config with "submodule--helper config"' '
(cd super &&
echo "new_url" >expect &&
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 2/3] submodule--helper: teach config subcommand --unset
2019-02-07 10:18 ` [PATCH v3 2/3] submodule--helper: teach config subcommand --unset Denton Liu
@ 2019-02-07 20:05 ` Junio C Hamano
2019-02-07 22:29 ` Junio C Hamano
1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-02-07 20:05 UTC (permalink / raw)
To: Denton Liu; +Cc: git
Denton Liu <liu.denton@gmail.com> writes:
> This teaches submodule--helper config the --unset option, which removes
> the specified configuration key from the .gitmodule file.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> builtin/submodule--helper.c | 18 ++++++++++++------
> t/t7411-submodule-config.sh | 9 +++++++++
> 2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b80fc4ba3d..a86eacf167 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2148,17 +2148,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
> static int module_config(int argc, const char **argv, const char *prefix)
> {
> enum {
> - CHECK_WRITEABLE = 1
> - } command = 0;
> + NONE,
> + CHECK_WRITEABLE,
> + DO_UNSET
> + } command = NONE;
I do not think the above, except for addition of DO_UNSET, is a good
change. The language spec may guarantee that NONE is 0, but in the
way the original is written, it is much more obvious that integer
zero is a special value and the variable is initialized to that
special value, and that is important. The above rewrite makes it
look as if there are a bunch of symbolic constants defined by this
particular caller and one random value NONE, whose only significance
is that it is different from any other value, is picked as its
initial value---but that is a wrong impression to give to the
readers. parse-options.c::get_value() special cases integer zero as
"unset" for any CMDMODE, so inventing a symbolic constant used by
this particular user is counter-productive. Needless to say, it is
not such a great idea to use such an overly generic word NONE here.
The way the original did to make sure all enum values are non-zero
(by explicitly documenting that its first value is 1) and used
literal 0 as "not specified" is much better aligned to the way how
OPT_CMDMODE() is to be used, I think.
>
> struct option module_config_options[] = {
> OPT_CMDMODE(0, "check-writeable", &command,
> N_("check if it is safe to write to the .gitmodules file"),
> CHECK_WRITEABLE),
> + OPT_CMDMODE(0, "unset", &command,
> + N_("unset the config in the .gitmodules file"),
> + DO_UNSET),
> OPT_END()
> };
> const char *const git_submodule_helper_usage[] = {
> - N_("git submodule--helper config name [value]"),
> + N_("git submodule--helper config name [--unset] [value]"),
That gives an impression that "config name --unset value" is a valid
input; isn't it more like "you can unset, or you can set to a
value"? The way to spell it would be "... config name [--unset | value]"
but it raises a larger question. What if you want to set to a value
that is a string "--unset"? Actually, allowing an option that comes
after "name" (which is an arbitrary end-user supplied thing) is one
thing, but writing it in the documentation as if we are encouraging
it is probably not a good idea. Shouldn't it be "config --unset name"?
In any case, seeing the new test in 7411, I think the best way to
write the above usage text would be to add one new line without
mucking with the existing "show it, or set it to the given value"
and add
git submodule--helper config --unset name
as a separate item to the list.
> diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
> index 89690b7adb..fcc0fb82d8 100755
> --- a/t/t7411-submodule-config.sh
> +++ b/t/t7411-submodule-config.sh
> @@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
> )
> '
>
> +test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
> + (cd super &&
> + git submodule--helper config --unset submodule.submodule.url &&
> + git submodule--helper config submodule.submodule.url >actual &&
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 2/3] submodule--helper: teach config subcommand --unset
2019-02-07 10:18 ` [PATCH v3 2/3] submodule--helper: teach config subcommand --unset Denton Liu
2019-02-07 20:05 ` Junio C Hamano
@ 2019-02-07 22:29 ` Junio C Hamano
1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-02-07 22:29 UTC (permalink / raw)
To: Denton Liu; +Cc: git
Denton Liu <liu.denton@gmail.com> writes:
> + if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
> if (!is_writing_gitmodules_ok())
> die(_("please make sure that the .gitmodules file is in the working tree"));
>
> - return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
> + const char *value = (argc == 3) ? argv[2] : NULL;
This introduces decl-after-stmt. Move it before the "is it OK to
write?" check.
> + return config_set_in_gitmodules_file_gently(argv[1], value);
> }
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 3/3] submodule: teach set-branch subcommand
2019-02-07 10:18 ` [PATCH v3 0/3] Teach submodule " Denton Liu
2019-02-07 10:18 ` [PATCH v3 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master' Denton Liu
2019-02-07 10:18 ` [PATCH v3 2/3] submodule--helper: teach config subcommand --unset Denton Liu
@ 2019-02-07 10:19 ` Denton Liu
2019-02-07 22:26 ` Junio C Hamano
2019-02-07 18:01 ` [PATCH v3 0/3] Teach submodule " Junio C Hamano
2019-02-08 11:21 ` [PATCH v4 " Denton Liu
4 siblings, 1 reply; 25+ messages in thread
From: Denton Liu @ 2019-02-07 10:19 UTC (permalink / raw)
To: git; +Cc: gitster
This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
Documentation/git-submodule.txt | 7 ++
contrib/completion/git-completion.bash | 5 +-
git-submodule.sh | 75 +++++++++++++++++++--
t/t7419-submodule-set-branch.sh | 93 ++++++++++++++++++++++++++
4 files changed, 175 insertions(+), 5 deletions(-)
create mode 100755 t/t7419-submodule-set-branch.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4150148fa3..6c608b3b8c 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
'git submodule' [--quiet] summary [<options>] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
--
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+ Sets the default remote tracking branch for the submodule. The
+ `--branch` option allows the remote branch to be specified. The
+ `--default` option removes the submodule.<name>.branch configuration
+ key, which causes the tracking branch to default to 'master'.
+--
summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
Show commit summary between the given commit (defaults to HEAD) and
working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index de56879960..0fccadfc97 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
{
__git_has_doubledash && return
- local subcommands="add status init deinit update summary foreach sync absorbgitdirs"
+ local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
--force --rebase --merge --reference --depth --recursive --jobs
"
;;
+ set-branch,--*)
+ __gitcomp "--default --branch"
+ ;;
summary,--*)
__gitcomp "--cached --files --summary-limit"
;;
diff --git a/git-submodule.sh b/git-submodule.sh
index b5f2beee60..470f681573 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
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] set-branch (--default|--branch <branch>) [--] <path>
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
}
}
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+ unset_branch=false
+ branch=
+
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ # we don't do anything with this but we need to accept it
+ ;;
+ -d|--default)
+ unset_branch=true
+ ;;
+ -b|--branch)
+ case "$2" in '') usage ;; esac
+ branch=$2
+ shift
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ if test $# -ne 1
+ then
+ usage
+ fi
+
+ # we can't use `git submodule--helper name` here because internally, it
+ # hashes the path so a trailing slash could lead to an unintentional no match
+ name="$(git submodule--helper list "$1" | cut -f2)"
+ if test -z "$name"
+ then
+ exit 1
+ fi
+
+ test -n "$branch"; has_branch=$?
+ test "$unset_branch" = true; has_unset_branch=$?
+
+ if test $((!$has_branch != !$has_unset_branch)) -eq 0
+ then
+ usage
+ fi
+
+ if test $has_branch -eq 0
+ then
+ git submodule--helper config submodule."$name".branch "$branch"
+ else
+ git submodule--helper config --unset submodule."$name".branch
+ fi
+}
+
#
# Show commit summary for submodules in index or working tree
#
@@ -983,7 +1050,7 @@ cmd_absorbgitdirs()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+ add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
command=$1
;;
-q|--quiet)
@@ -1024,8 +1091,8 @@ then
fi
fi
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
then
usage
fi
@@ -1036,4 +1103,4 @@ then
usage
fi
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+ mkdir submodule &&
+ (cd submodule &&
+ git init &&
+ echo a >a &&
+ git add . &&
+ git commit -ma &&
+ git checkout -b topic &&
+ echo b >a &&
+ git add . &&
+ git commit -mb
+ ) &&
+ mkdir super &&
+ (cd super &&
+ git init &&
+ git submodule add ../submodule &&
+ git commit -m "add submodule"
+ )
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+ (cd super &&
+ test_must_fail grep branch .gitmodules
+ )
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+ (cd super &&
+ git submodule set-branch --branch topic submodule &&
+ grep "branch = topic" .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ b
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch --default' '
+ (cd super &&
+ git submodule set-branch --default submodule &&
+ test_must_fail grep branch .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ a
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch -b' '
+ (cd super &&
+ git submodule set-branch -b topic submodule &&
+ grep "branch = topic" .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ b
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch -d' '
+ (cd super &&
+ git submodule set-branch -d submodule &&
+ test_must_fail grep branch .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ a
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_done
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 3/3] submodule: teach set-branch subcommand
2019-02-07 10:19 ` [PATCH v3 3/3] submodule: teach set-branch subcommand Denton Liu
@ 2019-02-07 22:26 ` Junio C Hamano
0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-02-07 22:26 UTC (permalink / raw)
To: Denton Liu; +Cc: git
Denton Liu <liu.denton@gmail.com> writes:
> @@ -168,6 +169,12 @@ submodule with the `--init` option.
> If `--recursive` is specified, this command will recurse into the
> registered submodules, and update any nested submodules within.
> --
> +set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
> + Sets the default remote tracking branch for the submodule. The
> + `--branch` option allows the remote branch to be specified. The
> + `--default` option removes the submodule.<name>.branch configuration
> + key, which causes the tracking branch to default to 'master'.
> +--
> summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
> Show commit summary between the given commit (defaults to HEAD) and
> working tree/index. For a submodule in question, a series of commits
The double-dash line you added must be replaced with an empty line.
The existing double-dash is *not* a separator between the previous
entry (i.e. update) and the next entry (i.e. summary). Rather, the
body for 'update' is enclosed in a pair of double-dash lines as it
has many paragraphs.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 0/3] Teach submodule set-branch subcommand
2019-02-07 10:18 ` [PATCH v3 0/3] Teach submodule " Denton Liu
` (2 preceding siblings ...)
2019-02-07 10:19 ` [PATCH v3 3/3] submodule: teach set-branch subcommand Denton Liu
@ 2019-02-07 18:01 ` Junio C Hamano
2019-02-08 5:31 ` Denton Liu
2019-02-08 11:21 ` [PATCH v4 " Denton Liu
4 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2019-02-07 18:01 UTC (permalink / raw)
To: Denton Liu; +Cc: git
Denton Liu <liu.denton@gmail.com> writes:
> I rebased the changes onto the latest 'next' because if this branch gets
> merged into 'next', there'll be merge conflicts from
> 'dl/complete-submodule-absorbgitdirs'.
Please don't do that.
A topic that depends on everything in 'next' cannot graduate to
'master' until everything that is cooking in 'next' does.
When
- the "conflict" that would arise is so trivial to resolve,
- there is no semantic crashes between the new topic and existing
ones, and
- the topic is usable even before other topics graduate (or even
when they get discarded)
please make it a habit to avoid making your topic (i.e. this one)
hostage to another topic (i.e. the absorbgitdirs one), and certainly
not hostage to the whole of 'next'. A trivial conflict resolution
in this case, if you revert the rebasing and then attempt to merge
the result to 'next', would look like this.
diff --cc contrib/completion/git-completion.bash
index 8b3b5a9d34,de56879960..0000000000
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@@ -2573,7 -2573,7 +2573,7 @@@ _git_submodule (
{
__git_has_doubledash && return
- local subcommands="add status init deinit update set-branch summary foreach sync"
- local subcommands="add status init deinit update summary foreach sync absorbgitdirs"
++ local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
case "$cur" in
By the way, if conflicts worry you so much, one thing you could do
is this. Instead of maintaining the ever-growing list of
subcommands and having to worry about textual conflicts, the
completion script could use a clean-up to reduce the need of textual
conflict resolution, when it is quiescent, perhaps like this (I am
assuming that in some future, there is a quiescent period _after_
both of these two topics landed, and this illustration patch is to
be used in such a future).
Merging two topics, each of which adds a new element by inserting a
new line with the element (and the element alone) is on it, is
certainly a lot easier and simpler than having to see what word is
getting inserted by each topic on a single long string on a line.
contrib/completion/git-completion.bash | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0fccadfc97..5d7d4ebacc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,18 @@ _git_submodule ()
{
__git_has_doubledash && return
- local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
+ local subcommands="
+ add
+ status
+ init
+ deinit
+ update
+ set-branch
+ summary
+ foreach
+ sync
+ absorbgitdirs
+ "
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
case "$cur" in
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 0/3] Teach submodule set-branch subcommand
2019-02-07 18:01 ` [PATCH v3 0/3] Teach submodule " Junio C Hamano
@ 2019-02-08 5:31 ` Denton Liu
2019-02-08 18:43 ` Junio C Hamano
0 siblings, 1 reply; 25+ messages in thread
From: Denton Liu @ 2019-02-08 5:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Thu, Feb 07, 2019 at 10:01:40AM -0800, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
>
> > I rebased the changes onto the latest 'next' because if this branch gets
> > merged into 'next', there'll be merge conflicts from
> > 'dl/complete-submodule-absorbgitdirs'.
>
> Please don't do that.
>
> A topic that depends on everything in 'next' cannot graduate to
> 'master' until everything that is cooking in 'next' does.
>
> When
>
> - the "conflict" that would arise is so trivial to resolve,
>
> - there is no semantic crashes between the new topic and existing
> ones, and
>
> - the topic is usable even before other topics graduate (or even
> when they get discarded)
>
> please make it a habit to avoid making your topic (i.e. this one)
> hostage to another topic (i.e. the absorbgitdirs one), and certainly
> not hostage to the whole of 'next'. A trivial conflict resolution
> in this case, if you revert the rebasing and then attempt to merge
> the result to 'next', would look like this.
My mistake, even though I've made a few contributions already, I'm still
getting used to the git.git workflow.
Thanks so much for helping me out and please don't hesitate to let me
know if I'm doing anything else wrong.
By the way, just for the record, how would you like me to handle
patchsets that cause merge conflicts?
Thanks,
Denton
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 0/3] Teach submodule set-branch subcommand
2019-02-08 5:31 ` Denton Liu
@ 2019-02-08 18:43 ` Junio C Hamano
0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2019-02-08 18:43 UTC (permalink / raw)
To: Denton Liu; +Cc: git
Denton Liu <liu.denton@gmail.com> writes:
> By the way, just for the record, how would you like me to handle
> patchsets that cause merge conflicts?
When it happens, I may ask you to rebase onto a specific commit.
https://public-inbox.org/git/xmqq5ztxstkh.fsf@gitster-ct.c.googlers.com/
is a recent example.
My preference (read: I'd be grateful if contributors tried to stick
to it, but it won't be the reason for patch rejection if they don't)
is:
1. Choose the right base:
a) For a fix to a bug that already exists in the last feature
release (i.e. v2.20.0), build on 'maint' and make sure it
merges cleanly to 'master', and the merge result builds and
passes tests. Depending on the severity of the bug, we might
want to make sure that the fix applies maintenance track even
older, but it would be something you would be sending patches
to git-security mailing list---over there we'll figure out the
right base on case-by-case basis.
b) For a new feature, build on 'master', if you can.
c) If you need to use something (i.e. a new helper function,
updated type, etc.) in 'next' that is not yet in 'master',
identify the topic(s) that introduces these things you need,
merge them to 'master' and build on top of it. If you did so,
please note in the cover letter what topics you depend on.
d) When updating a topic that is already in my tree
(i.e. rerolling), stick to the same base as the previous
round, if possible. You can find out the commit the previous
round was applied on top by fetching from me. Your reroll may
start using something the previous round did not use from
'next', in which case you may end up doing c) above, which is
OK. Just make a note in the cover letter to let reviewers
know.
2. Make sure the result builds and passes tests.
3. Make sure the result merges cleanly to 'next', and the merge
result builds and passes tests.
4. In any of the above steps where you are asked to "make sure it
merges cleanly", you may see merge conflicts.
a) If they are too much to resolve for you, for a change that is
not a bugfix, you may have to depend on the other topic(s).
Identify the topic(s) that touches these lines that heavily
conflict with your changes, merge them to 'master' and base
your topic on top of it (i.e. going back to step 1.c).
b) If they are easy to resolve for you, do not worry too much
about them. It may be helpful to note in the cover letter
"this may have minor textual conflict with these other
topics".
c) If you are left with huge conflict while working on a bugfix
change, we need to resolve it on case-by-case basis, so send
it out with the chosen base. Noting that it conflicts heavily
with 'master' or 'next' would be very much appreciated when
this happens.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 0/3] Teach submodule set-branch subcommand
2019-02-07 10:18 ` [PATCH v3 0/3] Teach submodule " Denton Liu
` (3 preceding siblings ...)
2019-02-07 18:01 ` [PATCH v3 0/3] Teach submodule " Junio C Hamano
@ 2019-02-08 11:21 ` Denton Liu
2019-02-08 11:21 ` [PATCH v4 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master' Denton Liu
` (2 more replies)
4 siblings, 3 replies; 25+ messages in thread
From: Denton Liu @ 2019-02-08 11:21 UTC (permalink / raw)
To: git; +Cc: gitster
Thanks so much for the feedback, Junio. I've updated the patchset based
on your comments.
Currently, there is no way to set the branch of a submodule without
manually manipulating the .gitmodules file. This patchset introduces a
porcelain command that enables this.
Changes since v1:
* Fixed incorrect usage of OPT_CMDMODE
Changes since v2:
* Corrected missing argument for -b/--branch in git-submodule.txt
* Rebased onto latest next
Changes since v3:
* Rebased back onto master
* Address Junio's comments and improve clarity of submodule--helper patch
* Fix incorrect documentation formatting in git-submodule.txt
Denton Liu (3):
git-submodule.txt: "--branch <branch>" option defaults to 'master'
submodule--helper: teach config subcommand --unset
submodule: teach set-branch subcommand
Documentation/git-submodule.txt | 14 +++-
builtin/submodule--helper.c | 17 +++--
contrib/completion/git-completion.bash | 5 +-
git-submodule.sh | 75 +++++++++++++++++++--
t/t7411-submodule-config.sh | 9 +++
t/t7419-submodule-set-branch.sh | 93 ++++++++++++++++++++++++++
6 files changed, 200 insertions(+), 13 deletions(-)
create mode 100755 t/t7419-submodule-set-branch.sh
--
2.20.1.522.g5f42c252e9
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master'
2019-02-08 11:21 ` [PATCH v4 " Denton Liu
@ 2019-02-08 11:21 ` Denton Liu
2019-02-08 11:21 ` [PATCH v4 2/3] submodule--helper: teach config subcommand --unset Denton Liu
2019-02-08 11:21 ` [PATCH v4 3/3] submodule: teach set-branch subcommand Denton Liu
2 siblings, 0 replies; 25+ messages in thread
From: Denton Liu @ 2019-02-08 11:21 UTC (permalink / raw)
To: git; +Cc: gitster
This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.
Also, add the missed argument to the -b/--branch option.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Documentation/git-submodule.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..4150148fa3 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -255,13 +255,14 @@ OPTIONS
This option is only valid for the deinit command. Unregister all
submodules in the working tree.
--b::
---branch::
+-b <branch>::
+--branch <branch>::
Branch of repository to add as submodule.
The name of the branch is recorded as `submodule.<name>.branch` in
`.gitmodules` for `update --remote`. A special value of `.` is used to
indicate that the name of the branch in the submodule should be the
- same name as the current branch in the current repository.
+ same name as the current branch in the current repository. If the
+ option is not specified, it defaults to 'master'.
-f::
--force::
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 2/3] submodule--helper: teach config subcommand --unset
2019-02-08 11:21 ` [PATCH v4 " Denton Liu
2019-02-08 11:21 ` [PATCH v4 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master' Denton Liu
@ 2019-02-08 11:21 ` Denton Liu
2019-02-08 11:21 ` [PATCH v4 3/3] submodule: teach set-branch subcommand Denton Liu
2 siblings, 0 replies; 25+ messages in thread
From: Denton Liu @ 2019-02-08 11:21 UTC (permalink / raw)
To: git; +Cc: gitster
This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
builtin/submodule--helper.c | 17 ++++++++++++-----
t/t7411-submodule-config.sh | 9 +++++++++
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0e140f176c..449e56f30c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2147,17 +2147,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
static int module_config(int argc, const char **argv, const char *prefix)
{
enum {
- CHECK_WRITEABLE = 1
+ CHECK_WRITEABLE = 1,
+ DO_UNSET = 2
} command = 0;
struct option module_config_options[] = {
OPT_CMDMODE(0, "check-writeable", &command,
N_("check if it is safe to write to the .gitmodules file"),
CHECK_WRITEABLE),
+ OPT_CMDMODE(0, "unset", &command,
+ N_("unset the config in the .gitmodules file"),
+ DO_UNSET),
OPT_END()
};
const char *const git_submodule_helper_usage[] = {
- N_("git submodule--helper config name [value]"),
+ N_("git submodule--helper config <name> [<value>]"),
+ N_("git submodule--helper config --unset <name>"),
N_("git submodule--helper config --check-writeable"),
NULL
};
@@ -2169,15 +2174,17 @@ static int module_config(int argc, const char **argv, const char *prefix)
return is_writing_gitmodules_ok() ? 0 : -1;
/* Equivalent to ACTION_GET in builtin/config.c */
- if (argc == 2)
+ if (argc == 2 && command != DO_UNSET)
return print_config_from_gitmodules(the_repository, argv[1]);
/* Equivalent to ACTION_SET in builtin/config.c */
- if (argc == 3) {
+ if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
+ const char *value = (argc == 3) ? argv[2] : NULL;
+
if (!is_writing_gitmodules_ok())
die(_("please make sure that the .gitmodules file is in the working tree"));
- return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+ return config_set_in_gitmodules_file_gently(argv[1], value);
}
usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
)
'
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+ (cd super &&
+ git submodule--helper config --unset submodule.submodule.url &&
+ git submodule--helper config submodule.submodule.url >actual &&
+ test_must_be_empty actual
+ )
+'
+
+
test_expect_success 'writing submodules config with "submodule--helper config"' '
(cd super &&
echo "new_url" >expect &&
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v4 3/3] submodule: teach set-branch subcommand
2019-02-08 11:21 ` [PATCH v4 " Denton Liu
2019-02-08 11:21 ` [PATCH v4 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master' Denton Liu
2019-02-08 11:21 ` [PATCH v4 2/3] submodule--helper: teach config subcommand --unset Denton Liu
@ 2019-02-08 11:21 ` Denton Liu
2019-02-08 23:51 ` Denton Liu
2 siblings, 1 reply; 25+ messages in thread
From: Denton Liu @ 2019-02-08 11:21 UTC (permalink / raw)
To: git; +Cc: gitster
This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
Documentation/git-submodule.txt | 7 ++
contrib/completion/git-completion.bash | 5 +-
git-submodule.sh | 75 +++++++++++++++++++--
t/t7419-submodule-set-branch.sh | 93 ++++++++++++++++++++++++++
4 files changed, 175 insertions(+), 5 deletions(-)
create mode 100755 t/t7419-submodule-set-branch.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4150148fa3..4daf144180 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
'git submodule' [--quiet] summary [<options>] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
--
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+ Sets the default remote tracking branch for the submodule. The
+ `--branch` option allows the remote branch to be specified. The
+ `--default` option removes the submodule.<name>.branch configuration
+ key, which causes the tracking branch to default to 'master'.
+
summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
Show commit summary between the given commit (defaults to HEAD) and
working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..8b3b5a9d34 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
{
__git_has_doubledash && return
- local subcommands="add status init deinit update summary foreach sync"
+ local subcommands="add status init deinit update set-branch summary foreach sync"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
--force --rebase --merge --reference --depth --recursive --jobs
"
;;
+ set-branch,--*)
+ __gitcomp "--default --branch"
+ ;;
summary,--*)
__gitcomp "--cached --files --summary-limit"
;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..43ec756602 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
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] set-branch (--default|--branch <branch>) [--] <path>
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
}
}
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+ unset_branch=false
+ branch=
+
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ # we don't do anything with this but we need to accept it
+ ;;
+ -d|--default)
+ unset_branch=true
+ ;;
+ -b|--branch)
+ case "$2" in '') usage ;; esac
+ branch=$2
+ shift
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ if test $# -ne 1
+ then
+ usage
+ fi
+
+ # we can't use `git submodule--helper name` here because internally, it
+ # hashes the path so a trailing slash could lead to an unintentional no match
+ name="$(git submodule--helper list "$1" | cut -f2)"
+ if test -z "$name"
+ then
+ exit 1
+ fi
+
+ test -n "$branch"; has_branch=$?
+ test "$unset_branch" = true; has_unset_branch=$?
+
+ if test $((!$has_branch != !$has_unset_branch)) -eq 0
+ then
+ usage
+ fi
+
+ if test $has_branch -eq 0
+ then
+ git submodule--helper config submodule."$name".branch "$branch"
+ else
+ git submodule--helper config --unset submodule."$name".branch
+ fi
+}
+
#
# Show commit summary for submodules in index or working tree
#
@@ -980,7 +1047,7 @@ cmd_absorbgitdirs()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+ add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
command=$1
;;
-q|--quiet)
@@ -1021,8 +1088,8 @@ then
fi
fi
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
then
usage
fi
@@ -1033,4 +1100,4 @@ then
usage
fi
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+ mkdir submodule &&
+ (cd submodule &&
+ git init &&
+ echo a >a &&
+ git add . &&
+ git commit -ma &&
+ git checkout -b topic &&
+ echo b >a &&
+ git add . &&
+ git commit -mb
+ ) &&
+ mkdir super &&
+ (cd super &&
+ git init &&
+ git submodule add ../submodule &&
+ git commit -m "add submodule"
+ )
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+ (cd super &&
+ test_must_fail grep branch .gitmodules
+ )
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+ (cd super &&
+ git submodule set-branch --branch topic submodule &&
+ grep "branch = topic" .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ b
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch --default' '
+ (cd super &&
+ git submodule set-branch --default submodule &&
+ test_must_fail grep branch .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ a
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch -b' '
+ (cd super &&
+ git submodule set-branch -b topic submodule &&
+ grep "branch = topic" .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ b
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch -d' '
+ (cd super &&
+ git submodule set-branch -d submodule &&
+ test_must_fail grep branch .gitmodules &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ a
+ EOF
+ git -C submodule show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_done
--
2.20.1.522.g5f42c252e9
^ permalink raw reply related [flat|nested] 25+ messages in thread