Re: [PATCH] add -e: ignore dirty submodules
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:00
Johannes Schindelin [off-list ref] writes:
We cannot add untracked/modified files in submodules anyway.
I can see the updated code would not break "git apply" that will be run on
this output, but the above cannot be the whole story. It is unclear to me
what it is trying to achieve (in other words, "this patch does not break
the system" is not the whole purpose of the patch).
When a submodule is updated and is dirty, we would get:
diff --git a/submodule b/submodule
@@ -1,+1 @@
-Subproject commit XXXX...
+Subproject commit YYYY...-dirty
and leaving this diff in the edited patch adds YYYY... for submodule, even
though "-dirty" suffix is there. So it is not fixing "the user tries to
update but fails because we do not filter dirty submodules" bug, or somesuch.
Besides, showing -dirty may be a good reminder that submodule has further
changes on top of what is going to be committed in this case.
When a submodule is only dirty, we would see:
diff --git a/submodule b/submodule
@@ -1,+1 @@
-Subproject commit XXXX...
+Subproject commit XXXX...-dirty
and leaving this diff in the edited patch keeps the submodule at XXXX...,
again without failing, so it is not fixing "the user gets unnecessary
error message" bug, or somesuch. In this case, leaving this diff will be
a no-op so it is wasteful and distracting to the user who edits the patch.
Is that what this patch is about? "For a submodule that is unchanged but
is dirty, submodule diff whose difference is only the '-dirty' suffix is
given but the user cannot update the index with such a diff anyway, so it
is a waste of space", or something like that?
That is the best guess I arrived at, but I suspect that it cannot be it,
as that discards the "-dirty" clue from the output when the submodule path
does have difference, as we saw in the earlier example. So there must be
something I am missing.
So I am out of ideas guessing what this patch is trying to achieve. The
commit log shouldn't force the readers of the history to _guess_ like the
above.
quoted hunk
builtin/add.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)diff --git a/builtin/add.c b/builtin/add.c index 1c42900..b79336d 100644 --- a/builtin/add.c +++ b/builtin/add.c@@ -280,6 +280,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix) argc = setup_revisions(argc, argv, &rev, NULL); rev.diffopt.output_format = DIFF_FORMAT_PATCH; + DIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES); out = open(file, O_CREAT | O_WRONLY, 0644); if (out < 0) die (_("Could not open '%s' for writing."), file);