Re: [PATCH] submodule: Fix documentation of update subcommand
Jens Lehmann writes: > Yup, but we should also mention '--merge' and '--rebase' here. This has been sitting in the Stalled pile for quite a while and I am getting tired of waiting. How does this look? -- >8 -- From: Michal Sojka Date: Mon, 3 Nov 2014 11:09:51 +0100 Subject: [PATCH] submodule: clarify documentation for update subcommand e6a1c43a (document submdule.$name.update=none option for gitmodules, 2012-05-10) meant to say "Unlike the case where your .update configuration is set to either 'rebase' or 'merge', when it is set to 'none', the tip of the submodule would never move. You can use the --checkout option if you want the contents of the submodule to be updated to some other commit." But the resulting text made it sound as if using "--checkout" would have no effect when .update configuration is set to 'rebase' or 'merge', which was misleading. In fact, with the "--checkout" option, the tip of the submodule moves to the exact commit that is recorded in the superproject tree, regardless of .update configuration. Signed-off-by: Michal Sojka Signed-off-by: Junio C Hamano --- Documentation/git-submodule.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..9bfcdf5 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,8 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. The configuration can be overridden by + specifying `--rebase`, `--merge`, or `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. -- 2.3.0-301-g71e72fe -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Jens Lehmann writes: > So I believe that gitmodules.txt should describe all ćonfig > options that can be provided by upstream (and e.g. mention that > the 'url' and 'update' values are copied into .git/config on > init), while all settings that can be overridden locally should > be documented in config.txt (which will be a subset of those > documented in gitmodules.txt). Right; thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Am 04.11.2014 um 00:08 schrieb Junio C Hamano: Michal Sojka writes: Or something perhaps? Or the detailed description of submodule.$name.update should be dropped from here and refer the reader to config.txt instead? I guess you mean gitmodules.txt. Actually, I do mean the configuration. .gitmodules is just a template to help the user populate .git/config, and the latter of which should be the sole source of truth. This is an important principle, and it becomes even more important once we start talking about security sensitive possiblity like allowing !command as the value. Not quite. You're definitely right about the !command value for the 'update' setting; this should never be taken from .gitmodules but only from .git/config. But apart from that following this principle would hurt submodule users a lot. The only thing that should be set in stone in .git/config is the 'url' setting, because an older url might not even exist anmore. But e.g. the 'branch' setting must be taken from .gitmodules. Otherwise we could not change it on a per-superproject-branch basis. And if the 'path' setting would only be taken from .git/config instead of .gitmodules, we wouldn't even be able to rename submodules (which is exactly what this setting was added for in the first place). The same applies to 'ignore' and 'fetch'. So I believe that gitmodules.txt should describe all ćonfig options that can be provided by upstream (and e.g. mention that the 'url' and 'update' values are copied into .git/config on init), while all settings that can be overridden locally should be documented in config.txt (which will be a subset of those documented in gitmodules.txt). The `!command` form is not documented in gitmodules.txt. Maybe it would be best to fully document .update in gitmodules.txt and just refer to there. Having documentation at two places seems to be confusing not only for users, but also for those who send patches :) I'm no longer able to formulate my proposal properly as a patch tonight, but if needed I'll try it later. That is fine. People have lived with the current text for more than two years without problems, so we are obviously not in a hurry. Yup. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Michal Sojka writes: > This sounds good, but it doesn't mention the `!command` value of > .update. That part is unchanged by what I did. My rewrite was up to ... by specifying `--checkout`. of the existing text. >> Or something perhaps? Or the detailed description of >> submodule.$name.update should be dropped from here and refer the >> reader to config.txt instead? > > I guess you mean gitmodules.txt. Actually, I do mean the configuration. .gitmodules is just a template to help the user populate .git/config, and the latter of which should be the sole source of truth. This is an important principle, and it becomes even more important once we start talking about security sensitive possiblity like allowing !command as the value. > The `!command` form is not documented in gitmodules.txt. Maybe it would > be best to fully document .update in gitmodules.txt and just refer to > there. Having documentation at two places seems to be confusing not only > for users, but also for those who send patches :) > > I'm no longer able to formulate my proposal properly as a patch tonight, > but if needed I'll try it later. That is fine. People have lived with the current text for more than two years without problems, so we are obviously not in a hurry. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
On Mon, Nov 03 2014, Junio C Hamano wrote: > Jens Lehmann writes: > >> This was introduced in e6a1c43aaf (document submdule.$name.update=none >> option for gitmodules), and I agree with Michal that we should fix it. >> But I think we should rather say "This can be overridden by specifying >> '--merge', '--rebase' or `--checkout`." here, as the other two options >> also override the update setting. So I think we should queue this: >> >> diff --git a/Documentation/git-submodule.txt >> b/Documentation/git-submodule.txt >> index 8e6af65..84ab577 100644 >> --- a/Documentation/git-submodule.txt >> +++ b/Documentation/git-submodule.txt >> @@ -158,7 +158,7 @@ update:: >> checkout the commit specified in the index of the containing repository. >> This will make the submodules HEAD be detached unless `--rebase` or >> `--merge` is specified or the key `submodule.$name.update` is set to >> -`rebase`, `merge` or `none`. `none` can be overridden by specifying >> +`rebase`, `merge` or `none`. This can be overridden by using '--merge', >> +'--rebase' or >> `--checkout`. Setting the key `submodule.$name.update` to `!command` >> will cause `command` to be run. `command` can be any arbitrary shell >> command that takes a single argument, namely the sha1 to update to. >> >> Apart from that I'm all for it. > > But read the whole thing again. Isn't that a bit roundabout and > tortuous? > > The paragraph is about the "update" subcommand, and then mentions > how the subcommand is affected by options and configuration. And > "OVERRIDING" the topic of this thread is only about configuration. > > Disecting what each sentence in the existing paragraph says: > > - This is about updating the submodule working tree to match > what the superproject expects. > > - There can be three ways how it is "updated" (and one way to > leave it not updated), by setting submodule.$name.update > and/or giving --rebase, --merge or --checkout option, and one > way to leave it not "updated" by setting .update=none. > > - The .update=none can be defeated with --checkout > > which I think is a mess. > > It is a fairly common and uniform pattern that command line options > override configured defaults, so I think it could be argued that > "you can override .update=none or .update=anything with command line > option" is not even worth saying. Definitely not by piling yet > another "oh by the way, if you have this, things behave differently > again" on top of existing description. > > Update the registered submodules to match what the superproject > expects by cloning missing submodules and updating the > working tree of the submodules. The "updating" can take > various forms: > > (1) By default, or by explicitly giving `--checkout` option, > the HEAD of the submodules are detached to the exact > commit recorded by the superproject. > > (2) By giving `--rebase` or `--merge` option, the commit > that happens to be checked out in the submodule's > working tree is integrated with the commit recorded by > the superproject by rebasing or merging, respectively. > > Setting submodule.$name.update configuration to `rebase` or > `merge` will make `git submodule update` without these > command line options to default to `--rebase` or `--merge`, > respectively. > > Also, setting submodule.$name.update configuration to `none` > marks the named submodule not updated by "submodule update" > by default (you can still use `--checkout`, `--merge`, or > `--rebase`). This sounds good, but it doesn't mention the `!command` value of .update. I'd call this form (3). But then different update forms would mix config settings and command line options. > Or something perhaps? Or the detailed description of > submodule.$name.update should be dropped from here and refer the > reader to config.txt instead? I guess you mean gitmodules.txt. The `!command` form is not documented in gitmodules.txt. Maybe it would be best to fully document .update in gitmodules.txt and just refer to there. Having documentation at two places seems to be confusing not only for users, but also for those who send patches :) I'm no longer able to formulate my proposal properly as a patch tonight, but if needed I'll try it later. -Michal -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Jens Lehmann writes: > This was introduced in e6a1c43aaf (document submdule.$name.update=none > option for gitmodules), and I agree with Michal that we should fix it. > But I think we should rather say "This can be overridden by specifying > '--merge', '--rebase' or `--checkout`." here, as the other two options > also override the update setting. So I think we should queue this: > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index 8e6af65..84ab577 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -158,7 +158,7 @@ update:: > checkout the commit specified in the index of the containing repository. > This will make the submodules HEAD be detached unless `--rebase` or > `--merge` is specified or the key `submodule.$name.update` is set to > - `rebase`, `merge` or `none`. `none` can be overridden by specifying > + `rebase`, `merge` or `none`. This can be overridden by using '--merge', > + '--rebase' or > `--checkout`. Setting the key `submodule.$name.update` to `!command` > will cause `command` to be run. `command` can be any arbitrary shell > command that takes a single argument, namely the sha1 to update to. > > Apart from that I'm all for it. But read the whole thing again. Isn't that a bit roundabout and tortuous? The paragraph is about the "update" subcommand, and then mentions how the subcommand is affected by options and configuration. And "OVERRIDING" the topic of this thread is only about configuration. Disecting what each sentence in the existing paragraph says: - This is about updating the submodule working tree to match what the superproject expects. - There can be three ways how it is "updated" (and one way to leave it not updated), by setting submodule.$name.update and/or giving --rebase, --merge or --checkout option, and one way to leave it not "updated" by setting .update=none. - The .update=none can be defeated with --checkout which I think is a mess. It is a fairly common and uniform pattern that command line options override configured defaults, so I think it could be argued that "you can override .update=none or .update=anything with command line option" is not even worth saying. Definitely not by piling yet another "oh by the way, if you have this, things behave differently again" on top of existing description. Update the registered submodules to match what the superproject expects by cloning missing submodules and updating the working tree of the submodules. The "updating" can take various forms: (1) By default, or by explicitly giving `--checkout` option, the HEAD of the submodules are detached to the exact commit recorded by the superproject. (2) By giving `--rebase` or `--merge` option, the commit that happens to be checked out in the submodule's working tree is integrated with the commit recorded by the superproject by rebasing or merging, respectively. Setting submodule.$name.update configuration to `rebase` or `merge` will make `git submodule update` without these command line options to default to `--rebase` or `--merge`, respectively. Also, setting submodule.$name.update configuration to `none` marks the named submodule not updated by "submodule update" by default (you can still use `--checkout`, `--merge`, or `--rebase`). Or something perhaps? Or the detailed description of submodule.$name.update should be dropped from here and refer the reader to config.txt instead? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
On Mon, Nov 03 2014, Junio C Hamano wrote: > I did a bit more digging of the history, and came up with this, > which would be with a clearer and fairer description. Also to > clarify, I spelled what Michal's "This" meant to refer to. > > -- >8 -- > From: Michal Sojka > Date: Mon, 3 Nov 2014 11:09:51 +0100 > Subject: [PATCH] submodule: clarify documentation for update subcommand > > e6a1c43a (document submdule.$name.update=none option for gitmodules, > 2012-05-10) meant to say "Unlike the case where your .update > configuration is set to either 'rebase' or 'merge', when it is set > to 'none', the tip of the submodule would never move. You can use > the --checkout option if you want the contents of the submodule to > be updated to some other commit." > > But the resulting text made it sound as if using "--checkout" would > have no effect when .update configuration is set to 'rebase' or > 'merge', which was misleading. In fact, with the "--checkout" > option, the tip of the submodule moves to the exact commit that is > recorded in the superproject tree, regardless of .update > configuration. This is much better description than I was able to put together. Thanks. I also agree with Jens that mentioning --merge and --rebase is worthwhile. -Michal -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Am 03.11.2014 um 21:53 schrieb Junio C Hamano: I did a bit more digging of the history, and came up with this, which would be with a clearer and fairer description. Also to clarify, I spelled what Michal's "This" meant to refer to. -- >8 -- From: Michal Sojka Date: Mon, 3 Nov 2014 11:09:51 +0100 Subject: [PATCH] submodule: clarify documentation for update subcommand e6a1c43a (document submdule.$name.update=none option for gitmodules, 2012-05-10) meant to say "Unlike the case where your .update configuration is set to either 'rebase' or 'merge', when it is set to 'none', the tip of the submodule would never move. You can use the --checkout option if you want the contents of the submodule to be updated to some other commit." But the resulting text made it sound as if using "--checkout" would have no effect when .update configuration is set to 'rebase' or 'merge', which was misleading. In fact, with the "--checkout" option, the tip of the submodule moves to the exact commit that is recorded in the superproject tree, regardless of .update configuration. Signed-off-by: Michal Sojka Signed-off-by: Junio C Hamano --- Documentation/git-submodule.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..648323f 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,7 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. The configuration can be overridden by specifying `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. Yup, but we should also mention '--merge' and '--rebase' here. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
I did a bit more digging of the history, and came up with this, which would be with a clearer and fairer description. Also to clarify, I spelled what Michal's "This" meant to refer to. -- >8 -- From: Michal Sojka Date: Mon, 3 Nov 2014 11:09:51 +0100 Subject: [PATCH] submodule: clarify documentation for update subcommand e6a1c43a (document submdule.$name.update=none option for gitmodules, 2012-05-10) meant to say "Unlike the case where your .update configuration is set to either 'rebase' or 'merge', when it is set to 'none', the tip of the submodule would never move. You can use the --checkout option if you want the contents of the submodule to be updated to some other commit." But the resulting text made it sound as if using "--checkout" would have no effect when .update configuration is set to 'rebase' or 'merge', which was misleading. In fact, with the "--checkout" option, the tip of the submodule moves to the exact commit that is recorded in the superproject tree, regardless of .update configuration. Signed-off-by: Michal Sojka Signed-off-by: Junio C Hamano --- Documentation/git-submodule.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..648323f 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,7 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. The configuration can be overridden by specifying `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. -- 2.2.0-rc0-43-g5b91d12 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Am 03.11.2014 um 20:02 schrieb Junio C Hamano: Michal Sojka writes: The documentation says that submodule.$name.update can be overridden by --checkout only if its value is `none`. This is not true, because both implementation and documentation of --checkout specifies that the override applies to all possible values. Signed-off-by: Michal Sojka --- Documentation/git-submodule.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..84ab577 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,7 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. This can be overridden by specifying `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. Thanks. This looks sensible, judging only from the text (iow I didn't check if there were legitimate reason why rebase/merge settings should not be overriden from the command line). This was introduced in e6a1c43aaf (document submdule.$name.update=none option for gitmodules), and I agree with Michal that we should fix it. But I think we should rather say "This can be overridden by specifying '--merge', '--rebase' or `--checkout`." here, as the other two options also override the update setting. So I think we should queue this: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..84ab577 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,7 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. This can be overridden by using '--merge', + '--rebase' or `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. Apart from that I'm all for it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Fix documentation of update subcommand
Michal Sojka writes: > The documentation says that submodule.$name.update can be overridden by > --checkout only if its value is `none`. This is not true, because both > implementation and documentation of --checkout specifies that the > override applies to all possible values. > > Signed-off-by: Michal Sojka > --- > Documentation/git-submodule.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index 8e6af65..84ab577 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -158,7 +158,7 @@ update:: > checkout the commit specified in the index of the containing repository. > This will make the submodules HEAD be detached unless `--rebase` or > `--merge` is specified or the key `submodule.$name.update` is set to > - `rebase`, `merge` or `none`. `none` can be overridden by specifying > + `rebase`, `merge` or `none`. This can be overridden by specifying > `--checkout`. Setting the key `submodule.$name.update` to `!command` > will cause `command` to be run. `command` can be any arbitrary shell > command that takes a single argument, namely the sha1 to update to. Thanks. This looks sensible, judging only from the text (iow I didn't check if there were legitimate reason why rebase/merge settings should not be overriden from the command line). Jens? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] submodule: Fix documentation of update subcommand
The documentation says that submodule.$name.update can be overridden by --checkout only if its value is `none`. This is not true, because both implementation and documentation of --checkout specifies that the override applies to all possible values. Signed-off-by: Michal Sojka --- Documentation/git-submodule.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..84ab577 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -158,7 +158,7 @@ update:: checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying + `rebase`, `merge` or `none`. This can be overridden by specifying `--checkout`. Setting the key `submodule.$name.update` to `!command` will cause `command` to be run. `command` can be any arbitrary shell command that takes a single argument, namely the sha1 to update to. -- 2.1.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
