Documented option "--deepen" is unknown to git-pull

Documented option "--deepen" is unknown to git-pull

[天几]

Option "--deepen" is documented in online doc of git-pull, Sec.
_Options related to fetching_, see
    https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---deepenltdepthgt
But executing "git pull --deepen=100 origin master" returns
    error: unknown option `deepen'

More tests show option "--deepen" is known to git-fetch but not
git-pull, although the doc of git-pull says
    git pull runs git fetch with the given parameters and calls git
    merge to merge the retrieved branch heads into the current branch.
    https://git-scm.com/docs/git-pull#_description
which suggests that every valid option of "git fetch" would also valid
for "git pull".

Is this a documentation or an implementation problem? (I use git v2.25.1.)


muzimuzhi

Re: Documented option "--deepen" is unknown to git-pull

[René Scharfe]

Am 08.03.20 um 20:17 schrieb 天几:
> Option "--deepen" is documented in online doc of git-pull, Sec.
> _Options related to fetching_, see
>     https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---deepenltdepthgt
> But executing "git pull --deepen=100 origin master" returns
>     error: unknown option `deepen'
>
> More tests show option "--deepen" is known to git-fetch but not
> git-pull, although the doc of git-pull says
>     git pull runs git fetch with the given parameters and calls git
>     merge to merge the retrieved branch heads into the current branch.
>     https://git-scm.com/docs/git-pull#_description
> which suggests that every valid option of "git fetch" would also valid
> for "git pull".
>
> Is this a documentation or an implementation problem? (I use git v2.25.1.)

Looks like an oversight to me: pull has --depth, so why not --deepen as well?

Are there more?  Let's take compare short help and manpage:

   LANG=C git pull -h | awk '
      /^Options related to fetching/ {show=1}
      /^ *-/ && show {sub(/^ */, ""); sub(/, /, "\n"); sub(/[ [=].*/, ""); print}
   ' | sort >shorthelp
   awk -v pull=1 -v show=1 '
      /^ifdef::git-pull/ {show=pull; next}
      /^ifndef::git-pull/ {show=!pull; next}
      /^endif/ {show=1; next}
      /::/ && show {sub(/::/, ""); sub(/[ [=].*/, ""); print}
   ' Documentation/fetch-options.txt | sort >longhelp
   diff shorthelp longhelp | grep '^[<>]' | LANG=C sort

   < --dry-run
   < --jobs
   < --prune
   < --refmap
   < --set-upstream
   < --tags
   < -j
   < -p
   < -t
   > --deepen
   > --negotiation-tip
   > --no-show-forced-updates
   > --no-tags
   > --progress
   > --server-option
   > --shallow-exclude
   > --shallow-since
   > --update-head-ok
   > -o
   > -u

Hmm, that shows a few false positives: -t/--tags actually pairs with
--no-tags and --show-forced-updates from the short help is mentioned in
the manpage along with --no-show-forced-updates.  And --progress is
actually mentioned in pull's short help, just not in the fetching
section.

If we ignore those cases, the following options are documented in its
manpage but not actually accepted by git pull:

   --deepen
   --negotiation-tip
   --server-option
   --shallow-exclude
   --shallow-since
   --update-head-ok
   -o
   -u

Side note: There must be a better way to keep short help and manpage
in sync than the above ad-hoc AWK scripts.

Here's a patch to fix the easy part:

-- >8 --
Subject: [PATCH] pull: document more passthru options

git pull accepts the options --dry-run, -p/--prune, --refmap, and
-t/--tags since a32975f516 (pull: pass git-fetch's options to git-fetch,
2015-06-18), -j/--jobs since 62104ba14a (submodules: allow parallel
fetching, add tests and documentation, 2015-12-15), and --set-upstream
since 24bc1a1292 (pull, fetch: add --set-upstream option, 2019-08-19).
Update its documentation to match.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 Documentation/fetch-options.txt | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index a115a1ae0e..00d03ec8c3 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,10 +61,8 @@ this option multiple times, one for each matching ref name.
 See also the `fetch.negotiationAlgorithm` configuration variable
 documented in linkgit:git-config[1].

-ifndef::git-pull[]
 --dry-run::
 	Show what would be done, without making any changes.
-endif::git-pull[]

 -f::
 --force::
@@ -95,6 +93,7 @@ ifndef::git-pull[]
 --[no-]write-commit-graph::
 	Write a commit-graph after fetching. This overrides the config
 	setting `fetch.writeCommitGraph`.
+endif::git-pull[]

 -p::
 --prune::
@@ -107,6 +106,7 @@ ifndef::git-pull[]
 	was cloned with the --mirror option), then they are also
 	subject to pruning. Supplying `--prune-tags` is a shorthand for
 	providing the tag refspec.
+ifndef::git-pull[]
 +
 See the PRUNING section below for more details.

@@ -133,7 +133,6 @@ endif::git-pull[]
 	behavior for a remote may be specified with the remote.<name>.tagOpt
 	setting. See linkgit:git-config[1].

-ifndef::git-pull[]
 --refmap=<refspec>::
 	When fetching refs listed on the command line, use the
 	specified refspec (can be given more than once) to map the
@@ -154,6 +153,7 @@ ifndef::git-pull[]
 	is used (though tags may be pruned anyway if they are also the
 	destination of an explicit refspec; see `--prune`).

+ifndef::git-pull[]
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
 	populated submodules should be fetched too. It can be used as a
@@ -164,6 +164,7 @@ ifndef::git-pull[]
 	when the superproject retrieves a commit that updates the submodule's
 	reference to a commit that isn't already in the local submodule
 	clone.
+endif::git-pull[]

 -j::
 --jobs=<n>::
@@ -177,9 +178,11 @@ parallel. To control them independently, use the config settings
 Typically, parallel recursive and multi-remote fetches will be faster. By
 default fetches are performed sequentially, not in parallel.

+ifndef::git-pull[]
 --no-recurse-submodules::
 	Disable recursive fetching of submodules (this has the same effect as
 	using the `--recurse-submodules=no` option).
+endif::git-pull[]

 --set-upstream::
 	If the remote is fetched successfully, pull and add upstream
@@ -188,6 +191,7 @@ default fetches are performed sequentially, not in parallel.
 	see `branch.<name>.merge` and `branch.<name>.remote` in
 	linkgit:git-config[1].

+ifndef::git-pull[]
 --submodule-prefix=<path>::
 	Prepend <path> to paths printed in informative messages
 	such as "Fetching submodule foo".  This option is used
--
2.25.1

[PATCH 2/1] pull: remove --update-head-ok from documentation

[René Scharfe]

'git pull' implicitly passes --update-head-ok to 'git fetch', but
doesn't itself accept that option from users.  That makes sense, as it
wouldn't work without the possibility to update HEAD.  Remove the option
from the command's documentation to match its actual behavior.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 Documentation/fetch-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 00d03ec8c3..05709f67a1 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -204,7 +204,6 @@ ifndef::git-pull[]
 	recursion (such as settings in linkgit:gitmodules[5] and
 	linkgit:git-config[1]) override this option, as does
 	specifying --[no-]recurse-submodules directly.
-endif::git-pull[]

 -u::
 --update-head-ok::
@@ -214,6 +213,7 @@ endif::git-pull[]
 	to communicate with 'git fetch', and unless you are
 	implementing your own Porcelain you are not supposed to
 	use it.
+endif::git-pull[]

 --upload-pack <upload-pack>::
 	When given, and the repository to fetch from is handled
--
2.26.0

[PATCH 3/1] pull: pass documented fetch options on

[René Scharfe]

The fetch options --deepen, --negotiation-tip, --server-option,
--shallow-exclude, and --shallow-since are documented for git pull as
well, but are not actually accepted by that command.  Pass them on to
make the code match its documentation.

Reported-by: 天几 <muzimuzhi@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/pull.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index e42665b681..880eb29852 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -110,6 +110,7 @@ static char *opt_ipv4;
 static char *opt_ipv6;
 static int opt_show_forced_updates = -1;
 static char *set_upstream;
+static struct argv_array opt_fetch = ARGV_ARRAY_INIT;

 static struct option pull_options[] = {
 	/* Shared options */
@@ -207,6 +208,15 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU(0, "depth", &opt_depth, N_("depth"),
 		N_("deepen history of shallow clone"),
 		0),
+	OPT_PASSTHRU_ARGV(0, "shallow-since", &opt_fetch, N_("time"),
+		N_("deepen history of shallow repository based on time"),
+		0),
+	OPT_PASSTHRU_ARGV(0, "shallow-exclude", &opt_fetch, N_("revision"),
+		N_("deepen history of shallow clone, excluding rev"),
+		0),
+	OPT_PASSTHRU_ARGV(0, "deepen", &opt_fetch, N_("n"),
+		N_("deepen history of shallow clone"),
+		0),
 	OPT_PASSTHRU(0, "unshallow", &opt_unshallow, NULL,
 		N_("convert to a complete repository"),
 		PARSE_OPT_NONEG | PARSE_OPT_NOARG),
@@ -216,12 +226,19 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU(0, "refmap", &opt_refmap, N_("refmap"),
 		N_("specify fetch refmap"),
 		PARSE_OPT_NONEG),
+	OPT_PASSTHRU_ARGV('o', "server-option", &opt_fetch,
+		N_("server-specific"),
+		N_("option to transmit"),
+		0),
 	OPT_PASSTHRU('4',  "ipv4", &opt_ipv4, NULL,
 		N_("use IPv4 addresses only"),
 		PARSE_OPT_NOARG),
 	OPT_PASSTHRU('6',  "ipv6", &opt_ipv6, NULL,
 		N_("use IPv6 addresses only"),
 		PARSE_OPT_NOARG),
+	OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
+		N_("report that we have only objects reachable from this object"),
+		0),
 	OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
 		 N_("check for forced-updates on all updated branches")),
 	OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
@@ -567,6 +584,7 @@ static int run_fetch(const char *repo, const char **refspecs)
 		argv_array_push(&args, "--no-show-forced-updates");
 	if (set_upstream)
 		argv_array_push(&args, set_upstream);
+	argv_array_pushv(&args, opt_fetch.argv);

 	if (repo) {
 		argv_array_push(&args, repo);
--
2.26.0

Re: [PATCH 2/1] pull: remove --update-head-ok from documentation

[Taylor Blau]

Hi René,

This patch (and the other one attached to this thread) both look good.
However, the numbering on this patch is a little weird, (2/1? 3/1?). Is
there a third patch missing, and/or are your subject lines mangled?

Please share if there is more, or if I should be interpreting this
series differently. Otherwise this patch makes sense.

On Sat, Mar 28, 2020 at 03:48:33PM +0100, René Scharfe wrote:
> 'git pull' implicitly passes --update-head-ok to 'git fetch', but
> doesn't itself accept that option from users.  That makes sense, as it
> wouldn't work without the possibility to update HEAD.  Remove the option
> from the command's documentation to match its actual behavior.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  Documentation/fetch-options.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 00d03ec8c3..05709f67a1 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -204,7 +204,6 @@ ifndef::git-pull[]
>  	recursion (such as settings in linkgit:gitmodules[5] and
>  	linkgit:git-config[1]) override this option, as does
>  	specifying --[no-]recurse-submodules directly.
> -endif::git-pull[]
>
>  -u::
>  --update-head-ok::
> @@ -214,6 +213,7 @@ endif::git-pull[]
>  	to communicate with 'git fetch', and unless you are
>  	implementing your own Porcelain you are not supposed to
>  	use it.
> +endif::git-pull[]
>
>  --upload-pack <upload-pack>::
>  	When given, and the repository to fetch from is handled
> --
> 2.26.0

Thanks,
Taylor

Re: [PATCH 2/1] pull: remove --update-head-ok from documentation

[René Scharfe]

Am 31.03.20 um 23:48 schrieb Taylor Blau:
> This patch (and the other one attached to this thread) both look good.
> However, the numbering on this patch is a little weird, (2/1? 3/1?). Is
> there a third patch missing, and/or are your subject lines mangled?
>
> Please share if there is more, or if I should be interpreting this
> series differently. Otherwise this patch makes sense.

The first patch was
https://lore.kernel.org/git/a406c273-9a2c-72ed-c5e3-4c19848f0f94@web.de/
and I meant to only send that one.  A few weeks later I couldn't help
but follow up with two more patches.  I don't plan to continue the
series any further.

René