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

Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable

From
Jens Lehmann <jens.lehmann@web.de>
Date
Dec 27, 2010, 11:14 UTC
Message-ID
<4D187511.3090104@web.de>
In-Reply-To
<7vd3ooz6qd.fsf@alter.siamese.dyndns.org>
Am 26.12.2010 20:14, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> So are there any reasons for the plumbing diff commands not to honor
>> the diff.ignoreSubmodules setting?
Thank you for explaining those reasons in detail.
Show 21 quoted lines
> One class of plumbing users is scripts that is about automation and
> mechanization that want to control what they do precisely (think cron
> jobs) without getting affected by the user preference stored in the
> repository configuration.  This class could either:
> 
>  (1) state what they want explicitly from the command line; or
>  (2) rely on built-in defaults not changing underneath them.
> 
> The behaviour of diff recursively inspecting submodule dirtiness has an
> unfortunate history, in that the behaviour changed over time, and in each
> step when we made a change, we thought we were making an unquestionable
> improvement.  Originally we only said "submodule HEAD is different from
> what we have in the index/superproject HEAD".  Later we added different
> kind of dirtiness like untracked files or modified contents in submodules,
> decided perhaps mistakenly that majority of users do want to see them as
> dirtiness and made that the default and allowed them to be ignored by an
> explicit request.  At that point, in order not to break existing scripts
> (mostly of the "mechanization" class, written back when there was no such
> extra dirtyness hence with no "explicit refusal" route available to them
> without rewriting), hence "no configuration should affect plumbing
> randomly" policy.

Good point. But unfortunately the diff plumbing commands are affected by the "submodule.<name>.ignore" ignore settings I introduced in August in aee9c7d6 and 302ad7a9. Maybe we should revert the part of these patches that changed the plumbing commands?

Show 18 quoted lines
> On the other hand, you may write user facing Porcelain in scripts and run
> plumbing from there.  This class of plumbing users could either:
> 
>  (1) inspect the config itself, interpret the customization and pass
>      an explicit command line flag; or
> 
>  (2) allow the plumbing honor the end user configuration stored in the
>      repository or user configuration files.
> 
> It is argurably more convenient for these users if the plumbing blindly
> honored the configurations, as it would have allowed the latter
> implementation.  That way, we can be more lazy when writing our scripts,
> and ignore having to worry about new kinds of customization added to
> underlying git after a script is written---but new kinds of customization
> may break your script's expectation of what will and what will not be made
> customizable, and you would end up giving an explicit "do not use that
> feature" in some cases, so the being able to be lazy is not necessarily
> always a win.
Agreed.
Show 14 quoted lines
> Things may have been a bit different if the original feature change to
> inspect submodules deeper, command line flags to control that behaviour
> and configuration to default the flags came at the same time, but
> unfortunately they happend over time.  I think we have been slowly getting
> better at this, but in the case of this particular feature, the original
> introduction of --ignore-submodules was in May 2008, deeper submodule
> inspection and the richer --ignore-submodules=<kind> option came much
> later in June 2010, and the configuration was invented later in August
> 2010, which would mean that allowing the plumbing to honor configuration
> would have broken scripts written in the 2 years and 3 months period.
> 
> And no, this does not call for a blanket "do / do not honor configuration"
> option to plumbing commands.  A more selective "do / do not honor these
> configuration variables" option might be an option, though.

What about a new "--ignore-submodules=config" option to tell the plumbing that it should honor the config?

And it looks like the PS1 problem that started this discussion is a valid example for mixed usage of porcelain and plumbing commands. In a first attempt to fix the problem by using "git diff --cached" instead of "git diff-index --cached" I noticed that those two commands give different results when new submodules were created and had been added to the index. "git diff --cached" ignores them while "git diff-index --cached" shows them. Anything I am missing here?

Previous: Алексей ШумкинNext: Johannes Schindelin
Message 12 of 20 in “Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal”
  1. 1/3 Fixes bug: git-diff: class methods are not detected in hunk headers for PascalZapped, Dec 25, 2010
  2. 2/3 Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-treeZapped, Dec 25, 2010
  3. Thomas RastJan 4, 2011
  4. Eric WongJan 4, 2011
  5. Alexey ShumkinFeb 3, 2011
  6. Re[2]: [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-treeАлексей Шумкин, Jan 5, 2011
  7. 3/3 Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variableZapped, Dec 25, 2010
  8. Jens LehmannDec 25, 2010
  9. Johannes SchindelinDec 25, 2010
  10. Junio C HamanoDec 26, 2010
  11. Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variableАлексей Шумкин, Dec 26, 2010
  12. Jens LehmannDec 27, 2010
  13. Johannes SchindelinDec 27, 2010
  14. Casey DahlinDec 27, 2010
  15. Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variableАлексей Крезов, Dec 26, 2010
  16. Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variableАлексей Шумкин, Dec 28, 2010
  17. Thomas RastJan 4, 2011
  18. Re[2]: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for PascalАлексей Шумкин, Jan 5, 2011
  19. Thomas RastJan 5, 2011
  20. Thomas RastJan 5, 2011

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.