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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 12, 2010, 20:07 UTC
Message-ID
<7vy6b6d8uj.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7vfwxeeu19.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>>>Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>> 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)
  
Previous: Junio C HamanoNext: Jens Lehmann
Message 11 of 14 in “builtin/describe.c: ignore untracked changes in submodules”
  1. builtin/describe.c: ignore untracked changes in submodulesBrandon Casey, Sep 9, 2010
  2. Junio C HamanoSep 10, 2010
  3. Jens LehmannSep 10, 2010
  4. Brandon CaseySep 12, 2010
  5. Jens LehmannSep 13, 2010
  6. yj2133011Sep 12, 2010
  7. Jens LehmannSep 11, 2010
  8. Junio C HamanoSep 11, 2010
  9. Jens LehmannSep 11, 2010
  10. Junio C HamanoSep 12, 2010
  11. Junio C HamanoSep 12, 2010
  12. Jens LehmannSep 13, 2010
  13. Junio C HamanoSep 13, 2010
  14. Jens LehmannSep 14, 2010

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.