From: Jacob Keller <jacob.kel...@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

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

Reply via email to