* [PATCH 06/14] update submodules: add a config option to determine if submodules are updated
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-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 9bbdd3ce7c..c0060c29f2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -17,6 +17,7 @@
#include "worktree.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;
@@ -545,6 +546,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 689033e538..c4e1ac828e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -58,6 +58,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.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 07/14] update submodules: introduce is_interesting_submodule
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-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.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 26 ++++++++++++++++++++++++++
submodule.h | 8 ++++++++
2 files changed, 34 insertions(+)
diff --git a/submodule.c b/submodule.c
index c0060c29f2..4c33374ae8 100644
--- a/submodule.c
+++ b/submodule.c
@@ -551,6 +551,32 @@ 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 is_interesting_submodule(const struct cache_entry *ce)
+{
+ const struct submodule *sub;
+
+ if (!S_ISGITLINK(ce->ce_mode))
+ return 0;
+
+ if (!submodules_interesting_for_update())
+ return 0;
+
+ sub = submodule_from_path(null_sha1, ce->name);
+ if (!sub)
+ return 0;
+
+ return sub->update_strategy.type != SM_UPDATE_NONE;
+}
+
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 c4e1ac828e..84b67a7c4a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -59,6 +59,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);
+
+/*
+ * Traditionally Git ignored changes made for submodules.
+ * This function checks if we are interested in the given submodule
+ * for any kind of operation.
+ */
+extern int submodules_interesting_for_update(void);
+extern int is_interesting_submodule(const struct cache_entry *ce);
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.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 11/14] unpack-trees: check if we can perform the operation for submodules
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
unpack-trees.h | 1 +
2 files changed, 90 insertions(+), 9 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 616a0ae4b2..6805cb9549 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -10,6 +10,7 @@
#include "attr.h"
#include "split-index.h"
#include "dir.h"
+#include "submodule.h"
/*
* Error messages expected by scripts out of plumbing commands such as
@@ -45,6 +46,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
/* ERROR_WOULD_LOSE_ORPHANED_REMOVED */
"Working tree file '%s' would be removed by sparse checkout update.",
+
+ /* ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE */
+ "Submodule '%s' cannot be deleted as it contains untracked files.",
};
#define ERRORMSG(o,type) \
@@ -161,6 +165,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
_("The following working tree files would be overwritten by sparse checkout update:\n%s");
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
_("The following working tree files would be removed by sparse checkout update:\n%s");
+ msgs[ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE] =
+ _("Submodule '%s' cannot be deleted as it contains untracked files.");
opts->show_all_errors = 1;
/* rejected paths may not have a static buffer */
@@ -240,12 +246,44 @@ static void display_error_msgs(struct unpack_trees_options *o)
fprintf(stderr, _("Aborting\n"));
}
+static int submodule_check_from_to(const struct cache_entry *ce, const char *old_id, const char *new_id, struct unpack_trees_options *o)
+{
+ if (submodule_go_from_to(ce->name, old_id,
+ new_id, 1, o->reset))
+ return o->gently ? -1 :
+ add_rejected_path(o, ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE, ce->name);
+ return 0;
+}
+
+static void reload_gitmodules_file(struct index_state *index,
+ struct checkout *state)
+{
+ int i;
+ for (i = 0; i < index->cache_nr; i++) {
+ struct cache_entry *ce = index->cache[i];
+ if (ce->ce_flags & CE_UPDATE) {
+
+ int r = strcmp(ce->name, ".gitmodules");
+ if (r < 0)
+ continue;
+ else if (r == 0) {
+ checkout_entry(ce, state, NULL);
+ } else
+ break;
+ }
+ }
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+}
+
/*
* Unlink the last component and schedule the leading directories for
* removal, such that empty directories get removed.
*/
static void unlink_entry(const struct cache_entry *ce)
{
+ if (is_interesting_submodule(ce))
+ submodule_go_from_to(ce->name, "HEAD", NULL, 0, 1);
if (!check_leading_path(ce->name, ce_namelen(ce)))
return;
if (remove_or_warn(ce->ce_mode, ce->name))
@@ -301,6 +339,9 @@ static int check_updates(struct unpack_trees_options *o)
remove_marked_cache_entries(index);
remove_scheduled_dirs();
+ if (submodules_interesting_for_update() && o->update && !o->dry_run)
+ reload_gitmodules_file(index, &state);
+
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
@@ -1358,17 +1399,27 @@ 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 (is_interesting_submodule(ce)) {
+ int r;
+ r = submodule_check_from_to(ce,
+ "HEAD", oid_to_hex(&ce->oid), o);
+ if (r)
+ return o->gently ? -1 :
+ add_rejected_path(o, error_type, ce->name);
+ return 0;
+ }
+
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.
+ * Historic default policy was to allow submodule to be out
+ * of sync wrt the superproject index. If the submodule was
+ * not considered interesting above, we don't care here.
*/
if (S_ISGITLINK(ce->ce_mode))
return 0;
+
errno = 0;
}
if (errno == ENOENT)
@@ -1412,7 +1463,12 @@ static int verify_clean_submodule(const char *old_sha1,
enum unpack_trees_error_types error_type,
struct unpack_trees_options *o)
{
- return 0;
+ if (!is_interesting_submodule(ce))
+ return 0;
+
+ return submodule_check_from_to(ce,
+ old_sha1,
+ oid_to_hex(&ce->oid), o);
}
static int verify_clean_subdirectory(const struct cache_entry *ce,
@@ -1578,9 +1634,15 @@ static int verify_absent_1(const struct cache_entry *ce,
path = xmemdupz(ce->name, len);
if (lstat(path, &st))
ret = error_errno("cannot stat '%s'", path);
- else
- ret = check_ok_to_remove(path, len, DT_UNKNOWN, NULL,
- &st, error_type, o);
+ else {
+ if (is_interesting_submodule(ce))
+ ret = submodule_check_from_to(ce,
+ oid_to_hex(&ce->oid),
+ NULL, o);
+ else
+ ret = check_ok_to_remove(path, len, DT_UNKNOWN, NULL,
+ &st, error_type, o);
+ }
free(path);
return ret;
} else if (lstat(ce->name, &st)) {
@@ -1588,6 +1650,10 @@ static int verify_absent_1(const struct cache_entry *ce,
return error_errno("cannot stat '%s'", ce->name);
return 0;
} else {
+ if (is_interesting_submodule(ce))
+ return submodule_check_from_to(ce, oid_to_hex(&ce->oid),
+ NULL, o);
+
return check_ok_to_remove(ce->name, ce_namelen(ce),
ce_to_dtype(ce), ce, &st,
error_type, o);
@@ -1643,6 +1709,16 @@ static int merged_entry(const struct cache_entry *ce,
return -1;
}
invalidate_ce_path(merge, o);
+
+ if (is_interesting_submodule(ce)) {
+ int ret = submodule_check_from_to(ce,
+ oid_to_hex(&old->oid),
+ oid_to_hex(&ce->oid),
+ o);
+ if (ret)
+ return ret;
+ }
+
} else if (!(old->ce_flags & CE_CONFLICTED)) {
/*
* See if we can re-use the old CE directly?
@@ -1663,6 +1739,10 @@ static int merged_entry(const struct cache_entry *ce,
update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
invalidate_ce_path(old, o);
}
+ if (is_interesting_submodule(ce)) {
+ if (submodule_check_from_to(ce, oid_to_hex(&old->oid), oid_to_hex(&ce->oid), o))
+ return -1;
+ }
} else {
/*
* Previously unmerged entry left as an existence
diff --git a/unpack-trees.h b/unpack-trees.h
index 36a73a6d00..c0427ce082 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -21,6 +21,7 @@ enum unpack_trees_error_types {
ERROR_SPARSE_NOT_UPTODATE_FILE,
ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN,
ERROR_WOULD_LOSE_ORPHANED_REMOVED,
+ ERROR_WOULD_LOSE_UNTRACKED_SUBMODULE,
NB_UNPACK_TREES_ERROR_TYPES
};
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 10/14] unpack-trees: pass old oid to verify_clean_submodule
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
The check (which uses the old oid) is yet to be implemented, but this part
is just a refactor, so it can go separately first.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 3a8ee19fe8..616a0ae4b2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1407,7 +1407,8 @@ static void invalidate_ce_path(const struct cache_entry *ce,
* 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,
+static int verify_clean_submodule(const char *old_sha1,
+ const struct cache_entry *ce,
enum unpack_trees_error_types error_type,
struct unpack_trees_options *o)
{
@@ -1427,16 +1428,18 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
struct dir_struct d;
char *pathbuf;
int cnt = 0;
- unsigned char sha1[20];
- if (S_ISGITLINK(ce->ce_mode) &&
- resolve_gitlink_ref(ce->name, "HEAD", sha1) == 0) {
- /* If we are not going to update the submodule, then
+ if (S_ISGITLINK(ce->ce_mode)) {
+ unsigned char sha1[20];
+ int sub_head = resolve_gitlink_ref(ce->name, "HEAD", sha1);
+ /*
+ * If we are not going to update the submodule, then
* we don't care.
*/
- if (!hashcmp(sha1, ce->oid.hash))
+ if (!sub_head && !hashcmp(sha1, ce->oid.hash))
return 0;
- return verify_clean_submodule(ce, error_type, o);
+ return verify_clean_submodule(sub_head ? NULL : sha1_to_hex(sha1),
+ ce, error_type, o);
}
/*
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 09/14] update submodules: add submodule_go_from_to
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
In later patches we introduce the options and flag for commands
that modify the working directory, e.g. git-checkout.
This piece of code will be used universally for
all these working tree modifications as it
* supports dry run to answer the question:
"Is it safe to change the submodule to this new state?"
e.g. is it overwriting untracked files or are there local
changes that would be overwritten?
* supports a force flag that can be used for resetting
the tree.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 5 ++
2 files changed, 156 insertions(+)
diff --git a/submodule.c b/submodule.c
index d3fc6c2a75..194cba9535 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1252,6 +1252,157 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
return ret;
}
+static int submodule_has_dirty_index(const struct submodule *sub)
+{
+ ssize_t len;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ int ret = 0;
+
+ prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+
+ cp.git_cmd = 1;
+ argv_array_pushl(&cp.args, "diff-index", "--cached", "HEAD", NULL);
+ cp.no_stdin = 1;
+ cp.out = -1;
+ cp.dir = sub->path;
+ if (start_command(&cp))
+ die("could not recurse into submodule %s", sub->path);
+
+ len = strbuf_read(&buf, cp.out, 1024);
+ if (len > 2)
+ ret = 1;
+
+ close(cp.out);
+ if (finish_command(&cp))
+ die("could not recurse into submodule %s", sub->path);
+
+ strbuf_release(&buf);
+ return ret;
+}
+
+void submodule_clean_index(const char *path)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+
+ argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
+ argv_array_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);
+
+ argv_array_push(&cp.args, EMPTY_TREE_SHA1_HEX);
+
+ if (run_command(&cp))
+ die("could not clean submodule index");
+}
+
+/**
+ * Moves a submodule at a given path from a given head to another new head.
+ * For edge cases (a submodule coming into existence or removing a submodule)
+ * pass NULL for old or new respectively.
+ *
+ * TODO: move dryrun and forced to flags.
+ */
+int submodule_go_from_to(const char *path,
+ const char *old,
+ const char *new,
+ int dry_run,
+ int force)
+{
+ int ret = 0;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const struct submodule *sub;
+
+ sub = submodule_from_path(null_sha1, path);
+
+ if (!sub)
+ die("BUG: could not get submodule information for '%s'", path);
+
+ if (!dry_run) {
+ if (old) {
+ if (!submodule_uses_gitfile(path))
+ absorb_git_dir_into_superproject("", path,
+ ABSORB_GITDIR_RECURSE_SUBMODULES);
+ } else {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%s/modules/%s",
+ get_git_common_dir(), sub->name);
+ connect_work_tree_and_git_dir(path, sb.buf);
+ strbuf_release(&sb);
+
+ /* make sure the index is clean as well */
+ submodule_clean_index(path);
+ }
+ }
+
+ if (old && !force) {
+ /* Check if the submodule has a dirty index. */
+ if (submodule_has_dirty_index(sub)) {
+ /* print a thing here? */
+ return -1;
+ }
+ }
+
+ prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+
+ argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
+ argv_array_pushl(&cp.args, "read-tree", NULL);
+
+ if (!dry_run)
+ argv_array_push(&cp.args, "-u");
+ else
+ argv_array_push(&cp.args, "-n");
+
+ if (force)
+ argv_array_push(&cp.args, "--reset");
+ else
+ argv_array_push(&cp.args, "-m");
+
+ argv_array_push(&cp.args, old ? old : EMPTY_TREE_SHA1_HEX);
+ argv_array_push(&cp.args, new ? new : EMPTY_TREE_SHA1_HEX);
+
+ if (run_command(&cp)) {
+ ret = -1;
+ goto out;
+ }
+
+ if (!dry_run) {
+ if (new) {
+ struct child_process cp1 = CHILD_PROCESS_INIT;
+ /* also set the HEAD accordingly */
+ cp1.git_cmd = 1;
+ cp1.no_stdin = 1;
+ cp1.dir = path;
+
+ argv_array_pushl(&cp1.args, "update-ref", "HEAD",
+ new ? new : EMPTY_TREE_SHA1_HEX, NULL);
+
+ if (run_command(&cp1)) {
+ ret = -1;
+ goto out;
+ }
+ } else {
+ struct strbuf sb = STRBUF_INIT;
+
+ strbuf_addf(&sb, "%s/.git", path);
+ unlink_or_warn(sb.buf);
+ strbuf_release(&sb);
+
+ if (is_empty_dir(path))
+ rmdir_or_warn(path);
+ }
+ }
+out:
+ return ret;
+}
+
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 84b67a7c4a..570953a351 100644
--- a/submodule.h
+++ b/submodule.h
@@ -91,6 +91,11 @@ extern int push_unpushed_submodules(struct sha1_array *commits,
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
extern int parallel_submodules(void);
+extern int submodule_go_from_to(const char *path,
+ const char *old,
+ const char *new,
+ int dry_run, int force);
+
/*
* Prepare the "env_array" parameter of a "struct child_process" for executing
* a submodule by clearing any repo-specific envirionment variables, but
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 04/14] connect_work_tree_and_git_dir: safely create leading directories
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
In a later patch we'll use connect_work_tree_and_git_dir when the
directory for the gitlink file doesn't exist yet. Safely create
the directory first.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/dir.c b/dir.c
index 4541f9e146..69ca3d1411 100644
--- a/dir.c
+++ b/dir.c
@@ -2735,6 +2735,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
+ if (safe_create_leading_directories_const(file_name.buf))
+ fprintf(stderr, "could not create directories for %s\n", file_name.buf);
write_file(file_name.buf, "gitdir: %s",
relative_path(git_dir, work_tree, &rel_path));
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 13/14] entry.c: update submodules when interesting
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
entry.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/entry.c b/entry.c
index c6eea240b6..bc6295d41a 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,13 @@ 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 (is_interesting_submodule(ce))
+ /*
+ * force=1 is ok for any case as we did a dry
+ * run before with appropriate force setting
+ */
+ return submodule_go_from_to(ce->name,
+ NULL, oid_to_hex(&ce->oid), 0, 1);
break;
default:
return error("unknown file mode for %s in index", path);
@@ -260,6 +268,26 @@ 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);
+ /*
+ * Needs to be checked before !changed returns early,
+ * as the possibly empty directory was not changed
+ */
+ if (is_interesting_submodule(ce)) {
+ int err;
+ if (!is_submodule_populated_gently(ce->name, &err)) {
+ struct stat sb;
+ if (lstat(ce->name, &sb))
+ die(_("could not stat file '%s'"), ce->name);
+ if (!(st.st_mode & S_IFDIR))
+ unlink_or_warn(ce->name);
+
+ return submodule_go_from_to(ce->name,
+ NULL, oid_to_hex(&ce->oid), 0, 1);
+ } else
+ return submodule_go_from_to(ce->name,
+ "HEAD", oid_to_hex(&ce->oid), 0, 1);
+ }
+
if (!changed)
return 0;
if (!state->force) {
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 14/14] builtin/checkout: add --recurse-submodules switch
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-checkout.txt | 7 +++++++
builtin/checkout.c | 28 ++++++++++++++++++++++++++++
t/lib-submodule-update.sh | 33 ++++++++++++++++++++++++---------
t/t2013-checkout-submodule.sh | 5 +++++
4 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8e2c0662dd..a0ea2c5651 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 f174f50303..207ce09771 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,12 +21,31 @@
#include "submodule-config.h"
#include "submodule.h"
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
static const char * const checkout_usage[] = {
N_("git checkout [<options>] <branch>"),
N_("git checkout [<options>] [<branch>] -- <file>..."),
NULL,
};
+int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
struct checkout_opts {
int patch_mode;
int quiet;
@@ -1163,6 +1182,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
N_("second guess 'git checkout <no-such-branch>'")),
OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the given ref")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
OPT_END(),
};
@@ -1193,6 +1215,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
+ if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+ git_config(submodule_config, NULL);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+ set_config_update_recurse_submodules(recurse_submodules);
+ }
+
if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
die(_("-b, -B and --orphan are mutually exclusive"));
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 7c8c557572..9c75a417d4 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -734,6 +734,11 @@ test_submodule_forced_switch () {
test_submodule_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -843,7 +848,7 @@ test_submodule_switch_recursing () {
'
# Replacing a submodule with files in a directory must succeeds
# when the submodule is clean
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -855,7 +860,7 @@ test_submodule_switch_recursing () {
)
'
# ... absorbing a .git directory.
- test_expect_success "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
+ test_expect_$RESULT "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -883,7 +888,7 @@ test_submodule_switch_recursing () {
'
# ... must check its local work tree for untracked files
- test_expect_success "$command: replace submodule with a file must fail with untracked files" '
+ test_expect_$RESULT "$command: replace submodule with a file must fail with untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -939,16 +944,21 @@ test_submodule_switch_recursing () {
)
'
+ # This test fails, due to missing setup, we do not clone sub2 into
+ # submodule_update, because it doesn't exist in the 'add_sub1' version
+ #
test_expect_success "$command: modified submodule updates submodule recursively" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
cd submodule_update &&
git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
- $command modify_sub1_recursively &&
- test_superproject_content origin/modify_sub1_recursively &&
- test_submodule_content sub1 origin/modify_sub1_recursively
- test_submodule_content sub1/sub2
+ test_must_fail $command modify_sub1_recursively &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ # test_superproject_content origin/modify_sub1_recursively &&
+ # test_submodule_content sub1 origin/modify_sub1_recursively &&
+ # test_submodule_content sub1/sub2 no_submodule
)
'
}
@@ -958,6 +968,11 @@ test_submodule_switch_recursing () {
# the superproject as well as the submodule is allowed.
test_submodule_forced_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear creates empty dir ...
test_expect_success "$command: added submodule is checked out" '
@@ -1052,7 +1067,7 @@ test_submodule_forced_switch_recursing () {
)
'
# Replacing a submodule with files in a directory ...
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -1103,7 +1118,7 @@ test_submodule_forced_switch_recursing () {
'
# ... but stops for untracked files that would be lost
- test_expect_success "$command: replace submodule with a file" '
+ test_expect_$RESULT "$command: replace submodule with a file stops for untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 6847f75822..aa35223369 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -63,6 +63,11 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+test_submodule_switch_recursing "git checkout --recurse-submodules"
+
+test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
+
test_submodule_switch "git checkout"
test_submodule_forced_switch "git checkout -f"
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 12/14] read-cache: remove_marked_cache_entries to wipe selected submodules.
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
read-cache.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 9054369dd0..c0776b93b0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -18,6 +18,7 @@
#include "varint.h"
#include "split-index.h"
#include "utf8.h"
+#include "submodule.h"
/* Mask for the name length in ce_flags in the on-disk index */
@@ -532,6 +533,8 @@ void remove_marked_cache_entries(struct index_state *istate)
for (i = j = 0; i < istate->cache_nr; i++) {
if (ce_array[i]->ce_flags & CE_REMOVE) {
+ if (is_interesting_submodule(ce_array[i]))
+ submodule_go_from_to(ce_array[i]->name, "HEAD", NULL, 0, 1);
remove_name_hash(istate, ce_array[i]);
save_or_free_index_entry(istate, ce_array[i]);
}
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 03/14] make is_submodule_populated gently
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
We need the gentle version in a later patch. As we have just one caller,
migrate the caller.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/grep.c | 2 +-
submodule.c | 4 ++--
submodule.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..b17835aed6 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -616,7 +616,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
{
if (!is_submodule_initialized(path))
return 0;
- if (!is_submodule_populated(path)) {
+ if (!is_submodule_populated_gently(path, NULL)) {
/*
* If searching history, check for the presense of the
* submodule's gitdir before skipping the submodule.
diff --git a/submodule.c b/submodule.c
index 3b98766a6b..9bbdd3ce7c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -237,12 +237,12 @@ int is_submodule_initialized(const char *path)
/*
* Determine if a submodule has been populated at a given 'path'
*/
-int is_submodule_populated(const char *path)
+int is_submodule_populated_gently(const char *path, int *return_error_code)
{
int ret = 0;
char *gitdir = xstrfmt("%s/.git", path);
- if (resolve_gitdir(gitdir))
+ if (resolve_gitdir_gently(gitdir, return_error_code))
ret = 1;
free(gitdir);
diff --git a/submodule.h b/submodule.h
index 05ab674f06..689033e538 100644
--- a/submodule.h
+++ b/submodule.h
@@ -41,7 +41,7 @@ extern int submodule_config(const char *var, const char *value, void *cb);
extern void gitmodules_config(void);
extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path);
-extern int is_submodule_populated(const char *path);
+extern int is_submodule_populated_gently(const char *path, int *return_error_code);
extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* [PATCH 08/14] update submodules: move up prepare_submodule_repo_env
From: Stefan Beller @ 2017-02-15 0:34 UTC (permalink / raw)
Cc: git, bmwill, jrnieder, sandals, gitster, Stefan Beller
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
In a later patch we need to prepare the submodule environment with
another git directory, so split up the function.
Also move it up in the file such that we do not need to declare the
function later before using it.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4c33374ae8..d3fc6c2a75 100644
--- a/submodule.c
+++ b/submodule.c
@@ -359,6 +359,23 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
strbuf_release(&sb);
}
+static void prepare_submodule_repo_env_no_git_dir(struct argv_array *out)
+{
+ const char * const *var;
+
+ for (var = local_repo_env; *var; var++) {
+ if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
+ argv_array_push(out, *var);
+ }
+}
+
+void prepare_submodule_repo_env(struct argv_array *out)
+{
+ prepare_submodule_repo_env_no_git_dir(out);
+ argv_array_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
+ DEFAULT_GIT_DIR_ENVIRONMENT);
+}
+
/* 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
@@ -1403,18 +1420,6 @@ int parallel_submodules(void)
return parallel_jobs;
}
-void prepare_submodule_repo_env(struct argv_array *out)
-{
- const char * const *var;
-
- for (var = local_repo_env; *var; var++) {
- if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
- argv_array_push(out, *var);
- }
- argv_array_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
- DEFAULT_GIT_DIR_ENVIRONMENT);
-}
-
/*
* Embeds a single submodules git directory into the superprojects git dir,
* non recursively.
--
2.12.0.rc0.16.gd1691994b4.dirty
^ permalink raw reply related
* Re: [PATCH 11/11] refs: split and make get_*_ref_store() public API
From: Duy Nguyen @ 2017-02-15 0:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, git@vger.kernel.org, Michael Haggerty,
Johannes Schindelin, David Turner
In-Reply-To: <xmqq8tp8aawb.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 15, 2017 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Direct call sites are just middle men though, e.g.
>> for_each_ref_submodule(). I'll attempt to clean this up at some point
>> in future (probably at the same time I attempt to kill *_submodule ref
>> api). But I think for now I'll just put a TODO or FIXME comment here.
>
> So we'd have get_*_ref_store() for various cases and then will have
> a unifying for_each_ref_in_refstore() that takes a ref-store, which
> supersedes all the ad-hoc iterators like for_each_ref_submodule(),
> for_each_namespaced_ref(), etc?
>
> That's a very sensible longer-term goal, methinks.
Ah I forgot about ref namespace. I'll need to try something out in that area.
> I am wondering what we should do to for_each_{tag,branch,...}_ref().
> Would that become
>
> ref_store = get_ref_main_store();
> tag_ref_store = filter_ref_store(ref_store, "refs/tags/");
> for_each_ref_in_refstore(tag_ref_store, ...);
>
> or do you plan to have some other pattern?
Long term, I think that's what Mike wants. Though the filter_ref_store
might return an (opaque) iterator instead of a ref store and
for_each_refstore() becomes a thin wrapper around the iterator
interface.
--
Duy
^ permalink raw reply
* Re: [PATCH v2] clean: use warning_errno() when appropriate
From: Duy Nguyen @ 2017-02-15 0:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Jeff King
In-Reply-To: <xmqqh93wabev.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 15, 2017 at 1:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> All these warning() calls are preceded by a system call. Report the
>> actual error to help the user understand why we fail to remove
>> something.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> v2 dances with errno
>
> Thanks.
>
>>
>> builtin/clean.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/clean.c b/builtin/clean.c
>> index d6bc3aaae..3569736f6 100644
>> --- a/builtin/clean.c
>> +++ b/builtin/clean.c
>> @@ -154,6 +154,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
>> struct strbuf quoted = STRBUF_INIT;
>> struct dirent *e;
>> int res = 0, ret = 0, gone = 1, original_len = path->len, len;
>> + int saved_errno;
>> struct string_list dels = STRING_LIST_INIT_DUP;
>>
>> *dir_gone = 1;
>> @@ -173,9 +174,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
>> if (!dir) {
>> /* an empty dir could be removed even if it is unreadble */
>> res = dry_run ? 0 : rmdir(path->buf);
>> + saved_errno = errno;
>> if (res) {
>> quote_path_relative(path->buf, prefix, "ed);
>
> I think this part should be more like
>
> res = ... : rmdir(...);
> if (res) {
> int saved_errno = errno;
> ... do other things that can touch errno ...
> errno = saved_errno;
> ... now we know what the original error was ...
>
> The reason to store the errno in saved_errno here is not because we
> want to help code after "if (res) {...}", but the patch sent as-is
> gives that impression and is confusing to the readers.
>
> Perhaps all hunks of this patch share the same issue? I could
> locally amend, of course, but I'd like to double check before doing
> so myself---perhaps you did it this way for a good reason that I am
> missing?
One thing I like about putting saved_errno right next to the related
syscall is, the syscall is visible from the diff (previously some are
out of context). This is really minor though. I briefly thought of
introducing rmdir_errno() and friends that return -errno on error, so
we could do
res = ... : rmdir_errno(..);
if (res) {
errno = -res;
warning_errno(...);
}
But that's more work and the errno = -res is not particularly pleasing
to read. I'm fine with moving saved_errno in the error handling
blocks.
--
Duy
^ permalink raw reply
* Re: [PATCH 11/11] refs: split and make get_*_ref_store() public API
From: Junio C Hamano @ 2017-02-15 1:16 UTC (permalink / raw)
To: Duy Nguyen
Cc: Stefan Beller, git@vger.kernel.org, Michael Haggerty,
Johannes Schindelin, David Turner
In-Reply-To: <CACsJy8AfwGK_Y8vH-mF4NXWts_4_CPZamO0L_rWD-1WR3=36Yg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Long term, I think that's what Mike wants. Though the filter_ref_store
> might return an (opaque) iterator instead of a ref store and
> for_each_refstore() becomes a thin wrapper around the iterator
> interface.
Ah, yes, an iterator would be a lot more suitable abstraction there.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] clean: use warning_errno() when appropriate
From: Junio C Hamano @ 2017-02-15 1:28 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Jeff King
In-Reply-To: <CACsJy8BXSAUr2knrkOfO0gXYAwQoJpL2hCXy44Q37H4GE_-yVA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Wed, Feb 15, 2017 at 1:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> The reason to store the errno in saved_errno here is not because we
>> want to help code after "if (res) {...}", but the patch sent as-is
>> gives that impression and is confusing to the readers.
>>
>> Perhaps all hunks of this patch share the same issue? I could
>> locally amend, of course, but I'd like to double check before doing
>> so myself---perhaps you did it this way for a good reason that I am
>> missing?
>
> One thing I like about putting saved_errno right next to the related
> syscall is, the syscall is visible from the diff (previously some are
> out of context). This is really minor though.
I agree that this is minor.
I care more about looking at errno ONLY after we saw our call to a
system library function indicated an error, and I wanted to avoid
doing:
res = dry_run ? 0 : rmdir(path);
saved_errno = errno;
if (res) {
... do something else ...
errno = saved_errno;
call_something_that_uses_errno();
When our call to rmdir() did not fail, or when we didn't even call
rmdir() at all, what is in errno has nothing to do with what we are
doing, and making a copy of it makes the code doubly confusing.
Rather, I'd prefer to see:
res = dry_run ? 0 : rmdir(path);
if (res) {
int saved_errno = errno;
... do something else ...
errno = saved_errno;
call_something_that_uses_errno();
which makes it clear what is going on when reading the resulting
code.
For now, I'll queue a separate SQUASH??? and wait for others to
comment.
Thanks.
^ permalink raw reply
* Re: [PATCH] completion: restore removed line continuating backslash
From: SZEDER Gábor @ 2017-02-15 1:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git mailing list
In-Reply-To: <xmqqfujhddl2.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 13, 2017 at 9:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> Recent commit 1cd23e9e0 (completion: don't use __gitdir() for git
>> commands, 2017-02-03) rewrapped a couple of long lines, and while
>> doing so it inadvertently removed a '\' from the end of a line, thus
>> breaking completion for 'git config remote.name.push <TAB>'.
>>
>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> ---
>>
>> I wanted this to be a fixup!, but then noticed that this series is
>> already in next, hence the proper commit message.
>
> We get "--format=... : command not found"?
Yeah, that's the one.
> Thanks for a set of sharp eyes.
Heh. Sharp?! It took over five minutes to notice after I first got
that error...
Furthermore, that '\' was already missing in v1, almost a year ago :)
>> I see the last What's cooking marks this series as "will merge to
>> master". That doesn't mean that it will be in v2.12, does it?
>
> I actually was hoping that these can go in. Others that can (or
> should) wait are marked as "Will cook in 'next'".
>
> If you feel uncomfortable and want these to cook longer, please tell
> me so.
Well, it was mainly my surprise that a 20+ patch series arriving so
late that it gets queued on top of -rc0 would still make it into the
release. However, I have been using this series with only minor
modifications for the better part of a year now, so it's unlikely that
there will be any big issues with it. Maybe something small, like
this missing '\', but we will deal with it when it arises.
Gábor
^ permalink raw reply
* Re: [PATCH v2] clean: use warning_errno() when appropriate
From: Jeff King @ 2017-02-15 1:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <xmqqfujg5jjv.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 14, 2017 at 05:28:36PM -0800, Junio C Hamano wrote:
> I care more about looking at errno ONLY after we saw our call to a
> system library function indicated an error, and I wanted to avoid
> doing:
>
> res = dry_run ? 0 : rmdir(path);
> saved_errno = errno;
> if (res) {
> ... do something else ...
> errno = saved_errno;
> call_something_that_uses_errno();
>
> When our call to rmdir() did not fail, or when we didn't even call
> rmdir() at all, what is in errno has nothing to do with what we are
> doing, and making a copy of it makes the code doubly confusing.
>
> Rather, I'd prefer to see:
>
> res = dry_run ? 0 : rmdir(path);
> if (res) {
> int saved_errno = errno;
> ... do something else ...
> errno = saved_errno;
> call_something_that_uses_errno();
>
> which makes it clear what is going on when reading the resulting
> code.
>
> For now, I'll queue a separate SQUASH??? and wait for others to
> comment.
I don't have a strong feeling either way, but I think your second
example there is probably preferable. The reason to save errno is
because of the "something else" that may affect it, and it puts the
saving close to that.
Duy's version above keeps the saved_errno close to the syscall that
caused it, which is nicer for making sure we're saving the right thing,
and didn't get fooled by:
res = rmdir(path);
... some other stuff ...
if (res) {
int saved_errno = errno;
... something else ...
errno = saved_errno;
That's wrong if "some other stuff" touches errno. But I think
"saved_errno" is not the right pattern there. It is "stuff away the
result _and_ errno for this thing so we can use it later".
IOW, I'd expect it to be more like:
rmdir_result = rmdir(path);
rmdir_errno = errno;
... some other stuff ...
if (rmdir_result)
show_error(rmdir_errno);
And that leads to the "gee, why don't we just encode error values as
negative integers" pattern. Which I agree is nicer, but I'm not sure I
want to get into wrapping every syscall to give it a better interface.
-Peff
^ permalink raw reply
* Re: [PATCH 01/14] lib-submodule-update.sh: reorder create_lib_submodule_repo
From: brian m. carlson @ 2017-02-15 1:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, gitster
In-Reply-To: <20170215003423.20245-2-sbeller@google.com>
[-- Attachment #1: Type: text/plain, Size: 613 bytes --]
On Tue, Feb 14, 2017 at 04:34:10PM -0800, Stefan Beller wrote:
> create_lib_submodule_repo () {
> git init submodule_update_repo &&
> (
> @@ -49,10 +54,11 @@ create_lib_submodule_repo () {
> git config submodule.sub1.ignore all &&
> git add .gitmodules &&
> git commit -m "Add sub1" &&
> - git checkout -b remove_sub1 &&
> +
> + git checkout -b remove_sub1 add_sub1&&
You're missing a space before the "&&".
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: Feature request: flagging “volatile” branches for integration/development
From: Junio C Hamano @ 2017-02-15 1:55 UTC (permalink / raw)
To: Herbert, Marc; +Cc: git, Josh Triplett
In-Reply-To: <6c7cb3da-714b-87ed-5885-220a433c646f@intel.com>
"Herbert, Marc" <marc.herbert@intel.com> writes:
>> The hard part may be policy (e.g. what if the user does not want a branch
>> to be treated volatile by various commands even if it receives such
>> flag from a git server).
>
> There would be instant, human-readable value in such a new "volatile"
> flag. Machine use and policies can be discussed later. These will be
> easier to prototype, experiment and refine once the flag exists.
We tend to avoid adding random cruft to the system whose semantics
is not well defined, so that we can avoid having to support an ill
defined feature forever.
> ... Now I bet this on the other hand must have been
> discussed (and rejected?) before, any pointer?
I suspect that people may have expressed vague wish from time to
time, but I do not think we saw a proposal that outlines the design
at a concrete enough level to let us rejecting in the past ;-)
Let me list some things that needs to be designed that comes to my
mind offhand:
* How would a user mark a ref as "volatile"? I am assuming that
anybody do so in her own local repository, but how does one mark
a ref as "volatile" at a hosting site, and who is allowed to do
so (one possibile design is "new option to 'git push' will do so,
and anybody who can push to the ref is allowed to", and I am fine
with that design, but you have to spell that out in a proposal)?
* How would a user learn if a ref is "volatile"? Does "ls-remote"
show that information?
* Does volatile-ness of a ref at the remote site propagate to your
remote-tracking ref that corresponds to it? What does it mean
that refs/remotes/origin/pu is marked as volatile in your local
repository? You cannot "checkout -b" based on it? Does "branch"
based on it need to be forbidden as well?
* Can a local ref be "volatile"? What does it mean (the same
subquestions as above)?
* If your local branch myA is set to build on a remote-tracking
branch A and push back to branch A at the remote, i.e.
$ git checkout -t -b myA refs/remotes/origin/A
$ ... work work work ...
$ git push
is set to result in their branch A updated with what you built in
myA, and if the branch A at the remote is marked as "volatile",
does it make your "myA" also "volatile"? How is the volatile-ness
inherited? From their A to your remotes/origin/A and then to
your myA? Any other useful rule that defines the propagation?
* Do we only care about "volatile"? If we are extending the system
to allow setting and propagating this new bit per ref (I am
blindly assuming that you do not have a strong reason to limit
this to branches), we may as well just design this extension to
allow the projects to assign arbitrary set of bits to refs. Some
projects may want to have different degree of volatile-ness and
have "volatile" refs, "unstable" refs and "iffy" refs, for
example.
Side note: even if we were to go with "any random label can be
assigned and the meaning for the labels can be defined by the
project convention" approach, it does not necessarily mean we are
adding a random cruft whose semantics is ill-defined. "Git does
not do anything special to a ref based on what labels it has--it
just remembers what labels the user told it to attach to the ref,
and shows what labels the ref has when asked" can be very well
defined semantics.
^ permalink raw reply
* Re: [PATCH 09/14] update submodules: add submodule_go_from_to
From: brian m. carlson @ 2017-02-15 2:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, gitster
In-Reply-To: <20170215003423.20245-10-sbeller@google.com>
[-- Attachment #1: Type: text/plain, Size: 885 bytes --]
On Tue, Feb 14, 2017 at 04:34:18PM -0800, Stefan Beller wrote:
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + cp.no_stdin = 1;
> + cp.dir = path;
> +
> + argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
> + argv_array_pushl(&cp.args, "read-tree", NULL);
> +
> + if (!dry_run)
> + argv_array_push(&cp.args, "-u");
> + else
> + argv_array_push(&cp.args, "-n");
I might write this as
if (dry_run)
argv_array_push(&cp.args, "-n");
else
argv_array_push(&cp.args, "-u");
In other words, avoiding the negation when you have an else branch. I
can also see an argument for keeping the condition identical to the
other branches, though.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH 14/14] builtin/checkout: add --recurse-submodules switch
From: brian m. carlson @ 2017-02-15 2:08 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, gitster
In-Reply-To: <20170215003423.20245-15-sbeller@google.com>
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
On Tue, Feb 14, 2017 at 04:34:23PM -0800, Stefan Beller wrote:
> +--[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)
I would say "unless" instead of "until". "Until" implies an ongoing
or repetitive action being interrupted, which isn't the case here.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [RFCv3 PATCH 00/14] Checkout aware of Submodules!
From: brian m. carlson @ 2017-02-15 2:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, gitster
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]
On Tue, Feb 14, 2017 at 04:34:09PM -0800, Stefan Beller wrote:
> 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!
Overall, I'm very pleased with this. I don't really have any
design-level comments at this point, only small nits. I'm considering
building and testing it out at work, where this would be extremely
useful. I'm therefore hoping to see how it works under real-world
conditions shortly.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH] completion: restore removed line continuating backslash
From: Junio C Hamano @ 2017-02-15 2:41 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Git mailing list
In-Reply-To: <CAM0VKjmU57saSfyRuoWfC+UZFNypH1Wp9X33VgzPq9fatD=qtg@mail.gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
>> If you feel uncomfortable and want these to cook longer, please tell
>> me so.
>
> Well, it was mainly my surprise that a 20+ patch series arriving so
> late that it gets queued on top of -rc0 would still make it into the
> release.
It all depends on what area the changes are about ;-)
Not meaning to make light of your contribution, but if the upcoming
version of Git shipped with a slightly broken completion, and the
breakage is severe enough, the only thing we need to do is to do a
maintenance release that reverts a single script. Distro packagers
may do that for us. While waiting, the user can choose not to load
the completion and can keep using Git just fine. A broken
"mergetool" would be similar in that a workaround to inspect "git
diff" is available. If we break "pull" or "commit" on the other
hand, the necessary workaround would be a lot more involved.
Some changes in low-impact areas can wait without reducing the
end-user value of the new release, but if the risk of regression is
small, I favour merging them, rather than postponing them for one
cycle, if only to reduce the number of patches that I need to hold
onto. Patches to clarify existing documentation fall into this
category.
My perception of "risk of regression" obviously is affected by the
size of the series, and 20+ is certainly on the larger side. But
other things also come into the picture. Patches from an author who
knows the area the patches touch very well and with track record of
not causing embarrassing regressions, would make me feel safer than
patches from others, for example.
^ permalink raw reply
* Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: SZEDER Gábor @ 2017-02-15 3:11 UTC (permalink / raw)
To: Cornelius Weig; +Cc: Git mailing list, Richard Wagner, j6t
In-Reply-To: <20170214212404.31469-2-cornelius.weig@tngtech.com>
On Tue, Feb 14, 2017 at 10:24 PM, <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Git-checkout completes words starting with '--' as options and other
> words as refs. Even after specifying a ref, further words not starting
> with '--' are completed as refs, which is invalid for git-checkout.
Refs completion is never attempted for words after the disambiguating
double-dash.
Even when refs completion is attempted, if it is unsuccessful, i.e.
there is no ref that matches the current word to be completed, then
Bash falls back to standard filename completion. No refs match
'./<TAB>'.
Furthermore, Bash performs filename completion on Alt-/ independently
from any completion function.
Granted, none of these will limit to only modified files... But that
might be a good thing, see below.
> This commit ensures that after specifying a ref, further non-option
> words are completed as paths. Four cases are considered:
>
> - If the word contains a ':', do not treat it as reference and use
> regular revlist completion.
> - If no ref is found on the command line, complete non-options as refs
> as before.
> - If the ref is HEAD or @, complete only with modified files because
> checking out unmodified files is a noop.
Here you use "modified" in the 'ls-files --modified' sense, but that
doesn't include modifications already staged in the index, see below.
> This case also applies if no ref is given, but '--' is present.
> - If a ref other than HEAD or @ is found, offer only valid paths from
> that revision.
>
> Note that one corner-case is not covered by the current implementation:
> if a refname contains a ':' and is followed by '--' the completion would
> not recognize the valid refname.
I'm not sure what you mean here. Refnames can't contain ':'.
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---
> contrib/completion/git-completion.bash | 39 +++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 4ab119d..df46f62 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1068,7 +1068,7 @@ _git_bundle ()
>
> _git_checkout ()
> {
> - __git_has_doubledash && return
> + local i c=2 ref="" seen_double_dash=""
>
> case "$cur" in
> --conflict=*)
> @@ -1081,13 +1081,36 @@ _git_checkout ()
> "
> ;;
> *)
> - # check if --track, --no-track, or --no-guess was specified
> - # if so, disable DWIM mode
> - local flags="--track --no-track --no-guess" track=1
> - if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> - track=''
> - fi
> - __gitcomp_nl "$(__git_refs '' $track)"
> + while [ $c -lt $cword ]; do
> + i="${words[c]}"
> + case "$i" in
> + --) seen_double_dash=1 ;;
> + -*|?*:*) ;;
> + *) ref="$i"; break ;;
I haven't tried it, but this would trigger on e.g. 'git checkout -b
new-feature <TAB>', wouldn't it?
> + esac
> + ((c++))
> + done
> +
> + case "$ref,$seen_double_dash,$cur" in
> + ,,*:*)
> + __git_complete_revlist_file
> + ;;
> + ,,*)
> + # check for --track, --no-track, or --no-guess
> + # if so, disable DWIM mode
> + local flags="--track --no-track --no-guess" track=1
> + if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> + track=''
> + fi
> + __gitcomp_nl "$(__git_refs '' $track)"
> + ;;
> + ,1,*|@,*|HEAD,*)
> + __git_complete_index_file "--modified"
What about
$ echo "unintentional change" >>tracked-file && git add -u
$ git rm important-file
$ git checkout HEAD <TAB>
? It seems it will offer neither 'tracked-file' nor 'important-file',
but I think it should offer both.
We have __git_complete_index_file() for a while now, but only use it
for commands that accept only --options and filenames, e.g. 'add',
'clean', 'rm'. This would be the first case when we would use it for
a command that accept both refs and filenames. Perhaps similar corner
cases and the easy ways to trigger filename completion are why no one
thought it's worth it.
> + ;;
> + *)
> + __git_complete_tree_file "$ref" "$cur"
Well, here you could go all-in, and say that this should complete only
files that are different from the version in $ref, because checking
out files that are still the same is a noop :)
> + ;;
> + esac
> ;;
> esac
> }
> --
> 2.10.2
>
^ permalink raw reply
* difflame improvements
From: Edmundo Carmona Antoranz @ 2017-02-15 5:19 UTC (permalink / raw)
To: Git List; +Cc: Jeff King
Hi!
I've been working on detecting revisions where a "real" deletion was
made and I think I advanced a lot in that front. I still have to work
on many scenarios (renamed files, for example... also performance) but
at least I'm using a few runs against git-scm history and the results
are "promising":
23:05 $ git blame -s --reverse -L 25,40 HEAD~20..HEAD -- versioncmp.c
066fb0494e 25) static int initialized;
066fb0494e 26)
066fb0494e 27) /*
8ec68d1ae2 28) * p1 and p2 point to the first different character in
two strings. If
8ec68d1ae2 29) * either p1 or p2 starts with a prerelease suffix, it
will be forced
8ec68d1ae2 30) * to be on top.
8ec68d1ae2 31) *
8ec68d1ae2 32) * If both p1 and p2 start with (different) suffix, the order is
8ec68d1ae2 33) * determined by config file.
066fb0494e 34) *
8ec68d1ae2 35) * Note that we don't have to deal with the situation
when both p1 and
8ec68d1ae2 36) * p2 start with the same suffix because the common
part is already
8ec68d1ae2 37) * consumed by the caller.
066fb0494e 38) *
066fb0494e 39) * Return non-zero if *diff contains the return value
for versioncmp()
066fb0494e 40) */
Lines 28-33:
23:05 $ git show --summary 8ec68d1ae2
commit 8ec68d1ae2863823b74d67c5e92297e38bbf97bc
Merge: e801be066 c48886779
Author: Junio C Hamano <>
Date: Mon Jan 23 15:59:21 2017 -0800
Merge branch 'vn/diff-ihc-config'
"git diff" learned diff.interHunkContext configuration variable
that gives the default value for its --inter-hunk-context option.
* vn/diff-ihc-config:
diff: add interhunk context config option
And this is not telling me the _real_ revision where the lines were
_deleted_ so it's not very helpful, as Peff has already mentioned.
Running difflame:
23:06 $ time ~/proyectos/git/difflame/difflame.py -bp=-s -w HEAD~20
HEAD -- versioncmp.c
diff --git a/versioncmp.c b/versioncmp.c
index 80bfd109f..9f81dc106 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -24,42 +24,83 @@
.
.
.
+b17846432d 33) static void find_better_matching_suffix(const char
*tagname, const char *suffix,
+b17846432d 34) int
suffix_len, int start, int conf_pos,
+b17846432d 35) struct
suffix_match *match)
+b17846432d 36) {
b17846432d 37) /*
c026557a3 versioncmp: generalize version sort suffix reordering
-c026557a3 (SZEDER 28) * p1 and p2 point to the first different
character in two strings. If
-c026557a3 (SZEDER 29) * either p1 or p2 starts with a prerelease
suffix, it will be forced
-c026557a3 (SZEDER 30) * to be on top.
-c026557a3 (SZEDER 31) *
-c026557a3 (SZEDER 32) * If both p1 and p2 start with (different)
suffix, the order is
-c026557a3 (SZEDER 33) * determined by config file.
b17846432 versioncmp: factor out helper for suffix matching
+b17846432d 38) * A better match either starts earlier or
starts at the same offset
+b17846432d 39) * but is longer.
+b17846432d 40) */
+b17846432d 41) int end = match->len < suffix_len ?
match->start : match->start-1;
.
.
.
Same range of (deleted) lines:
23:10 $ git --show --name-status c026557a3
commit c026557a37361b7019acca28f240a19f546739e9
Author: SZEDER Gábor <>
Date: Thu Dec 8 15:24:01 2016 +0100
versioncmp: generalize version sort suffix reordering
The 'versionsort.prereleaseSuffix' configuration variable, as its name
suggests, is supposed to only deal with tagnames with prerelease
.
.
.
Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
M Documentation/config.txt
M Documentation/git-tag.txt
M t/t7004-tag.sh
M versioncmp.c
This is the revision where the deletion happened.
That's it for the time being.
^ 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