{"thread":{"id":"35193","subject":"Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","startedAt":"2013-10-24T18:07:05Z","lastAt":"2013-10-25T07:25:56Z","messageCount":6,"participants":["yaneurabeya .","Jeff King","Johan Herland"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"229416","messageId":"CAGH67wRwb1A9CzAfod_XLRVFBRyoEron8tmM+NbMGOeKDVf2Hg@mail.gmail.com","threadId":"35193","inReplyTo":"CAGH67wSf_RQigCmqRZKOpHdV9ELqE=078mkpwA4dfnUr=AvGVQ@mail.gmail.com","subject":"Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"yaneurabeya .","fromEmail":"yanegomi@gmail.com","sentAt":"2013-10-24T18:07:05Z","receivedAt":"2013-10-24T18:07:05Z","isPatch":false,"sender":{"key":"yanegomi@gmail.com","avatar":null},"body":"Hi!\n I added an arbitrary upstream remote thinking that I could just\ngit diff the upstream remote's master. Turns out I needed to run git\npull --all in order to be able to diff the file (I forgot that step).\n Could this error message be improved for interactive commands by\nfirst checking to see whether or not the path starts with a remote,\nthen recommend that the remote be pulled? If this seems sane, I could\nwhip up a patch and post it in a github pull request.\nThanks!\n-Garrett\n\n$ git remote add pkohut [...]\n$ git diff upstream/master -- conf/ | wc -l\n16\n$ git diff pkohut/master -- conf/ | wc -l\nfatal: bad revision 'pkohut/master'\n0\n$ git pull --all\nFetching origin\nFetching upstream\nFetching pkohut\n...\n$ git diff pkohut/master -- conf/ | wc -l\n46\n$ git --version; uname -a\ngit version 1.7.9\nCYGWIN_NT-6.1 ZL00757 1.7.25(0.270/5/3) 2013-08-31 20:37 x86_64 Cygwin\n"},{"id":"229495","messageId":"20131025061407.GE11810@sigill.intra.peff.net","threadId":"35193","inReplyTo":"CAGH67wRwb1A9CzAfod_XLRVFBRyoEron8tmM+NbMGOeKDVf2Hg@mail.gmail.com","subject":"Re: Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-25T06:14:08Z","receivedAt":"2013-10-25T06:14:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 24, 2013 at 11:07:05AM -0700, yaneurabeya . wrote:\n\n> I added an arbitrary upstream remote thinking that I could just\n> git diff the upstream remote's master. Turns out I needed to run git\n> pull --all in order to be able to diff the file (I forgot that step).\n\nActually, you can just run \"git fetch\"; you just need to fetch the\ncommits from the remote, but you do not need to merge them (and pull is\na fetch plus a merge).\n\n> Could this error message be improved for interactive commands by\n> first checking to see whether or not the path starts with a remote,\n> then recommend that the remote be pulled?\n\nThat might be worth doing. We cannot definitely say the branch exists\nwithout hitting the network (which we would not want to do in the\ngeneral case), but I think it is reasonable for git to give suggestions\n(we could also give a \"did you mean X...\" for near-typos, as we do for\ntypo-ed commands like \"git dif\".\n\nIf you do try it, please don't just check for the remote name, but\nactually complete the right-hand side of the fetch refspec for each\nremote. They are equivalent in the default config, but aren't\nnecessarily so (and there has been talk of adjusting the layout of\nremote refspecs). I don't recall offhand what functions we have to help\nyou, but I believe Johan (cc'd) was working in this area recently and\nmight be able to say more.\n\n> If this seems sane, I could whip up a patch and post it in a github\n> pull request.\n\nIf you do write a patch, please send it to the list, as the github\nrepository is a mirror, and actual development happens here. Details are\nin Documentation/SubmittingPatches.\n\n-Peff\n"},{"id":"229496","messageId":"20131025061459.GA23524@sigill.intra.peff.net","threadId":"35193","inReplyTo":"20131025061407.GE11810@sigill.intra.peff.net","subject":"Re: Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-25T06:14:59Z","receivedAt":"2013-10-25T06:14:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 25, 2013 at 02:14:07AM -0400, Jeff King wrote:\n\n> > Could this error message be improved for interactive commands by\n> > first checking to see whether or not the path starts with a remote,\n> > then recommend that the remote be pulled?\n> \n> That might be worth doing. We cannot definitely say the branch exists\n> without hitting the network (which we would not want to do in the\n> general case), but I think it is reasonable for git to give suggestions\n> (we could also give a \"did you mean X...\" for near-typos, as we do for\n> typo-ed commands like \"git dif\".\n> \n> If you do try it, please don't just check for the remote name, but\n> actually complete the right-hand side of the fetch refspec for each\n> remote. They are equivalent in the default config, but aren't\n> necessarily so (and there has been talk of adjusting the layout of\n> remote refspecs). I don't recall offhand what functions we have to help\n> you, but I believe Johan (cc'd) was working in this area recently and\n> might be able to say more.\n\n...and of course I forgot to cc Johan in the original.\n\n-Peff\n"},{"id":"229501","messageId":"CALKQrgc0u8TA4mXc+2Hv+Fyo8EYNuiickF_janeCxkzFn+xAvA@mail.gmail.com","threadId":"35193","inReplyTo":"20131025061459.GA23524@sigill.intra.peff.net","subject":"Re: Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-10-25T07:03:41Z","receivedAt":"2013-10-25T07:03:41Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Fri, Oct 25, 2013 at 8:14 AM, Jeff King wrote:\n> On Fri, Oct 25, 2013 at 02:14:07AM -0400, Jeff King wrote:\n>> > Could this error message be improved for interactive commands by\n>> > first checking to see whether or not the path starts with a remote,\n>> > then recommend that the remote be pulled?\n>>\n>> That might be worth doing. We cannot definitely say the branch exists\n>> without hitting the network (which we would not want to do in the\n>> general case), but I think it is reasonable for git to give suggestions\n>> (we could also give a \"did you mean X...\" for near-typos, as we do for\n>> typo-ed commands like \"git dif\".\n>>\n>> If you do try it, please don't just check for the remote name, but\n>> actually complete the right-hand side of the fetch refspec for each\n>> remote. They are equivalent in the default config, but aren't\n>> necessarily so (and there has been talk of adjusting the layout of\n>> remote refspecs). I don't recall offhand what functions we have to help\n>> you, but I believe Johan (cc'd) was working in this area recently and\n>> might be able to say more.\n\nActually, I don't think there's much refspec stuff to be done here.\nWhen running \"git diff $remote/$branch\", there are 3 possible\noutcomes:\n\n - $remote is not a valid remote name, the user probably meant\nsomething different (like \"nested/branch\"). The current error message\nis fine.\n\n - $remote is a valid remote name, but $branch has not (yet) been\nfetched from there. Suggest the user run \"git fetch $remote\"\n\n - $remote/$branch is a valid remote-tracking branch. The diff works! No errors.\n\nSo, AFAICS, the patch should simply:\n\n 1. Split the input on the first '/' into $remote/$branch, and use the\npreceding part ($remote) as a potential remote name, and the following\npart ($branch) as a potential branch name. (Although it is\ntheoretically possible to have remote names containing slashes, I\ndon't think anybody uses them, and we have considered disallowing\nthem, mainly because of this very issue: it makes \"$remote/$branch\"\nparsing (even more) ambiguous)\n\n 2. See if a remote called $remote exists. If it does, suggest to the\nuser to run \"git fetch $remote\". If $remote does not exist, leave the\ncurrent error message in place.\n\nHope this helps,\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"229502","messageId":"20131025071048.GA16856@sigill.intra.peff.net","threadId":"35193","inReplyTo":"CALKQrgc0u8TA4mXc+2Hv+Fyo8EYNuiickF_janeCxkzFn+xAvA@mail.gmail.com","subject":"Re: Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-25T07:10:48Z","receivedAt":"2013-10-25T07:10:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 25, 2013 at 09:03:41AM +0200, Johan Herland wrote:\n\n> Actually, I don't think there's much refspec stuff to be done here.\n> When running \"git diff $remote/$branch\", there are 3 possible\n> outcomes:\n> \n> - $remote is not a valid remote name, the user probably meant\n> something different (like \"nested/branch\"). The current error message\n> is fine.\n> \n> - $remote is a valid remote name, but $branch has not (yet) been\n> fetched from there. Suggest the user run \"git fetch $remote\"\n> \n> - $remote/$branch is a valid remote-tracking branch. The diff works! No errors.\n\nRight, I think it is the second case we are talking about.\n\n> So, AFAICS, the patch should simply:\n> \n> 1. Split the input on the first '/' into $remote/$branch, and use the\n> preceding part ($remote) as a potential remote name, and the following\n> part ($branch) as a potential branch name. (Although it is\n> theoretically possible to have remote names containing slashes, I\n> don't think anybody uses them, and we have considered disallowing\n> them, mainly because of this very issue: it makes \"$remote/$branch\"\n> parsing (even more) ambiguous)\n\nWhat I specifically meant is that this breaks with a remote like:\n\n [remote \"foo\"]\n fetch = +refs/heads/*:refs/remotes/bar/*\n\nThe correct advice for \"bar/branch\" is to recommend \"git fetch foo\", and\nthe correct advice for \"foo/branch\" is nothing at all.\n\nI know such config is unusual, but I thought there was a recent push for\nus to be accurate about finding the local side of remote tracking\nbranches, rather than just assuming they start with \"$remote\". Maybe I\nam misremembering, though; I thought it was related to potentially\nshifting the default refspecs.\n\nThe procedure along those lines would be:\n\n for each remote\n for each fetch-refspec in remote\n if refspec.rhs contains \"refs/remotes/$failed_branch\"\n recommend \"git fetch $remote\"\n\nI was just wondering if we had something to make that \"does this refspec\ncontain this ref\" part easier.\n\n-Peff\n"},{"id":"229507","messageId":"CALKQrgdfcDzq+M4Pi=DdbSPXCDCcXRtk6qV7qhvyHviJoJXA8A@mail.gmail.com","threadId":"35193","inReplyTo":"20131025071048.GA16856@sigill.intra.peff.net","subject":"Re: Fwd: Errors when diff'ing arbitrary upstream remotes are not intuitive if git pull --all hasn't been done","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-10-25T07:25:56Z","receivedAt":"2013-10-25T07:25:56Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Fri, Oct 25, 2013 at 9:10 AM, Jeff King wrote:\n> On Fri, Oct 25, 2013 at 09:03:41AM +0200, Johan Herland wrote:\n>> 1. Split the input on the first '/' into $remote/$branch, and use the\n>> preceding part ($remote) as a potential remote name, and the following\n>> part ($branch) as a potential branch name. (Although it is\n>> theoretically possible to have remote names containing slashes, I\n>> don't think anybody uses them, and we have considered disallowing\n>> them, mainly because of this very issue: it makes \"$remote/$branch\"\n>> parsing (even more) ambiguous)\n>\n> What I specifically meant is that this breaks with a remote like:\n>\n> [remote \"foo\"]\n> fetch = +refs/heads/*:refs/remotes/bar/*\n>\n> The correct advice for \"bar/branch\" is to recommend \"git fetch foo\", and\n> the correct advice for \"foo/branch\" is nothing at all.\n>\n> I know such config is unusual, but I thought there was a recent push for\n> us to be accurate about finding the local side of remote tracking\n> branches, rather than just assuming they start with \"$remote\". Maybe I\n> am misremembering, though; I thought it was related to potentially\n> shifting the default refspecs.\n\nObviously, you're right. Sorry about that, haven't had my morning coffee yet. :(\n\n> The procedure along those lines would be:\n>\n> for each remote\n> for each fetch-refspec in remote\n> if refspec.rhs contains \"refs/remotes/$failed_branch\"\n> recommend \"git fetch $remote\"\n>\n> I was just wondering if we had something to make that \"does this refspec\n> contain this ref\" part easier.\n\nYes, I found the following code in branch.c (added in 41c21f2), which\ndoes a similar thing. Might want to refactor that into something more\ngeneral:\n\n+static int check_tracking_branch(struct remote *remote, void *cb_data)\n+{\n+ char *tracking_branch = cb_data;\n+ struct refspec query;\n+ memset(&query, 0, sizeof(struct refspec));\n+ query.dst = tracking_branch;\n+ return !(remote_find_tracking(remote, &query) ||\n+ prefixcmp(query.src, \"refs/heads/\"));\n+}\n+\n+static int validate_remote_tracking_branch(char *ref)\n+{\n+ return !for_each_remote(check_tracking_branch, ref);\n+}\n\n\nHope this helps,\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}