[PATCH v9 0/8] submodule show inline diff

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Jacob Keller <jacob.keller@xxxxxxxxx>

More suggestions from Junio and a few changes to support submodule name
lookup. Hopefully we're getting close to the goal!

interdiff between v8 and current:
diff --git c/builtin/rev-list.c w/builtin/rev-list.c
index 21cde8dd6b31..8479f6ed28aa 100644
--- c/builtin/rev-list.c
+++ w/builtin/rev-list.c
@@ -129,29 +129,29 @@ static void show_commit(struct commit *commit, void *data)
 			graph_show_commit_msg(revs->graph, stdout, &buf);
 
 			/*
-			* Add a newline after the commit message.
-			*
-			* Usually, this newline produces a blank
-			* padding line between entries, in which case
-			* we need to add graph padding on this line.
-			*
-			* However, the commit message may not end in a
-			* newline.  In this case the newline simply
-			* ends the last line of the commit message,
-			* and we don't need any graph output.  (This
-			* always happens with CMIT_FMT_ONELINE, and it
-			* happens with CMIT_FMT_USERFORMAT when the
-			* format doesn't explicitly end in a newline.)
-			*/
+			 * Add a newline after the commit message.
+			 *
+			 * Usually, this newline produces a blank
+			 * padding line between entries, in which case
+			 * we need to add graph padding on this line.
+			 *
+			 * However, the commit message may not end in a
+			 * newline.  In this case the newline simply
+			 * ends the last line of the commit message,
+			 * and we don't need any graph output.  (This
+			 * always happens with CMIT_FMT_ONELINE, and it
+			 * happens with CMIT_FMT_USERFORMAT when the
+			 * format doesn't explicitly end in a newline.)
+			 */
 			if (buf.len && buf.buf[buf.len - 1] == '\n')
 				graph_show_padding(revs->graph);
 			putchar('\n');
 		} else {
 			/*
-				* If the message buffer is empty, just show
-				* the rest of the graph output for this
-				* commit.
-				*/
+			 * If the message buffer is empty, just show
+			 * the rest of the graph output for this
+			 * commit.
+			 */
 			if (graph_show_remainder(revs->graph))
 				putchar('\n');
 			if (revs->commit_format == CMIT_FMT_ONELINE)
diff --git c/cache.h w/cache.h
index da9f0be67d7b..70428e92d7ed 100644
--- c/cache.h
+++ w/cache.h
@@ -953,24 +953,39 @@ static inline void oidclr(struct object_id *oid)
 #define EMPTY_TREE_SHA1_BIN_LITERAL \
 	 "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
 	 "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
-#define EMPTY_TREE_SHA1_BIN \
-	 ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
+extern const struct object_id empty_tree_oid;
+#define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
 
 #define EMPTY_BLOB_SHA1_HEX \
 	"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
 #define EMPTY_BLOB_SHA1_BIN_LITERAL \
 	"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
 	"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
-#define EMPTY_BLOB_SHA1_BIN \
-	((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
+extern const struct object_id empty_blob_oid;
+#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
 
-extern const struct object_id empty_tree_oid;
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
 	return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
 }
 
+static inline int is_empty_blob_oid(const struct object_id *oid)
+{
+	return !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);
+}
+
+static inline int is_empty_tree_sha1(const unsigned char *sha1)
+{
+	return !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);
+}
+
+static inline int is_empty_tree_oid(const struct object_id *oid)
+{
+	return !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);
+}
+
+
 int git_mkstemp(char *path, size_t n, const char *template);
 
 /* set default permissions by passing mode arguments to open(2) */
diff --git c/diff.h w/diff.h
index 192c0eedd0ff..ec76a90522ea 100644
--- c/diff.h
+++ w/diff.h
@@ -112,7 +112,7 @@ enum diff_words_type {
 enum diff_submodule_format {
 	DIFF_SUBMODULE_SHORT = 0,
 	DIFF_SUBMODULE_LOG,
-	DIFF_SUBMODULE_INLINE_DIFF,
+	DIFF_SUBMODULE_INLINE_DIFF
 };
 
 struct diff_options {
diff --git c/path.c w/path.c
index 188abfebbafe..081a22c1163c 100644
--- c/path.c
+++ w/path.c
@@ -6,6 +6,7 @@
 #include "string-list.h"
 #include "dir.h"
 #include "worktree.h"
+#include "submodule-config.h"
 
 static int get_st_mode_bits(const char *path, int *mode)
 {
@@ -474,6 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
 	const char *git_dir;
 	struct strbuf git_submodule_common_dir = STRBUF_INIT;
 	struct strbuf git_submodule_dir = STRBUF_INIT;
+	const struct submodule *submodule_config;
 
 	strbuf_addstr(buf, path);
 	strbuf_complete(buf, '/');
@@ -486,7 +488,16 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
 	}
 	if (!is_git_directory(buf->buf)) {
 		strbuf_reset(buf);
-		strbuf_git_path(buf, "%s/%s", "modules", path);
+		/*
+		 * Lookup the submodule name from the config. If that fails
+		 * fall back to assuming the path is the name.
+		 */
+		submodule_config = submodule_from_path(null_sha1, path);
+		if (submodule_config)
+			strbuf_git_path(buf, "%s/%s", "modules",
+					submodule_config->name);
+		else
+			strbuf_git_path(buf, "%s/%s", "modules", path);
 	}
 
 	strbuf_addch(buf, '/');
diff --git c/sha1_file.c w/sha1_file.c
index 10883d56a600..21cf923bcf1f 100644
--- c/sha1_file.c
+++ w/sha1_file.c
@@ -39,7 +39,10 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
 const unsigned char null_sha1[20];
 const struct object_id null_oid;
 const struct object_id empty_tree_oid = {
-	.hash = EMPTY_TREE_SHA1_BIN_LITERAL
+	EMPTY_TREE_SHA1_BIN_LITERAL
+};
+const struct object_id empty_blob_oid = {
+	EMPTY_BLOB_SHA1_BIN_LITERAL
 };
 
 /*
diff --git c/submodule.c w/submodule.c
index cecd3cd98de4..405fa9e4eb32 100644
--- c/submodule.c
+++ w/submodule.c
@@ -277,9 +277,10 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 }
 
 static int prepare_submodule_summary(struct rev_info *rev, const char *path,
-		struct commit *left, struct commit *right)
+		struct commit *left, struct commit *right,
+		struct commit_list *merge_bases)
 {
-	struct commit_list *merge_bases, *list;
+	struct commit_list *list;
 
 	init_revisions(rev, NULL);
 	setup_revisions(0, NULL, rev, NULL);
@@ -288,7 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
 	left->object.flags |= SYMMETRIC_LEFT;
 	add_pending_object(rev, &left->object, path);
 	add_pending_object(rev, &right->object, path);
-	merge_bases = get_merge_bases(left, right);
 	for (list = merge_bases; list; list = list->next) {
 		list->item->object.flags |= UNINTERESTING;
 		add_pending_object(rev, &list->item->object,
@@ -336,9 +336,9 @@ static void show_submodule_header(FILE *f, const char *path,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
 		const char *reset,
-		struct commit **left, struct commit **right)
+		struct commit **left, struct commit **right,
+		struct commit_list **merge_bases)
 {
-	struct commit_list *merge_bases;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
 	int fast_forward = 0, fast_backward = 0;
@@ -376,11 +376,11 @@ static void show_submodule_header(FILE *f, const char *path,
 	     (!is_null_oid(two) && !*right))
 		message = "(commits not present)";
 
-	merge_bases = get_merge_bases(*left, *right);
-	if (merge_bases) {
-		if (merge_bases->item == *left)
+	*merge_bases = get_merge_bases(*left, *right);
+	if (*merge_bases) {
+		if ((*merge_bases)->item == *left)
 			fast_forward = 1;
-		else if (merge_bases->item == *right)
+		else if ((*merge_bases)->item == *right)
 			fast_backward = 1;
 	}
 
@@ -412,9 +412,10 @@ void show_submodule_summary(FILE *f, const char *path,
 {
 	struct rev_info rev;
 	struct commit *left = NULL, *right = NULL;
+	struct commit_list *merge_bases = NULL;
 
 	show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
-			      meta, reset, &left, &right);
+			      meta, reset, &left, &right, &merge_bases);
 
 	/*
 	 * If we don't have both a left and a right pointer, there is no
@@ -422,15 +423,19 @@ void show_submodule_summary(FILE *f, const char *path,
 	 * all the information the user needs.
 	 */
 	if (!left || !right)
-		return;
+		goto out;
 
 	/* Treat revision walker failure the same as missing commits */
-	if (prepare_submodule_summary(&rev, path, left, right)) {
+	if (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {
 		fprintf(f, "%s(revision walker failed)\n", line_prefix);
-		return;
+		goto out;
 	}
 
 	print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+
+out:
+	if (merge_bases)
+		free_commit_list(merge_bases);
 	clear_commit_marks(left, ~0);
 	clear_commit_marks(right, ~0);
 }
@@ -444,11 +449,12 @@ void show_submodule_inline_diff(FILE *f, const char *path,
 {
 	const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
 	struct commit *left = NULL, *right = NULL;
+	struct commit_list *merge_bases = NULL;
 	struct strbuf submodule_dir = STRBUF_INIT;
 	struct child_process cp = CHILD_PROCESS_INIT;
 
 	show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
-			      meta, reset, &left, &right);
+			      meta, reset, &left, &right, &merge_bases);
 
 	/* We need a valid left and right commit to display a difference */
 	if (!(left || is_null_oid(one)) ||
@@ -467,7 +473,7 @@ void show_submodule_inline_diff(FILE *f, const char *path,
 	cp.no_stdin = 1;
 
 	/* TODO: other options may need to be passed here. */
-	argv_array_pushl(&cp.args, "diff");
+	argv_array_push(&cp.args, "diff");
 	argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
 	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
@@ -495,6 +501,12 @@ void show_submodule_inline_diff(FILE *f, const char *path,
 
 done:
 	strbuf_release(&submodule_dir);
+	if (merge_bases)
+		free_commit_list(merge_bases);
+	if (left)
+		clear_commit_marks(left, ~0);
+	if (right)
+		clear_commit_marks(right, ~0);
 }
 
 void set_config_fetch_recurse_submodules(int value)
diff --git c/t/t4059-diff-submodule-not-initialized.sh w/t/t4059-diff-submodule-not-initialized.sh
index cc787454033a..c8775854d3c2 100755
--- c/t/t4059-diff-submodule-not-initialized.sh
+++ w/t/t4059-diff-submodule-not-initialized.sh
@@ -9,7 +9,6 @@ This test tries to verify that add_submodule_odb works when the submodule was
 initialized previously but the checkout has since been removed.
 '
 
-TEST_NO_CREATE_REPO=1
 . ./test-lib.sh
 
 # Tested non-UTF-8 encoding
@@ -18,6 +17,7 @@ test_encoding="ISO8859-1"
 # String "added" in German (translated with Google Translate), encoded in UTF-8,
 # used in sample commit log messages in add_file() function below.
 added=$(printf "hinzugef\303\274gt")
+
 add_file () {
 	(
 		cd "$1" &&
@@ -35,44 +35,45 @@ add_file () {
 		git rev-parse --short --verify HEAD
 	)
 }
+
 commit_file () {
 	test_tick &&
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-test_create_repo sm2 &&
-add_file sm1 foo >/dev/null &&
-add_file sm2 foo1 foo2 >/dev/null &&
-smhead1=$(cd sm2; git rev-parse --short --verify HEAD) &&
-cd sm1
+test_expect_success 'setup - submodules' '
+	test_create_repo sm2 &&
+	add_file . foo &&
+	add_file sm2 foo1 foo2 &&
+	smhead1=$(git -C sm2 rev-parse --short --verify HEAD)
+'
 
-test_expect_success 'setup - submodule directory' '
-	git submodule add ../sm2 sm2 &&
-	commit_file sm2 &&
+test_expect_success 'setup - git submodule add' '
+	git submodule add ./sm2 sm1 &&
+	commit_file sm1 &&
 	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
-	Submodule sm2 0000000...$smhead1 (new submodule)
+	Submodule sm1 0000000...$smhead1 (new submodule)
 	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'submodule directory removed' '
-	rm -rf sm2 &&
+	rm -rf sm1 &&
 	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
-	Submodule sm2 0000000...$smhead1 (new submodule)
+	Submodule sm1 0000000...$smhead1 (new submodule)
 	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'setup - submodule multiple commits' '
-	git submodule update --checkout sm2 &&
-	smhead2=$(add_file sm2 foo3 foo4) &&
-	commit_file sm2 &&
+	git submodule update --checkout sm1 &&
+	smhead2=$(add_file sm1 foo3 foo4) &&
+	commit_file sm1 &&
 	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
-	Submodule sm2 $smhead1..$smhead2:
+	Submodule sm1 $smhead1..$smhead2:
 	  > Add foo4 ($added foo4)
 	  > Add foo3 ($added foo3)
 	EOF
@@ -80,26 +81,47 @@ test_expect_success 'setup - submodule multiple commits' '
 '
 
 test_expect_success 'submodule removed multiple commits' '
-	rm -rf sm2 &&
+	rm -rf sm1 &&
 	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
-	Submodule sm2 $smhead1..$smhead2:
+	Submodule sm1 $smhead1..$smhead2:
 	  > Add foo4 ($added foo4)
 	  > Add foo3 ($added foo3)
 	EOF
 	test_cmp expected actual
 '
 
-cd ..
-
 test_expect_success 'submodule not initialized in new clone' '
-	git clone sm1 sm3 &&
+	git clone . sm3 &&
 	git -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
-	Submodule sm2 $smhead1...$smhead2 (not initialized)
+	Submodule sm1 $smhead1...$smhead2 (not initialized)
 	EOF
 	test_cmp expected actual
+'
 
+test_expect_success 'setup submodule moved' '
+	git submodule update --checkout sm1 &&
+	git mv sm1 sm4 &&
+	commit_file sm4 &&
+	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+	cat >expected <<-EOF &&
+	Submodule sm4 0000000...$smhead2 (new submodule)
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'submodule moved then removed' '
+	smhead3=$(add_file sm4 foo6 foo7) &&
+	commit_file sm4 &&
+	rm -rf sm4 &&
+	git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+	cat >expected <<-EOF &&
+	Submodule sm4 $smhead2..$smhead3:
+	  > Add foo7 ($added foo7)
+	  > Add foo6 ($added foo6)
+	EOF
+	test_cmp expected actual
 '
 
 test_done
diff --git c/t/t4060-diff-submodule-option-diff-format.sh w/t/t4060-diff-submodule-option-diff-format.sh
index 836caef5c8ba..7e23b55ea4c5 100755
--- c/t/t4060-diff-submodule-option-diff-format.sh
+++ w/t/t4060-diff-submodule-option-diff-format.sh
@@ -18,6 +18,7 @@ test_encoding="ISO8859-1"
 # String "added" in German (translated with Google Translate), encoded in UTF-8,
 # used in sample commit log messages in add_file() function below.
 added=$(printf "hinzugef\303\274gt")
+
 add_file () {
 	(
 		cd "$1" &&
@@ -35,16 +36,18 @@ add_file () {
 		git rev-parse --short --verify HEAD
 	)
 }
+
 commit_file () {
 	test_tick &&
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
-fullhead1=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup repository' '
+	test_create_repo sm1 &&
+	add_file . foo &&
+	head1=$(add_file sm1 foo1 foo2) &&
+	fullhead1=$(git -C sm1 rev-parse --verify HEAD)
+'
 
 test_expect_success 'added submodule' '
 	git add sm1 &&

-------->8


Jacob Keller (7):
  cache: add empty_tree_oid object and helper function
  graph: add support for --line-prefix on all graph-aware output
  diff: prepare for additional submodule formats
  submodule: allow add_submodule_odb to work even if path is not checked
    out
  submodule: convert show_submodule_summary to use struct object_id *
  submodule: refactor show_submodule_summary with helper function
  diff: teach diff to display submodule difference with an inline diff

Junio C Hamano (1):
  diff.c: remove output_prefix_length field

 Documentation/diff-config.txt                      |   9 +-
 Documentation/diff-options.txt                     |  20 +-
 builtin/rev-list.c                                 |  70 +-
 cache.h                                            |  25 +-
 diff.c                                             |  64 +-
 diff.h                                             |  11 +-
 graph.c                                            | 100 ++-
 graph.h                                            |  22 +-
 log-tree.c                                         |   5 +-
 path.c                                             |  16 +
 sha1_file.c                                        |   6 +
 submodule.c                                        | 186 ++++-
 submodule.h                                        |   8 +-
 t/t4013-diff-various.sh                            |   6 +
 ...diff.diff_--line-prefix=abc_master_master^_side |  29 +
 t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
 t/t4059-diff-submodule-not-initialized.sh          | 127 ++++
 t/t4060-diff-submodule-option-diff-format.sh       | 749 +++++++++++++++++++++
 t/t4202-log.sh                                     | 323 +++++++++
 19 files changed, 1637 insertions(+), 154 deletions(-)
 create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
 create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
 create mode 100755 t/t4059-diff-submodule-not-initialized.sh
 create mode 100755 t/t4060-diff-submodule-option-diff-format.sh

-- 
2.10.0.rc0.259.g83512d9

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]