Re: [PATCH 19/24] submodule: use submodule repos for object lookup
On 11/2/2018 1:23 PM, Stefan Beller wrote: On Fri, Nov 2, 2018 at 6:03 AM Derrick Stolee wrote: On 10/30/2018 6:08 PM, Stefan Beller wrote: This converts the 'show_submodule_header' function to use the repository API properly, such that the submodule objects are not added to the main object store. Signed-off-by: Stefan Beller A couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1, including t4041-diff-submodule-option.sh. The failure bisects to this patch. Here is a verbose output of the first failure in that script:; expecting success: git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && Submodule sm1 $head2..$head3 (rewind): < Add foo3 ($added foo3) < Add foo2 ($added foo2) EOF test_cmp expected actual + git diff-index -p --submodule=log HEAD + cat + test_cmp expected actual + diff -u expected actual --- expected2018-11-02 12:58:43.429262380 + +++ actual 2018-11-02 12:58:43.429262380 + @@ -1,3 +1,5 @@ -Submodule sm1 30b9670..dafb207 (rewind): +Submodule sm1 30b9670...dafb207: < Add foo3 (hinzugefügt foo3) < Add foo2 (hinzugefügt foo2) + > Add foo1 (hinzugefügt foo1) + < Add foo1 (hinzugefügt foo1) error: last command exited with $?=1 not ok 9 - modified submodule(backward) I've been looking into the patch below to see if there is an obvious problem, but the best I can think is that open_submodule() creates an alternate 'struct repository' and somehow the commit-graph feature is interacting poorly with that struct. Stefan, do you know what's going on? Sure, see the last four patches of this series https://public-inbox.org/git/20181030220817.61691-1-sbel...@google.com/ (to which you also reply to? Junio did not queue this one, yet). Sorry! Got a bit mixed up looking at everything. I didn't realize that the current 'pu' didn't have your latest. Thanks!
Re: [PATCH 19/24] submodule: use submodule repos for object lookup
On Fri, Nov 2, 2018 at 6:03 AM Derrick Stolee wrote: > > On 10/30/2018 6:08 PM, Stefan Beller wrote: > > This converts the 'show_submodule_header' function to use > > the repository API properly, such that the submodule objects > > are not added to the main object store. > > > > Signed-off-by: Stefan Beller > > A couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1, > including t4041-diff-submodule-option.sh. The failure bisects to this patch. > > Here is a verbose output of the first failure in that script:; > > > expecting success: > git diff-index -p --submodule=log HEAD >actual && > cat >expected <<-EOF && > Submodule sm1 $head2..$head3 (rewind): >< Add foo3 ($added foo3) >< Add foo2 ($added foo2) > EOF > test_cmp expected actual > > + git diff-index -p --submodule=log HEAD > + cat > + test_cmp expected actual > + diff -u expected actual > --- expected2018-11-02 12:58:43.429262380 + > +++ actual 2018-11-02 12:58:43.429262380 + > @@ -1,3 +1,5 @@ > -Submodule sm1 30b9670..dafb207 (rewind): > +Submodule sm1 30b9670...dafb207: > < Add foo3 (hinzugefügt foo3) > < Add foo2 (hinzugefügt foo2) > + > Add foo1 (hinzugefügt foo1) > + < Add foo1 (hinzugefügt foo1) > error: last command exited with $?=1 > not ok 9 - modified submodule(backward) > > I've been looking into the patch below to see if there is an obvious > problem, but the best I can think is that open_submodule() creates an > alternate 'struct repository' and somehow the commit-graph feature is > interacting poorly with that struct. > > Stefan, do you know what's going on? Sure, see the last four patches of this series https://public-inbox.org/git/20181030220817.61691-1-sbel...@google.com/ (to which you also reply to? Junio did not queue this one, yet).
Re: [PATCH 19/24] submodule: use submodule repos for object lookup
On 10/30/2018 6:08 PM, Stefan Beller wrote:
This converts the 'show_submodule_header' function to use
the repository API properly, such that the submodule objects
are not added to the main object store.
Signed-off-by: Stefan Beller
A couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1,
including t4041-diff-submodule-option.sh. The failure bisects to this patch.
Here is a verbose output of the first failure in that script:;
expecting success:
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
Submodule sm1 $head2..$head3 (rewind):
< Add foo3 ($added foo3)
< Add foo2 ($added foo2)
EOF
test_cmp expected actual
+ git diff-index -p --submodule=log HEAD
+ cat
+ test_cmp expected actual
+ diff -u expected actual
--- expected 2018-11-02 12:58:43.429262380 +
+++ actual 2018-11-02 12:58:43.429262380 +
@@ -1,3 +1,5 @@
-Submodule sm1 30b9670..dafb207 (rewind):
+Submodule sm1 30b9670...dafb207:
< Add foo3 (hinzugefügt foo3)
< Add foo2 (hinzugefügt foo2)
+ > Add foo1 (hinzugefügt foo1)
+ < Add foo1 (hinzugefügt foo1)
error: last command exited with $?=1
not ok 9 - modified submodule(backward)
I've been looking into the patch below to see if there is an obvious
problem, but the best I can think is that open_submodule() creates an
alternate 'struct repository' and somehow the commit-graph feature is
interacting poorly with that struct.
Stefan, do you know what's going on?
Thanks,
-Stolee
---
submodule.c | 76 ++---
1 file changed, 61 insertions(+), 15 deletions(-)
diff --git a/submodule.c b/submodule.c
index d9d3046689..0fdda45252 100644
--- a/submodule.c
+++ b/submodule.c
@@ -443,7 +443,7 @@ static int prepare_submodule_summary(struct rev_info *rev,
const char *path,
return prepare_revision_walk(rev);
}
-static void print_submodule_summary(struct rev_info *rev, struct diff_options *o)
+static void print_submodule_summary(struct repository *r, struct rev_info
*rev, struct diff_options *o)
{
static const char format[] = " %m %s";
struct strbuf sb = STRBUF_INIT;
@@ -454,7 +454,8 @@ static void print_submodule_summary(struct rev_info *rev,
struct diff_options *o
ctx.date_mode = rev->date_mode;
ctx.output_encoding = get_log_output_encoding();
strbuf_setlen(, 0);
- format_commit_message(commit, format, , );
+ repo_format_commit_message(r, commit, format, ,
+ );
strbuf_addch(, '\n');
if (commit->object.flags & SYMMETRIC_LEFT)
diff_emit_submodule_del(o, sb.buf);
@@ -481,14 +482,47 @@ void prepare_submodule_repo_env(struct argv_array *out)
DEFAULT_GIT_DIR_ENVIRONMENT);
}
-/* 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
- * left and right pointers.
+/*
+ * Initialize 'out' based on the provided submodule path.
+ *
+ * Unlike repo_submodule_init, this tolerates submodules not present
+ * in .gitmodules. This function exists only to preserve historical behavior,
+ *
+ * Returns 0 on success, -1 when the submodule is not present.
*/
-static void show_submodule_header(struct diff_options *o, const char *path,
+static struct repository *open_submodule(const char *path)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct repository *out = xmalloc(sizeof(*out));
+
+ if (submodule_to_gitdir(, path) || repo_init(out, sb.buf, NULL)) {
+ strbuf_release();
+ free(out);
+ return NULL;
+ }
+
+ out->submodule_prefix = xstrfmt("%s%s/",
+ the_repository->submodule_prefix ?
+ the_repository->submodule_prefix :
+ "", path);
+
+ strbuf_release();
+ return out;
+}
+
+/*
+ * Helper function to display the submodule header line prior to the full
+ * summary output.
+ *
+ * If it can locate the submodule git directory it will create a repository
+ * handle for the submodule and lookup both the left and right commits and
+ * put them into the left and right pointers.
+ */
+static void show_submodule_header(struct diff_options *o,
+ const char *path,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule,
+ struct repository *sub,
struct commit **left, struct commit **right,
struct commit_list **merge_bases)
{
@@ -507,7 +541,7 @@ static void show_submodule_header(struct diff_options *o,
const char *path,
else if (is_null_oid(two))
[PATCH 19/24] submodule: use submodule repos for object lookup
This converts the 'show_submodule_header' function to use
the repository API properly, such that the submodule objects
are not added to the main object store.
Signed-off-by: Stefan Beller
---
submodule.c | 76 ++---
1 file changed, 61 insertions(+), 15 deletions(-)
diff --git a/submodule.c b/submodule.c
index d9d3046689..0fdda45252 100644
--- a/submodule.c
+++ b/submodule.c
@@ -443,7 +443,7 @@ static int prepare_submodule_summary(struct rev_info *rev,
const char *path,
return prepare_revision_walk(rev);
}
-static void print_submodule_summary(struct rev_info *rev, struct diff_options
*o)
+static void print_submodule_summary(struct repository *r, struct rev_info
*rev, struct diff_options *o)
{
static const char format[] = " %m %s";
struct strbuf sb = STRBUF_INIT;
@@ -454,7 +454,8 @@ static void print_submodule_summary(struct rev_info *rev,
struct diff_options *o
ctx.date_mode = rev->date_mode;
ctx.output_encoding = get_log_output_encoding();
strbuf_setlen(, 0);
- format_commit_message(commit, format, , );
+ repo_format_commit_message(r, commit, format, ,
+ );
strbuf_addch(, '\n');
if (commit->object.flags & SYMMETRIC_LEFT)
diff_emit_submodule_del(o, sb.buf);
@@ -481,14 +482,47 @@ void prepare_submodule_repo_env(struct argv_array *out)
DEFAULT_GIT_DIR_ENVIRONMENT);
}
-/* 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
- * left and right pointers.
+/*
+ * Initialize 'out' based on the provided submodule path.
+ *
+ * Unlike repo_submodule_init, this tolerates submodules not present
+ * in .gitmodules. This function exists only to preserve historical behavior,
+ *
+ * Returns 0 on success, -1 when the submodule is not present.
*/
-static void show_submodule_header(struct diff_options *o, const char *path,
+static struct repository *open_submodule(const char *path)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct repository *out = xmalloc(sizeof(*out));
+
+ if (submodule_to_gitdir(, path) || repo_init(out, sb.buf, NULL)) {
+ strbuf_release();
+ free(out);
+ return NULL;
+ }
+
+ out->submodule_prefix = xstrfmt("%s%s/",
+ the_repository->submodule_prefix ?
+ the_repository->submodule_prefix :
+ "", path);
+
+ strbuf_release();
+ return out;
+}
+
+/*
+ * Helper function to display the submodule header line prior to the full
+ * summary output.
+ *
+ * If it can locate the submodule git directory it will create a repository
+ * handle for the submodule and lookup both the left and right commits and
+ * put them into the left and right pointers.
+ */
+static void show_submodule_header(struct diff_options *o,
+ const char *path,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule,
+ struct repository *sub,
struct commit **left, struct commit **right,
struct commit_list **merge_bases)
{
@@ -507,7 +541,7 @@ static void show_submodule_header(struct diff_options *o,
const char *path,
else if (is_null_oid(two))
message = "(submodule deleted)";
- if (add_submodule_odb(path)) {
+ if (!sub) {
if (!message)
message = "(commits not present)";
goto output_header;
@@ -517,8 +551,8 @@ static void show_submodule_header(struct diff_options *o,
const char *path,
* Attempt to lookup the commit references, and determine if this is
* a fast forward or fast backwards update.
*/
- *left = lookup_commit_reference(the_repository, one);
- *right = lookup_commit_reference(the_repository, two);
+ *left = lookup_commit_reference(sub, one);
+ *right = lookup_commit_reference(sub, two);
/*
* Warn about missing commits in the submodule project, but only if
@@ -528,7 +562,7 @@ static void show_submodule_header(struct diff_options *o,
const char *path,
(!is_null_oid(two) && !*right))
message = "(commits not present)";
- *merge_bases = get_merge_bases(*left, *right);
+ *merge_bases = repo_get_merge_bases(sub, *left, *right);
if (*merge_bases) {
if ((*merge_bases)->item == *left)
fast_forward = 1;
@@ -562,16 +596,18 @@ void show_submodule_summary(struct diff_options *o, const
char *path,
struct rev_info rev;
struct commit *left =
