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

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

Paul Tan <pyokagan@gmail.com> writes:

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.

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(-)

On 05/12/2015 07:59 AM, Paul Tan wrote:

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

Hi,

On Wed, May 13, 2015 at 3:52 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

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