patchbuiltin/describe.c: ignore untracked changes in submodules

From: Brandon Casey <drafnel@gmail.com>

Since 'git describe' does not append -dirty to the version string it produces when untracked files exist in the working directory of the main repository, it should not do so for submodules either.

Add --ignore-submodules=untracked to the call to diff-index which is used to decide whether or not the '-dirty' string is necessary.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin/describe.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Brandon Casey <casey@nrlssc.navy.mil> writes:

Hmm, this changes the behaviour in a big way but it probably is for the better. At least it is consistent with the recent fixes to the interaction between diff and submodules.

Objections from submodule users?

Hmm, by default the diff family considers submodules with untracked files as dirty unless configured otherwise (and AFAICS the recent fixes to the interaction between diff and submodule were options to configure your own default).

So when git status tells you the subodule is modified, e.g. because of an untracked file, I would expect git describe to add '-dirty' to its output when requested. To get rid of that I would expect you either fix the .gitignore of the submodule or configure that you don't care about untracked files in submodules at all (either only for this submodule or in the config).

So if I didn't misunderstand something here I would rather vote against this change, git describe should append a '-dirty' when git status would show modifications, no?

On Fri, Sep 10, 2010 at 1:40 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:

Triple hmm. Perhaps a deeper level change is necessary than what I originally thought.

It appears to me now, that the behavior of the entire diff family is inconsistent with respect to how untracked content is handled at the super-project level and at the submodule level.

At the super-project level, git only considers as 'modified', changes to _tracked_ content. Any untracked content is ignored by git-describe, git-diff, and friends, and git-status places it in its own 'Untracked files' section.

At the submodule level, all files, tracked and untracked, are considered.

Do you agree that there is an inconsistency between how untracked content is treated at the super-project level and at the submodule level? Any thoughts about how the behavior should be made to be consistent?

Perhaps the default setting of submodule.<name>.ignore should be 'untracked'?

-Brandon

Yes, but I - and others included in that discussion some time ago - could not come up with a sane and simple solution to that problem.

The core of this issue is that for git a file is either untracked, modified or clean.
But submodules can have every combination of all these states - as they consist
of multiple files - and additionally their HEAD can differ from the commit recorded
in the superproject. So basically I see two ways to handle that:
a) add new states for an entry to represent all missing combinations of possible
   states for submodules and tell all porcelain to handle these.
b) simplify this problem by having a submodule show up as modified when
   either of these three conditions are met (and enable the user to choose what
   conditions she wants to see and what not).

Obviously a) will complicate all git by a large degree just for the sake of submodules. I am arguing for b), because submodules itself can be seen as a bunch of files which don't interest me as single entities until I want to take a closer look. I think the issue we are discussing here is the price we have to pay for this abstraction. I am very open to proposals how to better handle that but so far I haven't seen any.

I still vote for none. I think the default should be to not have untracked files in your projects (like you should not have warnings when compiling your project). If that is not wanted, just use the configuration options git provides to change it.

http://www.Gucci2u.com/ Gucci Shoes now produces flavor styles of shoes, loafers, clogs, and boots – and now even heels, Gucci Shoes have grown from the chief feature innate supplies and emphasizing durability, comfort, and strong feet, Gucci shoes for men makes men’s and women’s footwear. Having a group of high responsibility, especially inquiries department, authentic Gucci Shoes has put huge inquiries time studying how the time, Gucci changed by the recurring shock of daily walking and workmanship. Despite some worth issues over the person body is fitting a cult desired and is well on the person body. Sometimes mistaken for Birkenstocks, Discount Gucci Shoes will be assured that Gucci delivers the finest in value and operation. The self-proclaimed “finest walking shoes in the world”, http://www.Gucci2u.com/ Gucci Shoes takes an international success. Gucci women boots are physiological footwear intended.

----------------------------------------------------------------------------------------

Visit our site for reviews, news and information about flight simulator games , http://www.tomtop.com/toy-kids-baby/rc-electric-toys/helicopters-accessories/6ch-usb-3d-rc-helicopter-airplane-flight-simulator.html?aid=z helicopter simulator .A http://www.tomtop.com/mini-spy-car-dv-key-chain-camera-dvr-video-recorder-dc.html?aid=z keychain camera is a very small and inexpensive digital camera.

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?

Jens Lehmann <Jens.Lehmann@web.de> writes:

It is sensible to add the option, and handle_ignore_submodules_arg() call to grab "diff.ignoresubmodules" configuration) to the command, perhaps.

Both "status" and "diff" are described in the documentation as having "all" as the default value for --ignore-submodules option, but by default neither ignores changes in the submodules. So it seems consistent for the command to take submodule changes into account by default.

Ok, I'll look into that and prepare a patch.

Jens Lehmann <Jens.Lehmann@web.de> writes:

Thanks; and thanks for biting my sanity check weatherbaloon ;-)

Junio C Hamano <gitster@pobox.com> writes:

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)
  

I forgot to mention: I think untracked files should mark a submodule as modified because otherwise it is too easy to forget adding new files inside submodules, as they won't show up when the 'ignore' setting is 'untracked' .

Jens Lehmann <Jens.Lehmann@web.de> writes:

What makes untracked paths in the superproject different from the ones in a submodule? "git diff" cannot be it as it does not show untracked paths in the superproject, so you are talking about "the user cannot tell from the 'git status' output", right?

How about giving a new section in the status output that lists submodules with untracked paths, and do so only when ignore-submodules is set not to ignore them?

Am 14.09.2010 01:14, schrieb Junio C Hamano:

That you can have a different state for each path inside the superproject (modified, untracked etc.) while you can't have that for the paths in the submodule (when looked at from the superproject): There is only a single state available for the whole submodule, it's either modified or it isn't. So IMO a modified submodule should tell the user: "There is a change in this submodule so that when you commit/push your superproject now, others might run into problems when fetching it; you want to be sure this is not the case before doing that". And this is just the same thing you could say about a file in the superproject when it shows up as modified, no? And for submodules this definition must also include new yet untracked files, as they are very likely to be missing in every but your work tree.

Nope, it's "git diff" too. The thing that got me started working on this topic was that "git gui" and "gitk" were quiet about submodules which had modified tracked files and/or new untracked files, which lead to real world problems where I work. And both use diff-index and diff-files to get the paths they should display *and* to display the actual changes. (And as "git diff" uses the same machinery under the hood as "git status" does, everything fell into place pretty easily)

And I argue that this is sane behavior, as I'm sure other tools rely on "git diff" or "git status" too to check if there are modifications to the work tree (or they call run_diff_files() or run_diff_index() directly to do that). So all of these should agree on what they are saying about the state of a submodule, or things will get interesting. (Same goes for describe, it should append the "-dirty" when "git status" or "git diff" say a submodule is modified)

And this approach works really well at my dayjob. Since we are using it, me and my colleagues are really happy with it, because we can't forget to commit changes inside a submodule anymore. So judging from this real life experience "ignore=none" is a very sane default.

But I admit that this change in behavior can be strange for long time submodule users when they first encounter it. And if they still don't like the new behavior after some consideration, they can disable it easily using the new configuration options. But one of the advantages I really liked when I started using git was that is was not able to forget to commit new files anymore. So I suspect ignore=none is especially useful for new users of submodules, because it is on the safe side, and therefore should be the default setting. You can later turn the 'noise' down if you want (just like some users do when using the "-uno" option to "git status" if they don't want to be told about untracked files in the superproject or its submodules).