Re: [PATCH] Refactor dirty submodule detection in diff-lib.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jens Lehmann <Jens.Lehmann@xxxxxx> writes:

> Moving duplicated code into the new function match_stat_with_submodule().
> Replacing the implicit activation of detailed checks for the dirtiness of
> submodules when DIFF_FORMAT_PATCH was selected with explicitly setting
> the recently added DIFF_OPT_DIRTY_SUBMODULES option in diff_setup_done().
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@xxxxxx>
> ---
>
> All tests run fine with this patch. All other callsites where the option
> DIFF_FORMAT_PATCH is set look like they don't deal with the work tree
> or they call diff_setup_done() shortly after.
> I am pretty sure about builtin/log.c, as this can't use the work tree
> but only already commited stuff, no?
> But then there is edit_patch() in buitin/add.c, I am not sure it makes
> sense to set DIRTY_SUBMODULES there too. Opinions?

I like this particular refactoring to remove duplicated obscure logic.

I am not sure if it is the right thing to do to return "changed" if the
change is only "submodule is dirty even though its HEAD is the same as
what is recorded in the index of the superproject", though.  See my
patches in response to your "[PATCH] git status: Fix false positive "new
commits" output for".  I think it would be conceptually the right thing to
do to feed the same object names in the sha1 field for both sides of the
filepair, with the working tree side marked with "dirty-submodule".  I
also suspect that you would not need to re-resolve the gitlinks in later
stages when you generate patches in textual form if you do it that way.


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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]