Re: [PATCH 5/5] Teach checkout to recursively checkout submodules

2013-12-27 Thread Jens Lehmann
Am 26.12.2013 17:22, schrieb Jonathan Nieder:
> From: Jens Lehmann 
> Date: Wed, 13 Jun 2012 18:50:10 +0200
> 
> Signed-off-by: Jens Lehmann 
> Signed-off-by: Jonathan Nieder 
> ---
> This is the patch that actually introduces the --recurse-submodules
> option, which makes the rest work.
> 
> The tests indicate some future directions for improving this, but
> it works reasonably well already.  I think I'd be most comfortable
> applying these if they were rearranged a little to the following
> order:
> 
>  1. First, introducing the --recurse-submodules option, perhaps
> with no actual effect, with tests that it is parsed correctly,
> the default works, etc.
> 
>  2. Then, adding the desired behaviors one by one with corresponding
> tests (handling submodule modifications, removals, additions).
> 
>  3. Finally, any needed tweaks on top.
> 
> That way, it is easy to play around with early patches without
> worrying about the later ones at first, and the patches are easy
> to review to confirm that they at least don't break anything in
> the --no-recurse-submodules case.

Makes tons of sense.

The only feature I'm aware of that is currently missing is to
read the path <-> name mappings from the correct .gitmodules
blob, which is needed to populate submodules that appear.

> That said, Debian experimental has these applied as is already,
> and there haven't been any complaints yet.

Great!

I'm also using this code at $dayjob successfully for quite some
time now. Additionally I also enable unconditional recursive
checkout by putting a "return 1" in the first line of the
submodule_needs_update() function (Which is a hack to emulate
"autoupdate=true" while at the same time pretending to already
having added "--recurse-submodules=config" to all call sites in
git porcelain scripts that call plumbing themselves). Only in a
few corner cases submodules aren't properly updated, but we'll
weed those out while implementing the tests. And we need lots
of those to cover all important combinations ...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/5] Teach checkout to recursively checkout submodules

2013-12-26 Thread Jonathan Nieder
From: Jens Lehmann 
Date: Wed, 13 Jun 2012 18:50:10 +0200

Signed-off-by: Jens Lehmann 
Signed-off-by: Jonathan Nieder 
---
This is the patch that actually introduces the --recurse-submodules
option, which makes the rest work.

The tests indicate some future directions for improving this, but
it works reasonably well already.  I think I'd be most comfortable
applying these if they were rearranged a little to the following
order:

 1. First, introducing the --recurse-submodules option, perhaps
with no actual effect, with tests that it is parsed correctly,
the default works, etc.

 2. Then, adding the desired behaviors one by one with corresponding
tests (handling submodule modifications, removals, additions).

 3. Finally, any needed tweaks on top.

That way, it is easy to play around with early patches without
worrying about the later ones at first, and the patches are easy
to review to confirm that they at least don't break anything in
the --no-recurse-submodules case.

That said, Debian experimental has these applied as is already,
and there haven't been any complaints yet.

Thoughts?
Jonathan

 Documentation/git-checkout.txt |   8 ++
 builtin/checkout.c |  14 +++
 submodule.c|  14 +++
 submodule.h|   3 +
 t/t2013-checkout-submodule.sh  | 215 -
 5 files changed, 251 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 91294f8..aabcc65 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -225,6 +225,14 @@ This means that you can use `git checkout -p` to 
selectively discard
 edits from your current working tree. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
 
+--[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 until `-f` is used. If nothing (or --no-recurse-submodules)
+   is used, the work trees of submodules will not be updated, only the
+   hash recorded in the superproject will be changed.
+
 ::
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 5df3837..ac2f8d8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,6 +21,9 @@
 #include "submodule.h"
 #include "argv-array.h"
 
+static const char *recurse_submodules_default = "off";
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
 static const char * const checkout_usage[] = {
N_("git checkout [options] "),
N_("git checkout [options] [] -- ..."),
@@ -,6 +1114,12 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
 N_("do not limit pathspecs to sparse entries only")),
OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
N_("second guess 'git checkout 
no-such-branch'")),
+   { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+   "checkout", "control recursive updating of 
submodules",
+   PARSE_OPT_OPTARG, option_parse_update_submodules },
+   { OPTION_STRING, 0, "recurse-submodules-default",
+  &recurse_submodules_default, NULL,
+  "default mode for recursion", PARSE_OPT_HIDDEN },
OPT_END(),
};
 
@@ -1132,6 +1141,11 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
 
+   set_config_update_recurse_submodules(
+   
parse_fetch_recurse_submodules_arg("--recurse-submodules-default",
+  recurse_submodules_default),
+   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/submodule.c b/submodule.c
index 3365987..bdce1b2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -398,6 +398,20 @@ int parse_update_recurse_submodules_arg(const char *opt, 
const char *arg)
}
 }
 
+int option_parse_update_submodules(const struct option *opt,
+  const char *arg, int unset)
+{
+   if (unset) {
+   *(int *)opt->value = RECURSE_SUBMODULES_OFF;
+   } else {
+   if (arg)
+   *(int *)opt->value = 
parse_update_recurse_submodules_arg(opt->long_name, arg);
+   else
+   *(int *)opt->value = RECURSE_SUBMODULES_ON;
+   }
+