git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH] ls-files: respect 'submodule.recurse' config
Date: Wed, 09 Sep 2020 21:25:30 -0700	[thread overview]
Message-ID: <xmqqv9gms1fp.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.732.git.1599707259907.gitgitgadget@gmail.com> (Philippe Blain via GitGitGadget's message of "Thu, 10 Sep 2020 03:07:39 +0000")

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> `git ls-files` learned to recurse into submodules when given
> '--recurse-submodules' back in e77aa336f1 (ls-files: optionally recurse
> into submodules, 2016-10-07) but it does not respect the
> 'submodule.recurse' config option which was later introduced in
> 046b48239e (Introduce 'submodule.recurse' option for worktree
> manipulators, 2017-05-31).
>
> Add a 'git_ls_files_config' function to read this configuration
> variable, and adjust the documentation and tests accordingly.

I am afraid that this will break existing scripts big time, and I
would not be surprised if 046b48239e refrained to do the equivalent
of this patch very much on purpose to avoid such a regression.

Anybody who writes a script using ls-files _without_ passing the
--recurse-submodules option expects that the ls-files command will
stop at submodule boundary without recursing, that the script can
notice and pick up mode 160000 entries in the output from ls-files,
and that the script can decide if it wants to descend into
submodules it discovered that way.

It is easy to imagine that such a script will break badly when run
by a user who has the configuration variable set, I would think.

It also is easy to imagine a script derived from "git-submodule.sh"
back from the time before we started rewriting pieces of it in C.
The main "discover and list the immediate submodules we have" code
was ls-files piped to a loop that picks up entries with mode 160000,
and it was used to drive all the "git submodule" subcommands.  As
the scripted version was not the world's greatest code, it is quite
plausible that somebody forked and improved it, without feeding the
changes back to us.  Such a script is also a candidate to be broken
with this patch.

So, no.  I am not enthused to see this change.

  reply	other threads:[~2020-09-10  4:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10  3:07 [PATCH] ls-files: respect 'submodule.recurse' config Philippe Blain via GitGitGadget
2020-09-10  4:25 ` Junio C Hamano [this message]
2020-09-10  4:50   ` Junio C Hamano
2020-09-11 13:11     ` Philippe Blain
2020-09-11 14:30     ` Philippe Blain
2020-09-11 13:05   ` Philippe Blain
2020-09-11 19:19     ` Junio C Hamano
2020-09-13 10:47     ` Đoàn Trần Công Danh
2020-09-13 10:51       ` Đoàn Trần Công Danh
2020-09-11 13:48   ` Philippe Blain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv9gms1fp.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).