* [WIP/PATCH 0/5] git checkout --recurse-submodules
@ 2013-12-26 15:58 Jonathan Nieder
2013-12-26 16:02 ` [PATCH 1/5] submodule: prepare for recursive checkout of submodules Jonathan Nieder
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 15:58 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
Hi,
This patch series comes from
https://github.com/jlehmann/git-submod-enhancements branch
recursive_submodule_checkout. It needed some tiny tweaks to apply to
current "master" and build without warnings, but nothing major, and I
haven't sanity checked it much beyond that and letting the kind folks
that use Debian experimental play with it.
I'm sending it out now to get review and ideas for what needs to
happen next to get this series in shape to be included in git.git.
Thoughts of all kinds welcome.
Thanks,
Jonathan
Jens Lehmann (5):
submodule: prepare for recursive checkout of submodules
submodule: teach unpack_trees() to remove submodule contents
submodule: teach unpack_trees() to repopulate submodules
submodule: teach unpack_trees() to update submodules
Teach checkout to recursively checkout submodules
Documentation/git-checkout.txt | 8 ++
builtin/checkout.c | 14 +++
entry.c | 19 +++-
submodule.c | 217 ++++++++++++++++++++++++++++++++++++++++-
submodule.h | 11 +++
t/t2013-checkout-submodule.sh | 215 +++++++++++++++++++++++++++++++++++++++-
unpack-trees.c | 94 ++++++++++++++----
unpack-trees.h | 1 +
wrapper.c | 3 +
9 files changed, 556 insertions(+), 26 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] submodule: prepare for recursive checkout of submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
@ 2013-12-26 16:02 ` Jonathan Nieder
2013-12-27 13:42 ` Jens Lehmann
2013-12-26 16:12 ` [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents Jonathan Nieder
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 16:02 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
From: Jens Lehmann <Jens.Lehmann@web.de>
Date: Mon, 18 Jun 2012 22:17:59 +0200
This commit adds the functions needed for configuration, for setting the
default behavior and for determining if a submodule path should be updated
automatically.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Should probably be squashed into a patch that uses and documents this
configuration.
submodule.c | 36 ++++++++++++++++++++++++++++++++++++
submodule.h | 3 +++
2 files changed, 39 insertions(+)
diff --git a/submodule.c b/submodule.c
index 613857e..3f18d4d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,8 @@ static struct string_list config_name_for_path;
static struct string_list config_fetch_recurse_submodules_for_name;
static struct string_list config_ignore_for_name;
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
+static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static struct string_list changed_submodule_paths;
static int initialized_fetch_ref_tips;
static struct sha1_array ref_tips_before_fetch;
@@ -382,6 +384,34 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
}
}
+int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
+{
+ switch (git_config_maybe_bool(opt, arg)) {
+ case 1:
+ return RECURSE_SUBMODULES_ON;
+ case 0:
+ return RECURSE_SUBMODULES_OFF;
+ default:
+ if (!strcmp(arg, "checkout"))
+ return RECURSE_SUBMODULES_ON;
+ die("bad %s argument: %s", opt, arg);
+ }
+}
+
+int submodule_needs_update(const char *path)
+{
+ struct string_list_item *path_option;
+ path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+ if (!path_option)
+ return 0;
+
+ /* update can't be "none", "merge" or "rebase" */
+
+ if (option_update_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+ return 1;
+ return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
+}
+
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
@@ -589,6 +619,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
return ret;
}
+void set_config_update_recurse_submodules(int default_value, int option_value)
+{
+ config_update_recurse_submodules = default_value;
+ option_update_recurse_submodules = option_value;
+}
+
static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
{
int is_present = 0;
diff --git a/submodule.h b/submodule.h
index 7beec48..055918c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -22,12 +22,15 @@ void gitmodules_config(void);
int parse_submodule_config_option(const char *var, const char *value);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
+int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
+int submodule_needs_update(const char *path);
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
void set_config_fetch_recurse_submodules(int value);
+void set_config_update_recurse_submodules(int default_value, int option_value);
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
2013-12-26 16:02 ` [PATCH 1/5] submodule: prepare for recursive checkout of submodules Jonathan Nieder
@ 2013-12-26 16:12 ` Jonathan Nieder
2013-12-27 13:58 ` Jens Lehmann
2013-12-26 16:14 ` [PATCH 3/5] submodule: teach unpack_trees() to repopulate submodules Jonathan Nieder
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 16:12 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
From: Jens Lehmann <Jens.Lehmann@web.de>
Date: Tue, 19 Jun 2012 20:55:45 +0200
Implement the functionality needed to enable work tree manipulating
commands to that a deleted submodule should not only affect the index
(leaving all the files of the submodule in the work tree) but also to
remove the work tree of the superproject (including any untracked
files).
That will only work properly when the submodule uses a gitfile instead of
a .git directory and no untracked files are present. Otherwise the removal
will fail with a warning (which is just what happened until now).
Extend rmdir_or_warn() to remove the directories of those submodules which
are scheduled for removal. Also teach verify_clean_submodule() to check
that a submodule configured to be removed is not modified before scheduling
it for removal.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Should this share some code (or just the error message) with the 'git
rm' code that checks whether a submodule is safe to remove?
rmdir_or_warn is a pretty low-level function --- it feels odd to be
relying on the git repository layout here. On the other hand, that
function only has two callers, so it is possible to check quickly
whether it is safe.
I assume this is mostly for the sake of the caller in unpack-trees?
In builtin/apply.c, remove_file is used for deletion and rename
patches. Do we want this logic take effect there, too?
submodule.c | 37 +++++++++++++++++++++++++++++++++++++
submodule.h | 1 +
unpack-trees.c | 6 +++---
wrapper.c | 3 +++
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/submodule.c b/submodule.c
index 3f18d4d..a25db46 100644
--- a/submodule.c
+++ b/submodule.c
@@ -412,6 +412,43 @@ int submodule_needs_update(const char *path)
return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
}
+int depopulate_submodule(const char *path)
+{
+ struct strbuf dot_git = STRBUF_INIT;
+ struct child_process cp;
+ const char *argv[] = {"rm", "-rf", path, NULL};
+
+ /* Is it populated? */
+ strbuf_addf(&dot_git, "%s/.git", path);
+ if (!resolve_gitdir(dot_git.buf)) {
+ strbuf_release(&dot_git);
+ return 0;
+ }
+ strbuf_release(&dot_git);
+
+ /* Does it have a .git directory? */
+ if (!submodule_uses_gitfile(path)) {
+ warning(_("cannot remove submodule '%s' because it (or one of "
+ "its nested submodules) uses a .git directory"),
+ path);
+ return -1;
+ }
+
+ /* Remove the whole submodule directory */
+ memset(&cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.git_cmd = 0;
+ cp.no_stdin = 1;
+ if (run_command(&cp)) {
+ warning("Could not remove submodule %s", path);
+ strbuf_release(&dot_git);
+ return -1;
+ }
+
+ return 0;
+}
+
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
diff --git a/submodule.h b/submodule.h
index 055918c..df291cf 100644
--- a/submodule.h
+++ b/submodule.h
@@ -24,6 +24,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int submodule_needs_update(const char *path);
+int depopulate_submodule(const char *path);
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
diff --git a/unpack-trees.c b/unpack-trees.c
index ad3e9a0..89b506a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -8,6 +8,7 @@
#include "progress.h"
#include "refs.h"
#include "attr.h"
+#include "submodule.h"
/*
* Error messages expected by scripts out of plumbing commands such as
@@ -1263,14 +1264,13 @@ static void invalidate_ce_path(const struct cache_entry *ce,
/*
* Check that checking out ce->sha1 in subdir ce->name is not
* going to overwrite any working files.
- *
- * Currently, git does not checkout subprojects during a superproject
- * checkout, so it is not going to overwrite anything.
*/
static int verify_clean_submodule(const struct cache_entry *ce,
enum unpack_trees_error_types error_type,
struct unpack_trees_options *o)
{
+ if (submodule_needs_update(ce->name) && is_submodule_modified(ce->name, 0))
+ return 1;
return 0;
}
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..425a3fd 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -2,6 +2,7 @@
* Various trivial helper wrappers around standard functions
*/
#include "cache.h"
+#include "submodule.h"
static void do_nothing(size_t size)
{
@@ -409,6 +410,8 @@ int unlink_or_warn(const char *file)
int rmdir_or_warn(const char *file)
{
+ if (submodule_needs_update(file) && depopulate_submodule(file))
+ return -1;
return warn_if_unremovable("rmdir", file, rmdir(file));
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] submodule: teach unpack_trees() to repopulate submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
2013-12-26 16:02 ` [PATCH 1/5] submodule: prepare for recursive checkout of submodules Jonathan Nieder
2013-12-26 16:12 ` [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents Jonathan Nieder
@ 2013-12-26 16:14 ` Jonathan Nieder
2013-12-26 16:15 ` [PATCH 4/5] submodule: teach unpack_trees() to update submodules Jonathan Nieder
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 16:14 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
From: Jens Lehmann <Jens.Lehmann@web.de>
Date: Mon, 18 Jun 2012 22:11:45 +0200
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Neat. Would probably be clearer with some of the corresponding tests
squashed in to illustrate the intended behavior.
entry.c | 4 ++++
submodule.c | 44 +++++++++++++++++++++++++++++++++++++++++---
submodule.h | 1 +
unpack-trees.c | 19 +++++++++++++++++++
4 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/entry.c b/entry.c
index 7b7aa81..d1bf6ec 100644
--- a/entry.c
+++ b/entry.c
@@ -2,6 +2,7 @@
#include "blob.h"
#include "dir.h"
#include "streaming.h"
+#include "submodule.h"
static void create_directories(const char *path, int path_len,
const struct checkout *state)
@@ -203,6 +204,9 @@ static int write_entry(struct cache_entry *ce,
return error("cannot create temporary submodule %s", path);
if (mkdir(path, 0777) < 0)
return error("cannot create submodule directory %s", path);
+ if (submodule_needs_update(path) &&
+ populate_submodule(path, ce->sha1, state->force))
+ return error("cannot checkout submodule %s", path);
break;
default:
return error("unknown file mode for %s in index", path);
diff --git a/submodule.c b/submodule.c
index a25db46..06df5ae 100644
--- a/submodule.c
+++ b/submodule.c
@@ -412,6 +412,42 @@ int submodule_needs_update(const char *path)
return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
}
+int populate_submodule(const char *path, unsigned char sha1[20], int force)
+{
+ struct string_list_item *path_option;
+ const char *name, *real_git_dir;
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *argv[] = {"read-tree", force ? "--reset" : "-m", "-u", NULL, NULL};
+
+ path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+ if (!path_option)
+ return 0;
+
+ name = path_option->util;
+
+ strbuf_addf(&buf, "%s/modules/%s", resolve_gitdir(get_git_dir()), name);
+ real_git_dir = resolve_gitdir(buf.buf);
+ if (!real_git_dir)
+ goto out;
+ connect_work_tree_and_git_dir(path, real_git_dir);
+
+ /* Run read-tree --reset sha1 */
+ memset(&cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ argv[3] = sha1_to_hex(sha1);
+ if (run_command(&cp))
+ warning(_("Checking out submodule %s failed"), path);
+
+out:
+ strbuf_release(&buf);
+ return 0;
+}
+
int depopulate_submodule(const char *path)
{
struct strbuf dot_git = STRBUF_INIT;
@@ -1207,6 +1243,7 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
{
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
+ const char *real_git_dir = xstrdup(real_path(git_dir));
const char *real_work_tree = xstrdup(real_path(work_tree));
FILE *fp;
@@ -1215,15 +1252,15 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
fp = fopen(file_name.buf, "w");
if (!fp)
die(_("Could not create git link %s"), file_name.buf);
- fprintf(fp, "gitdir: %s\n", relative_path(git_dir, real_work_tree,
+ fprintf(fp, "gitdir: %s\n", relative_path(real_git_dir, real_work_tree,
&rel_path));
fclose(fp);
/* Update core.worktree setting */
strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
if (git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
+ relative_path(real_work_tree, real_git_dir,
&rel_path)))
die(_("Could not set core.worktree in %s"),
file_name.buf);
@@ -1231,4 +1268,5 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
strbuf_release(&file_name);
strbuf_release(&rel_path);
free((void *)real_work_tree);
+ free((void *)real_git_dir);
}
diff --git a/submodule.h b/submodule.h
index df291cf..3657ca8 100644
--- a/submodule.h
+++ b/submodule.h
@@ -24,6 +24,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int submodule_needs_update(const char *path);
+int populate_submodule(const char *path, unsigned char sha1[20], int force);
int depopulate_submodule(const char *path);
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
diff --git a/unpack-trees.c b/unpack-trees.c
index 89b506a..ed48d41 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1367,6 +1367,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
struct unpack_trees_options *o)
{
const struct cache_entry *result;
+ char *name_copy, *separator;
/*
* It may be that the 'lstat()' succeeded even though
@@ -1409,6 +1410,24 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
return 0;
}
+ /*
+ * If the path lies inside a to be added submodule it
+ * is ok to remove it.
+ */
+ name_copy = xstrdup(name);
+ while ((separator = strrchr(name_copy, '/'))) {
+ int i;
+ *separator = '\0';
+ for (i = 0; i < the_index.cache_nr; i++) {
+ struct cache_entry *ce = the_index.cache[i];
+ if (!strcmp(ce->name, name_copy) && S_ISGITLINK(ce->ce_mode)) {
+ free(name_copy);
+ return 0;
+ }
+ }
+ }
+ free(name_copy);
+
return o->gently ? -1 :
add_rejected_path(o, error_type, name);
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] submodule: teach unpack_trees() to update submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
` (2 preceding siblings ...)
2013-12-26 16:14 ` [PATCH 3/5] submodule: teach unpack_trees() to repopulate submodules Jonathan Nieder
@ 2013-12-26 16:15 ` Jonathan Nieder
2013-12-26 16:22 ` [PATCH 5/5] Teach checkout to recursively checkout submodules Jonathan Nieder
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 16:15 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
From: Jens Lehmann <Jens.Lehmann@web.de>
Date: Fri, 5 Apr 2013 18:35:27 +0200
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Also neat, also would benefit from documentation or tests.
entry.c | 15 ++++++++--
submodule.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 3 ++
unpack-trees.c | 69 ++++++++++++++++++++++++++++++++++++----------
unpack-trees.h | 1 +
5 files changed, 157 insertions(+), 17 deletions(-)
diff --git a/entry.c b/entry.c
index d1bf6ec..61a2767 100644
--- a/entry.c
+++ b/entry.c
@@ -265,7 +265,7 @@ int checkout_entry(struct cache_entry *ce,
if (!check_path(path, len, &st, state->base_dir_len)) {
unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
- if (!changed)
+ if (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))
return 0;
if (!state->force) {
if (!state->quiet)
@@ -280,9 +280,18 @@ int checkout_entry(struct cache_entry *ce,
* just do the right thing)
*/
if (S_ISDIR(st.st_mode)) {
- /* If it is a gitlink, leave it alone! */
- if (S_ISGITLINK(ce->ce_mode))
+ if (S_ISGITLINK(ce->ce_mode)) {
+ if (submodule_needs_update(ce->name)) {
+ if (is_submodule_populated(ce->name)) {
+ if (update_submodule(ce->name, ce->sha1, state->force))
+ return error("cannot checkout submodule %s", path);
+ } else {
+ if (populate_submodule(path, ce->sha1, state->force))
+ return error("cannot populate submodule %s", path);
+ }
+ }
return 0;
+ }
if (!state->force)
return error("%s is a directory", path);
remove_subtree(path);
diff --git a/submodule.c b/submodule.c
index 06df5ae..3365987 100644
--- a/submodule.c
+++ b/submodule.c
@@ -485,6 +485,42 @@ int depopulate_submodule(const char *path)
return 0;
}
+int update_submodule(const char *path, const unsigned char sha1[20], int force)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *hex_sha1 = sha1_to_hex(sha1);
+ const char *argv[] = {
+ "checkout",
+ force ? "-fq" : "-q",
+ hex_sha1,
+ NULL,
+ };
+ const char *git_dir;
+
+ strbuf_addf(&buf, "%s/.git", path);
+ git_dir = read_gitfile(buf.buf);
+ if (!git_dir)
+ git_dir = buf.buf;
+ if (!is_directory(git_dir)) {
+ strbuf_release(&buf);
+ /* The submodule is not populated, so we can't check it out */
+ return 0;
+ }
+ strbuf_release(&buf);
+
+ memset(&cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path; /* GIT_WORK_TREE doesn't work for git checkout */
+ if (run_command(&cp))
+ return error("Could not checkout submodule %s", path);
+
+ return 0;
+}
+
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
@@ -926,6 +962,17 @@ out:
return result;
}
+int is_submodule_populated(const char *path)
+{
+ int retval = 0;
+ struct strbuf gitdir = STRBUF_INIT;
+ strbuf_addf(&gitdir, "%s/.git", path);
+ if (resolve_gitdir(gitdir.buf))
+ retval = 1;
+ strbuf_release(&gitdir);
+ return retval;
+}
+
unsigned is_submodule_modified(const char *path, int ignore_untracked)
{
ssize_t len;
@@ -1075,6 +1122,45 @@ int ok_to_remove_submodule(const char *path)
return ok_to_remove;
}
+unsigned is_submodule_checkout_safe(const char *path, const unsigned char sha1[20])
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *hex_sha1 = sha1_to_hex(sha1);
+ const char *argv[] = {
+ "read-tree",
+ "-n",
+ "-m",
+ "HEAD",
+ hex_sha1,
+ NULL,
+ };
+ const char *git_dir;
+
+ strbuf_addf(&buf, "%s/.git", path);
+ git_dir = read_gitfile(buf.buf);
+ if (!git_dir)
+ git_dir = buf.buf;
+ if (!is_directory(git_dir)) {
+ strbuf_release(&buf);
+ /* The submodule is not populated, it's safe to check it out */
+ /*
+ * TODO: When git learns to re-populate submodules, a check must be
+ * added here to assert that no local files will be overwritten.
+ */
+ return 1;
+ }
+ strbuf_release(&buf);
+
+ memset(&cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ return run_command(&cp) == 0;
+}
+
static int find_first_merges(struct object_array *result, const char *path,
struct commit *a, struct commit *b)
{
diff --git a/submodule.h b/submodule.h
index 3657ca8..b42ae91 100644
--- a/submodule.h
+++ b/submodule.h
@@ -26,6 +26,7 @@ int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int submodule_needs_update(const char *path);
int populate_submodule(const char *path, unsigned char sha1[20], int force);
int depopulate_submodule(const char *path);
+int update_submodule(const char *path, const unsigned char sha1[20], int force);
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
@@ -37,9 +38,11 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20]);
int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
int quiet);
+int is_submodule_populated(const char *path);
unsigned is_submodule_modified(const char *path, int ignore_untracked);
int submodule_uses_gitfile(const char *path);
int ok_to_remove_submodule(const char *path);
+unsigned is_submodule_checkout_safe(const char *path, const unsigned char sha1[20]);
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
const unsigned char a[20], const unsigned char b[20], int search);
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
diff --git a/unpack-trees.c b/unpack-trees.c
index ed48d41..fc8855e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -27,6 +27,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
/* ERROR_NOT_UPTODATE_DIR */
"Updating '%s' would lose untracked files in it",
+ /* ERROR_NOT_UPTODATE_SUBMODULE */
+ "Updating submodule '%s' would lose modifications in it",
+
/* ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN */
"Untracked working tree file '%s' would be overwritten by merge.",
@@ -71,6 +74,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
msgs[ERROR_NOT_UPTODATE_DIR] =
"Updating the following directories would lose untracked files in it:\n%s";
+ msgs[ERROR_NOT_UPTODATE_SUBMODULE] =
+ "Updating the following submodules would lose modifications in it:\n%s";
if (advice_commit_before_merge)
msg = "The following untracked working tree files would be %s by %s:\n%%s"
@@ -1221,17 +1226,15 @@ static int verify_uptodate_1(const struct cache_entry *ce,
if (!lstat(ce->name, &st)) {
int flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;
unsigned changed = ie_match_stat(o->src_index, ce, &st, flags);
- if (!changed)
- return 0;
- /*
- * NEEDSWORK: the current default policy is to allow
- * submodule to be out of sync wrt the superproject
- * index. This needs to be tightened later for
- * submodules that are marked to be automatically
- * checked out.
- */
- if (S_ISGITLINK(ce->ce_mode))
- return 0;
+ if (!changed) {
+ if (!S_ISGITLINK(ce->ce_mode) || !submodule_needs_update(ce->name) ||
+ (ce_stage(ce) ? is_submodule_checkout_safe(ce->name, ce->sha1)
+ : !is_submodule_modified(ce->name, 1)))
+ return 0;
+ } else
+ if (S_ISGITLINK(ce->ce_mode) && !submodule_needs_update(ce->name))
+ return 0;
+
errno = 0;
}
if (errno == ENOENT)
@@ -1254,6 +1257,36 @@ static int verify_uptodate_sparse(const struct cache_entry *ce,
return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
}
+/*
+ * When a submodule gets turned into an unmerged entry, we want it to be
+ * up-to-date regarding the merge changes.
+ */
+static int verify_uptodate_submodule(const struct cache_entry *old,
+ const struct cache_entry *new,
+ struct unpack_trees_options *o)
+{
+ struct stat st;
+
+ if (o->index_only || (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old)) && (o->reset || ce_uptodate(old))))
+ return 0;
+ if (!lstat(old->name, &st)) {
+ unsigned changed = ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
+ if (!changed) {
+ if (!S_ISGITLINK(old->ce_mode) ||
+ !submodule_needs_update(new->name) ||
+ is_submodule_checkout_safe(new->name, new->sha1))
+ return 0;
+ } else
+ if (S_ISGITLINK(old->ce_mode) && !submodule_needs_update(new->name))
+ return 0;
+ errno = 0;
+ }
+ if (errno == ENOENT)
+ return 0;
+ return o->gently ? -1 :
+ add_rejected_path(o, ERROR_NOT_UPTODATE_SUBMODULE, old->name);
+}
+
static void invalidate_ce_path(const struct cache_entry *ce,
struct unpack_trees_options *o)
{
@@ -1532,9 +1565,17 @@ static int merged_entry(const struct cache_entry *ce,
copy_cache_entry(merge, old);
update = 0;
} else {
- if (verify_uptodate(old, o)) {
- free(merge);
- return -1;
+ if (S_ISGITLINK(old->ce_mode) ||
+ S_ISGITLINK(merge->ce_mode)) {
+ if (verify_uptodate_submodule(old, merge, o)) {
+ free(merge);
+ return -1;
+ }
+ } else {
+ if (verify_uptodate(old, o)) {
+ free(merge);
+ return -1;
+ }
}
/* Migrate old flags over */
update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
diff --git a/unpack-trees.h b/unpack-trees.h
index 36a73a6..bee8740 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -15,6 +15,7 @@ enum unpack_trees_error_types {
ERROR_WOULD_OVERWRITE = 0,
ERROR_NOT_UPTODATE_FILE,
ERROR_NOT_UPTODATE_DIR,
+ ERROR_NOT_UPTODATE_SUBMODULE,
ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN,
ERROR_WOULD_LOSE_UNTRACKED_REMOVED,
ERROR_BIND_OVERLAP,
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] Teach checkout to recursively checkout submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
` (3 preceding siblings ...)
2013-12-26 16:15 ` [PATCH 4/5] submodule: teach unpack_trees() to update submodules Jonathan Nieder
@ 2013-12-26 16:22 ` Jonathan Nieder
2013-12-27 14:14 ` Jens Lehmann
2013-12-26 19:58 ` [WIP/PATCH 0/5] git checkout --recurse-submodules Junio C Hamano
2013-12-27 13:34 ` Jens Lehmann
6 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2013-12-26 16:22 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt
From: Jens Lehmann <Jens.Lehmann@web.de>
Date: Wed, 13 Jun 2012 18:50:10 +0200
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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>::
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] <branch>"),
N_("git checkout [options] [<branch>] -- <file>..."),
@@ -1111,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;
+ }
+ return 0;
+}
+
int submodule_needs_update(const char *path)
{
struct string_list_item *path_option;
diff --git a/submodule.h b/submodule.h
index b42ae91..9841da3 100644
--- a/submodule.h
+++ b/submodule.h
@@ -3,6 +3,7 @@
struct diff_options;
struct argv_array;
+struct option;
enum {
RECURSE_SUBMODULES_ON_DEMAND = -1,
@@ -23,6 +24,8 @@ int parse_submodule_config_option(const char *var, const char *value);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
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);
int submodule_needs_update(const char *path);
int populate_submodule(const char *path, unsigned char sha1[20], int force);
int depopulate_submodule(const char *path);
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 06b18f8..bc3e1ca 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -4,17 +4,57 @@ test_description='checkout can handle submodules'
. ./test-lib.sh
+submodule_creation_must_succeed() {
+ # checkout base ($1)
+ git checkout -f --recurse-submodules $1 &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached $1 &&
+
+ # checkout target ($2)
+ if test -d submodule; then
+ echo change>>submodule/first.t &&
+ test_must_fail git checkout --recurse-submodules $2 &&
+ git checkout -f --recurse-submodules $2
+ else
+ git checkout --recurse-submodules $2
+ fi &&
+ test -e submodule/.git &&
+ test -f submodule/first.t &&
+ test -f submodule/second.t &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached $2
+}
+
+submodule_removal_must_succeed() {
+ # checkout base ($1)
+ git checkout -f --recurse-submodules $1 &&
+ git submodule update -f &&
+ test -e submodule/.git &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached $1 &&
+
+ # checkout target ($2)
+ echo change>>submodule/first.t &&
+ test_must_fail git checkout --recurse-submodules $2 &&
+ git checkout -f --recurse-submodules $2 &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached $2 &&
+ ! test -d submodule
+}
+
test_expect_success 'setup' '
mkdir submodule &&
(cd submodule &&
git init &&
test_commit first) &&
- git add submodule &&
+ echo first > file &&
+ git add file submodule &&
test_tick &&
git commit -m superproject &&
(cd submodule &&
test_commit second) &&
- git add submodule &&
+ echo second > file &&
+ git add file submodule &&
test_tick &&
git commit -m updated.superproject
'
@@ -36,7 +76,8 @@ test_expect_success '"checkout <submodule>" updates the index only' '
git checkout HEAD^ submodule &&
test_must_fail git diff-files --quiet &&
git checkout HEAD submodule &&
- git diff-files --quiet
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
'
test_expect_success '"checkout <submodule>" honors diff.ignoreSubmodules' '
@@ -62,4 +103,172 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+test_expect_success '"checkout --recurse-submodules" removes deleted submodule' '
+ git config -f .gitmodules submodule.submodule.path submodule &&
+ git config -f .gitmodules submodule.submodule.url submodule.bare &&
+ (cd submodule && git clone --bare . ../submodule.bare) &&
+ echo submodule.bare >>.gitignore &&
+ git config submodule.submodule.ignore none &&
+ git add .gitignore .gitmodules submodule &&
+ git submodule update --init &&
+ git commit -m "submodule registered" &&
+ git checkout -b base &&
+ git checkout -b delete_submodule &&
+ rm -rf submodule &&
+ git rm submodule &&
+ git commit -m "submodule deleted" &&
+ submodule_removal_must_succeed base delete_submodule
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
+ submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule in existing directory' '
+ git checkout --recurse-submodules delete_submodule &&
+ mkdir submodule &&
+ submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_dir &&
+ git update-index --force-remove submodule &&
+ rm -rf submodule/.git .gitmodules &&
+ git add .gitmodules submodule/* &&
+ git commit -m "submodule replaced" &&
+ git checkout -f base &&
+ git submodule update -f &&
+ git checkout --recurse-submodules replace_submodule_with_dir &&
+ test -d submodule &&
+ ! test -e submodule/.git &&
+ test -f submodule/first.t &&
+ test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes files and repopulates submodule' '
+ submodule_creation_must_succeed replace_submodule_with_dir base
+'
+
+test_expect_failure '"checkout --recurse-submodules" replaces submodule with a file' '
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_file &&
+ git update-index --force-remove submodule &&
+ rm -rf submodule .gitmodules &&
+ echo content >submodule &&
+ git add .gitmodules submodule &&
+ git commit -m "submodule replaced with file" &&
+ git checkout -f base &&
+ git submodule update -f &&
+ git checkout --recurse-submodules replace_submodule_with_file &&
+ test -d submodule &&
+ ! test -e submodule/.git &&
+ test -f submodule/first.t &&
+ test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes the file and repopulates submodule' '
+ submodule_creation_must_succeed replace_submodule_with_file base
+'
+
+test_expect_failure '"checkout --recurse-submodules" replaces submodule with a link' '
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_link &&
+ git update-index --force-remove submodule &&
+ rm -rf submodule .gitmodules &&
+ ln -s submodule &&
+ git add .gitmodules submodule &&
+ git commit -m "submodule replaced with link" &&
+ git checkout -f base &&
+ git submodule update -f &&
+ git checkout --recurse-submodules replace_submodule_with_link &&
+ test -d submodule &&
+ ! test -e submodule/.git &&
+ test -f submodule/first.t &&
+ test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes the link and repopulates submodule' '
+ submodule_creation_must_succeed replace_submodule_with_link base
+'
+
+test_expect_success '"checkout --recurse-submodules" updates recursively' '
+ git checkout --recurse-submodules base &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout -b updated_submodule &&
+ (cd submodule &&
+ echo x >>first.t &&
+ git add first.t &&
+ test_commit third) &&
+ git add submodule &&
+ test_tick &&
+ git commit -m updated.superproject &&
+ git checkout --recurse-submodules base &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f to update a modifed submodule commit' '
+ (
+ cd submodule &&
+ git checkout --recurse-submodules HEAD^
+ ) &&
+ test_must_fail git checkout --recurse-submodules master &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f to update modifed submodule content' '
+ echo modified >submodule/second.t &&
+ test_must_fail git checkout --recurse-submodules HEAD^ &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git checkout --recurse-submodules -f HEAD^ &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" ignores modified submodule content that would not be changed' '
+ echo modified >expected &&
+ cp expected submodule/first.t &&
+ git checkout --recurse-submodules HEAD^ &&
+ test_cmp expected submodule/first.t &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" does not care about untracked submodule content' '
+ echo untracked >submodule/untracked &&
+ git checkout --recurse-submodules master &&
+ git diff-files --quiet --ignore-submodules=untracked &&
+ git diff-index --quiet --cached HEAD &&
+ rm submodule/untracked
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f when submodule commit is not present (but does fail anyway)' '
+ git checkout --recurse-submodules -b bogus_commit master &&
+ git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submodule
+ BOGUS_TREE=$(git write-tree) &&
+ BOGUS_COMMIT=$(echo "bogus submodule commit" | git commit-tree $BOGUS_TREE) &&
+ git commit -m "bogus submodule commit" &&
+ git checkout --recurse-submodules -f master &&
+ test_must_fail git checkout --recurse-submodules bogus_commit &&
+ git diff-files --quiet &&
+ test_must_fail git checkout --recurse-submodules -f bogus_commit &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master
+'
+
test_done
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [WIP/PATCH 0/5] git checkout --recurse-submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
` (4 preceding siblings ...)
2013-12-26 16:22 ` [PATCH 5/5] Teach checkout to recursively checkout submodules Jonathan Nieder
@ 2013-12-26 19:58 ` Junio C Hamano
2013-12-27 13:34 ` Jens Lehmann
6 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2013-12-26 19:58 UTC (permalink / raw)
To: Jens Lehmann, Jonathan Nieder; +Cc: git, Heiko Voigt
Jonathan Nieder <jrnieder@gmail.com> writes:
> Hi,
>
> This patch series comes from
> https://github.com/jlehmann/git-submod-enhancements branch
> recursive_submodule_checkout. It needed some tiny tweaks to apply to
> current "master" and build without warnings, but nothing major, and I
> haven't sanity checked it much beyond that and letting the kind folks
> that use Debian experimental play with it.
>
> I'm sending it out now to get review and ideas for what needs to
> happen next to get this series in shape to be included in git.git.
>
> Thoughts of all kinds welcome.
>
> Thanks,
> Jonathan
>
> Jens Lehmann (5):
> submodule: prepare for recursive checkout of submodules
> submodule: teach unpack_trees() to remove submodule contents
> submodule: teach unpack_trees() to repopulate submodules
> submodule: teach unpack_trees() to update submodules
> Teach checkout to recursively checkout submodules
>
> Documentation/git-checkout.txt | 8 ++
> builtin/checkout.c | 14 +++
> entry.c | 19 +++-
> submodule.c | 217 ++++++++++++++++++++++++++++++++++++++++-
> submodule.h | 11 +++
> t/t2013-checkout-submodule.sh | 215 +++++++++++++++++++++++++++++++++++++++-
> unpack-trees.c | 94 ++++++++++++++----
> unpack-trees.h | 1 +
> wrapper.c | 3 +
> 9 files changed, 556 insertions(+), 26 deletions(-)
Looks reasonably clean from a cursory read. Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [WIP/PATCH 0/5] git checkout --recurse-submodules
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
` (5 preceding siblings ...)
2013-12-26 19:58 ` [WIP/PATCH 0/5] git checkout --recurse-submodules Junio C Hamano
@ 2013-12-27 13:34 ` Jens Lehmann
6 siblings, 0 replies; 11+ messages in thread
From: Jens Lehmann @ 2013-12-27 13:34 UTC (permalink / raw)
To: Jonathan Nieder, git; +Cc: Heiko Voigt
Am 26.12.2013 16:58, schrieb Jonathan Nieder:
> This patch series comes from
> https://github.com/jlehmann/git-submod-enhancements branch
> recursive_submodule_checkout. It needed some tiny tweaks to apply to
> current "master" and build without warnings, but nothing major, and I
> haven't sanity checked it much beyond that and letting the kind folks
> that use Debian experimental play with it.
Cool! Thanks for rebasing this series and great to hear that more
people are using it.
> I'm sending it out now to get review and ideas for what needs to
> happen next to get this series in shape to be included in git.git.
Excellent timing, Heiko and I wanted to work on this topic in the
coming days anyway.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] submodule: prepare for recursive checkout of submodules
2013-12-26 16:02 ` [PATCH 1/5] submodule: prepare for recursive checkout of submodules Jonathan Nieder
@ 2013-12-27 13:42 ` Jens Lehmann
0 siblings, 0 replies; 11+ messages in thread
From: Jens Lehmann @ 2013-12-27 13:42 UTC (permalink / raw)
To: Jonathan Nieder, git; +Cc: Heiko Voigt
Am 26.12.2013 17:02, schrieb Jonathan Nieder:
> From: Jens Lehmann <Jens.Lehmann@web.de>
> Date: Mon, 18 Jun 2012 22:17:59 +0200
>
> This commit adds the functions needed for configuration, for setting the
> default behavior and for determining if a submodule path should be updated
> automatically.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Should probably be squashed into a patch that uses and documents this
> configuration.
I'm not so sure. At the end of the day at least am, bisect, checkout,
checkout-index, cherry-pick, merge, pull, read-tree, rebase, reset
& stash should have learned this option (and all porcelain must honor
the autoupdate setting by default, plumbing only when requested by
giving the "--recurse-submodules=config" option to make life easier
for scripting). And I think they should learn this one command per
commit, making this one - and the yet-to-be-programmed one introducing
the autoupdate flag - the base for them.
> submodule.c | 36 ++++++++++++++++++++++++++++++++++++
> submodule.h | 3 +++
> 2 files changed, 39 insertions(+)
>
> diff --git a/submodule.c b/submodule.c
> index 613857e..3f18d4d 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -16,6 +16,8 @@ static struct string_list config_name_for_path;
> static struct string_list config_fetch_recurse_submodules_for_name;
> static struct string_list config_ignore_for_name;
> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
> +static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
> +static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> static struct string_list changed_submodule_paths;
> static int initialized_fetch_ref_tips;
> static struct sha1_array ref_tips_before_fetch;
> @@ -382,6 +384,34 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
> }
> }
>
> +int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
> +{
> + switch (git_config_maybe_bool(opt, arg)) {
> + case 1:
> + return RECURSE_SUBMODULES_ON;
> + case 0:
> + return RECURSE_SUBMODULES_OFF;
> + default:
> + if (!strcmp(arg, "checkout"))
> + return RECURSE_SUBMODULES_ON;
> + die("bad %s argument: %s", opt, arg);
> + }
> +}
> +
> +int submodule_needs_update(const char *path)
> +{
> + struct string_list_item *path_option;
> + path_option = unsorted_string_list_lookup(&config_name_for_path, path);
> + if (!path_option)
> + return 0;
> +
> + /* update can't be "none", "merge" or "rebase" */
> +
> + if (option_update_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
> + return 1;
> + return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
> +}
> +
> void show_submodule_summary(FILE *f, const char *path,
> const char *line_prefix,
> unsigned char one[20], unsigned char two[20],
> @@ -589,6 +619,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
> return ret;
> }
>
> +void set_config_update_recurse_submodules(int default_value, int option_value)
> +{
> + config_update_recurse_submodules = default_value;
> + option_update_recurse_submodules = option_value;
> +}
> +
> static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
> {
> int is_present = 0;
> diff --git a/submodule.h b/submodule.h
> index 7beec48..055918c 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -22,12 +22,15 @@ void gitmodules_config(void);
> int parse_submodule_config_option(const char *var, const char *value);
> void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
> int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
> +int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
> +int submodule_needs_update(const char *path);
> void show_submodule_summary(FILE *f, const char *path,
> const char *line_prefix,
> unsigned char one[20], unsigned char two[20],
> unsigned dirty_submodule, const char *meta,
> const char *del, const char *add, const char *reset);
> void set_config_fetch_recurse_submodules(int value);
> +void set_config_update_recurse_submodules(int default_value, int option_value);
> void check_for_new_submodule_commits(unsigned char new_sha1[20]);
> int fetch_populated_submodules(const struct argv_array *options,
> const char *prefix, int command_line_option,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents
2013-12-26 16:12 ` [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents Jonathan Nieder
@ 2013-12-27 13:58 ` Jens Lehmann
0 siblings, 0 replies; 11+ messages in thread
From: Jens Lehmann @ 2013-12-27 13:58 UTC (permalink / raw)
To: Jonathan Nieder, git; +Cc: Heiko Voigt
Am 26.12.2013 17:12, schrieb Jonathan Nieder:
> From: Jens Lehmann <Jens.Lehmann@web.de>
> Date: Tue, 19 Jun 2012 20:55:45 +0200
>
> Implement the functionality needed to enable work tree manipulating
> commands to that a deleted submodule should not only affect the index
> (leaving all the files of the submodule in the work tree) but also to
> remove the work tree of the superproject (including any untracked
> files).
>
> That will only work properly when the submodule uses a gitfile instead of
> a .git directory and no untracked files are present. Otherwise the removal
> will fail with a warning (which is just what happened until now).
>
> Extend rmdir_or_warn() to remove the directories of those submodules which
> are scheduled for removal. Also teach verify_clean_submodule() to check
> that a submodule configured to be removed is not modified before scheduling
> it for removal.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Should this share some code (or just the error message) with the 'git
> rm' code that checks whether a submodule is safe to remove?
Yes, that would make sense.
> rmdir_or_warn is a pretty low-level function --- it feels odd to be
> relying on the git repository layout here. On the other hand, that
> function only has two callers, so it is possible to check quickly
> whether it is safe.
>
> I assume this is mostly for the sake of the caller in unpack-trees?
Yup.
> In builtin/apply.c, remove_file is used for deletion and rename
> patches. Do we want this logic take effect there, too?
I think so. Recursive update should also affect apply and am when
requested via command line or configuration. But the apply
documentation states that it also handles files outside a git
repository, so we would have to disable this logic in that case.
> submodule.c | 37 +++++++++++++++++++++++++++++++++++++
> submodule.h | 1 +
> unpack-trees.c | 6 +++---
> wrapper.c | 3 +++
> 4 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 3f18d4d..a25db46 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -412,6 +412,43 @@ int submodule_needs_update(const char *path)
> return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
> }
>
> +int depopulate_submodule(const char *path)
> +{
> + struct strbuf dot_git = STRBUF_INIT;
> + struct child_process cp;
> + const char *argv[] = {"rm", "-rf", path, NULL};
> +
> + /* Is it populated? */
> + strbuf_addf(&dot_git, "%s/.git", path);
> + if (!resolve_gitdir(dot_git.buf)) {
> + strbuf_release(&dot_git);
> + return 0;
> + }
> + strbuf_release(&dot_git);
> +
> + /* Does it have a .git directory? */
> + if (!submodule_uses_gitfile(path)) {
> + warning(_("cannot remove submodule '%s' because it (or one of "
> + "its nested submodules) uses a .git directory"),
> + path);
> + return -1;
> + }
> +
> + /* Remove the whole submodule directory */
> + memset(&cp, 0, sizeof(cp));
> + cp.argv = argv;
> + cp.env = local_repo_env;
> + cp.git_cmd = 0;
> + cp.no_stdin = 1;
> + if (run_command(&cp)) {
> + warning("Could not remove submodule %s", path);
> + strbuf_release(&dot_git);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> void show_submodule_summary(FILE *f, const char *path,
> const char *line_prefix,
> unsigned char one[20], unsigned char two[20],
> diff --git a/submodule.h b/submodule.h
> index 055918c..df291cf 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -24,6 +24,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
> int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
> int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
> int submodule_needs_update(const char *path);
> +int depopulate_submodule(const char *path);
> void show_submodule_summary(FILE *f, const char *path,
> const char *line_prefix,
> unsigned char one[20], unsigned char two[20],
> diff --git a/unpack-trees.c b/unpack-trees.c
> index ad3e9a0..89b506a 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -8,6 +8,7 @@
> #include "progress.h"
> #include "refs.h"
> #include "attr.h"
> +#include "submodule.h"
>
> /*
> * Error messages expected by scripts out of plumbing commands such as
> @@ -1263,14 +1264,13 @@ static void invalidate_ce_path(const struct cache_entry *ce,
> /*
> * Check that checking out ce->sha1 in subdir ce->name is not
> * going to overwrite any working files.
> - *
> - * Currently, git does not checkout subprojects during a superproject
> - * checkout, so it is not going to overwrite anything.
> */
> static int verify_clean_submodule(const struct cache_entry *ce,
> enum unpack_trees_error_types error_type,
> struct unpack_trees_options *o)
> {
> + if (submodule_needs_update(ce->name) && is_submodule_modified(ce->name, 0))
> + return 1;
> return 0;
> }
>
> diff --git a/wrapper.c b/wrapper.c
> index 0cc5636..425a3fd 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -2,6 +2,7 @@
> * Various trivial helper wrappers around standard functions
> */
> #include "cache.h"
> +#include "submodule.h"
>
> static void do_nothing(size_t size)
> {
> @@ -409,6 +410,8 @@ int unlink_or_warn(const char *file)
>
> int rmdir_or_warn(const char *file)
> {
> + if (submodule_needs_update(file) && depopulate_submodule(file))
> + return -1;
> return warn_if_unremovable("rmdir", file, rmdir(file));
> }
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] Teach checkout to recursively checkout submodules
2013-12-26 16:22 ` [PATCH 5/5] Teach checkout to recursively checkout submodules Jonathan Nieder
@ 2013-12-27 14:14 ` Jens Lehmann
0 siblings, 0 replies; 11+ messages in thread
From: Jens Lehmann @ 2013-12-27 14:14 UTC (permalink / raw)
To: Jonathan Nieder, git; +Cc: Heiko Voigt
Am 26.12.2013 17:22, schrieb Jonathan Nieder:
> From: Jens Lehmann <Jens.Lehmann@web.de>
> Date: Wed, 13 Jun 2012 18:50:10 +0200
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> 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 ...
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-12-27 14:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-26 15:58 [WIP/PATCH 0/5] git checkout --recurse-submodules Jonathan Nieder
2013-12-26 16:02 ` [PATCH 1/5] submodule: prepare for recursive checkout of submodules Jonathan Nieder
2013-12-27 13:42 ` Jens Lehmann
2013-12-26 16:12 ` [PATCH 2/5] submodule: teach unpack_trees() to remove submodule contents Jonathan Nieder
2013-12-27 13:58 ` Jens Lehmann
2013-12-26 16:14 ` [PATCH 3/5] submodule: teach unpack_trees() to repopulate submodules Jonathan Nieder
2013-12-26 16:15 ` [PATCH 4/5] submodule: teach unpack_trees() to update submodules Jonathan Nieder
2013-12-26 16:22 ` [PATCH 5/5] Teach checkout to recursively checkout submodules Jonathan Nieder
2013-12-27 14:14 ` Jens Lehmann
2013-12-26 19:58 ` [WIP/PATCH 0/5] git checkout --recurse-submodules Junio C Hamano
2013-12-27 13:34 ` Jens Lehmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).