* [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-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/t2013-checkout-submodule.sh | 277 +++++++++++++++++++++++++++++++++++++++--
3 files changed, 306 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 512492aad9..5db0d933d1 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/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 6847f75822..33fb2f5de3 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,6 +70,260 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+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
+}
+
+test_expect_success 'setup the submodule config' '
+ 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" &&
+ git config submodule.submodule.url ./submodule.bare
+'
+
+test_expect_success '"checkout --recurse-submodules" migrates submodule git dir before deleting' '
+ git checkout -b base &&
+ git checkout -b delete_submodule &&
+ git update-index --force-remove submodule &&
+ git config -f .gitmodules --unset submodule.submodule.path &&
+ git config -f .gitmodules --unset submodule.submodule.url &&
+ git add .gitmodules &&
+ git commit -m "submodule deleted" &&
+ git checkout base &&
+ git checkout --recurse-submodules delete_submodule 2>output.err 1>output.out &&
+ test_i18ngrep "Migrating git directory" output.out &&
+ ! test -d submodule
+'
+
+test_expect_success '"check --recurse-submodules" removes deleted submodule' '
+ # Make sure we have the submodule here and ready.
+ git checkout base &&
+ git submodule embedgitdirs &&
+ git submodule update -f . &&
+ test -e submodule/.git &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached base &&
+
+ # Check if the checkout deletes the submodule.
+ echo change >>submodule/first.t &&
+ test_must_fail git checkout --recurse-submodules delete_submodule &&
+ git checkout -f --recurse-submodules delete_submodule &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached delete_submodule &&
+ ! test -d submodule
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
+ submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule in existing directory' '
+ git checkout --recurse-submodules delete_submodule &&
+ mkdir submodule &&
+ submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
+ git checkout -f base &&
+ git checkout -b replace_submodule_with_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' '
+ submodule_creation_must_succeed replace_submodule_with_file base
+'
+
+test_expect_success '"checkout --recurse-submodules" replaces submodule with a directory' '
+ 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' '
+ submodule_creation_must_succeed replace_submodule_with_dir base
+'
+
+test_expect_success SYMLINKS '"checkout --recurse-submodules" replaces submodule with a link' '
+ 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' '
+ submodule_creation_must_succeed replace_submodule_with_link base
+'
+
+test_expect_success '"checkout --recurse-submodules" updates the submodule' '
+ 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
+'
+
+# In 293ab15eea34 we considered untracked ignored files in submodules
+# expendable, we may want to revisit this decision by adding user as
+# well as command specific configuration for it.
+# When building in-tree the untracked ignored files are probably ok to remove
+# in a case as tested here, but e.g. when git.git is a submodule, then a user
+# may not want to lose a well crafted (but ignored by default) "config.mak"
+# Commands like "git rm" may care less about untracked files in a submodule
+# as the checkout command that removes a submodule as well.
+test_expect_failure 'untracked file is not deleted' '
+ 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' '
+ 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' '
+ 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' '
+ 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' '
+ 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' '
+ git -C submodule checkout --recurse-submodules HEAD^ &&
+ test_must_fail git checkout --recurse-submodules master &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f to update modifed submodule content' '
+ echo modified >submodule/second.t &&
+ test_must_fail git checkout --recurse-submodules HEAD^ &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git checkout --recurse-submodules -f HEAD^ &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" ignores modified submodule content that would not be changed' '
+ echo modified >expected &&
+ cp expected submodule/first.t &&
+ git checkout --recurse-submodules HEAD^ &&
+ test_cmp expected submodule/first.t &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master &&
+ git diff-files --quiet &&
+ git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" does not care about untracked submodule content' '
+ echo untracked >submodule/untracked &&
+ git checkout --recurse-submodules master &&
+ git diff-files --quiet --ignore-submodules=untracked &&
+ git diff-index --quiet --cached HEAD &&
+ rm submodule/untracked
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f when submodule commit is not present (but does fail anyway)' '
+ git checkout --recurse-submodules -b bogus_commit master &&
+ git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submodule &&
+ BOGUS_TREE=$(git write-tree) &&
+ BOGUS_COMMIT=$(echo "bogus submodule commit" | git commit-tree $BOGUS_TREE) &&
+ git commit -m "bogus submodule commit" &&
+ git checkout --recurse-submodules -f master &&
+ test_must_fail git checkout --recurse-submodules bogus_commit &&
+ git diff-files --quiet &&
+ test_must_fail git checkout --recurse-submodules -f bogus_commit &&
+ test_must_fail git diff-files --quiet submodule &&
+ git diff-files --quiet file &&
+ git diff-index --quiet --cached HEAD &&
+ git checkout --recurse-submodules -f master
+'
+
test_submodule_switch "git checkout"
test_submodule_forced_switch "git checkout -f"
--
2.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 03/17] update submodules: move up prepare_submodule_repo_env
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-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 | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/submodule.c b/submodule.c
index eb80b0c5ad..78b69b5a55 100644
--- a/submodule.c
+++ b/submodule.c
@@ -307,6 +307,22 @@ 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_push(out, "GIT_DIR=.git");
+}
+
/* 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
@@ -1246,14 +1262,3 @@ 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_push(out, "GIT_DIR=.git");
-}
--
2.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 16/17] completion: add '--recurse-submodules' to checkout
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
contrib/completion/git-completion.bash | 2 +-
t/t9902-completion.sh | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8df..28acfdb377 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1068,7 +1068,7 @@ _git_checkout ()
--*)
__gitcomp "
--quiet --ours --theirs --track --no-track --merge
- --conflict= --orphan --patch
+ --conflict= --orphan --patch --recurse-submodules
"
;;
*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2ba62fbc17..d2d1102a3d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -447,6 +447,7 @@ test_expect_success 'double dash "git checkout"' '
--conflict=
--orphan Z
--patch Z
+ --recurse-submodules Z
EOF
'
--
2.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 01/17] submodule.h: add extern keyword to functions
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-1-sbeller@google.com>
As the upcoming series will add a lot of functions to the submodule
header, let's first make the header consistent to the rest of the project
by adding the extern keyword to functions.
As per the CodingGuidelines we try to stay below 80 characters per line,
so adapt all those functions to stay below 80 characters that are already
using more than one line. Those function using just one line are better
kept in one line than breaking them up into multiple lines just for the
goal of staying below the character limit as it makes grepping
for functions easier if they are one liners.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.h | 57 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/submodule.h b/submodule.h
index d9e197a948..5db0b53b3f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,50 +29,53 @@ 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 *,
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 *, 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.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 14/17] submodule: teach unpack_trees() to update submodules
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
entry.c | 7 +++---
unpack-trees.c | 76 +++++++++++++++++++++++++++++++++++++++++++++-------------
unpack-trees.h | 1 +
3 files changed, 65 insertions(+), 19 deletions(-)
diff --git a/entry.c b/entry.c
index a668025b8e..3ed885b886 100644
--- a/entry.c
+++ b/entry.c
@@ -267,7 +267,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)
@@ -284,9 +284,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 db03293347..8b0f6dfd1a 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.",
@@ -81,6 +84,9 @@ 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 them:\n%s");
+
if (!strcmp(cmd, "checkout"))
msg = advice_commit_before_merge
? _("The following untracked working tree files would be removed by checkout:\n%%s"
@@ -1320,19 +1326,17 @@ 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)) {
+ if (!submodule_is_interesting(ce->name))
+ return 0;
+ if (ce_stage(ce) ? is_submodule_checkout_safe(ce->name, &ce->oid)
+ : !is_submodule_modified(ce->name, 1))
+ return 0;
+ } else {
+ int flags = CE_MATCH_IGNORE_VALID | CE_MATCH_IGNORE_SKIP_WORKTREE;
+ if (!ie_match_stat(o->src_index, ce, &st, flags))
+ return 0;
+ }
errno = 0;
}
if (errno == ENOENT)
@@ -1355,6 +1359,38 @@ 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))
+ return 0;
+
+ if (lstat(old->name, &st)) {
+ if (errno == ENOENT)
+ return 0;
+ return o->gently ? -1 :
+ add_rejected_path(o, ERROR_NOT_UPTODATE_SUBMODULE,
+ old->name);
+ }
+
+ if (S_ISGITLINK(new->ce_mode))
+ return !is_submodule_checkout_safe(new->name, &new->oid);
+ else
+ return !ok_to_remove_submodule(old->name);
+}
+
static void invalidate_ce_path(const struct cache_entry *ce,
struct unpack_trees_options *o)
{
@@ -1616,9 +1652,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 36a73a6d00..bee874088a 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.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 06/17] update submodules: add a config option to determine if submodules are updated
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-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 c29153e9ff..1ba398ba3b 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;
@@ -510,6 +511,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 a9eabcc3d0..21236b095c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -53,6 +53,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.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 13/17] entry: write_entry to write populate submodules
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-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 02c4ac9f22..a668025b8e 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)
@@ -208,6 +209,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.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 11/17] unpack-trees: teach verify_clean_submodule to inspect submodules
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-1-sbeller@google.com>
As a later patch will modify submodules, if they are interesting
we need to see if the submodule is clean in case they are
interesting.
If they are not interesting, then we do not care about the submodule
keeping historic behavior.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ea6bdd20e0..22e32eca96 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,15 @@ 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;
+ if (!submodule_is_interesting(ce->name))
+ return 0;
+
+ return !is_submodule_modified(ce->name, 0);
}
static int verify_clean_subdirectory(const struct cache_entry *ce,
--
2.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default
From: Stefan Beller @ 2016-12-03 0:30 UTC (permalink / raw)
To: bmwill, David.Turner; +Cc: git, sandals, hvoigt, gitster, Stefan Beller
In-Reply-To: <20161203003022.29797-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 | 16 ++++++++++++++++
4 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66aae7..67e0714358 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 a0ea2c5651..819c430b6e 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 4253f7f044..e4e326bce4 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 33fb2f5de3..673021fb80 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -137,6 +137,22 @@ test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
submodule_creation_must_succeed delete_submodule base
'
+test_expect_success 'option checkout.recurseSubmodules updates submodule' '
+ test_config checkout.recurseSubmodules 1 &&
+ 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' '
git checkout --recurse-submodules delete_submodule &&
mkdir submodule &&
--
2.11.0.rc2.28.g2673dad
^ permalink raw reply related
* [BUG] Index.lock error message regression in git 2.11.0
From: Robbie Iannucci @ 2016-12-03 1:44 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
Hello,
I just upgraded to 2.11.0 from 2.10.2, and I noticed that some
commands no longer print an error message when the `index.lock` file
exists (such as `git merge --ff-only`).
It appears this bug was introduced in
55f5704da69d3e6836620f01bee0093ad5e331e8 (sequencer: lib'ify
checkout_fast_forward()). I determined this by running the attached
bisect script (on OS X, but I don't think that matters; I've also seen
the error message missing on Linux):
$ cd /path/to/git/src
$ git bisect start v2.11.0-rc0 v2.10.2
$ git bisect run /path/to/bisect.test.sh
(my original version of the script also includes some other makefile
parameters to get a modern version of gettext and openssl too, but
they're not relevant to this ML).
I'm not certain that I have enough context to propose a meaningful
patch though :/.
Cheers,
Robbie
[-- Attachment #2: bisect.test.sh --]
[-- Type: application/octet-stream, Size: 527 bytes --]
#!/bin/bash
make -j 8 2>&1 > /dev/null
if [[ $? != 0 ]]; then
# skip this version
exit 125
fi
git=`realpath ./git`
rm -rf .test_repo || true
mkdir .test_repo
cd .test_repo
$git init
echo HELLO > a
$git add a
$git commit -m 'initial_commit'
$git branch --track new_branch
$git checkout master
echo HELLO >> a
$git add a
$git commit -m 'second_commit'
$git checkout new_branch
touch .git/index.lock
if $git merge --ff-only master 2>&1 | grep -F "index.lock" ; then
echo OK
exit 0
fi
echo Message is missing
exit 1
^ permalink raw reply
* Re: [BUG] Index.lock error message regression in git 2.11.0
From: Robbie Iannucci @ 2016-12-03 1:52 UTC (permalink / raw)
To: git
In-Reply-To: <CA+q_oBdHytoeSD-hmLx_N473M8XinjqckvE35Re3eNpQRWYjHQ@mail.gmail.com>
Apparently I'm not supposed to send attachments >_<. Here's the script
in non-attachement form:
--------
#!/bin/bash
make -j 8 2>&1 > /dev/null
if [[ $? != 0 ]]; then
# skip this version
exit 125
fi
git=`realpath ./git`
rm -rf .test_repo || true
mkdir .test_repo
cd .test_repo
$git init
echo HELLO > a
$git add a
$git commit -m 'initial_commit'
$git branch --track new_branch
$git checkout master
echo HELLO >> a
$git add a
$git commit -m 'second_commit'
$git checkout new_branch
touch .git/index.lock
if $git merge --ff-only master 2>&1 | grep -F "index.lock" ; then
echo OK
exit 0
fi
echo Message is missing
exit 1
On Fri, Dec 2, 2016 at 5:44 PM, Robbie Iannucci <iannucci@google.com> wrote:
> Hello,
>
> I just upgraded to 2.11.0 from 2.10.2, and I noticed that some
> commands no longer print an error message when the `index.lock` file
> exists (such as `git merge --ff-only`).
>
> It appears this bug was introduced in
> 55f5704da69d3e6836620f01bee0093ad5e331e8 (sequencer: lib'ify
> checkout_fast_forward()). I determined this by running the attached
> bisect script (on OS X, but I don't think that matters; I've also seen
> the error message missing on Linux):
>
> $ cd /path/to/git/src
> $ git bisect start v2.11.0-rc0 v2.10.2
> $ git bisect run /path/to/bisect.test.sh
>
> (my original version of the script also includes some other makefile
> parameters to get a modern version of gettext and openssl too, but
> they're not relevant to this ML).
>
> I'm not certain that I have enough context to propose a meaningful
> patch though :/.
>
> Cheers,
> Robbie
^ permalink raw reply
* Re: [PATCH] commit: make --only --allow-empty work without paths
From: Jeff King @ 2016-12-03 4:32 UTC (permalink / raw)
To: Andreas Krey; +Cc: git, Junio C Hamano
In-Reply-To: <20161202221513.GA5370@inner.h.apk.li>
On Fri, Dec 02, 2016 at 11:15:13PM +0100, Andreas Krey wrote:
> --only is implied when paths are present, and required
> them unless --amend. But with --allow-empty it should
> be allowed as well - it is the only way to create an
> empty commit in the presence of staged changes.
OK. I'm not sure why you would want to create an empty commit in such a
case. But I do agree that this seems like a natural outcome for "--only
--allow-empty". So whether it is particularly useful or not, it seems
like the right thing to do. The patch itself looks good to me.
> Arguably, requiring paths with --only is
> pointless anyway because it is implicit
> in that case, but I'm happy when it works
> like in this patch.
I think the point is just to warn the user that what they've asked for
is by definition a noop (and that's why there's already an exception for
--amend, which _does_ make it do something). The fact that --only is
implicit with paths is mostly historical; at one point it was not the
default. These days it's unnecessary, but retained for backwards
compatibility.
> (The interdepence of the tests is a strange thing;
> making --run=N somewhat pointless.)
Yes, I think --run is a misfeature (I actually had to look it up, as I
had completely forgotten that it was added). It's too hard to know which
tests are required setup for later ones, and often the dependency is
implicit. If a single test script is annoyingly long to run, I'd argue
it should be broken out into its own script (and that will let it run in
parallel when the full suite is run, too). I don't know that t7501
qualifies, though; it runs in about 800ms on my machine.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 8976c3d29..89b66816f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1206,7 +1206,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>
> if (also + only + all + interactive > 1)
> die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
> - if (argc == 0 && (also || (only && !amend)))
> + if (argc == 0 && (also || (only && !amend && !allow_empty)))
> die(_("No paths with --include/--only does not make sense."));
> if (argc == 0 && only && amend)
> only_include_assumed = _("Clever... amending the last one with dirty index.");
I think this should be sufficient. Obviously we'll end up with an empty
commit, but allow_empty should cover that case later on.
> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> index d84897a67..0d8d89309 100755
> --- a/t/t7501-commit.sh
> +++ b/t/t7501-commit.sh
> @@ -155,6 +155,15 @@ test_expect_success 'amend --only ignores staged contents' '
> git diff --exit-code
> '
>
> +test_expect_success 'allow-empty --only ignores staged contents' '
> + echo changed-again >file &&
> + git add file &&
> + git commit --allow-empty --only -m "empty" &&
> + git cat-file blob HEAD:file >file.actual &&
> + test_cmp file.expect file.actual &&
> + git diff --exit-code
> +'
> +
Usually we'd put new tests at the end. I guess you wanted this here to
match the "--amend --only" test before it. That kind of sticks this
oddball in the middle of a bunch of --amend tests, but I'm not sure it
would go better anywhere else. So I'm fine with it here.
-Peff
^ permalink raw reply
* git reset --hard should not irretrievably destroy new files
From: Julian de Bhal @ 2016-12-03 5:04 UTC (permalink / raw)
To: git
If you `git add new_file; git reset --hard`, new_file is gone forever.
This is totally what git says it will do on the box, but it caught me out.
It might seem a little less stupid if I explain what I was doing: I was
breaking apart a chunk of work into smaller changes:
git commit -a -m 'tmp' # You feel pretty safe now, right?
git checkout -b backup/my-stuff # Not necessary, just a convenience
git checkout -
git reset HEAD^ # mixed
git add new_file
git add -p # also not necessary, but distracting
git reset --hard # decided copy from backed up diff
# boom. new_file is gone forever
Now, again, this is totally what git says it's going to do, and that was
pretty stupid, but that file is gone for good, and it feels bad.
Everything that was committed is safe, and the other untracked files in
my local directory are also fine, but that particular file is
permanently destroyed. This is the first time I've lost something since I
discovered the reflog a year or two ago.
The behaviour that would make the most sense to me (personally) would be
for a hard reset to unstage new files, but I'd be nearly as happy if a
commit was added to the reflog when the reset happens (I can probably make
that happen with some configuration now that I've been bitten).
If there's support for this idea but no-one is keen to write the code, let
me know and I could have a crack at it.
Cheers,
Julian de Bhál
^ permalink raw reply
* Re: [PATCH 1/4] shallow.c: make paint_alloc slightly more robust
From: Jeff King @ 2016-12-03 5:14 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1480710664-26290-1-git-send-email-rv@rasmusvillemoes.dk>
On Fri, Dec 02, 2016 at 09:31:01PM +0100, Rasmus Villemoes wrote:
> I have no idea if this is a real issue, but it's not obvious to me that
> paint_alloc cannot be called with info->nr_bits greater than about
> 4M (\approx 8*COMMIT_SLAB_SIZE). In that case the new slab would be too
> small. So just round up the allocation to the maximum of
> COMMIT_SLAB_SIZE and size.
I had trouble understanding what the problem is from this description,
but I think i figured it out from the code.
Let me try to restate it to make sure I understand.
The paint_alloc() may be asked to allocate a certain number of bits,
which it does across a series of independently allocated slabs. Each
slab holds a fixed size, but we only allocate a single slab. If the
number we need to allocate is larger than fits in a single slab, then at
the end we'll have under-allocated.
Your solution is to make the slab we allocate bigger. But that seems
odd to me. Usually when we are using COMMIT_SLAB_SIZE, we are allocating
a series of slabs that make up a virtual array, and we know that each
slab has the same size. So if you need to find the k-th item, and each
slab has length n, then you'd look at slab (k / n), and then at item (k
% n) within that slab.
In other words, I think the solution isn't to make the one slab bigger,
but to allocate slabs until we have enough of them to meet the request.
But I don't really know how this code is used, or why it is using
COMMIT_SLAB_SIZE in the first place. That's generally supposed to be an
internal detail of the commit-slab.h infrastructure. Why is it being
used directly, instead of just using the functions that commit-slab
defines?
-Peff
^ permalink raw reply
* Re: [PATCH 2/4] shallow.c: avoid theoretical pointer wrap-around
From: Jeff King @ 2016-12-03 5:17 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1480710664-26290-2-git-send-email-rv@rasmusvillemoes.dk>
On Fri, Dec 02, 2016 at 09:31:02PM +0100, Rasmus Villemoes wrote:
> The expression info->free+size is technically undefined behaviour in
> exactly the case we want to test for. Moreover, the compiler is likely
> to translate the expression to
>
> (unsigned long)info->free + size > (unsigned long)info->end
>
> where there's at least a theoretical chance that the LHS could wrap
> around 0, giving a false negative.
>
> This might as well be written using pointer subtraction avoiding these
> issues.
> [...]
>
> - if (!info->slab_count || info->free + size > info->end) {
> + if (!info->slab_count || size > info->end - info->free) {
Yeah, I agree the correct way to write this is to compare the sizes
directly. That is how overflow checks _must_ be written. This one is
less likely to overflow, but even computing the value more than one past
the end of the array is technically undefined.
-Peff
^ permalink raw reply
* Re: [PATCH 3/4] shallow.c: bit manipulation tweaks
From: Jeff King @ 2016-12-03 5:21 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1480710664-26290-3-git-send-email-rv@rasmusvillemoes.dk>
On Fri, Dec 02, 2016 at 09:31:03PM +0100, Rasmus Villemoes wrote:
> First of all, 1 << 31 is technically undefined behaviour, so let's just
> use an unsigned literal.
It took me a second to realize that you weren't talking about the
unsigned parameter here. You mean using "1U". It might be worth saying:
...use an unsigned literal, "1U".
to make it more obvious.
> If i is 'signed int' and gcc doesn't know that i is positive, gcc
> generates code to compute the C99-mandated values of "i / 32" and "i %
> 32", which is a lot more complicated than simple a simple shifts/mask.
Right, that makes sense (though it is a separate issue).
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] shallow.c: remove useless test
From: Jeff King @ 2016-12-03 5:24 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1480710664-26290-4-git-send-email-rv@rasmusvillemoes.dk>
On Fri, Dec 02, 2016 at 09:31:04PM +0100, Rasmus Villemoes wrote:
> It seems to be odd to do x=y if x==y. Maybe there's a bug somewhere near
> this, but as is this is somewhat confusing.
Yeah, this code is definitely wrong, but I'm not sure what it's trying
to do. This is the first time I've looked at it.
-Peff
^ permalink raw reply
* Re: [RFC PATCH 00/16] Checkout aware of Submodules!
From: Xiaodong Qi @ 2016-12-03 6:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, gitster, jrnieder, mogulguy10, David.Turner
In-Reply-To: <20161115230651.23953-1-sbeller@google.com>
I found this patch on Reddit and personally support this idea to
simplify the submodule update and checkout process. I don't know how
other users handle the submodule update process, I do sometimes forget
to checkout in superprojects with submodules and get a lot of trouble in
using the submodule function. The patch seems aims to making the process
easier than before, although I am not qualified to review the code in
detail. I suggest experts in this area to review the code promptly and
work out the nuts and bolts toward the goal of this patch. Thank you for
listening.
Regards,
Xiaodong Qi
On 11/15/2016 04:06 PM, Stefan Beller wrote:
> 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(-)
>
^ permalink raw reply
* git add -p doesn't honor diff.noprefix config
From: paddor @ 2016-12-03 6:45 UTC (permalink / raw)
To: git
Hi all
I set the config diff.noprefix = true because I don't like the a/ and b/ prefixes, which nicely changed the output of `git diff`. Unfortunately, the filenames in the output of `git add --patch` are still prefixed.
To me, this seems like a bug. Or there's a config option missing.
Best regards,
Patrik
^ permalink raw reply
* Re: [PATCH] commit: make --only --allow-empty work without paths
From: Andreas Krey @ 2016-12-03 6:59 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20161203043254.7ozjyucfn6uivnsh@sigill.intra.peff.net>
On Fri, 02 Dec 2016 23:32:55 +0000, Jeff King wrote:
> On Fri, Dec 02, 2016 at 11:15:13PM +0100, Andreas Krey wrote:
>
> > --only is implied when paths are present, and required
> > them unless --amend. But with --allow-empty it should
> > be allowed as well - it is the only way to create an
> > empty commit in the presence of staged changes.
>
> OK. I'm not sure why you would want to create an empty commit in such a
> case.
User: Ok tool, make me a pullreq.
Tool: But you haven't mentioned any issue
in your commit messages. Which are they?
User: Ok, that would be A-123.
Tool: git commit --allow-empty -m 'FIX: A-123'
Originally we checked that the status output was
empty, and later added an option for 'yes, I know
that there are uncommitted changes; I don't want
them included'.
And then someone had staged changes, which lead me here,
because there is no way now to create an empty commit
(just for the commit message) in that situation.
Amending the previous commit wouldn't fly with us
because of a local ban on non-fast-forward pushes.
...
> > (The interdepence of the tests is a strange thing;
> > making --run=N somewhat pointless.)
>
> Yes, I think --run is a misfeature (I actually had to look it up, as I
...
> implicit. If a single test script is annoyingly long to run, I'd argue
It wasn't about runtime but about output. I would have
liked to see only the output of my still-failing test;
a 'stop after test X' would be helpful there.
Andreas
--
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800
^ permalink raw reply
* Re: git reset --hard should not irretrievably destroy new files
From: Johannes Sixt @ 2016-12-03 7:49 UTC (permalink / raw)
To: Julian de Bhal; +Cc: git
In-Reply-To: <CAJZCeG1Eu+5DfaxavX_WGUCa+SY+yepDWZhPXxiFcV__h0xjrw@mail.gmail.com>
Am 03.12.2016 um 06:04 schrieb Julian de Bhal:
> If you `git add new_file; git reset --hard`, new_file is gone forever.
AFAIC, this is a feature ;-) I occasionally use it to remove a file when
I already have git-gui in front of me. Then it's often less convenient
to type the path in a shell, or to pointy-click around in a file browser.
> git add new_file
Because of this ...
> git add -p # also not necessary, but distracting
> git reset --hard # decided copy from backed up diff
> # boom. new_file is gone forever
... it is not. The file is still among the dangling blobs in the
repository until you clean it up with 'git gc'. Use 'git fsck --lost-found':
--lost-found
Write dangling objects into .git/lost-found/commit/ or
.git/lost-found/other/, depending on type. If the object is a blob, the
contents are written into the file, rather than its object name.
-- Hannes
^ permalink raw reply
* Re: git reset --hard should not irretrievably destroy new files
From: Christian Couder @ 2016-12-03 8:11 UTC (permalink / raw)
To: Julian de Bhal; +Cc: git
In-Reply-To: <CAJZCeG1Eu+5DfaxavX_WGUCa+SY+yepDWZhPXxiFcV__h0xjrw@mail.gmail.com>
On Sat, Dec 3, 2016 at 6:04 AM, Julian de Bhal <julian.debhal@gmail.com> wrote:
> If you `git add new_file; git reset --hard`, new_file is gone forever.
>
> This is totally what git says it will do on the box, but it caught me out.
Yeah, you are not the first one, and probably not the last
unfortunately, to be caught by it, see for example the last discussion
about it:
https://public-inbox.org/git/loom.20160523T023140-975@post.gmane.org/
which itself refers to this previous discussion:
https://public-inbox.org/git/CANWD=rX-MEiS4cNzDWr2wwkshz2zu8-L31UrKwbZrJSBcJX-nQ@mail.gmail.com/
> It might seem a little less stupid if I explain what I was doing: I was
> breaking apart a chunk of work into smaller changes:
>
> git commit -a -m 'tmp' # You feel pretty safe now, right?
> git checkout -b backup/my-stuff # Not necessary, just a convenience
> git checkout -
> git reset HEAD^ # mixed
> git add new_file
> git add -p # also not necessary, but distracting
> git reset --hard # decided copy from backed up diff
> # boom. new_file is gone forever
>
>
> Now, again, this is totally what git says it's going to do, and that was
> pretty stupid, but that file is gone for good, and it feels bad.
Yeah, I agree that it feels bad even if there are often ways to get
back your data as you can see from the links in Yotam's email above.
> Everything that was committed is safe, and the other untracked files in
> my local directory are also fine, but that particular file is
> permanently destroyed. This is the first time I've lost something since I
> discovered the reflog a year or two ago.
>
> The behaviour that would make the most sense to me (personally) would be
> for a hard reset to unstage new files,
This has already been proposed last time...
> but I'd be nearly as happy if a
> commit was added to the reflog when the reset happens (I can probably make
> that happen with some configuration now that I've been bitten).
Not sure if this has been proposed. Perhaps it would be simpler to
just output the sha1, and maybe the filenames too, of the blobs, that
are no more referenced from the trees, somewhere (in a bloblog?).
> If there's support for this idea but no-one is keen to write the code, let
> me know and I could have a crack at it.
Not sure if your report and your offer will make us more likely to
agree to do something, but thanks for trying!
^ permalink raw reply
* [PATCH 3/3] unicode_width.h: fix the double_width[] table
From: Beat Bolli @ 2016-12-03 10:35 UTC (permalink / raw)
To: git; +Cc: Beat Bolli
In-Reply-To: <1480713995-16157-1-git-send-email-dev+git@drbeat.li>
The function bisearch() in utf8.c does a pure binary search in
double_width. It does not care about the 17 plane offsets which
unicode/uniset/uniset prepends. Leaving the plane offsets in the table
may cause wrong results.
Filter out the plane offsets in the update-unicode.sh and regenerate
the table.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
unicode_width.h | 17 -----------------
update_unicode.sh | 2 +-
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/unicode_width.h b/unicode_width.h
index 73b5fd6..02207be 100644
--- a/unicode_width.h
+++ b/unicode_width.h
@@ -297,23 +297,6 @@ static const struct interval zero_width[] = {
{ 0xE0100, 0xE01EF }
};
static const struct interval double_width[] = {
-{ /* plane */ 0x0, 0x3D },
-{ /* plane */ 0x3D, 0x68 },
-{ /* plane */ 0x68, 0x69 },
-{ /* plane */ 0x69, 0x6A },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
{ 0x1100, 0x115F },
{ 0x231A, 0x231B },
{ 0x2329, 0x232A },
diff --git a/update_unicode.sh b/update_unicode.sh
index 3c84270..4c1ec8d 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -30,7 +30,7 @@ fi &&
grep -v plane)
};
static const struct interval double_width[] = {
- $(uniset/uniset --32 eaw:F,W)
+ $(uniset/uniset --32 eaw:F,W | grep -v plane)
};
EOF
)
--
2.7.2
^ permalink raw reply related
* [PATCH v2 1/3] update-unicode.sh: automatically download newer definition files
From: Beat Bolli @ 2016-12-03 10:53 UTC (permalink / raw)
To: git; +Cc: Beat Bolli, Torsten Bögershausen
In-Reply-To: <1480713995-16157-1-git-send-email-dev+git@drbeat.li>
Checking just for the unicode data files' existence is not sufficient;
we should also download them if a newer version exists on the Unicode
consortium's servers. Option -N of wget does this nicely for us.
Cc: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Diff to v1:
- reword the commit message
- add Thorsten's Cc:
update_unicode.sh | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/update_unicode.sh b/update_unicode.sh
index 27af77c..3c84270 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -10,12 +10,8 @@ if ! test -d unicode; then
mkdir unicode
fi &&
( cd unicode &&
- if ! test -f UnicodeData.txt; then
- wget http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt
- fi &&
- if ! test -f EastAsianWidth.txt; then
- wget http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt
- fi &&
+ wget -N http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt \
+ http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt &&
if ! test -d uniset; then
git clone https://github.com/depp/uniset.git
fi &&
--
2.7.2
^ permalink raw reply related
* [PATCH v2 3/3] unicode_width.h: fix the double_width[] table
From: Beat Bolli @ 2016-12-03 10:53 UTC (permalink / raw)
To: git; +Cc: Beat Bolli, Torsten Bögershausen
In-Reply-To: <1480762392-28731-1-git-send-email-dev+git@drbeat.li>
The function bisearch() in utf8.c does a pure binary search in
double_width. It does not care about the 17 plane offsets which
unicode/uniset/uniset prepends. Leaving the plane offsets in the table
may cause wrong results.
Filter out the plane offsets in update-unicode.sh and regenerate the
table.
Cc: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Diff to v1:
- add Thorsten's Cc:
unicode_width.h | 17 -----------------
update_unicode.sh | 2 +-
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/unicode_width.h b/unicode_width.h
index 73b5fd6..02207be 100644
--- a/unicode_width.h
+++ b/unicode_width.h
@@ -297,23 +297,6 @@ static const struct interval zero_width[] = {
{ 0xE0100, 0xE01EF }
};
static const struct interval double_width[] = {
-{ /* plane */ 0x0, 0x3D },
-{ /* plane */ 0x3D, 0x68 },
-{ /* plane */ 0x68, 0x69 },
-{ /* plane */ 0x69, 0x6A },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
-{ /* plane */ 0x0, 0x0 },
{ 0x1100, 0x115F },
{ 0x231A, 0x231B },
{ 0x2329, 0x232A },
diff --git a/update_unicode.sh b/update_unicode.sh
index 3c84270..4c1ec8d 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -30,7 +30,7 @@ fi &&
grep -v plane)
};
static const struct interval double_width[] = {
- $(uniset/uniset --32 eaw:F,W)
+ $(uniset/uniset --32 eaw:F,W | grep -v plane)
};
EOF
)
--
2.7.2
^ 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