git-pull --tags with no merge candidates case gives confusing error message

git-pull --tags with no merge candidates case gives confusing error message

[Paul Tan]

Hi all,

Calling git-pull --tags, and hitting the no merge candidates case,
currently gives the following error message:

    It doesn't make sense to pull all tags; you probably meant:
        git fetch --tags

This error message comes from the following code block in git-pull.sh:

    for opt
    do
        case "$opt" in
        -t|--t|--ta|--tag|--tags)
            echo "It doesn't make sense to pull all tags; you probably meant:"
            echo "  git fetch --tags"
            exit 1
        esac
    done

This behavior was introduced in 441ed41 ("git pull --tags": error out
with a better message., 2007-12-28), which stated that:

    In the longer term, it would be a better approach to change the
    semantics of --tags option to make "git fetch" and "git pull"
    to:

     (1) behave as if no --tags was given (so an explicit refspec on
         the command line overrides configured ones, or no explicit
         refspecs on the command line takes configured ones); but

     (2) no auto-following of tags is made even when using
         configured refspecs; and

     (3) fetch all tags as not-for-merge entries".

    Then we would not need to have this separate error message, as
    the ordinary merge will happen even with the --tags option.

Given that as of c5a84e9 (fetch --tags: fetch tags *in addition to*
other stuff, 2013-10-30), git-pull --tags will fetch tags in addition
to the configured refspecs, so if there are no merge candidates, it
would not be because --tags was specified on the command line.

As such, I wonder if the error message should be removed, since it
conceals the actual reason of why there are no merge candidates.
Unless there is a reason why this special error message was kept?

Thanks,
Paul

Re: git-pull --tags with no merge candidates case gives confusing error message

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> This behavior was introduced in 441ed41 ("git pull --tags": error out
> with a better message., 2007-12-28), which stated that:
>
>     In the longer term, it would be a better approach to change the
>     semantics of --tags option to make "git fetch" and "git pull"
>     to:
>     ...
>     Then we would not need to have this separate error message, as
>     the ordinary merge will happen even with the --tags option.
>
> Given that as of c5a84e9 (fetch --tags: fetch tags *in addition to*
> other stuff, 2013-10-30), git-pull --tags will fetch tags in addition
> to the configured refspecs, so if there are no merge candidates, it
> would not be because --tags was specified on the command line.
>
> As such, I wonder if the error message should be removed, since it
> conceals the actual reason of why there are no merge candidates.

I love it when people carefully analyse why the things are in the
way they are, and some things no longer make sense in today's world
order.

I agree 100% with your analysis.  c5a84e9 should have tweaked this
part, but was not careful enough, and made the heuristic used in
441ed41 when diagnosing the error is a bad one now.  It should go.

[PATCH] pull: remove --tags error in no merge candidates case

[Paul Tan]

Since 441ed41 ("git pull --tags": error out with a better message.,
2007-12-28), git pull --tags would print a different error message if
git-fetch did not return any merge candidates:

   It doesn't make sense to pull all tags; you probably meant:
        git fetch --tags

This is because at that time, git-fetch --tags would override any
configured refspecs, and thus there would be no merge candidates. The
error message was thus introduced to prevent confusion.

However, since c5a84e9 (fetch --tags: fetch tags *in addition to*
other stuff, 2013-10-30), git fetch --tags would fetch tags in addition
to any configured refspecs. Hence, if any no merge candidates situation
occurs, it is not because --tags was set. As such, this special error
message is now irrelevant.

To prevent confusion, remove this error message.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 git-pull.sh | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 9ed01fd..9005171 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -190,15 +190,6 @@ esac
 
 error_on_no_merge_candidates () {
 	exec >&2
-	for opt
-	do
-		case "$opt" in
-		-t|--t|--ta|--tag|--tags)
-			echo "It doesn't make sense to pull all tags; you probably meant:"
-			echo "  git fetch --tags"
-			exit 1
-		esac
-	done
 
 	if test true = "$rebase"
 	then
-- 
2.1.4

Re: git-pull --tags with no merge candidates case gives confusing error message

[Michael Haggerty]

On 05/12/2015 07:59 AM, Paul Tan wrote:
> Calling git-pull --tags, and hitting the no merge candidates case,
> currently gives the following error message:
> 
>     It doesn't make sense to pull all tags; you probably meant:
>         git fetch --tags
> 
> [...]
> Given that as of c5a84e9 (fetch --tags: fetch tags *in addition to*
> other stuff, 2013-10-30), git-pull --tags will fetch tags in addition
> to the configured refspecs, so if there are no merge candidates, it
> would not be because --tags was specified on the command line.
> 
> As such, I wonder if the error message should be removed, since it
> conceals the actual reason of why there are no merge candidates.
> Unless there is a reason why this special error message was kept?

Thanks for the bug report and the careful analysis.

I never use pull so I'm not really acquainted with its semantics. But it
seems to me that when you remove the special "--tags" error message, you
might also have to adjust the logic later in the function that looks at
"$#". Specifically, unless the presence of a "--tags" option can provide
candidates for merging, then in "[ $# -gt 1 ]", "$#" might need to be
changed to "the number of arguments *not including --tags arguments*".

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu

Re: git-pull --tags with no merge candidates case gives confusing error message

[Paul Tan]

Hi,

On Wed, May 13, 2015 at 3:52 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> I never use pull so I'm not really acquainted with its semantics. But it
> seems to me that when you remove the special "--tags" error message, you
> might also have to adjust the logic later in the function that looks at
> "$#". Specifically, unless the presence of a "--tags" option can provide
> candidates for merging, then in "[ $# -gt 1 ]", "$#" might need to be
> changed to "the number of arguments *not including --tags arguments*".

Yes, I'm aware of the problems of using "$@" and "$#" including
git-fetch's options in git-pull[1]. It's not just enough to skip over
arguments that look like options, though, as we do not know if the
options take values or not, so the whole logic may become complicated.
I'm planning to solve it in a later patch series by explicitly parsing
git-fetch's options as well.

[1] http://thread.gmane.org/gmane.comp.version-control.git/268510/focus=268565

Thanks,
Paul