From: Jacob Keller <jacob.e.keller@intel.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Stefan Beller <stefanbeller@gmail.com>, Jeff King <peff@peff.net>,
Johannes Sixt <j6t@kdbg.org>,
Jacob Keller <jacob.keller@gmail.com>
Subject: [PATCH v9 0/8] submodule show inline diff
Date: Fri, 19 Aug 2016 16:34:24 -0700 [thread overview]
Message-ID: <20160819233432.15188-1-jacob.e.keller@intel.com> (raw)
From: Jacob Keller <jacob.keller@gmail.com>
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
next reply other threads:[~2016-08-19 23:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-19 23:34 Jacob Keller [this message]
2016-08-19 23:34 ` [PATCH v9 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
2016-08-19 23:34 ` [PATCH v9 2/8] diff.c: remove output_prefix_length field Jacob Keller
2016-08-19 23:34 ` [PATCH v9 3/8] graph: add support for --line-prefix on all graph-aware output Jacob Keller
2016-08-19 23:34 ` [PATCH v9 4/8] diff: prepare for additional submodule formats Jacob Keller
2016-08-19 23:34 ` [PATCH v9 5/8] submodule: allow add_submodule_odb to work even if path is not checked out Jacob Keller
2016-08-19 23:34 ` [PATCH v9 6/8] submodule: convert show_submodule_summary to use struct object_id * Jacob Keller
2016-08-19 23:34 ` [PATCH v9 7/8] submodule: refactor show_submodule_summary with helper function Jacob Keller
2016-08-19 23:34 ` [PATCH v9 8/8] diff: teach diff to display submodule difference with an inline diff Jacob Keller
2016-08-20 0:02 ` [PATCH v9 0/8] submodule show " Stefan Beller
2016-08-20 6:16 ` Jacob Keller
2016-08-22 17:43 ` Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160819233432.15188-1-jacob.e.keller@intel.com \
--to=jacob.e.keller@intel.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jacob.keller@gmail.com \
--cc=peff@peff.net \
--cc=stefanbeller@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).