Git development
 help / color / mirror / Atom feed
* [PATCH 1/6] submodule.h: add extern keyword to functions
From: Stefan Beller @ 2016-12-13  1:40 UTC (permalink / raw)
  To: gitster; +Cc: git, David.Turner, bmwill, Stefan Beller
In-Reply-To: <20161213014055.14268-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 | 55 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/submodule.h b/submodule.h
index 6229054b99..61fb610749 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,50 +29,55 @@ 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);
-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);
 
 #define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
 extern void absorb_git_dir_into_superproject(const char *prefix,
-- 
2.11.0.rc2.35.g7af3268


^ permalink raw reply related

* [PATCH 3/6] submodule: add flags to ok_to_remove_submodule
From: Stefan Beller @ 2016-12-13  1:40 UTC (permalink / raw)
  To: gitster; +Cc: git, David.Turner, bmwill, Stefan Beller
In-Reply-To: <20161213014055.14268-1-sbeller@google.com>

In different contexts the question whether deleting a submodule is ok
to remove may be answered differently.

In 293ab15eea (submodule: teach rm to remove submodules unless they
contain a git directory, 2012-09-26) a case was made that we can safely
ignore ignored untracked files for removal as we explicitely ask for the
removal of the submodule.

In a later patch we want to remove submodules even when the user doesn't
explicitly ask for it (e.g. checking out a tree-ish in which the submodule
doesn't exist).  In that case we want to be more careful when it comes
to deletion of untracked files. As of this patch it is unclear how this
will be implemented exactly, so we'll offer flags in which the caller
can specify how the different untracked files ought to be handled.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/rm.c |  3 ++-
 submodule.c  | 23 +++++++++++++++++++----
 submodule.h  |  5 ++++-
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 3f3e24eb36..fdd7183f61 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -187,7 +187,8 @@ static int check_local_mod(struct object_id *head, int index_only)
 		 */
 		if (ce_match_stat(ce, &st, 0) ||
 		    (S_ISGITLINK(ce->ce_mode) &&
-		     !ok_to_remove_submodule(ce->name)))
+		     !ok_to_remove_submodule(ce->name,
+				SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)))
 			local_changes = 1;
 
 		/*
diff --git a/submodule.c b/submodule.c
index 9f0b544ebe..2d13744b06 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1019,7 +1019,7 @@ int submodule_uses_gitfile(const char *path)
 	return 1;
 }
 
-int ok_to_remove_submodule(const char *path)
+int ok_to_remove_submodule(const char *path, unsigned flags)
 {
 	ssize_t len;
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -1032,15 +1032,27 @@ int ok_to_remove_submodule(const char *path)
 	if (!submodule_uses_gitfile(path))
 		return 0;
 
-	argv_array_pushl(&cp.args, "status", "--porcelain", "-u",
+	argv_array_pushl(&cp.args, "status", "--porcelain",
 				   "--ignore-submodules=none", NULL);
+
+	if (flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED)
+		argv_array_push(&cp.args, "-uno");
+	else
+		argv_array_push(&cp.args, "-uall");
+
+	if (!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED))
+		argv_array_push(&cp.args, "--ignored");
+
 	prepare_submodule_repo_env(&cp.env_array);
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;
 	cp.out = -1;
 	cp.dir = path;
 	if (start_command(&cp))
-		die(_("could not run 'git status --porcelain -u --ignore-submodules=none' in submodule %s"), path);
+		die(_("could not run 'git status --porcelain --ignore-submodules=none %s %s' in submodule '%s'"),
+			(flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED) ? "-uno" : "-uall",
+			(!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)) ? "--ignored" : "",
+			path);
 
 	len = strbuf_read(&buf, cp.out, 1024);
 	if (len > 2)
@@ -1048,7 +1060,10 @@ int ok_to_remove_submodule(const char *path)
 	close(cp.out);
 
 	if (finish_command(&cp))
-		die(_("'git status --porcelain -u --ignore-submodules=none' failed in submodule %s"), path);
+		die(_("'git status --porcelain --ignore-submodules=none %s %s' failed in submodule '%s'"),
+			(flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED) ? "-uno" : "-uall",
+			(!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)) ? "--ignored" : "",
+			path);
 
 	strbuf_release(&buf);
 	return ok_to_remove;
diff --git a/submodule.h b/submodule.h
index 61fb610749..3ed3aa479a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -59,7 +59,10 @@ extern int fetch_populated_submodules(const struct argv_array *options,
 			       int quiet, int max_parallel_jobs);
 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);
+
+#define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<0)
+#define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<1)
+extern int ok_to_remove_submodule(const char *path, unsigned flags);
 extern int merge_submodule(unsigned char result[20], const char *path,
 			   const unsigned char base[20],
 			   const unsigned char a[20],
-- 
2.11.0.rc2.35.g7af3268


^ permalink raw reply related

* [PATCH 6/6] rm: add absorb a submodules git dir before deletion
From: Stefan Beller @ 2016-12-13  1:40 UTC (permalink / raw)
  To: gitster; +Cc: git, David.Turner, bmwill, Stefan Beller
In-Reply-To: <20161213014055.14268-1-sbeller@google.com>

When deleting a submodule we need to keep the actual git directory around,
such that we do not lose local changes in there and at a later checkout
of the submodule we don't need to clone it again.

Implement `depopulate_submodule`, that migrates the git directory before
deletion of a submodule and afterwards the equivalent of "rm -rf", which
is already found in entry.c, so expose that and for clarity add a suffix
"_or_dir" to it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/rm.c  | 18 ++++++------------
 cache.h       |  2 ++
 entry.c       |  5 +++++
 submodule.c   | 31 +++++++++++++++++++++++++++++++
 submodule.h   |  6 ++++++
 t/t3600-rm.sh | 41 ++++++++++++++++-------------------------
 6 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index fdd7183f61..f8c5e9b6c6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -400,18 +400,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 						continue;
 					}
 				} else {
-					strbuf_reset(&buf);
-					strbuf_addstr(&buf, path);
-					if (!remove_dir_recursively(&buf, 0)) {
-						removed = 1;
-						if (!remove_path_from_gitmodules(path))
-							gitmodules_modified = 1;
-						strbuf_release(&buf);
-						continue;
-					} else if (!file_exists(path))
-						/* Submodule was removed by user */
-						if (!remove_path_from_gitmodules(path))
-							gitmodules_modified = 1;
+					if (file_exists(path))
+						depopulate_submodule(path);
+					removed = 1;
+					if (!remove_path_from_gitmodules(path))
+						gitmodules_modified = 1;
+					continue;
 					/* Fallthrough and let remove_path() fail. */
 				}
 			}
diff --git a/cache.h b/cache.h
index a50a61a197..b645ca2f9a 100644
--- a/cache.h
+++ b/cache.h
@@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);
  */
 void safe_create_dir(const char *dir, int share);
 
+extern void remove_directory_or_die(struct strbuf *path);
+
 #endif /* CACHE_H */
diff --git a/entry.c b/entry.c
index c6eea240b6..02c4ac9f22 100644
--- a/entry.c
+++ b/entry.c
@@ -73,6 +73,11 @@ static void remove_subtree(struct strbuf *path)
 		die_errno("cannot rmdir '%s'", path->buf);
 }
 
+void remove_directory_or_die(struct strbuf *path)
+{
+	remove_subtree(path);
+}
+
 static int create_file(const char *path, unsigned int mode)
 {
 	mode = (mode & 0100) ? 0777 : 0666;
diff --git a/submodule.c b/submodule.c
index e42efa2337..3770ecb7b9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -308,6 +308,37 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
 	strbuf_release(&sb);
 }
 
+void depopulate_submodule(const char *path)
+{
+	struct strbuf pathbuf = STRBUF_INIT;
+	char *dot_git = xstrfmt("%s/.git", path);
+
+	/* Is it populated? */
+	if (!resolve_gitdir(dot_git))
+		goto out;
+
+	/* Does it have a .git directory? */
+	if (!submodule_uses_gitfile(path)) {
+		absorb_git_dir_into_superproject("", path,
+			ABSORB_GITDIR_RECURSE_SUBMODULES);
+
+		if (!submodule_uses_gitfile(path)) {
+			/*
+			 * We should be using a gitfile by now. Let's double
+			 * check as losing the git dir would be fatal.
+			 */
+			die("BUG: could not absorb git directory for '%s'", path);
+		}
+	}
+
+	strbuf_addstr(&pathbuf, path);
+	remove_directory_or_die(&pathbuf);
+
+out:
+	strbuf_release(&pathbuf);
+	free(dot_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
diff --git a/submodule.h b/submodule.h
index 3ed3aa479a..516e377a12 100644
--- a/submodule.h
+++ b/submodule.h
@@ -53,6 +53,12 @@ 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);
+
+/*
+ * Removes a submodule from a given path. When the submodule contains its
+ * git directory instead of a gitlink, migrate that first into the superproject.
+ */
+extern void depopulate_submodule(const char *path);
 extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 extern int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5e5a16c863..5aa6db584c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -569,26 +569,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
 	test_cmp expect actual
 '
 
-test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
+test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
 	git checkout -f master &&
 	git reset --hard &&
 	git submodule update &&
 	(cd submod &&
 		rm .git &&
 		cp -R ../.git/modules/sub .git &&
-		GIT_WORK_TREE=. git config --unset core.worktree
+		GIT_WORK_TREE=. git config --unset core.worktree &&
+		rm -r ../.git/modules/sub
 	) &&
-	test_must_fail git rm submod &&
-	test -d submod &&
-	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	test_must_fail git rm -f submod &&
-	test -d submod &&
-	test -d submod/.git &&
+	git rm submod 2>output.err &&
+	! test -d submod &&
+	! test -d submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	rm -rf submod
+	test -s actual &&
+	test_i18ngrep Migrating output.err
 '
 
 cat >expect.deepmodified <<EOF
@@ -667,27 +663,22 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
 	git submodule update --recursive &&
 	(cd submod/subsubmod &&
 		rm .git &&
-		cp -R ../../.git/modules/sub/modules/sub .git &&
+		mv ../../.git/modules/sub/modules/sub .git &&
 		GIT_WORK_TREE=. git config --unset core.worktree
 	) &&
-	test_must_fail git rm submod &&
-	test -d submod &&
-	test -d submod/subsubmod/.git &&
-	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	test_must_fail git rm -f submod &&
-	test -d submod &&
-	test -d submod/subsubmod/.git &&
+	git rm submod 2>output.err &&
+	! test -d submod &&
+	! test -d submod/subsubmod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	rm -rf submod
+	test -s actual &&
+	test_i18ngrep Migrating output.err
 '
 
 test_expect_success 'checking out a commit after submodule removal needs manual updates' '
-	git commit -m "submodule removal" submod &&
+	git commit -m "submodule removal" submod .gitmodules &&
 	git checkout HEAD^ &&
 	git submodule update &&
-	git checkout -q HEAD^ 2>actual &&
+	git checkout -q HEAD^ &&
 	git checkout -q master 2>actual &&
 	test_i18ngrep "^warning: unable to rmdir submod:" actual &&
 	git status -s submod >actual &&
-- 
2.11.0.rc2.35.g7af3268


^ permalink raw reply related

* [PATCH 5/6] t3600: slightly modernize style
From: Stefan Beller @ 2016-12-13  1:40 UTC (permalink / raw)
  To: gitster; +Cc: git, David.Turner, bmwill, Stefan Beller
In-Reply-To: <20161213014055.14268-1-sbeller@google.com>

Remove the space between redirection and file name.
Also remove unnecessary invocations of subshells, such as

	(cd submod &&
		echo X >untracked
	) &&

as there is no point of having the shell for functional purposes.
In case of a single Git command use the `-C` option to let Git cd into
the directory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t3600-rm.sh | 122 ++++++++++++++++++++++++----------------------------------
 1 file changed, 50 insertions(+), 72 deletions(-)

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 14f0edca2b..5e5a16c863 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -111,21 +111,21 @@ test_expect_success 'Remove nonexistent file with --ignore-unmatch' '
 '
 
 test_expect_success '"rm" command printed' '
-	echo frotz > test-file &&
+	echo frotz >test-file &&
 	git add test-file &&
 	git commit -m "add file for rm test" &&
-	git rm test-file > rm-output &&
+	git rm test-file >rm-output &&
 	test $(grep "^rm " rm-output | wc -l) = 1 &&
 	rm -f test-file rm-output &&
 	git commit -m "remove file from rm test"
 '
 
 test_expect_success '"rm" command suppressed with --quiet' '
-	echo frotz > test-file &&
+	echo frotz >test-file &&
 	git add test-file &&
 	git commit -m "add file for rm --quiet test" &&
-	git rm --quiet test-file > rm-output &&
-	test $(wc -l < rm-output) = 0 &&
+	git rm --quiet test-file >rm-output &&
+	test_must_be_empty rm-output &&
 	rm -f test-file rm-output &&
 	git commit -m "remove file from rm --quiet test"
 '
@@ -221,7 +221,7 @@ test_expect_success 'Call "rm" from outside the work tree' '
 	mkdir repo &&
 	(cd repo &&
 	 git init &&
-	 echo something > somefile &&
+	 echo something >somefile &&
 	 git add somefile &&
 	 git commit -m "add a file" &&
 	 (cd .. &&
@@ -287,7 +287,7 @@ test_expect_success 'rm removes empty submodules from work tree' '
 	git commit -m "add submodule" &&
 	git rm submod &&
 	test ! -e submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -298,7 +298,7 @@ test_expect_success 'rm removes removed submodule from index and .gitmodules' '
 	git submodule update &&
 	rm -rf submod &&
 	git rm submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -309,7 +309,7 @@ test_expect_success 'rm removes work tree of unmodified submodules' '
 	git submodule update &&
 	git rm submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -320,7 +320,7 @@ test_expect_success 'rm removes a submodule with a trailing /' '
 	git submodule update &&
 	git rm submod/ &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -335,17 +335,15 @@ test_expect_success 'rm succeeds when given a directory with a trailing /' '
 test_expect_success 'rm of a populated submodule with different HEAD fails unless forced' '
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		git checkout HEAD^
-	) &&
+	git -C submod checkout HEAD^ &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -418,34 +416,30 @@ test_expect_success 'rm issues a warning when section is not found in .gitmodule
 test_expect_success 'rm of a populated submodule with modifications fails unless forced' '
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		echo X >empty
-	) &&
+	echo X >submod/empty &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'rm of a populated submodule with untracked files fails unless forced' '
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		echo X >untracked
-	) &&
+	echo X >submod/untracked &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -461,16 +455,12 @@ test_expect_success 'setup submodule conflict' '
 	git add nitfol &&
 	git commit -m "added nitfol 2" &&
 	git checkout -b conflict1 master &&
-	(cd submod &&
-		git fetch &&
-		git checkout branch1
-	) &&
+	git -C submod fetch &&
+	git -C submod checkout branch1 &&
 	git add submod &&
 	git commit -m "submod 1" &&
 	git checkout -b conflict2 master &&
-	(cd submod &&
-		git checkout branch2
-	) &&
+	git -C submod checkout branch2 &&
 	git add submod &&
 	git commit -m "submod 2"
 '
@@ -486,7 +476,7 @@ test_expect_success 'rm removes work tree of unmodified conflicted submodule' '
 	test_must_fail git merge conflict2 &&
 	git rm submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -494,18 +484,16 @@ test_expect_success 'rm of a conflicted populated submodule with different HEAD
 	git checkout conflict1 &&
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		git checkout HEAD^
-	) &&
+	git -C submod checkout HEAD^ &&
 	test_must_fail git merge conflict2 &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.conflict actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -515,18 +503,16 @@ test_expect_success 'rm of a conflicted populated submodule with modifications f
 	git checkout conflict1 &&
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		echo X >empty
-	) &&
+	echo X >submod/empty &&
 	test_must_fail git merge conflict2 &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.conflict actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual &&
 	test_must_fail git config -f .gitmodules submodule.sub.url &&
 	test_must_fail git config -f .gitmodules submodule.sub.path
@@ -536,18 +522,16 @@ test_expect_success 'rm of a conflicted populated submodule with untracked files
 	git checkout conflict1 &&
 	git reset --hard &&
 	git submodule update &&
-	(cd submod &&
-		echo X >untracked
-	) &&
+	echo X >submod/untracked &&
 	test_must_fail git merge conflict2 &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.conflict actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -564,12 +548,12 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.conflict actual &&
 	test_must_fail git rm -f submod &&
 	test -d submod &&
 	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.conflict actual &&
 	git merge --abort &&
 	rm -rf submod
@@ -581,7 +565,7 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
 	test_must_fail git merge conflict2 &&
 	git rm submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -597,12 +581,12 @@ test_expect_success 'rm of a populated submodule with a .git directory fails eve
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	! test -s actual &&
 	test_must_fail git rm -f submod &&
 	test -d submod &&
 	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	! test -s actual &&
 	rm -rf submod
 '
@@ -629,58 +613,52 @@ test_expect_success 'setup subsubmodule' '
 test_expect_success 'rm recursively removes work tree of unmodified submodules' '
 	git rm submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'rm of a populated nested submodule with different nested HEAD fails unless forced' '
 	git reset --hard &&
 	git submodule update --recursive &&
-	(cd submod/subsubmod &&
-		git checkout HEAD^
-	) &&
+	git -C submod/subsubmod checkout HEAD^ &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'rm of a populated nested submodule with nested modifications fails unless forced' '
 	git reset --hard &&
 	git submodule update --recursive &&
-	(cd submod/subsubmod &&
-		echo X >empty
-	) &&
+	echo X >submod/subsubmod/empty &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'rm of a populated nested submodule with nested untracked files fails unless forced' '
 	git reset --hard &&
 	git submodule update --recursive &&
-	(cd submod/subsubmod &&
-		echo X >untracked
-	) &&
+	echo X >submod/subsubmod/untracked &&
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -f submod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect.modified actual &&
 	git rm -f submod &&
 	test ! -d submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	test_cmp expect actual
 '
 
@@ -695,12 +673,12 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
 	test_must_fail git rm submod &&
 	test -d submod &&
 	test -d submod/subsubmod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	! test -s actual &&
 	test_must_fail git rm -f submod &&
 	test -d submod &&
 	test -d submod/subsubmod/.git &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	! test -s actual &&
 	rm -rf submod
 '
@@ -716,7 +694,7 @@ test_expect_success 'checking out a commit after submodule removal needs manual
 	echo "?? submod/" >expected &&
 	test_cmp expected actual &&
 	rm -rf submod &&
-	git status -s -uno --ignore-submodules=none > actual &&
+	git status -s -uno --ignore-submodules=none >actual &&
 	! test -s actual
 '
 
-- 
2.11.0.rc2.35.g7af3268


^ permalink raw reply related

* Re: [PATCH 6/6] rm: add absorb a submodules git dir before deletion
From: brian m. carlson @ 2016-12-13  3:28 UTC (permalink / raw)
  To: Stefan Beller; +Cc: gitster, git, David.Turner, bmwill
In-Reply-To: <20161213014055.14268-7-sbeller@google.com>

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

On Mon, Dec 12, 2016 at 05:40:55PM -0800, Stefan Beller wrote:
> When deleting a submodule we need to keep the actual git directory around,
> such that we do not lose local changes in there and at a later checkout
> of the submodule we don't need to clone it again.
> 
> Implement `depopulate_submodule`, that migrates the git directory before
> deletion of a submodule and afterwards the equivalent of "rm -rf", which
> is already found in entry.c, so expose that and for clarity add a suffix
> "_or_dir" to it.

I think you might have meant "_or_die" here.

Other than that, I didn't see anything that stuck out to me in this
series as obviously wrong or broken, but this isn't an area of the code
I'm super familiar with.
-- 
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 1/3] update_unicode.sh: update the uniset repo if it exists
From: Torsten Bögershausen @ 2016-12-13  6:16 UTC (permalink / raw)
  To: Beat Bolli, git
In-Reply-To: <954eed6b-c899-4f4c-eb3d-2b6d2ff4385d@drbeat.li>


>> Sure, and I'd rather see the update-unicode.sh script moved
>> somewhere in contrib/ while at it.  Those who are interested in
>> keeping up with the unicode standard are tiny minority of the
>> developer population, and most of us would treat the built width
>> table as the source (after all, that is what we ship).
>>
>> To be bluntly honest, I'd rather not to see "update-unicode.sh"
>> download and build uniset at all.  It's as if po/ hierarchy shipping
>> with its own script to download and build msgmerge--that's madness.
>> Needless to say, shipping the sources for uniset embedded in our
>> project tree (either as a snapshot-fork or as a submodule) is even
>> worse.  Those who want to muck with po/ are expected to have
>> msgmerge and friends.  Why not expect the same for those who want to
>> update the unicode width table?
>>
>> I'd rather see a written instruction telling which snapshot to get
>> and from where to build and place on their $PATH in the README file,
>> sitting next to the update-unicode.sh script in contrib/uniwidth/
>> directory, for those who are interested in building the width table
>> "from the source", and the update-unicode.sh script to assume that
>> uniset is available.
OK with the contrib - that's an improvement.
About the instructions how to download and compile:
(we don't need to change the  $PATH, do we ?)
I don't know.
The typical instructions I have seen are a sequence of shell commands
to be executed, which hopefully simply work by doing "copy-and-paste".
I find this error-prone, as you you may loose the last character while
moving the mouse, or don't check the error message or return codes.
Having a pre-baked shell script, which does use "&&" is in that way more 
attractive,
and the README can be as simple as run "update-unicode.sh" and that's it.

uniset is a small project and where should we put it ?
a) inside the Git tree?
b) /tmp ?
c) into the $HOME directory ?
d) /usr/local

a) is quick and dirty
b) probably OK
c) Not sure about tha
d) Needs super user rights
Can we try to find a good place ?

"contrib/uniwidth/" may be different to find, how about contrib/update-unicode ?
  

> OK. So please don't merge bb/unicode-9.0 to next yet; I'll prepare a
> reroll following your description.
>
> Torsten, is this alright with you?

sure

> Cheers, Beat



^ permalink raw reply

* Re: [PATCHv2 5/7] versioncmp: cope with common part overlapping with prerelease suffix
From: Junio C Hamano @ 2016-12-13  6:39 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git
In-Reply-To: <CAM0VKjnPdt3keodXRFNit9=WKeY330N4T2t_dJuArgch7L6BNg@mail.gmail.com>

SZEDER Gábor <szeder.dev@gmail.com> writes:

>>> -             if (i1 == -1 && starts_with(s1 + off, suffix))
>>> -                     i1 = i;
>>> -             if (i2 == -1 && starts_with(s2 + off, suffix))
>>> -                     i2 = i;
>>> +             int j, start, suffix_len = strlen(suffix);
>>> +             if (suffix_len < off)
>>> +                     start = off - suffix_len + 1;
>>> +             else
>>> +                     start = 0;
>>
>> Now that this function has to rewind the beginning of the comparison
>> earlier than the given 'off', it makes me wonder if it still makes
>> sense for the caller to compute it in the first place.
>
> The caller has to compute it anyway, because it must deal with all the
> cases when the two compared tagnames are not reordered based on their
> (prerelease)suffix.

Sure.

^ permalink raw reply

* Re: [PATCH v3 4/4] real_path: have callers use real_pathdup and strbuf_realpath
From: Junio C Hamano @ 2016-12-13  6:39 UTC (permalink / raw)
  To: Brandon Williams
  Cc: Stefan Beller, git@vger.kernel.org, Jeff King, Jacob Keller,
	Ramsay Jones, Torsten Bögershausen, Johannes Sixt,
	Duy Nguyen
In-Reply-To: <20161213011529.GC193413@google.com>

Brandon Williams <bmwill@google.com> writes:

> On 12/12, Stefan Beller wrote:
>> On Mon, Dec 12, 2016 at 3:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Junio C Hamano <gitster@pobox.com> writes:
>> >
>> >> Brandon Williams <bmwill@google.com> writes:
>> >>
>> >>> Migrate callers of real_path() who duplicate the retern value to use
>> >>> real_pathdup or strbuf_realpath.
>> >>
>> >> Looks good.
>> >
>> > This has small non-textual conflicts with Stefan's embed^Wabsorption
>> > topic; please holler if you spot my mismerge.  Thanks.
>> 
>> 5f6a003727 (Merge branch 'bw/realpath-wo-chdir' into jch)
>> looks good to me.
>
> Looks good to me as well.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/3] update_unicode.sh: update the uniset repo if it exists
From: Junio C Hamano @ 2016-12-13  6:42 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Beat Bolli, git
In-Reply-To: <d91b5e69-9b90-e21e-0dcb-8eace00ddd74@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

> The typical instructions I have seen are a sequence of shell commands
> to be executed, which hopefully simply work by doing "copy-and-paste".
> I find this error-prone, as you you may loose the last character while
> moving the mouse, or don't check the error message or return codes.
> Having a pre-baked shell script, which does use "&&" is in that way
> more attractive,
> and the README can be as simple as run "update-unicode.sh" and that's it.

That's OK as well.

> "contrib/uniwidth/" may be different to find, how about contrib/update-unicode ?

This, too.  And as long as .gitignore pattern is set up correctly
there, I do not think we terribly mind "git clone ..from..there.."
into it, either.

^ permalink raw reply

* Re: [PATCH] t3600: slightly modernize style
From: Junio C Hamano @ 2016-12-13  7:11 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git
In-Reply-To: <20161212235455.13796-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> Remove the space between redirection and file name.
> Also remove unnecessary invocations of subshells, such as
>
> 	(cd submod &&
> 		echo X >untracked
> 	) &&
>
> as there is no point of having the shell for functional purposes.
> In case of a single Git command use the `-C` option to let Git cd into
> the directory.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Looks good.  Thanks.

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Junio C Hamano @ 2016-12-13  7:28 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, David.Turner, bmwill
In-Reply-To: <20161213014055.14268-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> The "checkout --recurse-submodules" series got too large to comfortably send
> it out for review, so I had to break it up into smaller series'; this is the
> first subseries, but it makes sense on its own.
>
> This series teaches git-rm to absorb the git directory of a submodule instead
> of failing and complaining about the git directory preventing deletion.
>  
> It applies on origin/sb/submodule-embed-gitdir.

Thanks.  I probably should rename the topic again with s/embed/absorb/;

> Any feedback welcome!
>
> Thanks,
> Stefan
>
> Stefan Beller (6):
>   submodule.h: add extern keyword to functions
>   submodule: modernize ok_to_remove_submodule to use argv_array
>   submodule: add flags to ok_to_remove_submodule
>   ok_to_remove_submodule: absorb the submodule git dir
>   t3600: slightly modernize style
>   rm: add absorb a submodules git dir before deletion
>
>  builtin/rm.c  |  21 +++-----
>  cache.h       |   2 +
>  entry.c       |   5 ++
>  submodule.c   |  77 +++++++++++++++++++++++-----
>  submodule.h   |  64 ++++++++++++++---------
>  t/t3600-rm.sh | 159 +++++++++++++++++++++++----------------------------------
>  6 files changed, 182 insertions(+), 146 deletions(-)

^ permalink raw reply

* Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit'
From: Chris Packham @ 2016-12-13  8:33 UTC (permalink / raw)
  To: Markus Hitter; +Cc: GIT, Jeff King, Jacob Keller, Junio C Hamano
In-Reply-To: <b814932a-b395-2b27-979f-cd170ba363ee@jump-ing.de>

On Mon, Dec 12, 2016 at 10:02 PM, Markus Hitter <mah@jump-ing.de> wrote:
> Am 12.12.2016 um 09:34 schrieb Chris Packham:
>> Teach 'git merge' the --continue option which allows 'continuing' a
>> merge by completing it. The traditional way of completing a merge after
>> resolving conflicts is to use 'git commit'. Now with commands like 'git
>> rebase' and 'git cherry-pick' having a '--continue' option adding such
>> an option to 'git merge' presents a consistent UI.
>
> Like.
>
> While Junio is entirely right that this is redundant, the inner workings of Git are just voodoo for a (guessed) 95% of users out there, so a consistent UI is important.
>
>>  DESCRIPTION
>>  -----------
>> @@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
>>  discouraged: while possible, it may leave you in a state that is hard to
>>  back out of in the case of a conflict.
>>
>> +The fourth syntax ("`git merge --continue`") can only be run after the
>> +merge has resulted in conflicts. 'git merge --continue' will take the
>> +currently staged changes and complete the merge.
>
> I think this should mention the equivalence to 'git commit'.
>

It is mentioned in the OPTIONS section where the --continue option is
documented. I could move it here but the OPTIONS section is where the
--abort synonym also has a reference to git reset --merge.

>
> Markus
>
> --
> - - - - - - - - - - - - - - - - - - -
> Dipl. Ing. (FH) Markus Hitter
> http://www.jump-ing.de/

^ permalink raw reply

* log: range_set_append: Assertion failed with -L:funcname:
From: Kevin Locke @ 2016-12-13  8:41 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

Hi all,

I encountered the following assertion failure when running in a local
clone of the https://github.com/nodejs/node.git repository:

$ git log -L:writeFileSync:lib/fs.js -L:appendFileSync:lib/fs.js aa67b1^..aa67b1
git: line-log.c:71: range_set_append: Assertion `rs->nr == 0 || rs->ranges[rs->nr-1].end <= a' failed.
Aborted

I was able to provoke the same assertion failure in the git repository
with the following:

$ git log -L:print_one_push_status:transport.c -L:transport_summary_width:transport.c f9db0c^..f9db0c

(FWIW, f9db0c is a merge commit while aa67b1 is not.)

I was able to reproduce the error with git built from 13b8f6 (the
commit which introduced git log -L:pattern:file) as well as current
next (91ed5b).  The backtrace from next is attached.

Any assistance to avoid or fix the error would be greatly appreciated.

Thanks,
Kevin

P.S.  Please CC me in replies as I am not subscribed to this ML.

-- 
Kevin Locke  |  kevin@kevinlocke.name    | XMPP: kevin@kevinlocke.name
             |  https://kevinlocke.name  | IRC:   kevinoid on freenode

[-- Attachment #2: backtrace.txt --]
[-- Type: text/plain, Size: 2461 bytes --]

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:58
#1  0x00007ffff6fa040a in __GI_abort () at abort.c:89
#2  0x00007ffff6f97e47 in __assert_fail_base (fmt=<optimized out>, 
    assertion=assertion@entry=0x555555754400 "rs->nr == 0 || rs->ranges[rs->nr-1].end <= a", file=file@entry=0x5555557543eb "line-log.c", line=line@entry=71, 
    function=function@entry=0x555555754730 <__PRETTY_FUNCTION__.29201> "range_set_append") at assert.c:92
#3  0x00007ffff6f97ef2 in __GI___assert_fail (
    assertion=0x555555754400 "rs->nr == 0 || rs->ranges[rs->nr-1].end <= a", 
    file=0x5555557543eb "line-log.c", line=71, 
    function=0x555555754730 <__PRETTY_FUNCTION__.29201> "range_set_append")
    at assert.c:101
#4  0x00005555556677dd in range_set_append (rs=0x7fffffffd320, a=421, b=434)
    at line-log.c:71
#5  0x00005555556686f8 in range_set_shift_diff (out=0x7fffffffd320, 
    rs=0x7fffffffd330, diff=0x7fffffffd3a0) at line-log.c:442
#6  0x00005555556687aa in range_set_map_across_diff (out=0x7fffffffd3c0, 
    rs=0x555555ad9698, diff=0x7fffffffd3a0, touched_out=0x7fffffffd410)
    at line-log.c:465
#7  0x000055555566a33e in process_diff_filepair (rev=0x7fffffffd5f0, 
    pair=0x555555a60830, range=0x555555ad9680, diff_out=0x7fffffffd410)
    at line-log.c:1036
#8  0x000055555566a4ec in process_all_files (range_out=0x555555a5ba10, 
    rev=0x7fffffffd5f0, queue=0x555555a078a0, range=0x555555a347a0)
    at line-log.c:1076
#9  0x000055555566a852 in process_ranges_merge_commit (rev=0x7fffffffd5f0, 
    commit=0x555555a19f70, range=0x555555a347a0) at line-log.c:1158
#10 0x000055555566aa37 in process_ranges_arbitrary_commit (rev=0x7fffffffd5f0, 
    commit=0x555555a19f70) at line-log.c:1202
#11 0x000055555566ab4b in line_log_filter (rev=0x7fffffffd5f0)
    at line-log.c:1236
#12 0x00005555556c59f1 in prepare_revision_walk (revs=0x7fffffffd5f0)
    at revision.c:2773
#13 0x00005555555ac8a9 in cmd_log_walk (rev=0x7fffffffd5f0)
    at builtin/log.c:347
#14 0x00005555555ad9e1 in cmd_log (argc=4, argv=0x7fffffffe0d8, prefix=0x0)
    at builtin/log.c:682
#15 0x0000555555566079 in run_builtin (p=0x5555559b5ba0 <commands+1152>, 
    argc=4, argv=0x7fffffffe0d8) at git.c:373
#16 0x00005555555662fa in handle_builtin (argc=4, argv=0x7fffffffe0d8)
    at git.c:572
#17 0x0000555555566534 in cmd_main (argc=4, argv=0x7fffffffe0d8) at git.c:677
#18 0x00005555555fd6bf in main (argc=4, argv=0x7fffffffe0d8)
    at common-main.c:40

^ permalink raw reply

* [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'
From: Chris Packham @ 2016-12-13  8:48 UTC (permalink / raw)
  To: git; +Cc: mah, peff, jacob.keller, gitster, Chris Packham
In-Reply-To: <20161212083413.7334-1-judge.packham@gmail.com>

Teach 'git merge' the --continue option which allows 'continuing' a
merge by completing it. The traditional way of completing a merge after
resolving conflicts is to use 'git commit'. Now with commands like 'git
rebase' and 'git cherry-pick' having a '--continue' option adding such
an option to 'git merge' presents a consistent UI.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

Notes:
    Changes in v2:
    - add --continue to builtin_merge_usage
    - verify that no other arguments are present when --continue is used.
    - add basic test

 Documentation/git-merge.txt | 13 ++++++++++++-
 builtin/merge.c             | 22 +++++++++++++++++++++-
 t/t7600-merge.sh            |  8 ++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d5556..765b0f26e 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
+'git merge' --continue
 
 DESCRIPTION
 -----------
@@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
 discouraged: while possible, it may leave you in a state that is hard to
 back out of in the case of a conflict.
 
+The fourth syntax ("`git merge --continue`") can only be run after the
+merge has resulted in conflicts. 'git merge --continue' will take the
+currently staged changes and complete the merge.
 
 OPTIONS
 -------
@@ -99,6 +103,12 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
+--continue::
+	Take the currently staged changes and complete the merge.
++
+'git merge --continue' is equivalent to 'git commit' when
+`MERGE_HEAD` is present.
+
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
@@ -277,7 +287,8 @@ After seeing a conflict, you can do two things:
 
  * Resolve the conflicts.  Git will mark the conflicts in
    the working tree.  Edit the files into shape and
-   'git add' them to the index.  Use 'git commit' to seal the deal.
+   'git add' them to the index.  Use 'git merge --continue' to seal the
+   deal.
 
 You can work through the conflict with a number of tools:
 
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87..379685223 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
 	N_("git merge [<options>] [<commit>...]"),
 	N_("git merge [<options>] <msg> HEAD <commit>"),
 	N_("git merge --abort"),
+	N_("git merge --continue"),
 	NULL
 };
 
@@ -65,6 +66,7 @@ static int option_renormalize;
 static int verbosity;
 static int allow_rerere_auto;
 static int abort_current_merge;
+static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
@@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
+	OPT_BOOL(0, "continue", &continue_current_merge,
+		N_("continue the current in-progress merge")),
 	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
 		 N_("allow merging unrelated histories")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
@@ -739,7 +743,7 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
 	if (err_msg)
 		error("%s", err_msg);
 	fprintf(stderr,
-		_("Not committing merge; use 'git commit' to complete the merge.\n"));
+		_("Not committing merge; use 'git merge --continue' to complete the merge.\n"));
 	write_merge_state(remoteheads);
 	exit(1);
 }
@@ -1166,6 +1170,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		goto done;
 	}
 
+	if (continue_current_merge) {
+		int nargc = 1;
+		const char *nargv[] = {"commit", NULL};
+
+		if (argc)
+			usage_msg_opt("--continue expects no arguments",
+			      builtin_merge_usage, builtin_merge_options);
+
+		if (!file_exists(git_path_merge_head()))
+			die(_("There is no merge in progress (MERGE_HEAD missing)."));
+
+		/* Invoke 'git commit' */
+		ret = cmd_commit(nargc, nargv, prefix);
+		goto done;
+	}
+
 	if (read_cache_unmerged())
 		die_resolve_conflict("merge");
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 85248a14b..44b34ef3a 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,7 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge -s foobar c1 &&
 	test_must_fail git merge -s=foobar c1 &&
 	test_must_fail git merge -m &&
+	test_must_fail git merge --continue foobar &&
 	test_must_fail git merge
 '
 
@@ -763,4 +764,11 @@ test_expect_success 'merge nothing into void' '
 	)
 '
 
+test_expect_success 'merge can be completed with --continue' '
+	git reset --hard c0 &&
+	git merge --no-ff --no-commit c1 &&
+	git merge --continue &&
+	verify_parents $c0 $c1
+'
+
 test_done
-- 
2.11.0.24.ge6920cf


^ permalink raw reply related

* [PATCHv2 2/2] completion: add --continue option for merge
From: Chris Packham @ 2016-12-13  8:48 UTC (permalink / raw)
  To: git; +Cc: mah, peff, jacob.keller, gitster, Chris Packham
In-Reply-To: <20161213084859.13426-1-judge.packham@gmail.com>

Add 'git merge --continue' option when completing.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

Notes:
    Changes in v2:
    - new.

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..1f97ffae1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1552,7 +1552,7 @@ _git_merge ()
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_merge_options
-			--rerere-autoupdate --no-rerere-autoupdate --abort"
+			--rerere-autoupdate --no-rerere-autoupdate --abort --continue"
 		return
 	esac
 	__gitcomp_nl "$(__git_refs)"
-- 
2.11.0.24.ge6920cf


^ permalink raw reply related

* [RFC/PATCH] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-13  9:22 UTC (permalink / raw)
  To: git; +Cc: gitter.spiros, Chris Packham

Add cppcheck target to Makefile. Cppcheck is a static
analysis tool for C/C++ code. Cppcheck primarily detects
the types of bugs that the compilers normally do not detect.
It is an useful target for doing QA analysis.

Based-on-patch-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
I had been playing with cppcheck for some other projects and happened to
notice [1] in the archives. This is my attempt to resolve the feedback
that Junio made at the time.

In terms of errors that are actually reported there are only a few

$ make cppcheck
cppcheck --force --quiet --inline-suppr  .
[compat/nedmalloc/malloc.c.h:4093]: (error) Possible null pointer dereference: sp
[compat/nedmalloc/malloc.c.h:4106]: (error) Possible null pointer dereference: sp
[compat/nedmalloc/nedmalloc.c:551]: (error) Expression '*(&p.mycache)=TlsAlloc(),TLS_OUT_OF_INDEXES==*(&p.mycache)' depends on order of evaluation of side effects
[compat/regex/regcomp.c:3086]: (error) Memory leak: sbcset
[compat/regex/regcomp.c:3634]: (error) Memory leak: sbcset
[compat/regex/regcomp.c:3086]: (error) Memory leak: mbcset
[compat/regex/regcomp.c:3634]: (error) Memory leak: mbcset
[compat/regex/regcomp.c:2802]: (error) Uninitialized variable: table_size
[compat/regex/regcomp.c:2805]: (error) Uninitialized variable: table_size
[compat/regex/regcomp.c:532]: (error) Memory leak: fastmap
[t/t4051/appended1.c:3]: (error) Invalid number of character '{' when these macros are defined: ''.
[t/t4051/appended2.c:35]: (error) Invalid number of character '{' when these macros are defined: ''.

The last 2 are just false positives from test data. I haven't looked
into any of the others.

I've also provisioned for enabling extra checks by passing CPPCHECK_ADD
in the make invocation.

$ make cppcheck CPPCHECK_ADD=--enable=all
... lots of output
    
[1] - http://public-inbox.org/git/1390993371-2431-1-git-send-email-gitter.spiros@gmail.com/#t

 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index f53fcc90d..8b5976d88 100644
--- a/Makefile
+++ b/Makefile
@@ -2635,3 +2635,7 @@ cover_db: coverage-report
 cover_db_html: cover_db
 	cover -report html -outputdir cover_db_html cover_db
 
+.PHONY: cppcheck
+
+cppcheck:
+	cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) .
-- 
2.11.0.24.ge6920cf


^ permalink raw reply related

* Re: [RFC/PATCH] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-13  9:32 UTC (permalink / raw)
  To: GIT; +Cc: Elia Pinto, Chris Packham
In-Reply-To: <20161213092225.15299-1-judge.packham@gmail.com>

On Tue, Dec 13, 2016 at 10:22 PM, Chris Packham <judge.packham@gmail.com> wrote:
> Add cppcheck target to Makefile. Cppcheck is a static
> analysis tool for C/C++ code. Cppcheck primarily detects
> the types of bugs that the compilers normally do not detect.
> It is an useful target for doing QA analysis.
>
> Based-on-patch-by: Elia Pinto <gitter.spiros@gmail.com>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> I had been playing with cppcheck for some other projects and happened to
> notice [1] in the archives. This is my attempt to resolve the feedback
> that Junio made at the time.
>
> In terms of errors that are actually reported there are only a few
>
> $ make cppcheck
> cppcheck --force --quiet --inline-suppr  .
> [compat/nedmalloc/malloc.c.h:4093]: (error) Possible null pointer dereference: sp
> [compat/nedmalloc/malloc.c.h:4106]: (error) Possible null pointer dereference: sp
> [compat/nedmalloc/nedmalloc.c:551]: (error) Expression '*(&p.mycache)=TlsAlloc(),TLS_OUT_OF_INDEXES==*(&p.mycache)' depends on order of evaluation of side effects
> [compat/regex/regcomp.c:3086]: (error) Memory leak: sbcset
> [compat/regex/regcomp.c:3634]: (error) Memory leak: sbcset
> [compat/regex/regcomp.c:3086]: (error) Memory leak: mbcset
> [compat/regex/regcomp.c:3634]: (error) Memory leak: mbcset
> [compat/regex/regcomp.c:2802]: (error) Uninitialized variable: table_size
> [compat/regex/regcomp.c:2805]: (error) Uninitialized variable: table_size
> [compat/regex/regcomp.c:532]: (error) Memory leak: fastmap
> [t/t4051/appended1.c:3]: (error) Invalid number of character '{' when these macros are defined: ''.
> [t/t4051/appended2.c:35]: (error) Invalid number of character '{' when these macros are defined: ''.
>
> The last 2 are just false positives from test data. I haven't looked
> into any of the others.
>
> I've also provisioned for enabling extra checks by passing CPPCHECK_ADD
> in the make invocation.
>
> $ make cppcheck CPPCHECK_ADD=--enable=all
> ... lots of output
>
> [1] - http://public-inbox.org/git/1390993371-2431-1-git-send-email-gitter.spiros@gmail.com/#t
>
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index f53fcc90d..8b5976d88 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2635,3 +2635,7 @@ cover_db: coverage-report
>  cover_db_html: cover_db
>         cover -report html -outputdir cover_db_html cover_db
>
> +.PHONY: cppcheck
> +
> +cppcheck:
> +       cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) .

If I'm permitted a little GNU make-ism the following might make
CPPCHECK_ADD a bit more usable

+       cppcheck --force --quiet --inline-suppr $(if
$(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD)) .

Which would take us from

$ make cppcheck CPPCHECK_ADD=--enable=all

to

$ make cppcheck CPPCHECK_ADD=all

^ permalink raw reply

* Re: [RFC/PATCH] Makefile: add cppcheck target
From: stefan.naewe @ 2016-12-13  9:37 UTC (permalink / raw)
  To: judge.packham, git; +Cc: gitter.spiros
In-Reply-To: <CAFOYHZAZAH9Rt1o73cx2uFvtr4weL00J+Yktei3h2GN1JgbY=A@mail.gmail.com>

Am 13.12.2016 um 10:32 schrieb Chris Packham:
> On Tue, Dec 13, 2016 at 10:22 PM, Chris Packham <judge.packham@gmail.com> wrote:
>> Add cppcheck target to Makefile. Cppcheck is a static
>> analysis tool for C/C++ code. Cppcheck primarily detects
>> the types of bugs that the compilers normally do not detect.
>> It is an useful target for doing QA analysis.
>>
>> Based-on-patch-by: Elia Pinto <gitter.spiros@gmail.com>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> ---
>> I had been playing with cppcheck for some other projects and happened to
>> notice [1] in the archives. This is my attempt to resolve the feedback
>> that Junio made at the time.
>>
>> In terms of errors that are actually reported there are only a few
>>
>> $ make cppcheck
>> cppcheck --force --quiet --inline-suppr  .
>> [compat/nedmalloc/malloc.c.h:4093]: (error) Possible null pointer dereference: sp
>> [compat/nedmalloc/malloc.c.h:4106]: (error) Possible null pointer dereference: sp
>> [compat/nedmalloc/nedmalloc.c:551]: (error) Expression '*(&p.mycache)=TlsAlloc(),TLS_OUT_OF_INDEXES==*(&p.mycache)' depends on order of evaluation of side effects
>> [compat/regex/regcomp.c:3086]: (error) Memory leak: sbcset
>> [compat/regex/regcomp.c:3634]: (error) Memory leak: sbcset
>> [compat/regex/regcomp.c:3086]: (error) Memory leak: mbcset
>> [compat/regex/regcomp.c:3634]: (error) Memory leak: mbcset
>> [compat/regex/regcomp.c:2802]: (error) Uninitialized variable: table_size
>> [compat/regex/regcomp.c:2805]: (error) Uninitialized variable: table_size
>> [compat/regex/regcomp.c:532]: (error) Memory leak: fastmap
>> [t/t4051/appended1.c:3]: (error) Invalid number of character '{' when these macros are defined: ''.
>> [t/t4051/appended2.c:35]: (error) Invalid number of character '{' when these macros are defined: ''.
>>
>> The last 2 are just false positives from test data. I haven't looked
>> into any of the others.
>>
>> I've also provisioned for enabling extra checks by passing CPPCHECK_ADD
>> in the make invocation.
>>
>> $ make cppcheck CPPCHECK_ADD=--enable=all
>> ... lots of output
>>
>> [1] - http://public-inbox.org/git/1390993371-2431-1-git-send-email-gitter.spiros@gmail.com/#t
>>
>>  Makefile | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index f53fcc90d..8b5976d88 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2635,3 +2635,7 @@ cover_db: coverage-report
>>  cover_db_html: cover_db
>>         cover -report html -outputdir cover_db_html cover_db
>>
>> +.PHONY: cppcheck
>> +
>> +cppcheck:
>> +       cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) .
> 
> If I'm permitted a little GNU make-ism the following might make
> CPPCHECK_ADD a bit more usable
> 
> +       cppcheck --force --quiet --inline-suppr $(if
> $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD)) .
> 
> Which would take us from
> 
> $ make cppcheck CPPCHECK_ADD=--enable=all
> 
> to
> 
> $ make cppcheck CPPCHECK_ADD=all

Hhhmmm....but this allows for only specifying options to '--enable'.
The other version is much more flexible (i.e. allows for other complete options as well).

Just my 0,02€

Stefan
-- 
----------------------------------------------------------------
/dev/random says: I'd love to, but I prefer to remain an enigma.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

^ permalink raw reply

* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Vasco Almeida @ 2016-12-13 11:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, git, Jiang Xin,
	Ævar Arnfjörð Bjarmason, Jean-Noël AVILA,
	Jakub Narębski, David Aguilar
In-Reply-To: <xmqqy3zno2qv.fsf@gitster.mtv.corp.google.com>

A Sáb, 10-12-2016 às 14:09 -0800, Junio C Hamano escreveu:
> We only update comment_line_char from the default "#" when the
> configured value is a single-byte character and we ignore incorrect
> values in the configuration file.  So I think the patch you sent is
> correct after all.

I am still not sure what version do we prefer.

Check whether core.commentchar is a single character. If not, use '#'
as the $comment_line_char.

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       $comment_line_char = '#' if (length($comment_line_char) != 1);
+       return $comment_line_char;
+}

Check whether core.commentchar is a single character. If not, use the
first character of the core.commentchar value, mirroring the "rebase
-i" behavior introduced recently.

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       if (length($comment_line_char) != 1) {
+               # use first character
+               $comment_line_char = substr($comment_line_char, 0, 1);
+       }
+       return $comment_line_char;
+}

Or akin to what I had in the first patch related to handling 'auto'
value of core.commentchar configuration variable:

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       return $comment_line_char;
+}

Which assumes that the value of core.commentchar configuration variable
is either 'auto' or one single character, or the variable is not
defined.

^ permalink raw reply

* Re: [PATCHv2] git-p4: support git worktrees
From: Duy Nguyen @ 2016-12-13 11:17 UTC (permalink / raw)
  To: Luke Diamand
  Cc: Git Users, Vinicius Kursancew, Lars Schneider, Junio C Hamano
In-Reply-To: <CAE5ih7_6Ap_dY3mRb3Hk2yzDRMkZ3HnnQOaikF=ybx_XNdVWhQ@mail.gmail.com>

On Sun, Dec 11, 2016 at 2:19 PM, Luke Diamand <luke@diamand.org> wrote:
> On 10 December 2016 at 21:57, Luke Diamand <luke@diamand.org> wrote:
>> git-p4 would attempt to find the git directory using
>> its own specific code, which did not know about git
>> worktrees. This caused git operations to fail needlessly.
>>
>> Rework it to use "git rev-parse --git-dir" instead, which
>> knows about worktrees.
>
> Actually this doesn't work as well as the original version. "git
> rev-parse --git-dir" won't go and find the ".git" subdirectory. The
> previous version would go looking for it, so this would introduce a
> regression.

Maybe git rev-parse --git-path HEAD, then strip out the /HEAD part?
-- 
Duy

^ permalink raw reply

* Re: [PATCH 1/2] alternates: accept double-quoted paths
From: Duy Nguyen @ 2016-12-13 11:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, Klaus Ethgen, Git Mailing List
In-Reply-To: <20161212195222.rxnabok6amklt2zf@sigill.intra.peff.net>

On Tue, Dec 13, 2016 at 2:52 AM, Jeff King <peff@peff.net> wrote:
> Instead, let's treat names as unquoted unless they begin
> with a double-quote, in which case they are interpreted via
> our usual C-stylke quoting rules. This also breaks
> backwards-compatibility, but in a smaller way: it only
> matters if your file has a double-quote as the very _first_
> character in the path (whereas an escape character is a
> problem anywhere in the path).  It's also consistent with
> many other parts of git, which accept either a bare pathname
> or a double-quoted one, and the sender can choose to quote
> or not as required.

At least attr has the same problem and is going the same direction
[1]. Cool. (I actually thought the patch was in and evidence that this
kind of backward compatibility breaking was ok, turns out the patch
has stayed around for years)

[1] http://public-inbox.org/git/%3C20161110203428.30512-18-sbeller@google.com%3E/
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Jeff King @ 2016-12-13 11:44 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <d83cd58f-9b52-cbc5-04dd-5aafe2822533@kdbg.org>

On Mon, Dec 12, 2016 at 09:53:11PM +0100, Johannes Sixt wrote:

> > The appropriate check there would be ";" anyway, but I am not
> > sure _that_ is allowed in paths, either.
> 
> That was also my line of thought. I tested earlier today whether a file name
> can have ";", and the OS did not reject it bluntly. Let me see whether I can
> adjust the test case for Windows.

The naive conversion is just:

diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 6275ec807..e195e168b 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -33,8 +33,13 @@ test_expect_success 'rejected objects are removed' '
 	test_cmp expect actual
 '
 
-# MINGW does not allow colons in pathnames in the first place
-test_expect_success !MINGW 'push to repo path with colon' '
+if test_have_prereq MINGW
+then
+	path_sep=';'
+else
+	path_sep=':'
+fi
+test_expect_success 'push to repo path with (semi-)colon separator' '
 	# The interesting failure case here is when the
 	# receiving end cannot access its original object directory,
 	# so make it likely for us to generate a delta by having
@@ -43,13 +48,13 @@ test_expect_success !MINGW 'push to repo path with colon' '
 	test-genrandom foo 4096 >file.bin &&
 	git add file.bin &&
 	git commit -m bin &&
-	git clone --bare . xxx:yyy.git &&
+	git clone --bare . xxx${path_sep}yyy.git &&
 
 	echo change >>file.bin &&
 	git commit -am change &&
 	# Note that we have to use the full path here, or it gets confused
 	# with the ssh host:path syntax.
-	git push "$PWD/xxx:yyy.git" HEAD
+	git push "$PWD/xxx${path_sep}yyy.git" HEAD
 '
 
 test_done

Does that work?

-Peff

^ permalink raw reply related

* Re: [PATCH 0/2] handling alternates paths with colons
From: Jeff King @ 2016-12-13 11:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <xmqqvauo7p0r.fsf@gitster.mtv.corp.google.com>

On Mon, Dec 12, 2016 at 02:37:08PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So here are patches that do that. It kicks in only when the first
> > character of a path is a double-quote, and then expects the usual
> > C-style quoting.
> 
> The quote being per delimited component is what makes this fifth
> approach a huge win.  
> 
> All sane components on a list-valued environment are still honored
> and an insane component that has either a colon in the middle or
> begins with a double-quote gets quoted.  As long as nobody used a
> path that begins with a double-quote as an element in such a
> list-valued environment (and they cannot be, as using a non-absolute
> path as an element does not make much sense), this will be safe, and
> a path with a colon didn't work with Git unaware of the new quoting
> rule anyway.  Nice.

We do support non-absolute paths, both in alternates files and
environment variables. It's a nice feature if you want to have a
relocatable family of shared repositories. I'd imagine that most cases
start with "../", though.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] alternates: accept double-quoted paths
From: Jeff King @ 2016-12-13 11:52 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Junio C Hamano, Johannes Sixt, Klaus Ethgen, Git Mailing List
In-Reply-To: <CACsJy8B52ZDRTUjGLqub_1wELtugv99xbDnBg1PX1LUTb6nVMQ@mail.gmail.com>

On Tue, Dec 13, 2016 at 06:30:15PM +0700, Duy Nguyen wrote:

> On Tue, Dec 13, 2016 at 2:52 AM, Jeff King <peff@peff.net> wrote:
> > Instead, let's treat names as unquoted unless they begin
> > with a double-quote, in which case they are interpreted via
> > our usual C-stylke quoting rules. This also breaks
> > backwards-compatibility, but in a smaller way: it only
> > matters if your file has a double-quote as the very _first_
> > character in the path (whereas an escape character is a
> > problem anywhere in the path).  It's also consistent with
> > many other parts of git, which accept either a bare pathname
> > or a double-quoted one, and the sender can choose to quote
> > or not as required.
> 
> At least attr has the same problem and is going the same direction
> [1]. Cool. (I actually thought the patch was in and evidence that this
> kind of backward compatibility breaking was ok, turns out the patch
> has stayed around for years)
> 
> [1] http://public-inbox.org/git/%3C20161110203428.30512-18-sbeller@google.com%3E/

Thanks for digging that up. As soon as I came up with the idea[1], I
wanted to use the attr code as an example of a similar problem and
solution, but I couldn't find it in the code. Which makes sense if it
wasn't merged.

I do think it's a pretty reasonable approach in general, and would be OK
for the attributes code.

-Peff

[1] One could argue that I did not come up with the idea at all, but
    rather just remembered that somebody else had done so. :)

^ permalink raw reply

* Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'
From: Jeff King @ 2016-12-13 11:59 UTC (permalink / raw)
  To: Chris Packham; +Cc: git, mah, jacob.keller, gitster
In-Reply-To: <20161213084859.13426-1-judge.packham@gmail.com>

On Tue, Dec 13, 2016 at 09:48:58PM +1300, Chris Packham wrote:

> +	if (continue_current_merge) {
> +		int nargc = 1;
> +		const char *nargv[] = {"commit", NULL};
> +
> +		if (argc)
> +			usage_msg_opt("--continue expects no arguments",
> +			      builtin_merge_usage, builtin_merge_options);

This checks that we don't have:

  git merge --continue foobar

but still allows:

  git merge --continue --some-option

because parse_options() decrements argc.

It would be insane to check individually which options might have been
set. But I wonder if we could do something like:

  int orig_argc = argc;
  ...
  argc = parse_options(argc, argv, ...);

  if (continue_current_merge) {
	if (orig_argc != 1) /* maybe 2, to account for argv[0] ? */
		usage_msg_opt("--continue expects no arguments", ...);
  }

That gets trickier if there ever is an option that's OK to use with
--continue. We might want to forward along "--quiet", for example. On
the other hand, we silently ignore it now, so maybe it is better to
complain and then let --quiet get added later if somebody cares.

Whatever we do here, I think "--abort" should get the same treatment
(probably as a separate patch).

> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 85248a14b..44b34ef3a 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -154,6 +154,7 @@ test_expect_success 'test option parsing' '
>  	test_must_fail git merge -s foobar c1 &&
>  	test_must_fail git merge -s=foobar c1 &&
>  	test_must_fail git merge -m &&
> +	test_must_fail git merge --continue foobar &&
>  	test_must_fail git merge
>  '

Your tests look good, though obviously if you check for options above,
that should be covered in this test.

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox