allow do_submodule_path to work even if submodule isn't checked out
Currently, do_submodule_path will attempt locating the .git directory by using read_gitfile on <path>/.git. If this fails it just assumes the <path>/.git is actually a git directory. This is good because it allows for handling submodules which were cloned in a regular manner first before being added to the superproject. Unfortunately this fails if the <path> is not actually checked out any longer, such as by removing the directory. Fix this by checking if the directory we found is actually a gitdir. In the case it is not, attempt to lookup the submodule configuration and find the name of where it is stored in the .git/modules/ directory of the superproject. If we can't locate the submodule configuration, this might occur because for example a submodule gitlink was added but the corresponding .gitmodules file was not properly updated. A die() here would not be pleasant to the users of submodule diff formats, so instead, modify do_submodule_path() to return an error code: - git_pathdup_submodule() returns NULL when we fail to find a path. - strbuf_git_path_submodule() propagates the error code to the caller. Modify the callers of these functions to check the error code and fail properly. This ensures we don't attempt to use a bad path that doesn't match the corresponding submodule. Because this change fixes add_submodule_odb() to work even if the submodule is not checked out, update the wording of the submodule log diff format to correctly display that the submodule is "not initialized" instead of "not checked out" Add tests to ensure this change works as expected. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller committed
Aug 31, 2016 at 16:27 UTC
99b43a61f2daf5fe183056e5bf2a75a8a81a28bf
5 files changed
+173
-11
cache.h
+2
-2
@@ -819,8 +819,8 @@ extern void strbuf_git_common_path(struct strbuf *sb, const char *fmt, ...)
819
__attribute__((format (printf, 2, 3)));
820
extern char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
821
__attribute__((format (printf, 2, 3)));
822
-extern void strbuf_git_path_submodule(struct strbuf *sb, const char *path,
823
- const char *fmt, ...)
822
+extern int strbuf_git_path_submodule(struct strbuf *sb, const char *path,
823
+ const char *fmt, ...)
824
__attribute__((format (printf, 3, 4)));
825
extern char *git_pathdup(const char *fmt, ...)
826
__attribute__((format (printf, 1, 2)));
path.c
+33
-6
@@ -6,6 +6,7 @@
6
#include "string-list.h"
7
#include "dir.h"
8
#include "worktree.h"
9
+#include "submodule-config.h"
10
11
static int get_st_mode_bits(const char *path, int *mode)
12
{
@@ -466,12 +467,16 @@ const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
467
return pathname->buf;
468
}
469
469
-static void do_submodule_path(struct strbuf *buf, const char *path,
470
- const char *fmt, va_list args)
470
+/* Returns 0 on success, negative on failure. */
471
+#define SUBMODULE_PATH_ERR_NOT_CONFIGURED -1
472
+static int do_submodule_path(struct strbuf *buf, const char *path,
473
+ const char *fmt, va_list args)
474
{
475
const char *git_dir;
476
struct strbuf git_submodule_common_dir = STRBUF_INIT;
477
struct strbuf git_submodule_dir = STRBUF_INIT;
478
+ const struct submodule *sub;
479
+ int err = 0;
480
481
strbuf_addstr(buf, path);
482
strbuf_complete(buf, '/');
@@ -482,6 +487,17 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
487
strbuf_reset(buf);
488
strbuf_addstr(buf, git_dir);
489
}
490
+ if (!is_git_directory(buf->buf)) {
491
+ gitmodules_config();
492
+ sub = submodule_from_path(null_sha1, path);
493
+ if (!sub) {
494
+ err = SUBMODULE_PATH_ERR_NOT_CONFIGURED;
495
+ goto cleanup;
496
+ }
497
+ strbuf_reset(buf);
498
+ strbuf_git_path(buf, "%s/%s", "modules", sub->name);
499
+ }
500
+
501
strbuf_addch(buf, '/');
502
strbuf_addbuf(&git_submodule_dir, buf);
503
@@ -492,27 +508,38 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
508
509
strbuf_cleanup_path(buf);
510
511
+cleanup:
512
strbuf_release(&git_submodule_dir);
513
strbuf_release(&git_submodule_common_dir);
514
+
515
+ return err;
516
}
517
518
char *git_pathdup_submodule(const char *path, const char *fmt, ...)
519
{
520
+ int err;
521
va_list args;
522
struct strbuf buf = STRBUF_INIT;
523
va_start(args, fmt);
504
- do_submodule_path(&buf, path, fmt, args);
524
+ err = do_submodule_path(&buf, path, fmt, args);
525
va_end(args);
526
+ if (err) {
527
+ strbuf_release(&buf);
528
+ return NULL;
529
+ }
530
return strbuf_detach(&buf, NULL);
531
}
532
509
-void strbuf_git_path_submodule(struct strbuf *buf, const char *path,
510
- const char *fmt, ...)
533
+int strbuf_git_path_submodule(struct strbuf *buf, const char *path,
534
+ const char *fmt, ...)
535
{
536
+ int err;
537
va_list args;
538
va_start(args, fmt);
514
- do_submodule_path(buf, path, fmt, args);
539
+ err = do_submodule_path(buf, path, fmt, args);
540
va_end(args);
541
+
542
+ return err;
543
}
544
545
static void do_git_common_path(struct strbuf *buf,
refs/files-backend.c
+7
-1
@@ -1225,13 +1225,19 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
1225
struct strbuf refname;
1226
struct strbuf path = STRBUF_INIT;
1227
size_t path_baselen;
1228
+ int err = 0;
1229
1230
if (*refs->name)
1230
- strbuf_git_path_submodule(&path, refs->name, "%s", dirname);
1231
+ err = strbuf_git_path_submodule(&path, refs->name, "%s", dirname);
1232
else
1233
strbuf_git_path(&path, "%s", dirname);
1234
path_baselen = path.len;
1235
1236
+ if (err) {
1237
+ strbuf_release(&path);
1238
+ return;
1239
+ }
1240
+
1241
d = opendir(path.buf);
1242
if (!d) {
1243
strbuf_release(&path);
submodule.c
+4
-2
@@ -127,7 +127,9 @@ static int add_submodule_odb(const char *path)
127
int ret = 0;
128
size_t alloc;
129
130
- strbuf_git_path_submodule(&objects_directory, path, "objects/");
130
+ ret = strbuf_git_path_submodule(&objects_directory, path, "objects/");
131
+ if (ret)
132
+ goto done;
133
if (!is_directory(objects_directory.buf)) {
134
ret = -1;
135
goto done;
@@ -348,7 +350,7 @@ void show_submodule_summary(FILE *f, const char *path,
350
if (is_null_sha1(two))
351
message = "(submodule deleted)";
352
else if (add_submodule_odb(path))
351
- message = "(not checked out)";
353
+ message = "(not initialized)";
354
else if (is_null_sha1(one))
355
message = "(new submodule)";
356
else if (!(left = lookup_commit_reference(one)) ||
t/t4059-diff-submodule-not-initialized.sh
new
+127
@@ -0,0 +1,127 @@
1
+#!/bin/sh
2
+#
3
+# Copyright (c) 2016 Jacob Keller, based on t4041 by Jens Lehmann
4
+#
5
+
6
+test_description='Test for submodule diff on non-checked out submodule
7
+
8
+This test tries to verify that add_submodule_odb works when the submodule was
9
+initialized previously but the checkout has since been removed.
10
+'
11
+
12
+. ./test-lib.sh
13
+
14
+# Tested non-UTF-8 encoding
15
+test_encoding="ISO8859-1"
16
+
17
+# String "added" in German (translated with Google Translate), encoded in UTF-8,
18
+# used in sample commit log messages in add_file() function below.
19
+added=$(printf "hinzugef\303\274gt")
20
+
21
+add_file () {
22
+ (
23
+ cd "$1" &&
24
+ shift &&
25
+ for name
26
+ do
27
+ echo "$name" >"$name" &&
28
+ git add "$name" &&
29
+ test_tick &&
30
+ # "git commit -m" would break MinGW, as Windows refuse to pass
31
+ # $test_encoding encoded parameter to git.
32
+ echo "Add $name ($added $name)" | iconv -f utf-8 -t $test_encoding |
33
+ git -c "i18n.commitEncoding=$test_encoding" commit -F -
34
+ done >/dev/null &&
35
+ git rev-parse --short --verify HEAD
36
+ )
37
+}
38
+
39
+commit_file () {
40
+ test_tick &&
41
+ git commit "$@" -m "Commit $*" >/dev/null
42
+}
43
+
44
+test_expect_success 'setup - submodules' '
45
+ test_create_repo sm2 &&
46
+ add_file . foo &&
47
+ add_file sm2 foo1 foo2 &&
48
+ smhead1=$(git -C sm2 rev-parse --short --verify HEAD)
49
+'
50
+
51
+test_expect_success 'setup - git submodule add' '
52
+ git submodule add ./sm2 sm1 &&
53
+ commit_file sm1 .gitmodules &&
54
+ git diff-tree -p --no-commit-id --submodule=log HEAD -- sm1 >actual &&
55
+ cat >expected <<-EOF &&
56
+ Submodule sm1 0000000...$smhead1 (new submodule)
57
+ EOF
58
+ test_cmp expected actual
59
+'
60
+
61
+test_expect_success 'submodule directory removed' '
62
+ rm -rf sm1 &&
63
+ git diff-tree -p --no-commit-id --submodule=log HEAD -- sm1 >actual &&
64
+ cat >expected <<-EOF &&
65
+ Submodule sm1 0000000...$smhead1 (new submodule)
66
+ EOF
67
+ test_cmp expected actual
68
+'
69
+
70
+test_expect_success 'setup - submodule multiple commits' '
71
+ git submodule update --checkout sm1 &&
72
+ smhead2=$(add_file sm1 foo3 foo4) &&
73
+ commit_file sm1 &&
74
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
75
+ cat >expected <<-EOF &&
76
+ Submodule sm1 $smhead1..$smhead2:
77
+ > Add foo4 ($added foo4)
78
+ > Add foo3 ($added foo3)
79
+ EOF
80
+ test_cmp expected actual
81
+'
82
+
83
+test_expect_success 'submodule removed multiple commits' '
84
+ rm -rf sm1 &&
85
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
86
+ cat >expected <<-EOF &&
87
+ Submodule sm1 $smhead1..$smhead2:
88
+ > Add foo4 ($added foo4)
89
+ > Add foo3 ($added foo3)
90
+ EOF
91
+ test_cmp expected actual
92
+'
93
+
94
+test_expect_success 'submodule not initialized in new clone' '
95
+ git clone . sm3 &&
96
+ git -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
97
+ cat >expected <<-EOF &&
98
+ Submodule sm1 $smhead1...$smhead2 (not initialized)
99
+ EOF
100
+ test_cmp expected actual
101
+'
102
+
103
+test_expect_success 'setup submodule moved' '
104
+ git submodule update --checkout sm1 &&
105
+ git mv sm1 sm4 &&
106
+ commit_file sm4 &&
107
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
108
+ cat >expected <<-EOF &&
109
+ Submodule sm4 0000000...$smhead2 (new submodule)
110
+ EOF
111
+ test_cmp expected actual
112
+'
113
+
114
+test_expect_success 'submodule moved then removed' '
115
+ smhead3=$(add_file sm4 foo6 foo7) &&
116
+ commit_file sm4 &&
117
+ rm -rf sm4 &&
118
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
119
+ cat >expected <<-EOF &&
120
+ Submodule sm4 $smhead2..$smhead3:
121
+ > Add foo7 ($added foo7)
122
+ > Add foo6 ($added foo6)
123
+ EOF
124
+ test_cmp expected actual
125
+'
126
+
127
+test_done