* [PATCH 04/16] update submodules: add is_submodule_populated
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
This is nearly same as Brandon sent out.
(First patch of origin/bw/grep-recurse-submodules,
will drop this patch once Brandons series is stable
enough to build on).
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 11 +++++++++++
submodule.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/submodule.c b/submodule.c
index c9d22e5..97eaf7c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -914,6 +914,17 @@ int fetch_populated_submodules(const struct argv_array *options,
return spf.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;
diff --git a/submodule.h b/submodule.h
index afc58d0..d44b4f1 100644
--- a/submodule.h
+++ b/submodule.h
@@ -61,6 +61,7 @@ extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
int quiet, int max_parallel_jobs);
extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
+extern int is_submodule_populated(const char *path);
extern int submodule_uses_gitfile(const char *path);
extern int ok_to_remove_submodule(const char *path);
extern int merge_submodule(unsigned char result[20], const char *path,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 03/16] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
This addresses a similar concern as in f8eaa0ba98b (submodule--helper,
module_clone: always operate on absolute paths, 2016-03-31)
When computing the relative path from one to another location, we
need to provide both locations as absolute paths to make sure the
computation of the relative path is correct.
While at it, change `real_work_tree` to be non const as we own
the memory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 53a6dbb..c9d22e5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1221,23 +1221,25 @@ 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_work_tree = xstrdup(real_path(work_tree));
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, real_work_tree, &rel_path));
+ relative_path(real_git_dir, real_work_tree, &rel_path));
/* 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);
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));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(real_work_tree);
+ free(real_git_dir);
}
int parallel_submodules(void)
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 07/16] update submodules: introduce submodule_is_interesting
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
In later patches we introduce the --recurse-submodule flag for commands
that modify the working directory, e.g. git-checkout.
It is potentially expensive to check if a submodule needs an update,
because a common theme to interact with submodules is to spawn a child
process for each interaction.
So let's introduce a function that pre checks if a submodule needs
to be checked for an update.
I am not particular happy with the name `submodule_is_interesting`,
in internal iterations I had `submodule_requires_check_for_update`
and `submodule_needs_update`, but I was even less happy with those
names. Maybe `submodule_interesting_for_update`?
Generally this is to answer "Am I allowed to touch the submodule
at all?" or: "Does the user expect me to touch it?"
which includes all of creation/deletion/update.
This patch is based off a prior attempt by Jens Lehmann to add
submodules to checkout.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 37 +++++++++++++++++++++++++++++++++++++
submodule.h | 8 ++++++++
2 files changed, 45 insertions(+)
diff --git a/submodule.c b/submodule.c
index 38b0573..d34b721 100644
--- a/submodule.c
+++ b/submodule.c
@@ -500,6 +500,43 @@ void set_config_update_recurse_submodules(int value)
config_update_recurse_submodules = value;
}
+int submodules_interesting_for_update(void)
+{
+ /*
+ * Update can't be "none", "merge" or "rebase",
+ * treat any value as OFF, except an explicit ON.
+ */
+ return config_update_recurse_submodules == RECURSE_SUBMODULES_ON;
+}
+
+int submodule_is_interesting(const char *path, const unsigned char *sha1)
+{
+ /*
+ * If we cannot load a submodule config, we cannot get the name
+ * of the submodule, so we'd need to follow the gitlink file
+ */
+ const struct submodule *sub;
+
+ if (!submodules_interesting_for_update())
+ return 0;
+
+ sub = submodule_from_path(sha1, path);
+ if (!sub)
+ return 0;
+
+ switch (sub->update_strategy.type) {
+ case SM_UPDATE_UNSPECIFIED:
+ case SM_UPDATE_CHECKOUT:
+ return 1;
+ case SM_UPDATE_REBASE:
+ case SM_UPDATE_MERGE:
+ case SM_UPDATE_NONE:
+ case SM_UPDATE_COMMAND:
+ return 0;
+ }
+ return 0;
+}
+
static int has_remote(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
diff --git a/submodule.h b/submodule.h
index 185ad18..3df6881 100644
--- a/submodule.h
+++ b/submodule.h
@@ -57,6 +57,14 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
const struct diff_options *opt);
extern void set_config_fetch_recurse_submodules(int value);
extern void set_config_update_recurse_submodules(int value);
+/**
+ * When updating the working tree, do we need to check if the submodule needs
+ * updating. We do not require a check if we are already sure that the
+ * submodule doesn't need updating, e.g. when we are not interested in submodules
+ * or the submodule is marked uninteresting by being not initialized.
+ */
+extern int submodule_is_interesting(const char *path, const unsigned char *sha1);
+extern int submodules_interesting_for_update(void);
extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 16/16] checkout: add config option to recurse into submodules by default
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
To make it easier for the user, who doesn't want to give the
`--recurse-submodules` option whenever they run checkout, have an
option for to set the default behavior for checkout to recurse into
submodules.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/config.txt | 6 ++++++
Documentation/git-checkout.txt | 5 +++--
submodule.c | 6 +++---
t/t2013-checkout-submodule.sh | 19 +++++++++++++++++++
4 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66a..67e0714 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -949,6 +949,12 @@ clean.requireForce::
A boolean to make git-clean do nothing unless given -f,
-i or -n. Defaults to true.
+checkout.recurseSubmodules::
+ This option can be set to a boolean value.
+ When set to true checkout will recurse into submodules and
+ update them. When set to false, which is the default, checkout
+ will leave submodules untouched.
+
color.branch::
A boolean to enable/disable color in the output of
linkgit:git-branch[1]. May be set to `always`,
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index a0ea2c5..819c430 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -260,8 +260,9 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
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.
+ will fail until `-f` is used. If `--no-recurse-submodules` is used,
+ the work trees of submodules will not be updated. If no command line
+ argument is given, `checkout.recurseSubmodules` is used as a default.
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
diff --git a/submodule.c b/submodule.c
index 2149ef7..0c807d9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -160,10 +160,10 @@ int submodule_config(const char *var, const char *value, void *cb)
return 0;
} else if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
- else if (!strcmp(var, "fetch.recursesubmodules")) {
+ else if (!strcmp(var, "fetch.recursesubmodules"))
config_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);
- return 0;
- }
+ else if (!strcmp(var, "checkout.recursesubmodules"))
+ config_update_recurse_submodules = parse_update_recurse_submodules_arg(var, value);
return 0;
}
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 60f6987..788c59d 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -149,6 +149,25 @@ test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
)
'
+test_expect_success 'option checkout.recurseSubmodules updates submodule' '
+ test_config -C super checkout.recurseSubmodules 1 &&
+ (
+ cd super &&
+ git checkout base &&
+ git checkout -b advanced-base &&
+ git -C submodule commit --allow-empty -m "empty commit" &&
+ git add submodule &&
+ git commit -m "advance submodule" &&
+ git checkout base &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached base &&
+ git checkout advanced-base &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached advanced-base &&
+ git checkout --recurse-submodules base
+ )
+'
+
test_expect_success '"checkout --recurse-submodules" repopulates submodule in existing directory' '
(
cd super &&
--
2.10.1.469.g00a8914
^ permalink raw reply related
* RE: [PATCH 02/16] submodule: modernize ok_to_remove_submodule to use argv_array
From: David Turner @ 2016-11-15 23:11 UTC (permalink / raw)
To: 'Stefan Beller'
Cc: git@vger.kernel.org, bmwill@google.com, gitster@pobox.com,
jrnieder@gmail.com, mogulguy10@gmail.com
In-Reply-To: <20161115230651.23953-3-sbeller@google.com>
> - "-u",
...
> + argv_array_pushl(&cp.args, "status", "--porcelain", "-uall",
This also changes -u to -uall, which is not mentioned in the commit message. That should probably be called out.
^ permalink raw reply
* [PATCH 13/16] submodule: teach unpack_trees() to update submodules
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
entry.c | 7 +++--
unpack-trees.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++----------
unpack-trees.h | 1 +
3 files changed, 86 insertions(+), 19 deletions(-)
diff --git a/entry.c b/entry.c
index 2330b6e..dd829ec 100644
--- a/entry.c
+++ b/entry.c
@@ -270,7 +270,7 @@ int checkout_entry(struct cache_entry *ce,
if (!check_path(path.buf, 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)
@@ -287,9 +287,10 @@ 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)) {
+ schedule_submodule_for_update(ce, 1);
return 0;
+ }
if (!state->force)
return error("%s is a directory", path.buf);
remove_subtree(&path);
diff --git a/unpack-trees.c b/unpack-trees.c
index 576e1d5..c5c22ed 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -29,6 +29,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.",
@@ -80,6 +83,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 (!strcmp(cmd, "checkout"))
msg = advice_commit_before_merge
@@ -1315,19 +1320,18 @@ static int verify_uptodate_1(const struct cache_entry *ce,
return 0;
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 (!S_ISGITLINK(ce->ce_mode)) {
+ 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;
+ } else {
+ if (!submodule_is_interesting(ce->name, null_sha1))
+ return 0;
+ if (ce_stage(ce) ? is_submodule_checkout_safe(ce->name, &ce->oid)
+ : !is_submodule_modified(ce->name, 1))
+ return 0;
+ }
errno = 0;
}
if (errno == ENOENT)
@@ -1350,6 +1354,59 @@ 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 (!submodule_is_interesting(new->name, null_sha1))
+ 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) {
+ /* old is always a submodule */
+ if (S_ISGITLINK(new->ce_mode)) {
+ /*
+ * new is also a submodule, so check if we care
+ * and then if can checkout the new sha1 safely
+ */
+ if (submodule_is_interesting(old->name, null_sha1)
+ && is_submodule_checkout_safe(new->name, &new->oid))
+ return 0;
+ } else {
+ /*
+ * new is not a submodule any more, so only
+ * care if we care:
+ */
+ if (submodule_is_interesting(old->name, null_sha1)
+ && ok_to_remove_submodule(old->name))
+ return 0;
+ }
+ } else {
+ if (S_ISGITLINK(new->ce_mode))
+ return !is_submodule_checkout_safe(new->name, &new->oid);
+ else
+ return !ok_to_remove_submodule(old->name);
+ }
+ 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)
{
@@ -1608,9 +1665,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,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 14/16] checkout: recurse into submodules if asked to
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
Allow checkout to recurse into submodules via
the command line option --[no-]recurse-submodules.
The flag for recurse-submodules in its current form
could be an OPT_BOOL, but eventually we may want to have
it as:
git checkout --recurse-submodules=rebase|merge| \
cherry-pick|checkout|none
which resembles the submodule.<name>.update options,
so naturally a value such as
"as-configured-or-X-as-fallback" would also come in handy.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-checkout.txt | 7 +
builtin/checkout.c | 31 +++-
t/lib-submodule-update.sh | 10 +-
t/t2013-checkout-submodule.sh | 325 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 362 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8e2c066..a0ea2c5 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -256,6 +256,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
out anyway. In other words, the ref can be held by more than one
worktree.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject. If
+ local modifications in a submodule would be overwritten the checkout
+ will fail until `-f` is used. If nothing (or --no-recurse-submodules)
+ is used, the work trees of submodules will not be updated.
+
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9b2a5b3..2a626a3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,12 +21,31 @@
#include "submodule-config.h"
#include "submodule.h"
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
static const char * const checkout_usage[] = {
N_("git checkout [<options>] <branch>"),
N_("git checkout [<options>] [<branch>] -- <file>..."),
NULL,
};
+int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
struct checkout_opts {
int patch_mode;
int quiet;
@@ -826,7 +845,8 @@ static int switch_branches(const struct checkout_opts *opts,
parse_commit_or_die(new->commit);
}
- ret = merge_working_tree(opts, &old, new, &writeout_error);
+ ret = merge_working_tree(opts, &old, new, &writeout_error) ||
+ update_submodules(opts->force);
if (ret) {
free(path_to_free);
return ret;
@@ -1160,6 +1180,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
N_("second guess 'git checkout <no-such-branch>'")),
OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the given ref")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
OPT_END(),
};
@@ -1190,6 +1213,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
+ if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+ git_config(submodule_config, NULL);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+ set_config_update_recurse_submodules(recurse_submodules);
+ }
+
if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
die(_("-b, -B and --orphan are mutually exclusive"));
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 79cdd34..e0773c6 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -634,7 +634,13 @@ test_submodule_forced_switch () {
########################## Modified submodule #########################
# Updating a submodule sha1 doesn't update the submodule's work tree
- test_expect_success "$command: modified submodule does not update submodule work tree" '
+ if test "$KNOWN_FAILURE_RECURSE_SUBMODULE_SERIES_BREAKS_REPLACE_SUBMODULE_TEST" = 1
+ then
+ RESULT="failure"
+ else
+ RESULT="success"
+ fi
+ test_expect_$RESULT "$command: modified submodule does not update submodule work tree" '
prolog &&
reset_work_tree_to add_sub1 &&
(
@@ -649,7 +655,7 @@ test_submodule_forced_switch () {
'
# Updating a submodule to an invalid sha1 doesn't update the
# submodule's work tree, subsequent update will fail
- test_expect_success "$command: modified submodule does not update submodule work tree to invalid commit" '
+ test_expect_$RESULT "$command: modified submodule does not update submodule work tree to invalid commit" '
prolog &&
reset_work_tree_to add_sub1 &&
(
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 6847f75..60f6987 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -7,15 +7,21 @@ test_description='checkout can handle submodules'
test_expect_success 'setup' '
mkdir submodule &&
- (cd submodule &&
- git init &&
- test_commit first) &&
- git add submodule &&
+ (
+ cd submodule &&
+ git init &&
+ test_commit first
+ ) &&
+ echo first >file &&
+ git add file submodule &&
test_tick &&
git commit -m superproject &&
- (cd submodule &&
- test_commit second) &&
- git add submodule &&
+ (
+ cd submodule &&
+ test_commit second
+ ) &&
+ echo second > file &&
+ git add file submodule &&
test_tick &&
git commit -m updated.superproject
'
@@ -37,7 +43,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' '
@@ -63,8 +70,310 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+# TODO: hardcode $2
+# use flag instead of checking for directory.
+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
+}
+
+# TODO: inline this:
+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 '"check --recurse-submodules" removes deleted submodule' '
+ # first some setup:
+ git config -f .gitmodules submodule.submodule.path submodule &&
+ git config -f .gitmodules submodule.submodule.url ./submodule.bare &&
+ git -C submodule clone --bare . ../submodule.bare &&
+ echo submodule.bare >>.gitignore &&
+ git add .gitignore .gitmodules submodule &&
+ git commit -m "submodule registered with a gitmodules file" &&
+
+ # for testing this case, do it in a fresh clone with the submodules
+ # git dir inside the superprojects .git/modules dir.
+ git clone --recurse-submodules . super &&
+ (
+ cd super &&
+ 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" does not delete submodule with .git dir inside' '
+ git fetch super delete_submodule &&
+ git checkout --recurse-submodules FETCH_HEAD 2>output.err &&
+ test_i18ngrep "cannot remove submodule" output.err &&
+ test -d submodule/.git
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
+ (
+ cd super &&
+ submodule_creation_must_succeed delete_submodule base
+ )
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule in existing directory' '
+ (
+ cd super &&
+ 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' '
+ (
+ cd super
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_file &&
+ git rm -f submodule &&
+ echo "file instead" >submodule &&
+ git add submodule &&
+ git commit -m "submodule replaced" &&
+ git checkout -f base &&
+ git submodule update -f . &&
+ git checkout --recurse-submodules replace_submodule_with_file &&
+ test -f submodule
+ )
+'
+
+test_expect_success '"checkout --recurse-submodules" removes files and repopulates submodule' '
+ (
+ cd super &&
+ submodule_creation_must_succeed replace_submodule_with_file base
+ )
+'
+
+test_expect_success '"checkout --recurse-submodules" replaces submodule with a directory' '
+ (
+ cd super
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_dir &&
+ git rm -f submodule &&
+ mkdir -p submodule/dir &&
+ echo content >submodule/dir/file &&
+ git add submodule &&
+ git commit -m "submodule replaced with a directory (file inside)" &&
+ 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 -d submodule/dir
+ )
+'
+
+test_expect_success '"checkout --recurse-submodules" removes the directory and repopulates submodule' '
+ (
+ cd super
+ submodule_creation_must_succeed replace_submodule_with_dir base
+ )
+'
+
+test_expect_success SYMLINKS '"checkout --recurse-submodules" replaces submodule with a link' '
+ (
+ cd super
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_link &&
+ git rm -f submodule &&
+ ln -s submodule &&
+ git add submodule &&
+ git commit -m "submodule replaced with a link" &&
+ git checkout -f base &&
+ git submodule update -f . &&
+ git checkout --recurse-submodules replace_submodule_with_link &&
+ test -L submodule
+ )
+'
+
+test_expect_success SYMLINKS '"checkout --recurse-submodules" removes the link and repopulates submodule' '
+ (
+ cd super
+ submodule_creation_must_succeed replace_submodule_with_link base
+ )
+'
+
+test_expect_success '"checkout --recurse-submodules" updates the submodule' '
+ (
+ cd super
+ 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 'untracked file is not deleted' '
+ (
+ cd super &&
+ git checkout --recurse-submodules base &&
+ echo important >submodule/untracked &&
+ test_must_fail git checkout --recurse-submodules delete_submodule &&
+ git checkout -f --recurse-submodules delete_submodule
+ )
+'
+
+test_expect_success 'ignored file works just fine' '
+ (
+ cd super &&
+ git checkout --recurse-submodules base &&
+ echo important >submodule/ignored &&
+ echo ignored >.git/modules/submodule/info/exclude &&
+ git checkout --recurse-submodules delete_submodule
+ )
+'
+
+test_expect_success 'dirty file file is not deleted' '
+ (
+ cd super &&
+ git checkout --recurse-submodules base &&
+ echo important >submodule/first.t &&
+ test_must_fail git checkout --recurse-submodules delete_submodule &&
+ git checkout -f --recurse-submodules delete_submodule
+ )
+'
+
+test_expect_success 'added to index is not deleted' '
+ (
+ cd super &&
+ git checkout --recurse-submodules base &&
+ echo important >submodule/to_index &&
+ git -C submodule add to_index &&
+ test_must_fail git checkout --recurse-submodules delete_submodule &&
+ git checkout -f --recurse-submodules delete_submodule
+ )
+'
+
+# This is ok in theory, we just need to make sure
+# the garbage collection doesn't eat the commit.
+test_expect_success 'different commit prevents from deleting' '
+ (
+ cd super &&
+ git checkout --recurse-submodules base &&
+ echo important >submodule/to_index &&
+ git -C submodule add to_index &&
+ test_must_fail git checkout --recurse-submodules delete_submodule &&
+ git checkout -f --recurse-submodules delete_submodule
+ )
+'
+
+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
+'
+
+KNOWN_FAILURE_RECURSE_SUBMODULE_SERIES_BREAKS_REPLACE_SUBMODULE_TEST=1
test_submodule_switch "git checkout"
+KNOWN_FAILURE_RECURSE_SUBMODULE_SERIES_BREAKS_REPLACE_SUBMODULE_TEST=
test_submodule_forced_switch "git checkout -f"
test_done
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 12/16] entry: write_entry to write populate submodules
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
entry.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/entry.c b/entry.c
index 019826b..2330b6e 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)
@@ -211,6 +212,7 @@ 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);
+ schedule_submodule_for_update(ce, 1);
break;
default:
return error("unknown file mode for %s in index", path);
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 09/16] update submodules: add scheduling to update submodules
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
The walker of a tree is only expected to call `schedule_submodule_for_update`
and once done, to run `update_submodules`. This avoids directory/file
conflicts and later we can parallelize all submodule actions if needed.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 4 +++
2 files changed, 121 insertions(+)
diff --git a/submodule.c b/submodule.c
index 0fa4613..2773151 100644
--- a/submodule.c
+++ b/submodule.c
@@ -308,6 +308,75 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
strbuf_release(&sb);
}
+static int update_submodule(const char *path, const struct object_id *oid,
+ int force, int is_new)
+{
+ const char *git_dir;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const struct submodule *sub = submodule_from_path(null_sha1, path);
+
+ if (!sub || !sub->name)
+ return -1;
+
+ git_dir = resolve_gitdir(git_common_path("modules/%s", sub->name));
+
+ if (!git_dir)
+ return -1;
+
+ if (is_new)
+ connect_work_tree_and_git_dir(path, git_dir);
+
+ /* update index via `read-tree --reset sha1` */
+ argv_array_pushl(&cp.args, "read-tree",
+ force ? "--reset" : "-m",
+ "-u", sha1_to_hex(oid->hash), NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ if (run_command(&cp)) {
+ warning(_("reading the index in submodule '%s' failed"), path);
+ child_process_clear(&cp);
+ return -1;
+ }
+
+ /* write index to working dir */
+ child_process_clear(&cp);
+ child_process_init(&cp);
+ argv_array_pushl(&cp.args, "checkout-index", "-a", NULL);
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ if (force)
+ argv_array_push(&cp.args, "-f");
+
+ if (run_command(&cp)) {
+ warning(_("populating the working directory in submodule '%s' failed"), path);
+ child_process_clear(&cp);
+ return -1;
+ }
+
+ /* get the HEAD right */
+ child_process_clear(&cp);
+ child_process_init(&cp);
+ argv_array_pushl(&cp.args, "checkout", "--recurse-submodules", NULL);
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ if (force)
+ argv_array_push(&cp.args, "-f");
+ argv_array_push(&cp.args, sha1_to_hex(oid->hash));
+
+ if (run_command(&cp)) {
+ warning(_("setting the HEAD in submodule '%s' failed"), path);
+ child_process_clear(&cp);
+ return -1;
+ }
+
+ child_process_clear(&cp);
+ return 0;
+}
+
int depopulate_submodule(const char *path)
{
int ret = 0;
@@ -1336,3 +1405,51 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+struct scheduled_submodules_update_type {
+ const char *path;
+ const struct object_id *oid;
+ /*
+ * Do we need to perform a complete checkout or just incremental
+ * update?
+ */
+ unsigned is_new:1;
+} *scheduled_submodules;
+#define SCHEDULED_SUBMODULES_INIT {NULL, NULL}
+
+int scheduled_submodules_nr, scheduled_submodules_alloc;
+
+void schedule_submodule_for_update(const struct cache_entry *ce, int is_new)
+{
+ struct scheduled_submodules_update_type *ssu;
+ ALLOC_GROW(scheduled_submodules,
+ scheduled_submodules_nr + 1,
+ scheduled_submodules_alloc);
+ ssu = &scheduled_submodules[scheduled_submodules_nr++];
+ ssu->path = ce->name;
+ ssu->oid = &ce->oid;
+ ssu->is_new = !!is_new;
+}
+
+int update_submodules(int force)
+{
+ int i;
+ gitmodules_config();
+
+ /*
+ * NEEDSWORK: As submodule updates can potentially take some
+ * time each and they do not overlap (i.e. no d/f conflicts;
+ * this can be parallelized using the run_commands.h API.
+ */
+ for (i = 0; i < scheduled_submodules_nr; i++) {
+ struct scheduled_submodules_update_type *ssu =
+ &scheduled_submodules[i];
+
+ if (submodule_is_interesting(ssu->path, null_sha1))
+ update_submodule(ssu->path, ssu->oid,
+ force, ssu->is_new);
+ }
+
+ scheduled_submodules_nr = 0;
+ return 0;
+}
diff --git a/submodule.h b/submodule.h
index 8518cf3..f01f87c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -94,4 +94,8 @@ extern int parallel_submodules(void);
*/
extern void prepare_submodule_repo_env(struct argv_array *out);
+extern void schedule_submodule_for_update(const struct cache_entry *ce,
+ int new);
+extern int update_submodules(int force);
+
#endif
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 11/16] teach unpack_trees() to remove submodule contents
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
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: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 6 ++----
wrapper.c | 4 ++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ea6bdd2..576e1d5 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -9,6 +9,7 @@
#include "refs.h"
#include "attr.h"
#include "split-index.h"
+#include "submodule.h"
#include "dir.h"
/*
@@ -1361,15 +1362,12 @@ 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)
{
- return 0;
+ return submodule_is_interesting(ce->name, null_sha1) && is_submodule_modified(ce->name, 0);
}
static int verify_clean_subdirectory(const struct cache_entry *ce,
diff --git a/wrapper.c b/wrapper.c
index e7f1979..17c08de 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)
{
@@ -592,6 +593,9 @@ int unlink_or_warn(const char *file)
int rmdir_or_warn(const char *file)
{
+ if (submodule_is_interesting(file, null_sha1)
+ && depopulate_submodule(file))
+ return -1;
return warn_if_unremovable("rmdir", file, rmdir(file));
}
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 08/16] update submodules: add depopulate_submodule
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
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).
To do so, we need an equivalent of "rm -rf", which is already found in
entry.c, so expose that and for clarity add a suffix "_or_dir" to it.
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).
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 2 ++
entry.c | 8 ++++++++
submodule.c | 25 +++++++++++++++++++++++++
submodule.h | 1 +
4 files changed, 36 insertions(+)
diff --git a/cache.h b/cache.h
index a50a61a..65c47e4 100644
--- a/cache.h
+++ b/cache.h
@@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);
*/
void safe_create_dir(const char *dir, int share);
+void remove_subtree_or_die(const char *path);
+
#endif /* CACHE_H */
diff --git a/entry.c b/entry.c
index c6eea24..019826b 100644
--- a/entry.c
+++ b/entry.c
@@ -73,6 +73,14 @@ static void remove_subtree(struct strbuf *path)
die_errno("cannot rmdir '%s'", path->buf);
}
+void remove_subtree_or_die(const char *path)
+{
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addstr(&sb, path);
+ remove_subtree(&sb);
+ strbuf_release(&sb);
+}
+
static int create_file(const char *path, unsigned int mode)
{
mode = (mode & 0100) ? 0777 : 0666;
diff --git a/submodule.c b/submodule.c
index d34b721..0fa4613 100644
--- a/submodule.c
+++ b/submodule.c
@@ -308,6 +308,31 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
strbuf_release(&sb);
}
+int depopulate_submodule(const char *path)
+{
+ int ret = 0;
+ char *dot_git = xstrfmt("%s/.git", path);
+
+ /* Is it populated? */
+ if (!resolve_gitdir(dot_git))
+ goto out;
+
+ /* 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);
+ ret = -1;
+ goto out;
+ }
+
+ remove_subtree_or_die(path);
+
+out:
+ free(dot_git);
+ return ret;
+}
+
/* Helper function to display the submodule header line prior to the full
* summary output. If it can locate the submodule objects directory it will
* attempt to lookup both the left and right commits and put them into the
diff --git a/submodule.h b/submodule.h
index 3df6881..8518cf3 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,6 +65,7 @@ extern void set_config_update_recurse_submodules(int value);
*/
extern int submodule_is_interesting(const char *path, const unsigned char *sha1);
extern int submodules_interesting_for_update(void);
+extern int depopulate_submodule(const char *path);
extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 06/16] update submodules: add a config option to determine if submodules are updated
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
In later patches we introduce the options and flag for commands
that modify the working directory, e.g. git-checkout.
Have a central place to store such settings whether we want to update
a submodule.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 6 ++++++
submodule.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/submodule.c b/submodule.c
index 97eaf7c..38b0573 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,7 @@
#include "quote.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int parallel_jobs = 1;
static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;
static int initialized_fetch_ref_tips;
@@ -494,6 +495,11 @@ void set_config_fetch_recurse_submodules(int value)
config_fetch_recurse_submodules = value;
}
+void set_config_update_recurse_submodules(int value)
+{
+ config_update_recurse_submodules = value;
+}
+
static int has_remote(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
diff --git a/submodule.h b/submodule.h
index d44b4f1..185ad18 100644
--- a/submodule.h
+++ b/submodule.h
@@ -56,6 +56,7 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
const char *del, const char *add, const char *reset,
const struct diff_options *opt);
extern void set_config_fetch_recurse_submodules(int value);
+extern void set_config_update_recurse_submodules(int value);
extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 01/16] submodule.h: add extern keyword to functions, break line before 80
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
As the upcoming series will add a lot of functions to the submodule
header, it's good to start of a sane base. So format the header to
not exceed 80 characters a line and mark all functions to be extern.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.h | 60 ++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/submodule.h b/submodule.h
index d9e197a..afc58d0 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,50 +29,58 @@ struct submodule_update_strategy {
};
#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
-int is_staging_gitmodules_ok(void);
-int update_path_in_gitmodules(const char *oldpath, const char *newpath);
-int remove_path_from_gitmodules(const char *path);
-void stage_updated_gitmodules(void);
-void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
+extern int is_staging_gitmodules_ok(void);
+extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
+extern int remove_path_from_gitmodules(const char *path);
+extern void stage_updated_gitmodules(void);
+extern void set_diffopt_flags_from_submodule_config(
+ struct diff_options *diffopt,
const char *path);
-int submodule_config(const char *var, const char *value, void *cb);
-void gitmodules_config(void);
-int parse_submodule_update_strategy(const char *value,
+extern int submodule_config(const char *var, const char *value, void *cb);
+extern void gitmodules_config(void);
+extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
-const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
-void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
-void show_submodule_summary(FILE *f, const char *path,
+extern const char *submodule_strategy_to_string(
+ const struct submodule_update_strategy *s);
+extern void handle_ignore_submodules_arg(struct diff_options *diffopt,
+ const char *);
+extern void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
-void show_submodule_inline_diff(FILE *f, const char *path,
+extern void show_submodule_inline_diff(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset,
const struct diff_options *opt);
-void set_config_fetch_recurse_submodules(int value);
-void check_for_new_submodule_commits(unsigned char new_sha1[20]);
-int fetch_populated_submodules(const struct argv_array *options,
+extern void set_config_fetch_recurse_submodules(int value);
+extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
+extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
int quiet, int max_parallel_jobs);
-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);
-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,
- struct string_list *needs_pushing);
-int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
-int parallel_submodules(void);
+extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
+extern int submodule_uses_gitfile(const char *path);
+extern int ok_to_remove_submodule(const char *path);
+extern 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);
+extern int find_unpushed_submodules(unsigned char new_sha1[20],
+ const char *remotes_name,
+ struct string_list *needs_pushing);
+extern int push_unpushed_submodules(unsigned char new_sha1[20],
+ const char *remotes_name);
+extern void connect_work_tree_and_git_dir(const char *work_tree,
+ const char *git_dir);
+extern int parallel_submodules(void);
/*
* Prepare the "env_array" parameter of a "struct child_process" for executing
* a submodule by clearing any repo-specific envirionment variables, but
* retaining any config in the environment.
*/
-void prepare_submodule_repo_env(struct argv_array *out);
+extern void prepare_submodule_repo_env(struct argv_array *out);
#endif
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [RFC PATCH 00/16] Checkout aware of Submodules!
From: Stefan Beller @ 2016-11-15 23:06 UTC (permalink / raw)
Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner,
Stefan Beller
When working with submodules, nearly anytime after checking out
a different state of the projects, that has submodules changed
you'd run "git submodule update" with a current version of Git.
There are two problems with this approach:
* The "submodule update" command is dangerous as it
doesn't check for work that may be lost in the submodule
(e.g. a dangling commit).
* you may forget to run the command as checkout is supposed
to do all the work for you.
Integrate updating the submodules into git checkout, with the same
safety promises that git-checkout has, i.e. not throw away data unless
asked to. This is done by first checking if the submodule is at the same
sha1 as it is recorded in the superproject. If there are changes we stop
proceeding the checkout just like it is when checking out a file that
has local changes.
The integration happens in the code that is also used in other commands
such that it will be easier in the future to make other commands aware
of submodule.
This also solves d/f conflicts in case you replace a file/directory
with a submodule or vice versa.
The patches are still a bit rough, but the overall series seems
promising enough to me that I want to put it out here.
Any review, specifically on the design level welcome!
Thanks,
Stefan
Stefan Beller (16):
submodule.h: add extern keyword to functions, break line before 80
submodule: modernize ok_to_remove_submodule to use argv_array
submodule: use absolute path for computing relative path connecting
update submodules: add is_submodule_populated
update submodules: add submodule config parsing
update submodules: add a config option to determine if submodules are
updated
update submodules: introduce submodule_is_interesting
update submodules: add depopulate_submodule
update submodules: add scheduling to update submodules
update submodules: is_submodule_checkout_safe
teach unpack_trees() to remove submodule contents
entry: write_entry to write populate submodules
submodule: teach unpack_trees() to update submodules
checkout: recurse into submodules if asked to
completion: add '--recurse-submodules' to checkout
checkout: add config option to recurse into submodules by default
Documentation/config.txt | 6 +
Documentation/git-checkout.txt | 8 +
builtin/checkout.c | 31 ++-
cache.h | 2 +
contrib/completion/git-completion.bash | 2 +-
entry.c | 17 +-
submodule-config.c | 22 +++
submodule-config.h | 17 +-
submodule.c | 246 +++++++++++++++++++++--
submodule.h | 77 +++++---
t/lib-submodule-update.sh | 10 +-
t/t2013-checkout-submodule.sh | 344 ++++++++++++++++++++++++++++++++-
t/t9902-completion.sh | 1 +
unpack-trees.c | 103 ++++++++--
unpack-trees.h | 1 +
wrapper.c | 4 +
16 files changed, 806 insertions(+), 85 deletions(-)
--
2.10.1.469.g00a8914
^ permalink raw reply
* Re: [PATCH v3 4/4] submodule_needs_pushing() NEEDSWORK when we can not answer this question
From: Stefan Beller @ 2016-11-15 22:39 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, git@vger.kernel.org, Jeff King, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <1d87628994df89751afdcc7e180ebcdc29dde722.1479221071.git.hvoigt@hvoigt.net>
On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
> submodule.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/submodule.c b/submodule.c
> index e1196fd..29efee9 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -531,6 +531,14 @@ static int submodule_has_commits(const char *path, struct sha1_array *commits)
> static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
> {
> if (!submodule_has_commits(path, commits))
> + /* NEEDSWORK: The correct answer here is "We do not
style nit:
/*
* Usually we prefer comments with both the first and last line of the
comment "empty".
*/
/* or just a one liner */
AFAICT these are the only two modes that we prefer in Git.
For a discussion of all the other style, enjoy Linus' guidance. ;)
http://lkml.iu.edu/hypermail/linux/kernel/1607.1/00627.html
> "We do not know" ...
... because there is no way to check for us as we don't have the
submodule commits.
" We do consider it safe as no one in their sane mind would
have changed the submodule pointers without having the
submodule around. If a user did however change the submodules
without having the submodule commits around, this indicates an
expert who knows what they were doing."
> We currently
> + * proceed pushing here as if the submodules commits are
> + * available on a remote. Since we can not check the
> + * remote availability for this submodule we should
> + * consider changing this behavior to: Stop here and
> + * tell the user how to skip this check if wanted.
> + */
> return 0;
Thanks for adding the NEEDSWORK, I just wrote the above lines
to clarify my thought process, not as a suggestion for change.
Overall the series looks good to me; the nits are minor IMHO.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v3 3/4] batch check whether submodule needs pushing into one call
From: Stefan Beller @ 2016-11-15 22:28 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, git@vger.kernel.org, Jeff King, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <0f1aaa07e151f6be87eb61b434f8c9448f8dad75.1479221071.git.hvoigt@hvoigt.net>
On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> -static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
> +static int check_has_commit(const unsigned char sha1[20], void *data)
> {
> - if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> + int *has_commit = (int *) data;
nit: just as prior patches ;) void* can be cast implicitly.
^ permalink raw reply
* Re: [PATCH v3 2/4] serialize collection of refs that contain submodule changes
From: Stefan Beller @ 2016-11-15 22:20 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, git@vger.kernel.org, Jeff King, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <616038c9b59cfcccce180de480c2a168921b9a63.1479221071.git.hvoigt@hvoigt.net>
On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> +++ b/submodule.c
> @@ -500,6 +500,13 @@ static int has_remote(const char *refname, const struct object_id *oid,
> return 1;
> }
>
> +static int append_sha1_to_argv(const unsigned char sha1[20], void *data)
> +{
> + struct argv_array *argv = (struct argv_array *) data;
nit: no explicit cast needed when coming from a void pointer.
^ permalink raw reply
* Re: [PATCH v3 1/4] serialize collection of changed submodules
From: Stefan Beller @ 2016-11-15 22:15 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, git@vger.kernel.org, Jeff King, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <67e11474d4fd8ada2652809cf14f6c4d96be55ff.1479221071.git.hvoigt@hvoigt.net>
On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> @@ -560,6 +575,31 @@ static void find_unpushed_submodule_commits(struct commit *commit,
> diff_tree_combined_merge(commit, 1, &rev);
> }
>
> +struct collect_submodule_from_sha1s_data {
> + char *submodule_path;
> + struct string_list *needs_pushing;
> +};
> +
> +static int collect_submodules_from_sha1s(const unsigned char sha1[20],
> + void *data)
> +{
> + struct collect_submodule_from_sha1s_data *me =
> + (struct collect_submodule_from_sha1s_data *) data;
nit: no explicit cast needed when coming from void* ?
^ permalink raw reply
* Re: [PATCH v15 04/27] bisect--helper: `bisect_clean_state` shell function in C
From: Stephan Beyer @ 2016-11-15 21:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pranit Bauva, git
In-Reply-To: <xmqq37istoay.fsf@gitster.mtv.corp.google.com>
On 11/15/2016 10:40 PM, Junio C Hamano wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
>
>>> +int bisect_clean_state(void)
>>> +{
>>> + int result = 0;
>>> +
>>> + /* There may be some refs packed during bisection */
>>> + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
>>> + for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
>>> + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
>>> + result = delete_refs(&refs_for_removal, REF_NODEREF);
>>> + refs_for_removal.strdup_strings = 1;
>>> + string_list_clear(&refs_for_removal, 0);
>>
>> Does it have advantages to populate a list (with duplicated strings),
>> hand it to delete_refs(), and clear the list (and strings), instead of
>> just doing a single delete_ref() (or whatever name the singular function
>> has) in the callback?
>
> Depending on ref backends, removing multiple refs may be a lot more
> efficient than calling a single ref removal for the same set of
> refs, and the comment upfront I think hints that the code was
> written in the way exactly with that in mind. Removing N refs from
> a packed refs file will involve a loop that runs N times, each
> iteration loading the file, locating an entry among possibly 100s of
> refs to remove, and then rewriting the file.
Great, that's the reply I wanted to hear (and that I've considered but
wasn't sure of) ;)
[I did not want to dig into the sources and check if delete_refs() does
something smarter than invoking delete_ref() on each item of the list.]
~Stephan
^ permalink raw reply
* Re: [PATCH v15 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
From: Stephan Beyer @ 2016-11-15 21:40 UTC (permalink / raw)
To: Junio C Hamano, git
Cc: Pranit Bauva, Christian Couder, Matthieu Moy, Alex Henrie,
Antoine Delaite
In-Reply-To: <xmqqvawd7mnr.fsf@gitster.mtv.corp.google.com>
Hi,
On 10/27/2016 06:59 PM, Junio C Hamano wrote:
> Does any of you (and others on the list) have time and inclination
> to review this series?
Me, currently. ;)
Besides the things I'm mentioning in respective patch e-mails, I wonder
why several bisect--helper commands are prefixed by "bisect"; I'm
talking about:
git bisect--helper --bisect-clean-state
git bisect--helper --bisect-reset
git bisect--helper --bisect-write
git bisect--helper --bisect-check-and-set-terms
git bisect--helper --bisect-next-check
git bisect--helper --bisect-terms
git bisect--helper --bisect-start
etc.
instead of
git bisect--helper --clean-state
git bisect--helper --reset
git bisect--helper --write
git bisect--helper --check-and-set-terms
git bisect--helper --next-check
git bisect--helper --terms
git bisect--helper --start
etc.
Well, I know *why* they have these names: because the shell function
names are simply reused. But I don't know why these prefixes are kept in
the bisect--helper command options. On the other hand, these command
names are not exposed to the user and may hence not be that important.(?)
~Stephan
^ permalink raw reply
* Re: [PATCH v15 04/27] bisect--helper: `bisect_clean_state` shell function in C
From: Junio C Hamano @ 2016-11-15 21:40 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Pranit Bauva, git
In-Reply-To: <13aa642a-2272-c5b8-4a30-382ab5e73b98@gmx.net>
Stephan Beyer <s-beyer@gmx.net> writes:
>> +int bisect_clean_state(void)
>> +{
>> + int result = 0;
>> +
>> + /* There may be some refs packed during bisection */
>> + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
>> + for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
>> + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
>> + result = delete_refs(&refs_for_removal, REF_NODEREF);
>> + refs_for_removal.strdup_strings = 1;
>> + string_list_clear(&refs_for_removal, 0);
>
> Does it have advantages to populate a list (with duplicated strings),
> hand it to delete_refs(), and clear the list (and strings), instead of
> just doing a single delete_ref() (or whatever name the singular function
> has) in the callback?
Depending on ref backends, removing multiple refs may be a lot more
efficient than calling a single ref removal for the same set of
refs, and the comment upfront I think hints that the code was
written in the way exactly with that in mind. Removing N refs from
a packed refs file will involve a loop that runs N times, each
iteration loading the file, locating an entry among possibly 100s of
refs to remove, and then rewriting the file.
Besides, it is bad taste to delete each individual item being
iterated over in an interator in general, isn't it?
^ permalink raw reply
* Re: [PATCH v15 04/27] bisect--helper: `bisect_clean_state` shell function in C
From: Stephan Beyer @ 2016-11-15 21:09 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1a82-dc1c5b57-3e93-4996-87e7-4a1d83cb5817-000000@eu-west-1.amazonses.com>
Hi,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/bisect.c b/bisect.c
> index 6f512c2..45d598d 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -1040,3 +1046,40 @@ int estimate_bisect_steps(int all)
>
> return (e < 3 * x) ? n : n - 1;
> }
> +
> +static int mark_for_removal(const char *refname, const struct object_id *oid,
> + int flag, void *cb_data)
> +{
> + struct string_list *refs = cb_data;
> + char *ref = xstrfmt("refs/bisect%s", refname);
> + string_list_append(refs, ref);
> + return 0;
> +}
> +
> +int bisect_clean_state(void)
> +{
> + int result = 0;
> +
> + /* There may be some refs packed during bisection */
> + struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
> + for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
> + string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
> + result = delete_refs(&refs_for_removal, REF_NODEREF);
> + refs_for_removal.strdup_strings = 1;
> + string_list_clear(&refs_for_removal, 0);
Does it have advantages to populate a list (with duplicated strings),
hand it to delete_refs(), and clear the list (and strings), instead of
just doing a single delete_ref() (or whatever name the singular function
has) in the callback?
I am only seeing the disadvantage: a list with duped strings.
> + unlink_or_warn(git_path_bisect_expected_rev());
[...]
> + unlink_or_warn(git_path_bisect_start());
Comparing it with the original shell code (which uses "rm -f"), I was
wondering a little after reading the function name unlink_or_warn()
here... Looking it up helped: despite its name, unlink_or_warn() is
really the function you have to use here ;D
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 65cf519..4254d61 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -5,10 +5,15 @@
> #include "refs.h"
>
> static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
> +static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
> +static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
> +static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
> +static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
This is perhaps a non-issue, but you do not need any of these new path
functions in *this* patch. I think nobody really cares, though, because
you will need them later.
Cheers
Stephan
^ permalink raw reply
* Re* [PATCH v7 00/17] port branch.c to use ref-filter's printing options
From: Junio C Hamano @ 2016-11-15 20:57 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <xmqqbmxgtqxv.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Something like this needs to go before that step.
This time with a log message and an additional test. The second
paragraph of the proposed log message is written expecting that this
patch will come before your "branch: use ref-filter printing APIs",
which I think is a good place to avoid breakage in the series.
-- >8 --
Subject: [PATCH] for-each-ref: do not segv with %(HEAD) on an unborn branch
The code to flip between "*" and " " prefixes depending on what
branch is checked out used in --format='%(HEAD)' did not consider
that HEAD may resolve to an unborn branch and dereferenced a NULL.
This will become a lot easier to trigger as the codepath will now be
shared with "git branch [--list]".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
ref-filter.c | 2 +-
t/t3203-branch-output.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 944671af5a..c71d7360d2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1318,7 +1318,7 @@ static void populate_value(struct ref_array_item *ref)
head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
sha1, NULL);
- if (!strcmp(ref->refname, head))
+ if (head && !strcmp(ref->refname, head))
v->s = "*";
else
v->s = " ";
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index d8edaf27e9..1a8dbca8c8 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -37,6 +37,14 @@ test_expect_success 'git branch --list shows local branches' '
test_cmp expect actual
'
+test_expect_success 'same, but on an unborn branch' '
+ test_when_finished "git checkout master" &&
+ git checkout --orphan naster &&
+ git branch --list >actual &&
+ sed -e "s/\* / /" expect >expect-unborn &&
+ test_cmp expect-unborn actual
+'
+
cat >expect <<'EOF'
branch-one
branch-two
--
2.11.0-rc1-154-gcd2a643dcd
^ permalink raw reply related
* Re: [PATCH] worktree: fix a sparse 'Using plain integer as NULL pointer' warning
From: Ramsay Jones @ 2016-11-15 20:50 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <5b7d7d0b-8a6c-d516-4eb9-4e4ea13dce73@ramsayjones.plus.com>
On 15/11/16 20:28, Ramsay Jones wrote:
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>
> Hi Duy,
>
> If you need to re-roll your 'nd/worktree-move' branch, could you
> please squash this into the relevant patch [commit c49e92f5c
> ("worktree move: refuse to move worktrees with submodules", 12-11-2016)].
>
> Also, one of the new tests introduced by commit 31a8f3066 ("worktree move:
> new command", 12-11-2016), fails for me, thus:
>
> $ ./t2028-worktree-move.sh -i -v
> ...
> --- expected 2016-11-15 20:22:50.647241458 +0000
> +++ actual 2016-11-15 20:22:50.647241458 +0000
> @@ -1,3 +1,3 @@
> worktree /home/ramsay/git/t/trash directory.t2028-worktree-move
> -worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/destination
> worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/elsewhere
> +worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/destination
> not ok 12 - move worktree
> #
> # git worktree move source destination &&
> # test_path_is_missing source &&
> # git worktree list --porcelain | grep "^worktree" >actual &&
> # cat <<-EOF >expected &&
> # worktree $TRASH_DIRECTORY
> # worktree $TRASH_DIRECTORY/destination
> # worktree $TRASH_DIRECTORY/elsewhere
> # EOF
> # test_cmp expected actual &&
> # git -C destination log --format=%s >actual2 &&
> # echo init >expected2 &&
> # test_cmp expected2 actual2
> #
> $
>
> Is there an expectation that the submodules will be listed in
Er, that should read 'worktrees', of course! :(
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v7 00/17] port branch.c to use ref-filter's printing options
From: Junio C Hamano @ 2016-11-15 20:43 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <20161108201211.25213-1-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> This is part of unification of the commands 'git tag -l, git branch -l
> and git for-each-ref'. This ports over branch.c to use ref-filter's
> printing options.
>
> Karthik Nayak (17):
> ref-filter: implement %(if), %(then), and %(else) atoms
> ref-filter: include reference to 'used_atom' within 'atom_value'
> ref-filter: implement %(if:equals=<string>) and
> %(if:notequals=<string>)
> ref-filter: modify "%(objectname:short)" to take length
> ref-filter: move get_head_description() from branch.c
> ref-filter: introduce format_ref_array_item()
> ref-filter: make %(upstream:track) prints "[gone]" for invalid
> upstreams
> ref-filter: add support for %(upstream:track,nobracket)
> ref-filter: make "%(symref)" atom work with the ':short' modifier
> ref-filter: introduce refname_atom_parser_internal()
> ref-filter: introduce symref_atom_parser() and refname_atom_parser()
> ref-filter: make remote_ref_atom_parser() use
> refname_atom_parser_internal()
> ref-filter: add `:dir` and `:base` options for ref printing atoms
> ref-filter: allow porcelain to translate messages in the output
> branch, tag: use porcelain output
> branch: use ref-filter printing APIs
> branch: implement '--format' option
This is not a new issue, but --format='%(HEAD)' you stole from
for-each-ref is broken when you are on an unborn branch, and the
second patch from the tip makes "git branch" (no other args) on
an unborn branch to segfault, when there are real branches that
have commits.
Something like this needs to go before that step.
ref-filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 944671af5a..c71d7360d2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1318,7 +1318,7 @@ static void populate_value(struct ref_array_item *ref)
head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
sha1, NULL);
- if (!strcmp(ref->refname, head))
+ if (head && !strcmp(ref->refname, head))
v->s = "*";
else
v->s = " ";
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox