Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] builtin/describe.c: ignore untracked changes in submodules

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:32

Junio C Hamano [off-list ref] writes:
Jens Lehmann [off-list ref] writes:
quoted
quoted
Jens Lehmann [off-list ref] writes:
quoted
And maybe we should teach "git describe" the "--ignore-submodules" option, then
you could tell describe what to pass to the diff-index command. Thoughts?
It is sensible to add the option, and handle_ignore_submodules_arg() call
to grab "diff.ignoresubmodules" configuration) to the command, perhaps.
Ok, I'll look into that and prepare a patch.
By the way, I think that route of action would make the resulting git
internally consistent in that everything by default will report submodules
with untracked paths in its working tree as dirty, but sidesteps the
original issue Brandon raised, which I think is a valid concern.

 - In the "Untracked" section of "git status" output, we list an untracked
   path in the superproject (i.e. the one in which "git status" was run)
   to remind the user that the path might be a new file forgotten to be
   added (unless of course it is ignored).  But it does not make the
   working tree dirty.

 - When you have an untracked path in a submodule:

   - the submodule is listed in the "Changed but not updated" section.
     This also makes the working tree of the superproject dirty, even
     though the working tree of the submodule is _not_.

   - "git diff" output at the superproject level shows that the submodule
     has modifications (i.e. "-dirty" is shown), but when run inside the
     submodule, there is no change shown.

I think this is a misdesign at the UI level; reporting an untracked and
unignored path as potential mistake to remind the user is a good thing,
but the current way "status" and "diff" does so does not make much sense
to me.

In the "git status" output, there are three sections.  The way to view
them at the user level has always been:

 - "Changes to be committed": they are included if you say "git commit"
   without any pathspec nor option (aka "What you would commit");

 - "Changed but not updated": they are included if you say "git commit -a"
   (aka "What you could commit");

 - "Untracked files": they may be paths you forgot to add, and you might
   want to say "git add" on some of them and add ignore patterns to cover
   the others.

I have a suspicion that the change we made in 1.7.1 to say an untracked
path in a submodule counts as a "Changed but not updated" was a mistake.
In a project with submodules, if a "git commit" were to be run without any
other option, with the "-a" option, and with the "-A" option, shouldn't
the above three category behave exactly like how they behave in a project
without submodules?

IOW, for a submodule that is not pristine, shouldn't we be doing this?

 - "Changes to be committed": show a submodule path for which the
   superproject index records a commit that is different from what is
   recorded in the superproject HEAD tree (currently we are doing this
   correctly);

 - "Changed but not updated": show a submodule path for which the HEAD in
   the submodule differs from what is recorded in the superproject HEAD
   tree (we are contaminating this list with "untracked content");

 - "Untracked files": show a submodule path that is not in the index.

A submodule with untracked files may be worth knowing, but I don't think
they should fall into any of the above categories.  Perhaps they should be
listed in their own separate section (they may be listed in multiple
sections just like "edit F; git add F; edit F" may result in a path listed
both in to-be-committed and not-updated sections)?

I haven't formed an opinion as to what to do with "git diff" output that
adds "-dirty" for untracked paths.  c.f.

  8e08b41 (Teach diff that modified submodule directory is dirty, 2010-01-16)
  721ceec (Teach diff --submodule that modified submodule directory is dirty, 2010-01-24)
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help