git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Diffing submodule does not yield complete logs for merge commits

From
RDRobert Dailey <rcdailey.lists@gmail.com>
Date
May 15, 2015, 20:33 UTC
Message-ID
<CAHd499Do2aB5E_=aDzkoDssEbgz181rH36X28Oe7Zcok2f=zBQ@mail.gmail.com>
In-Reply-To
<37f399418bbebb3b53a50bf8daffcdc0@www.dscho.org>

On Tue, May 5, 2015 at 12:49 AM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 14 quoted lines
> Hi Robert,
>
> On 2015-05-04 22:21, Robert Dailey wrote:
>
>> Since I am not a linux user, I have implemented this feature against
>> the Git for Windows fork of git. I am not able to verify changes if I
>> make them directly against the Git repository.
>
> That is why I worked so hard on support of Vagrant: https://github.com/msysgit/msysgit/wiki/Vagrant -- in short, it makes it dead easy for you to set up a *minimal* Linux VM inside your Git SDK and interact with it via ssh.
>
> With the Vagrant solution, you can easily test Linux Git even on Windows.
>
> Ciao,
> Johannes

At the moment I have a "half-ass" patch attached. This implements the feature itself. I'm able to test this and it seems to be working. Please note I'm a C++ developer and straight C / Bash are not my strong suits. I apologize in advance for any mistakes. I am open to taking recommendations for corrections.

I'm not sure how I can verify the feature in a unit test. In addition to learning bash scripting well enough to write the test, I am not sure how to use git to check for the additional commits. Plus the repo for the test will need to handle a submodule change to a merge commit as well. Any advice on setting up a good test case for this? What conditions should I check for, as far as log output goes?

From 66db185ab76d0d893792d03dc0fc4913d868f218 Mon Sep 17 00:00:00 2001
From: Robert Dailey <rcdailey@gmail.com>
Date: Tue, 5 May 2015 21:56:26 +0100
Subject: [PATCH] Add 'full-log' option to diff.submodule
Like the 'log' option to `diff --submodule`, 'full-log' provides
logs without the `--first-parent` option.
---
 diff.c      | 16 ++++++++++++----
 diff.h      |  1 +
 submodule.c |  9 +++++----
 submodule.h |  3 ++-
 4 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/diff.c b/diff.c
index 7500c55..58c4872 100644
--- a/diff.c
+++ b/diff.c
@@ -128,10 +128,18 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
 
 static int parse_submodule_params(struct diff_options *options, const char *value)
 {
-	if (!strcmp(value, "log"))
+	if (!strcmp(value, "log")) {
 		DIFF_OPT_SET(options, SUBMODULE_LOG);
-	else if (!strcmp(value, "short"))
+		DIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);
+	}
+	else if (!strcmp(value, "full-log")) {
+		DIFF_OPT_SET(options, SUBMODULE_FULL_LOG);
+		DIFF_OPT_CLR(options, SUBMODULE_LOG);
+	}
+	else if (!strcmp(value, "short")) {
 		DIFF_OPT_CLR(options, SUBMODULE_LOG);
+		DIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);
+	}
 	else
 		return -1;
 	return 0;
@@ -2240,7 +2248,7 @@ static void builtin_diff(const char *name_a,
 	struct strbuf header = STRBUF_INIT;
 	const char *line_prefix = diff_line_prefix(o);
 
-	if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
+	if ((DIFF_OPT_TST(o, SUBMODULE_LOG) || DIFF_OPT_TST(o, SUBMODULE_FULL_LOG)) &&
 			(!one->mode || S_ISGITLINK(one->mode)) &&
 			(!two->mode || S_ISGITLINK(two->mode))) {
 		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
@@ -2248,7 +2256,7 @@ static void builtin_diff(const char *name_a,
 		show_submodule_summary(o->file, one->path ? one->path : two->path,
 				line_prefix,
 				one->sha1, two->sha1, two->dirty_submodule,
-				meta, del, add, reset);
+				meta, del, add, reset, DIFF_OPT_TST(o, SUBMODULE_FULL_LOG));
 		return;
 	}
 
diff --git a/diff.h b/diff.h
index b4a624d..95f319c 100644
--- a/diff.h
+++ b/diff.h
@@ -90,6 +90,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)
 #define DIFF_OPT_FUNCCONTEXT         (1 << 29)
 #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
+#define DIFF_OPT_SUBMODULE_FULL_LOG  (1 << 31)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)
diff --git a/submodule.c b/submodule.c
index d37d400..f98173e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -290,14 +290,14 @@ 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,
-		int *fast_forward, int *fast_backward)
+		int *fast_forward, int *fast_backward, unsigned full_log)
 {
 	struct commit_list *merge_bases, *list;
 
 	init_revisions(rev, NULL);
 	setup_revisions(0, NULL, rev, NULL);
 	rev->left_right = 1;
-	rev->first_parent_only = 1;
+	rev->first_parent_only = full_log ? 0 : 1;
 	left->object.flags |= SYMMETRIC_LEFT;
 	add_pending_object(rev, &left->object, path);
 	add_pending_object(rev, &right->object, path);
@@ -363,7 +363,8 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule, const char *meta,
-		const char *del, const char *add, const char *reset)
+		const char *del, const char *add, const char *reset,
+		unsigned full_log)
 {
 	struct rev_info rev;
 	struct commit *left = NULL, *right = NULL;
@@ -381,7 +382,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		 !(right = lookup_commit_reference(two)))
 		message = "(commits not present)";
 	else if (prepare_submodule_summary(&rev, path, left, right,
-					   &fast_forward, &fast_backward))
+					   &fast_forward, &fast_backward, full_log))
 		message = "(revision walker failed)";
 
 	if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
diff --git a/submodule.h b/submodule.h
index 7beec48..301358b 100644
--- a/submodule.h
+++ b/submodule.h
@@ -26,7 +26,8 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule, const char *meta,
-		const char *del, const char *add, const char *reset);
+		const char *del, const char *add, const char *reset,
+		unsigned full_log);
 void set_config_fetch_recurse_submodules(int value);
 void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 int fetch_populated_submodules(const struct argv_array *options,

-- 
2.4.1.windows.1
Previous: Johannes SchindelinNext: Heiko Voigt
Message 8 of 24 in “Diffing submodule does not yield complete logs for merge commits”
  1. Robert DaileyApr 29, 2015
  2. Heiko VoigtMay 1, 2015
  3. Robert DaileyMay 4, 2015
  4. Jens LehmannMay 4, 2015
  5. Robert DaileyMay 4, 2015
  6. Heiko VoigtMay 4, 2015
  7. Johannes SchindelinMay 5, 2015
  8. Robert DaileyMay 15, 2015
  9. Heiko VoigtMay 18, 2015
  10. Robert DaileyMay 18, 2015
  11. Heiko VoigtMay 19, 2015
  12. Robert DaileyMay 19, 2015
  13. Stefan BellerMay 19, 2015
  14. Roberto TyleyMay 22, 2015
  15. Heiko VoigtMay 21, 2015
  16. Robert DaileyMay 30, 2015
  17. Heiko VoigtMay 30, 2015
  18. Junio C HamanoMay 30, 2015
  19. Robert DaileyMay 30, 2015
  20. Robert DaileyMay 30, 2015
  21. Junio C HamanoMay 30, 2015
  22. Robert DaileyMay 30, 2015
  23. Heiko VoigtJun 2, 2015
  24. Junio C HamanoMay 4, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.