| Message ID | 20230713193342.1053968-1-pvutov@imap.cc (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/1] docs: highlight that .gitmodules does not support !command | expand |
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.
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"?
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.
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.
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.
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`
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:
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.
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.
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.
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