[PATCH] doc: remove mentions of .gitmodules !command syntax

[PATCH] doc: remove mentions of .gitmodules !command syntax

[pvutov]

From: Petar Vutov <pvutov@imap.cc>

To mitigate CVE-2019-19604, the capability to configure
`git submodule update` to execute custom commands was
removed in v2.20.2.

The git-submodule documentation still mentions the now-unsupported
syntax, which is misleading.

Remove the leftover documentation.

Signed-off-by: Petar Vutov <pvutov@imap.cc>
---
 Documentation/git-submodule.txt | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4d3ab6b9f9..b40ac72f75 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -163,12 +163,6 @@ checked out in the submodule.
 The following 'update' procedures are only available via the
 `submodule.<name>.update` configuration variable:
 
-	custom command;; arbitrary shell command that takes a single
-	    argument (the sha1 of the commit recorded in the
-	    superproject) is executed. When `submodule.<name>.update`
-	    is set to '!command', the remainder after the exclamation mark
-	    is the custom command.
-
 	none;; the submodule is not updated.
 
 If the submodule is not yet initialized, and you just want to use the
-- 
2.41.0

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Junio C Hamano]

pvutov@imap.cc writes:

> From: Petar Vutov <pvutov@imap.cc>
>
> To mitigate CVE-2019-19604, the capability to configure
> `git submodule update` to execute custom commands was
> removed in v2.20.2.
>
> The git-submodule documentation still mentions the now-unsupported
> syntax, which is misleading.
>
> Remove the leftover documentation.

The change during v2.20.2 timeperiod you have in mind may be
e904deb8 (submodule: reject submodule.update = !command in
.gitmodules, 2019-12-05).  The key phrase is "in .gitmodules"
as it did not forbid writing update command in the configuration.

The pre-context lines of your patch (see below) say that the 'custom
command' option and 'none' option are only available via the
`submodule.<name>.update` configuration variable.  IOW, this part of
the documentation does not talk about the .gitmodules file---it
talks about what you can say in the configuration file (which is
under your local control).

I think the existing text that came from fc01a5d2 (submodule update
documentation: don't repeat ourselves, 2016-12-27) may be
misleading, and may has room for improvement, but I do not think it
is wrong per-se.  If we remove it, there is nowhere else that teaches
users !cmd can be set in their configuration files, or is there?

Thanks.

> Signed-off-by: Petar Vutov <pvutov@imap.cc>
> ---
>  Documentation/git-submodule.txt | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 4d3ab6b9f9..b40ac72f75 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -163,12 +163,6 @@ checked out in the submodule.
>  The following 'update' procedures are only available via the
>  `submodule.<name>.update` configuration variable:
>  
> -	custom command;; arbitrary shell command that takes a single
> -	    argument (the sha1 of the commit recorded in the
> -	    superproject) is executed. When `submodule.<name>.update`
> -	    is set to '!command', the remainder after the exclamation mark
> -	    is the custom command.
> -
>  	none;; the submodule is not updated.
>  
>  If the submodule is not yet initialized, and you just want to use the

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Petar Vutov]

On 7/12/23 18:40, Junio C Hamano wrote:
> pvutov@imap.cc writes:
> 
>> From: Petar Vutov <pvutov@imap.cc>
>>
>> To mitigate CVE-2019-19604, the capability to configure
>> `git submodule update` to execute custom commands was
>> removed in v2.20.2.
>>
>> The git-submodule documentation still mentions the now-unsupported
>> syntax, which is misleading.
>>
>> Remove the leftover documentation.
> 
> The change during v2.20.2 timeperiod you have in mind may be
> e904deb8 (submodule: reject submodule.update = !command in
> .gitmodules, 2019-12-05).  The key phrase is "in .gitmodules"
> as it did not forbid writing update command in the configuration.
> 
> The pre-context lines of your patch (see below) say that the 'custom
> command' option and 'none' option are only available via the
> `submodule.<name>.update` configuration variable.  IOW, this part of
> the documentation does not talk about the .gitmodules file---it
> talks about what you can say in the configuration file (which is
> under your local control).
> 
> I think the existing text that came from fc01a5d2 (submodule update
> documentation: don't repeat ourselves, 2016-12-27) may be
> misleading, and may has room for improvement, but I do not think it
> is wrong per-se.  If we remove it, there is nowhere else that teaches
> users !cmd can be set in their configuration files, or is there?
> 
> Thanks.

Thanks for the review. I was not aware of the .gitconfig use case.
I hit that paragraph while trying to enforce sparse-checkout
via .gitmodules. Yet the gitmodules doc is clear enough:

"See description of update command in git-submodule[1] for their 
meaning. For security reasons, the !command form is not accepted here."

Clearly I followed the link in the first sentence without reading the 
second :)

Perhaps the term "configuration variable" in

"The following update procedures are only available via the 
submodule.<name>.update configuration variable:"

is more specific and technical than immediately obvious - I would have 
expected the contents of .gitmodules to be a form of (repository) 
configuration. But that is just bikeshedding.

> 
>> Signed-off-by: Petar Vutov <pvutov@imap.cc>
>> ---
>>   Documentation/git-submodule.txt | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 4d3ab6b9f9..b40ac72f75 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -163,12 +163,6 @@ checked out in the submodule.
>>   The following 'update' procedures are only available via the
>>   `submodule.<name>.update` configuration variable:
>>   
>> -	custom command;; arbitrary shell command that takes a single
>> -	    argument (the sha1 of the commit recorded in the
>> -	    superproject) is executed. When `submodule.<name>.update`
>> -	    is set to '!command', the remainder after the exclamation mark
>> -	    is the custom command.
>> -
>>   	none;; the submodule is not updated.
>>   
>>   If the submodule is not yet initialized, and you just want to use the

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> Perhaps the term "configuration variable" in
>
> "The following update procedures are only available via the
> submodule.<name>.update configuration variable:"
>
> is more specific and technical than immediately obvious - I would have
> expected the contents of .gitmodules to be a form of (repository)
> configuration. But that is just bikeshedding.

Not necessarily.  It is always good to learn how "normal people"
(read: those other than who develop and write documentation for Git)
would react to and understand what we write in the manual pages, as
that is the only way we can find what is confusing, ambiguous and
can be improved.

Perhaps

    "... available via the ... configuration variable, and cannot be
    used in the .gitmodules file"

would have helped you?

Thanks.

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Petar Vutov]

On 7/12/23 19:54, Junio C Hamano wrote:
> 
> Perhaps
> 
>      "... available via the ... configuration variable, and cannot be
>      used in the .gitmodules file"
> 
> would have helped you?
> 
> Thanks.

I think that addition is good and makes the intent clearer.

But it's slightly wrong - only 'custom command' cannot be used in 
.gitmodules. 'none' is legal (and mentioned in the gitmodules doc as such).

What about something like this?

 From 7b2fcd9a56b4954863cc74e1cf89a4b9d9d3ad52 Mon Sep 17 00:00:00 2001
From: Petar Vutov <pvutov@imap.cc>
Date: Wed, 12 Jul 2023 20:40:10 +0200
Subject: [PATCH] docs: highlight that .gitmodules does not support !command

The `custom command` and `none` entries are described as sharing the
same limitations, but one is allowed in .gitmodules and the other is
not. Instead, describe their limitations separately and with slightly
more detail.

Signed-off-by: Petar Vutov <pvutov@imap.cc>
---
  Documentation/git-submodule.txt | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt 
b/Documentation/git-submodule.txt
index 4d3ab6b9f9..eb024a1531 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -160,16 +160,19 @@ checked out in the submodule.
  	merge;; the commit recorded in the superproject will be merged
  	    into the current branch in the submodule.

-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
-
  	custom command;; arbitrary shell command that takes a single
  	    argument (the sha1 of the commit recorded in the
  	    superproject) is executed. When `submodule.<name>.update`
  	    is set to '!command', the remainder after the exclamation mark
  	    is the custom command.
++
+Custom commands are only available via the `submodule.<name>.update`
+configuration variable. They cannot be used in the .gitmodules file.

  	none;; the submodule is not updated.
++
+The `none` update procedure is only available via the .gitmodules file
+or the `submodule.<name>.update` configuration variable.

  If the submodule is not yet initialized, and you just want to use the
  setting as stored in `.gitmodules`, you can automatically initialize the
-- 
2.41.0

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> On 7/12/23 19:54, Junio C Hamano wrote:
>> Perhaps
>>      "... available via the ... configuration variable, and cannot
>> be
>>      used in the .gitmodules file"
>> would have helped you?
>> Thanks.
>
> I think that addition is good and makes the intent clearer.
>
> But it's slightly wrong - only 'custom command' cannot be used in
> .gitmodules. 'none' is legal (and mentioned in the gitmodules doc as
> such).

Hmph, that will be a bugfix for fc01a5d2 (submodule update
documentation: don't repeat ourselves, 2016-12-27).  I think you are
technically correct.

    Side note: but is there a useful use case to set it to 'none' in
    ".gitmodules" in the first place?  If there is not, saying that
    "'none' is only useful in the configuration files" is not quite
    wrong per-se.

>  	merge;; the commit recorded in the superproject will be merged
>  	    into the current branch in the submodule.
>
> -The following 'update' procedures are only available via the
> -`submodule.<name>.update` configuration variable:
> -
>  	custom command;; arbitrary shell command that takes a single
>  	    argument (the sha1 of the commit recorded in the
>  	    superproject) is executed. When `submodule.<name>.update`
>  	    is set to '!command', the remainder after the exclamation mark
>  	    is the custom command.
> ++
> +Custom commands are only available via the `submodule.<name>.update`
> +configuration variable. They cannot be used in the .gitmodules file.

Sounds good.  s/available/allowed/, perhaps.

>  	none;; the submodule is not updated.
> ++
> +The `none` update procedure is only available via the .gitmodules file
> +or the `submodule.<name>.update` configuration variable.

This side we do not need "only" anywhere in the sentence, do we?

Thanks.

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Petar Vutov]

On 7/12/23 22:33, Junio C Hamano wrote:
> 
>      Side note: but is there a useful use case to set it to 'none' in
>      ".gitmodules" in the first place?  If there is not, saying that
>      "'none' is only useful in the configuration files" is not quite
>      wrong per-se.

A twisted person might use it to commit a truly empty folder, without 
even a .gitkeep file. Whether that's a useful use case, I don't know.

> 
> Sounds good.  s/available/allowed/, perhaps.
> 
>>   	none;; the submodule is not updated.
>> ++
>> +The `none` update procedure is only available via the .gitmodules file
>> +or the `submodule.<name>.update` configuration variable.
> 
> This side we do not need "only" anywhere in the sentence, do we?
> 
> Thanks.

I will address both of those and include your bugfix blurb in v3. Still 
figuring out how to properly reference the previous patches using 
git-send-email..

Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> I will address both of those and include your bugfix blurb in
> v3. Still figuring out how to properly reference the previous patches
> using git-send-email..

$ git send-email --in-reply-to='<7090349c-4485-d5c4-1f26-190974864f72@imap.cc>' ...

[PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[pvutov]

From: Petar Vutov <pvutov@imap.cc>

Bugfix for fc01a5d2 (submodule update documentation: don't repeat
ourselves, 2016-12-27).

The `custom command` and `none` entries are described as sharing the
same limitations, but one is allowed in .gitmodules and the other is
not. Instead, describe their limitations separately and in slightly
more detail.

Signed-off-by: Petar Vutov <pvutov@imap.cc>
---

Changes from v1:
Don't delete the documentation for `!command`. Instead, highlight
the differences in the limitations of `none` and `!command`.

Changes from v2:
Improve phrasing.
Add the bugfix blurb in the commit message.

 Documentation/git-submodule.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4d3ab6b9f9..69ee2cd6b0 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -160,16 +160,19 @@ checked out in the submodule.
 	merge;; the commit recorded in the superproject will be merged
 	    into the current branch in the submodule.
 
-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
-
 	custom command;; arbitrary shell command that takes a single
 	    argument (the sha1 of the commit recorded in the
 	    superproject) is executed. When `submodule.<name>.update`
 	    is set to '!command', the remainder after the exclamation mark
 	    is the custom command.
++
+Custom commands are only allowed in the `submodule.<name>.update`
+git-config variable. They cannot be used in the .gitmodules file.
 
 	none;; the submodule is not updated.
++
+The `none` update procedure is allowed in the .gitmodules file
+or the `submodule.<name>.update` git-config variable.
 
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in `.gitmodules`, you can automatically initialize the
-- 
2.41.0

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

pvutov@imap.cc writes:

> From: Petar Vutov <pvutov@imap.cc>
>
> Bugfix for fc01a5d2 (submodule update documentation: don't repeat
> ourselves, 2016-12-27).
>
> The `custom command` and `none` entries are described as sharing the
> same limitations, but one is allowed in .gitmodules and the other is
> not. Instead, describe their limitations separately and in slightly
> more detail.

Sounds sensible.

>
> Signed-off-by: Petar Vutov <pvutov@imap.cc>
> ---
>
> Changes from v1:
> Don't delete the documentation for `!command`. Instead, highlight
> the differences in the limitations of `none` and `!command`.
>
> Changes from v2:
> Improve phrasing.
> Add the bugfix blurb in the commit message.
>
>  Documentation/git-submodule.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 4d3ab6b9f9..69ee2cd6b0 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -160,16 +160,19 @@ checked out in the submodule.
>  	merge;; the commit recorded in the superproject will be merged
>  	    into the current branch in the submodule.
>  
> -The following 'update' procedures are only available via the
> -`submodule.<name>.update` configuration variable:
> -
>  	custom command;; arbitrary shell command that takes a single
>  	    argument (the sha1 of the commit recorded in the
>  	    superproject) is executed. When `submodule.<name>.update`
>  	    is set to '!command', the remainder after the exclamation mark
>  	    is the custom command.
> ++
> +Custom commands are only allowed in the `submodule.<name>.update`
> +git-config variable. They cannot be used in the .gitmodules file.
>  
>  	none;; the submodule is not updated.
> ++
> +The `none` update procedure is allowed in the .gitmodules file
> +or the `submodule.<name>.update` git-config variable.

But the usual ones like "merge" are also allowed in both places.
Does this still need to be said?

I wonder if it makes more sense to swap the order of these two
entries, showing "none" without any additional text first, and
then describe "custom command" with the note, exactly like you
did above.

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Petar Vutov]

Oops, this was supposed to go under 
<7090349c-4485-d5c4-1f26-190974864f72@imap.cc>..

Side question in this side thread: I was tempted to change the mention 
of "configuration variable" with "git-config variable", to highlight the 
narrow meaning of the term. But I grepped and that term is used 
everywhere in the documentation. Changing it only in this section would 
be inconsistent.

If I were to go through all those mentions and figure out which 
"configuration variable" mentions refer exclusively to `git-config`, wis 
there interest in changing them to something like "git-config variable" 
in a potential future patch? Or do we like the term "configuration 
variable"?

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> Oops, this was supposed to go under
> <7090349c-4485-d5c4-1f26-190974864f72@imap.cc>..
>
> Side question in this side thread: I was tempted to change the mention
> of "configuration variable" with "git-config variable", to highlight
> the narrow meaning of the term. But I grepped and that term is used
> everywhere in the documentation. Changing it only in this section
> would be inconsistent.

Don't.  People should be familiar with "configuration variable", but
may not be with "git-config variable".

How about doing it this way?  I moved 'none' up, as it is effective
in `.gitmodules` and as a configuration variable, just like all
others, and then completely rewrote the somewhat awkward explanation
of the custom command thing.

----- >8 --------- >8 --------- >8 --------- >8 ----
Subject: submodule: clarify that "!custom command" is the only oddball

We singled out 'none' and 'custom command' as submodule update modes
that cannot be specified in the .gitmodules file, but 'none' can
appear there, and use of a custom command is the only oddball.

Move the description of 'none' up, clarify how the custom command is
used, and explicitly say it cannot be used in the `.gitmodules` file.

Strictly speaking, the last one should not be needed, as we already
say `configuration variable`, but to new readers, the distinction
between the configuration variable and settings that appear in the
.gitmodules file may not be apparent; hopefully the new text will
help them understand where it can(not) be used.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-submodule.txt | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git c/Documentation/git-submodule.txt w/Documentation/git-submodule.txt
index 4d3ab6b9f9..391ff0dbf2 100644
--- c/Documentation/git-submodule.txt
+++ w/Documentation/git-submodule.txt
@@ -160,17 +160,15 @@ checked out in the submodule.
 	merge;; the commit recorded in the superproject will be merged
 	    into the current branch in the submodule.
 
-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
-
-	custom command;; arbitrary shell command that takes a single
-	    argument (the sha1 of the commit recorded in the
-	    superproject) is executed. When `submodule.<name>.update`
-	    is set to '!command', the remainder after the exclamation mark
-	    is the custom command.
-
 	none;; the submodule is not updated.
 
+	custom command;; When the `submodule.<name>.update`
+	    configuration variable is set to `!custom command`, the
+	    object name of the commit recorded in the superproject
+	    for the submodule is appended to the `custom command`
+	    string and gets executed.  Note that this mechanism
+	    cannot be used in the `.gitmodules` file.
+
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in `.gitmodules`, you can automatically initialize the
 submodule with the `--init` option.

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Petar Vutov]

On 7/13/23 21:55, Junio C Hamano wrote:
> Petar Vutov <pvutov@imap.cc> writes:
> 
> Don't.  People should be familiar with "configuration variable", but
> may not be with "git-config variable".

Right, on closer inspection this collision with .gitmodules is rather 
niche to start with. Few things in .gitconfig are also legal in .gitmodules.

>   
> -The following 'update' procedures are only available via the
> -`submodule.<name>.update` configuration variable:
> -
> -	custom command;; arbitrary shell command that takes a single
> -	    argument (the sha1 of the commit recorded in the
> -	    superproject) is executed. When `submodule.<name>.update`
> -	    is set to '!command', the remainder after the exclamation mark
> -	    is the custom command.
> -
>   	none;; the submodule is not updated.
>   
> +	custom command;; When the `submodule.<name>.update`
> +	    configuration variable is set to `!custom command`, the
> +	    object name of the commit recorded in the superproject
> +	    for the submodule is appended to the `custom command`
> +	    string and gets executed.  Note that this mechanism
> +	    cannot be used in the `.gitmodules` file.
> +
>   If the submodule is not yet initialized, and you just want to use the
>   setting as stored in `.gitmodules`, you can automatically initialize the
>   submodule with the `--init` option.

I prefer the original description as it uses shorter sentences. I can't 
hold that 5-liner in my head :)

But now I really am bikeshedding, and I've taken enough of your time. I 
will send a v4 with just the .gitmodules disclaimer, no rewrite, in case 
you end up agreeing.

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> I prefer the original description as it uses shorter sentences. I
> can't hold that 5-liner in my head :)

But you are comparing oranges and apples, aren't you?  You are not
counting "oh by the way this cannot be in .gitmodules" as part of 5.

> But now I really am bikeshedding, and I've taken enough of your
> time. I will send a v4 with just the .gitmodules disclaimer, no
> rewrite, in case you end up agreeing.

After reading the original again and again, I hate more and more the
way the original wasted too many words to refer to one thing twice
(e.g. "arbitrary command" and then "is the custom command", "a
single argument" and then the parenthesized explanation of it)
without adding any clarity.

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

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

> After reading the original again and again, I hate more and more the
> way the original wasted too many words to refer to one thing twice
> (e.g. "arbitrary command" and then "is the custom command", "a
> single argument" and then the parenthesized explanation of it)
> without adding any clarity.

Another thing I noticed is that this section is ONLY talking about
the configuration variable, so everything both of us have been
saying misses the point.  The preamble before the choices
("checkout", "rebase", "merge", "custom" and "none") are listed read
like so:

    update [--init] [--remote] [-N|--no-f...
    +
    --
    Update the registered submodules to match what the superproject
    expects by cloning missing submodules, fetching missing commits
    in submodules and updating the working tree of
    the submodules. The "updating" can be done in several ways depending
    on command line options and the value of `submodule.<name>.update`
    configuration variable. The command line option takes precedence over
    the configuration variable. If neither is given, a 'checkout' is performed.
    The 'update' procedures supported both from the command line as well as
    through the `submodule.<name>.update` configuration are:

Then why do we even think about referring to ".gitmodules" here?  It
is because of this behaviour that is described elsewhere:

    init [--] [<path>...]::
            Initialize the submodules recorded in the index (which were
            added and committed elsewhere) by setting `submodule.$name.url`
            in .git/config. It uses the same setting from `.gitmodules` as
            a template. If the URL is relative, it will be resolved using
            the default remote. If there is no default remote, the current
            repository will be assumed to be upstream.
    +
    Optional <path> arguments limit which submodules will be initialized.
    If no path is specified and submodule.active has been configured, submodules
    configured to be active will be initialized, otherwise all submodules are
    initialized.
    +
    When present, it will also copy the value of `submodule.$name.update`.

and this description is very flawed.  It says "X is copied", but not
"X is copied from A to B".  It also does not say that even if you
have a custom command there, it does not get copied.

It copies submodule.<name>.update from the ".gitmodules" to the
configuraiton.  And that is the reason why, the configuration may
have "submodule.<name>.update" in it after running "git init" to
initialize a submodule.  And that is why the choices of update
methods listed in the part your patch touched talked about things
that can both appear in .gitmodules and the configuration.  But the
linkage is quite indirect.

So, here is another round, this time the primary change is to stop
talking about `.gitmodules` in the "update" section, but explain how
`.gitmodules` file is used in the "init" section.

 Documentation/git-submodule.txt | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git c/Documentation/git-submodule.txt w/Documentation/git-submodule.txt
index 4d3ab6b9f9..5248840b18 100644
--- c/Documentation/git-submodule.txt
+++ w/Documentation/git-submodule.txt
@@ -95,7 +95,7 @@ too (and can also report changes to a submodule's work tree).
 init [--] [<path>...]::
 	Initialize the submodules recorded in the index (which were
 	added and committed elsewhere) by setting `submodule.$name.url`
-	in .git/config. It uses the same setting from `.gitmodules` as
+	in `.git/config`, using the same setting from `.gitmodules` as
 	a template. If the URL is relative, it will be resolved using
 	the default remote. If there is no default remote, the current
 	repository will be assumed to be upstream.
@@ -105,9 +105,12 @@ If no path is specified and submodule.active has been configured, submodules
 configured to be active will be initialized, otherwise all submodules are
 initialized.
 +
-When present, it will also copy the value of `submodule.$name.update`.
-This command does not alter existing information in .git/config.
-You can then customize the submodule clone URLs in .git/config
+It will also copy the value of `submodule.$name.update`, if present in
+the `.gitmodules` file, to `.git/config`, but (1) this command does not
+alter existing information in `.git/config`, and (2) `submodule.$name.update`
+that is set to a custom command is *not* copied for security reasons.
++
+You can then customize the submodule clone URLs in `.git/config`
 for your local setup and proceed to `git submodule update`;
 you can also just use `git submodule update --init` without
 the explicit 'init' step if you do not intend to customize
@@ -143,6 +146,8 @@ the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule.<name>.update`
 configuration variable. The command line option takes precedence over
 the configuration variable. If neither is given, a 'checkout' is performed.
+(note: what is in `.gitmodules` file is irrelevant at this point;
+see `git submodule init` above for how `.gitmodules` is used).
 The 'update' procedures supported both from the command line as well as
 through the `submodule.<name>.update` configuration are:
 
@@ -160,9 +165,6 @@ checked out in the submodule.
 	merge;; the commit recorded in the superproject will be merged
 	    into the current branch in the submodule.
 
-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
-
 	custom command;; arbitrary shell command that takes a single
 	    argument (the sha1 of the commit recorded in the
 	    superproject) is executed. When `submodule.<name>.update`

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Petar Vutov]

Thanks. I'll digest this and reply tomorrow. Until then, I remembered 
something:

On 7/13/23 23:37, Junio C Hamano wrote:
>   
> -The following 'update' procedures are only available via the
> -`submodule.<name>.update` configuration variable:
> -

Originally I was trying to write a disclaimer about `none` because the 
current documentation we're removing seems to be trying to say something 
like this:

"The following `update` procedures are not supported via the command 
line: none, custom command"

It's referencing this bit at the top:

 >   The 'update' procedures supported both from the command line as well as
 >   through the `submodule.<name>.update` configuration are:

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> Thanks. I'll digest this and reply tomorrow.

Thanks.

> "The following `update` procedures are not supported via the command
> line: none, custom command"

Ah, that is a good thing to notice.  Yes, the command line override
is also something we need to explain.

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Petar Vutov]

On 7/13/23 23:37, Junio C Hamano wrote:
> So, here is another round, this time the primary change is to stop
> talking about `.gitmodules` in the "update" section, but explain how
> `.gitmodules` file is used in the "init" section.

Looks good to me.

I applied your patch and then made some additions, which I'll append at 
the end of this message. Summary:

* Added your rewrite from yesterday. I like that it's more precise than 
the current docs, but I struggle with the complexity of the first 
sentence. I wanted to make it easier to follow by splitting it in two 
somehow. Instead I ended up with an introductory sentence summarizing 
the functionality with small words.

* In gitmodules.txt, moved the security disclaimer so people are more 
likely to see it before they follow the link to git-submodule.txt.

* Explicitly called out `none` and `custom command` as being unusable on 
the command line (and in .gitmodules). I expect that you won't want that 
.gitmodules mention anymore, but I left it in for now, up to you.

By the way, I ran into an SO question where some people were discussing 
the same issue: https://stackoverflow.com/q/65744067/876832

The diff below is based on top of your patch from yesterday, since I 
assume that you don't want to look at a mishmash of both patches.

---
  Documentation/git-submodule.txt | 19 ++++++++++++-------
  Documentation/gitmodules.txt    |  6 +++---
  2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-submodule.txt 
b/Documentation/git-submodule.txt
index 5248840b18..695730609a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -165,13 +165,18 @@ checked out in the submodule.
  	merge;; the commit recorded in the superproject will be merged
  	    into the current branch in the submodule.

-	custom command;; arbitrary shell command that takes a single
-	    argument (the sha1 of the commit recorded in the
-	    superproject) is executed. When `submodule.<name>.update`
-	    is set to '!command', the remainder after the exclamation mark
-	    is the custom command.
-
-	none;; the submodule is not updated.
+The following update procedures have additional limitations:
+
+	custom command;; mechanism for running arbitrary commands with the
+	    commit ID as an argument. Specifically, if the
+	    `submodule.<name>.update` configuration variable is set to
+	    `!custom command`, the object name of the commit recorded in the
+	    superproject for the submodule is appended to the `custom command`
+	    string and executed. Note that this mechanism is not supported in
+	    the `.gitmodules` file or on the command line.
+
+	none;; the submodule is not updated. This update procedure is not
+	    allowed on the command line.

  If the submodule is not yet initialized, and you just want to use the
  setting as stored in `.gitmodules`, you can automatically initialize the
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index dcee09b500..d9bec8b187 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -43,9 +43,9 @@ submodule.<name>.update::
  	command in the superproject. This is only used by `git
  	submodule init` to initialize the configuration variable of
  	the same name. Allowed values here are 'checkout', 'rebase',
-	'merge' or 'none'. See description of 'update' command in
-	linkgit:git-submodule[1] for their meaning. For security
-	reasons, the '!command' form is not accepted here.
+	'merge' or 'none', but not '!command' (for security reasons).
+	See the description of the 'update' command in
+	linkgit:git-submodule[1] for more details.

  submodule.<name>.branch::
  	A remote branch name for tracking updates in the upstream submodule.
-- 
2.41.0

Re: [PATCH v3 1/1] docs: highlight that .gitmodules does not support !command

[Junio C Hamano]

Petar Vutov <pvutov@imap.cc> writes:

> I applied your patch and then made some additions, which I'll append
> at the end of this message. Summary:
>
> * Added your rewrite from yesterday. I like that it's more precise
>   than the current docs, but I struggle with the complexity of the
>   first sentence. I wanted to make it easier to follow by splitting it
>   in two somehow. Instead I ended up with an introductory sentence
>   summarizing the functionality with small words.
>
> * In gitmodules.txt, moved the security disclaimer so people are more
>   likely to see it before they follow the link to git-submodule.txt.
>
> * Explicitly called out `none` and `custom command` as being unusable
>   on the command line (and in .gitmodules). I expect that you won't
>   want that .gitmodules mention anymore, but I left it in for now, up
>  to you.
>
> The diff below is based on top of your patch from yesterday, since I
> assume that you don't want to look at a mishmash of both patches.

Sorry for a very slow response; I seem to have missed this one.

The end result looks good.  Care to wrap it up into a single patch
(iow, make a "mishmash of both patches") with a good log message so
we can move the topic forward?

Thanks.

> diff --git a/Documentation/git-submodule.txt
> b/Documentation/git-submodule.txt
> index 5248840b18..695730609a 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -165,13 +165,18 @@ checked out in the submodule.
>  	merge;; the commit recorded in the superproject will be merged
>  	    into the current branch in the submodule.
>
> -	custom command;; arbitrary shell command that takes a single
> -	    argument (the sha1 of the commit recorded in the
> -	    superproject) is executed. When `submodule.<name>.update`
> -	    is set to '!command', the remainder after the exclamation mark
> -	    is the custom command.
> -
> -	none;; the submodule is not updated.
> +The following update procedures have additional limitations:
> +
> +	custom command;; mechanism for running arbitrary commands with the
> +	    commit ID as an argument. Specifically, if the
> +	    `submodule.<name>.update` configuration variable is set to
> +	    `!custom command`, the object name of the commit recorded in the
> +	    superproject for the submodule is appended to the `custom command`
> +	    string and executed. Note that this mechanism is not supported in
> +	    the `.gitmodules` file or on the command line.
> +
> +	none;; the submodule is not updated. This update procedure is not
> +	    allowed on the command line.
>
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in `.gitmodules`, you can automatically initialize the
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index dcee09b500..d9bec8b187 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -43,9 +43,9 @@ submodule.<name>.update::
>  	command in the superproject. This is only used by `git
>  	submodule init` to initialize the configuration variable of
>  	the same name. Allowed values here are 'checkout', 'rebase',
> -	'merge' or 'none'. See description of 'update' command in
> -	linkgit:git-submodule[1] for their meaning. For security
> -	reasons, the '!command' form is not accepted here.
> +	'merge' or 'none', but not '!command' (for security reasons).
> +	See the description of the 'update' command in
> +	linkgit:git-submodule[1] for more details.
>
>  submodule.<name>.branch::
>  	A remote branch name for tracking updates in the upstream submodule.

[PATCH v5] doc: highlight that .gitmodules does not support !command

[pvutov]

From: Petar Vutov <pvutov@imap.cc>

Bugfix for fc01a5d2 (submodule update documentation: don't repeat
ourselves, 2016-12-27).

The `custom command` and `none` options are described as sharing the
same limitations, but one is allowed in .gitmodules and the other is
not.

Rewrite the description for custom commands to be more precise,
and make it easier for readers to notice that custom commands cannot
be used in the .gitmodules file.

Signed-off-by: Petar Vutov <pvutov@imap.cc>
---
Combines the custom command description rewrite by Junio C Hamano and
my proposed changes into a single patch.

Apologies for the "From:" line at the top - I don't know if it is safe to
remove it, and I can't figure out how set my name in git-send-email so
that the line is not automatically added.

 Documentation/git-submodule.txt | 31 +++++++++++++++++++------------
 Documentation/gitmodules.txt    |  6 +++---
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4d3ab6b9f9..695730609a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -95,7 +95,7 @@ too (and can also report changes to a submodule's work tree).
 init [--] [<path>...]::
 	Initialize the submodules recorded in the index (which were
 	added and committed elsewhere) by setting `submodule.$name.url`
-	in .git/config. It uses the same setting from `.gitmodules` as
+	in `.git/config`, using the same setting from `.gitmodules` as
 	a template. If the URL is relative, it will be resolved using
 	the default remote. If there is no default remote, the current
 	repository will be assumed to be upstream.
@@ -105,9 +105,12 @@ If no path is specified and submodule.active has been configured, submodules
 configured to be active will be initialized, otherwise all submodules are
 initialized.
 +
-When present, it will also copy the value of `submodule.$name.update`.
-This command does not alter existing information in .git/config.
-You can then customize the submodule clone URLs in .git/config
+It will also copy the value of `submodule.$name.update`, if present in
+the `.gitmodules` file, to `.git/config`, but (1) this command does not
+alter existing information in `.git/config`, and (2) `submodule.$name.update`
+that is set to a custom command is *not* copied for security reasons.
++
+You can then customize the submodule clone URLs in `.git/config`
 for your local setup and proceed to `git submodule update`;
 you can also just use `git submodule update --init` without
 the explicit 'init' step if you do not intend to customize
@@ -143,6 +146,8 @@ the submodules. The "updating" can be done in several ways depending
 on command line options and the value of `submodule.<name>.update`
 configuration variable. The command line option takes precedence over
 the configuration variable. If neither is given, a 'checkout' is performed.
+(note: what is in `.gitmodules` file is irrelevant at this point;
+see `git submodule init` above for how `.gitmodules` is used).
 The 'update' procedures supported both from the command line as well as
 through the `submodule.<name>.update` configuration are:
 
@@ -160,16 +165,18 @@ checked out in the submodule.
 	merge;; the commit recorded in the superproject will be merged
 	    into the current branch in the submodule.
 
-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
+The following update procedures have additional limitations:
 
-	custom command;; arbitrary shell command that takes a single
-	    argument (the sha1 of the commit recorded in the
-	    superproject) is executed. When `submodule.<name>.update`
-	    is set to '!command', the remainder after the exclamation mark
-	    is the custom command.
+	custom command;; mechanism for running arbitrary commands with the
+	    commit ID as an argument. Specifically, if the
+	    `submodule.<name>.update` configuration variable is set to
+	    `!custom command`, the object name of the commit recorded in the
+	    superproject for the submodule is appended to the `custom command`
+	    string and executed. Note that this mechanism is not supported in
+	    the `.gitmodules` file or on the command line.
 
-	none;; the submodule is not updated.
+	none;; the submodule is not updated. This update procedure is not
+	    allowed on the command line.
 
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in `.gitmodules`, you can automatically initialize the
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index dcee09b500..d9bec8b187 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -43,9 +43,9 @@ submodule.<name>.update::
 	command in the superproject. This is only used by `git
 	submodule init` to initialize the configuration variable of
 	the same name. Allowed values here are 'checkout', 'rebase',
-	'merge' or 'none'. See description of 'update' command in
-	linkgit:git-submodule[1] for their meaning. For security
-	reasons, the '!command' form is not accepted here.
+	'merge' or 'none', but not '!command' (for security reasons).
+	See the description of the 'update' command in
+	linkgit:git-submodule[1] for more details.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.41.0

Re: [PATCH v5] doc: highlight that .gitmodules does not support !command

[Junio C Hamano]

pvutov@imap.cc writes:

> From: Petar Vutov <pvutov@imap.cc>
>
> Bugfix for fc01a5d2 (submodule update documentation: don't repeat
> ourselves, 2016-12-27).
>
> The `custom command` and `none` options are described as sharing the
> same limitations, but one is allowed in .gitmodules and the other is
> not.
>
> Rewrite the description for custom commands to be more precise,
> and make it easier for readers to notice that custom commands cannot
> be used in the .gitmodules file.
>
> Signed-off-by: Petar Vutov <pvutov@imap.cc>
> ---
> Combines the custom command description rewrite by Junio C Hamano and
> my proposed changes into a single patch.

Thanks.  Willl queue.  Let's move it forward.

[PATCH v4] docs: highlight that .gitmodules does not support !command

[pvutov]

From: Petar Vutov <pvutov@imap.cc>

Bugfix for fc01a5d2 (submodule update documentation: don't repeat
ourselves, 2016-12-27).

The `custom command` and `none` entries are described as sharing the
same limitations, but one is allowed in .gitmodules and the other is
not. Instead, highlight the limitation of `custom command` only.

Signed-off-by: Petar Vutov <pvutov@imap.cc>
---

Changes from v1:
Don't delete the documentation for `!command`. Instead, highlight
the differences in the limitations of `none` and `!command`.

Changes from v2:
Improve phrasing.
Add the bugfix blurb in the commit message.

Changes from v3:
Keep the description of `none` unchanged and just move it up.

 Documentation/git-submodule.txt | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4d3ab6b9f9..b3303afb8f 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -160,16 +160,14 @@ checked out in the submodule.
 	merge;; the commit recorded in the superproject will be merged
 	    into the current branch in the submodule.
 
-The following 'update' procedures are only available via the
-`submodule.<name>.update` configuration variable:
+	none;; the submodule is not updated.
 
 	custom command;; arbitrary shell command that takes a single
 	    argument (the sha1 of the commit recorded in the
 	    superproject) is executed. When `submodule.<name>.update`
 	    is set to '!command', the remainder after the exclamation mark
-	    is the custom command.
-
-	none;; the submodule is not updated.
+	    is the custom command. Note that this mechanism
+	    cannot be used in the .gitmodules file.
 
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in `.gitmodules`, you can automatically initialize the
-- 
2.41.0