{"thread":{"id":"39306","subject":"git-pull --tags with no merge candidates case gives confusing error message","startedAt":"2015-05-12T05:59:04Z","lastAt":"2015-05-13T10:13:51Z","messageCount":5,"participants":["Paul Tan","Junio C Hamano","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"261039","messageId":"CACRoPnQvNvBv1_wBzWQH6A8XRvWZ+HURvtkqtgYpwjjNXuCERg@mail.gmail.com","threadId":"39306","inReplyTo":null,"subject":"git-pull --tags with no merge candidates case gives confusing error message","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-12T05:59:04Z","receivedAt":"2015-05-12T05:59:04Z","isPatch":false,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi all,\n\nCalling git-pull --tags, and hitting the no merge candidates case,\ncurrently gives the following error message:\n\n    It doesn't make sense to pull all tags; you probably meant:\n        git fetch --tags\n\nThis error message comes from the following code block in git-pull.sh:\n\n    for opt\n    do\n        case \"$opt\" in\n        -t|--t|--ta|--tag|--tags)\n            echo \"It doesn't make sense to pull all tags; you probably meant:\"\n            echo \"  git fetch --tags\"\n            exit 1\n        esac\n    done\n\nThis behavior was introduced in 441ed41 (\"git pull --tags\": error out\nwith a better message., 2007-12-28), which stated that:\n\n    In the longer term, it would be a better approach to change the\n    semantics of --tags option to make \"git fetch\" and \"git pull\"\n    to:\n\n     (1) behave as if no --tags was given (so an explicit refspec on\n         the command line overrides configured ones, or no explicit\n         refspecs on the command line takes configured ones); but\n\n     (2) no auto-following of tags is made even when using\n         configured refspecs; and\n\n     (3) fetch all tags as not-for-merge entries\".\n\n    Then we would not need to have this separate error message, as\n    the ordinary merge will happen even with the --tags option.\n\nGiven that as of c5a84e9 (fetch --tags: fetch tags *in addition to*\nother stuff, 2013-10-30), git-pull --tags will fetch tags in addition\nto the configured refspecs, so if there are no merge candidates, it\nwould not be because --tags was specified on the command line.\n\nAs such, I wonder if the error message should be removed, since it\nconceals the actual reason of why there are no merge candidates.\nUnless there is a reason why this special error message was kept?\n\nThanks,\nPaul\n"},{"id":"261054","messageId":"xmqq38311zi0.fsf@gitster.dls.corp.google.com","threadId":"39306","inReplyTo":"CACRoPnQvNvBv1_wBzWQH6A8XRvWZ+HURvtkqtgYpwjjNXuCERg@mail.gmail.com","subject":"Re: git-pull --tags with no merge candidates case gives confusing error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T17:23:19Z","receivedAt":"2015-05-12T17:23:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> This behavior was introduced in 441ed41 (\"git pull --tags\": error out\n> with a better message., 2007-12-28), which stated that:\n>\n>     In the longer term, it would be a better approach to change the\n>     semantics of --tags option to make \"git fetch\" and \"git pull\"\n>     to:\n>     ...\n>     Then we would not need to have this separate error message, as\n>     the ordinary merge will happen even with the --tags option.\n>\n> Given that as of c5a84e9 (fetch --tags: fetch tags *in addition to*\n> other stuff, 2013-10-30), git-pull --tags will fetch tags in addition\n> to the configured refspecs, so if there are no merge candidates, it\n> would not be because --tags was specified on the command line.\n>\n> As such, I wonder if the error message should be removed, since it\n> conceals the actual reason of why there are no merge candidates.\n\nI love it when people carefully analyse why the things are in the\nway they are, and some things no longer make sense in today's world\norder.\n\nI agree 100% with your analysis.  c5a84e9 should have tweaked this\npart, but was not careful enough, and made the heuristic used in\n441ed41 when diagnosing the error is a bad one now.  It should go.\n"},{"id":"261070","messageId":"555259E5.4040304@alum.mit.edu","threadId":"39306","inReplyTo":"CACRoPnQvNvBv1_wBzWQH6A8XRvWZ+HURvtkqtgYpwjjNXuCERg@mail.gmail.com","subject":"Re: git-pull --tags with no merge candidates case gives confusing error message","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-05-12T19:52:05Z","receivedAt":"2015-05-12T19:52:05Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/12/2015 07:59 AM, Paul Tan wrote:\n> Calling git-pull --tags, and hitting the no merge candidates case,\n> currently gives the following error message:\n> \n>     It doesn't make sense to pull all tags; you probably meant:\n>         git fetch --tags\n> \n> [...]\n> Given that as of c5a84e9 (fetch --tags: fetch tags *in addition to*\n> other stuff, 2013-10-30), git-pull --tags will fetch tags in addition\n> to the configured refspecs, so if there are no merge candidates, it\n> would not be because --tags was specified on the command line.\n> \n> As such, I wonder if the error message should be removed, since it\n> conceals the actual reason of why there are no merge candidates.\n> Unless there is a reason why this special error message was kept?\n\nThanks for the bug report and the careful analysis.\n\nI never use pull so I'm not really acquainted with its semantics. But it\nseems to me that when you remove the special \"--tags\" error message, you\nmight also have to adjust the logic later in the function that looks at\n\"$#\". Specifically, unless the presence of a \"--tags\" option can provide\ncandidates for merging, then in \"[ $# -gt 1 ]\", \"$#\" might need to be\nchanged to \"the number of arguments *not including --tags arguments*\".\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"261171","messageId":"1431511607-18251-1-git-send-email-pyokagan@gmail.com","threadId":"39306","inReplyTo":"xmqq38311zi0.fsf@gitster.dls.corp.google.com","subject":"[PATCH] pull: remove --tags error in no merge candidates case","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T10:06:47Z","receivedAt":"2015-05-13T10:06:47Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 441ed41 (\"git pull --tags\": error out with a better message.,\n2007-12-28), git pull --tags would print a different error message if\ngit-fetch did not return any merge candidates:\n\n   It doesn't make sense to pull all tags; you probably meant:\n        git fetch --tags\n\nThis is because at that time, git-fetch --tags would override any\nconfigured refspecs, and thus there would be no merge candidates. The\nerror message was thus introduced to prevent confusion.\n\nHowever, since c5a84e9 (fetch --tags: fetch tags *in addition to*\nother stuff, 2013-10-30), git fetch --tags would fetch tags in addition\nto any configured refspecs. Hence, if any no merge candidates situation\noccurs, it is not because --tags was set. As such, this special error\nmessage is now irrelevant.\n\nTo prevent confusion, remove this error message.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-pull.sh | 9 ---------\n 1 file changed, 9 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9ed01fd..9005171 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -190,15 +190,6 @@ esac\n \n error_on_no_merge_candidates () {\n \texec >&2\n-\tfor opt\n-\tdo\n-\t\tcase \"$opt\" in\n-\t\t-t|--t|--ta|--tag|--tags)\n-\t\t\techo \"It doesn't make sense to pull all tags; you probably meant:\"\n-\t\t\techo \"  git fetch --tags\"\n-\t\t\texit 1\n-\t\tesac\n-\tdone\n \n \tif test true = \"$rebase\"\n \tthen\n-- \n2.1.4\n"},{"id":"261172","messageId":"CACRoPnSjaE677xa9i2m-f28TYbfg7XbmWOP9r_-5ELhXOWY2fQ@mail.gmail.com","threadId":"39306","inReplyTo":"555259E5.4040304@alum.mit.edu","subject":"Re: git-pull --tags with no merge candidates case gives confusing error message","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-13T10:13:51Z","receivedAt":"2015-05-13T10:13:51Z","isPatch":false,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Wed, May 13, 2015 at 3:52 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> I never use pull so I'm not really acquainted with its semantics. But it\n> seems to me that when you remove the special \"--tags\" error message, you\n> might also have to adjust the logic later in the function that looks at\n> \"$#\". Specifically, unless the presence of a \"--tags\" option can provide\n> candidates for merging, then in \"[ $# -gt 1 ]\", \"$#\" might need to be\n> changed to \"the number of arguments *not including --tags arguments*\".\n\nYes, I'm aware of the problems of using \"$@\" and \"$#\" including\ngit-fetch's options in git-pull[1]. It's not just enough to skip over\narguments that look like options, though, as we do not know if the\noptions take values or not, so the whole logic may become complicated.\nI'm planning to solve it in a later patch series by explicitly parsing\ngit-fetch's options as well.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268510/focus=268565\n\nThanks,\nPaul\n"}]}