{"thread":{"id":"50504","subject":"[Bug report] git diff stat shows unrelated diff","startedAt":"2019-02-14T08:23:04Z","lastAt":"2019-02-18T04:34:59Z","messageCount":17,"participants":["Viresh Kumar","Johannes Sixt","Elijah Newren","Junio C Hamano","Denton Liu","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"369301","messageId":"20190214082258.3mh3hcr2l6dl3wuf@vireshk-i7","threadId":"50504","inReplyTo":null,"subject":"[Bug report] git diff stat shows unrelated diff","fromName":"Viresh Kumar","fromEmail":"viresh.kumar@linaro.org","sentAt":"2019-02-14T08:22:58Z","receivedAt":"2019-02-14T08:23:04Z","isPatch":false,"sender":{"key":"viresh.kumar@linaro.org","avatar":"https://gravatar.com/avatar/938b1abce576d6c6d2734c01fce1c75040618c8fc0d1b7e836277ecd9f35fb19?d=mp&s=160"},"body":"Hello,\n\nI am not sure if it is a bug or not, but the output I got wasn't what\nI was looking for. And so looking for some help. I was looking to send\npull request [1] to PM maintainer and was generating the request\nagainst his tree [2], which already has kernel upto v5.0-rc6 merged in\nit.\n\nThese are my local branches:\n\nBranch A:\n\n55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP\nf896d06665ec cpufreq: qcom-hw: Move to device_initcall\n1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2\n\nBranch B:\n\na4f342b9607d PM / OPP: Introduce a power estimation helper\n285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only()\nbfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1\n\npm/linux-next branch already has Branch B merged in it (with an\nearlier pull request).\n\nBranch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I\ntry to generate diff-stat with:\n\ngit diff -M --stat pm/linux-next..7c139d3f0f99\n\nIt shows me the diffstat between v5.0-rc1..v5.0-rc2 as well.\n\nIf I do\n\ngit diff -M --stat v5.0-rc2..7c139d3f0f99\n\nit shows only the files that have changed in patches in branch A and\nB.\n\nSince pm/linux-next already has Branch B and all the rcs upto rc6, I\nwas expecting the output of first diffstat to be similar to second one\n(without branch B stuff). Is the expectation incorrect ?\n\n-- \nviresh\n\n[1] My tree: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git\n[2] PM tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git\n\nTop commit in PM tree at the time I was trying it:\n\nd26ff2405272 (pm/testing, pm/linux-next) Merge branch 'pm-opp' into linux-next\n"},{"id":"369321","messageId":"d421fd13-5771-d103-5f77-d38a24afaef6@kdbg.org","threadId":"50504","inReplyTo":"20190214082258.3mh3hcr2l6dl3wuf@vireshk-i7","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-02-14T18:42:09Z","receivedAt":"2019-02-14T18:42:14Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.02.19 um 09:22 schrieb Viresh Kumar:\n> Hello,\n> \n> I am not sure if it is a bug or not, but the output I got wasn't what\n> I was looking for. And so looking for some help. I was looking to send\n> pull request [1] to PM maintainer and was generating the request\n> against his tree [2], which already has kernel upto v5.0-rc6 merged in\n> it.\n> \n> These are my local branches:\n> \n> Branch A:\n> \n> 55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP\n> f896d06665ec cpufreq: qcom-hw: Move to device_initcall\n> 1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2\n> \n> Branch B:\n> \n> a4f342b9607d PM / OPP: Introduce a power estimation helper\n> 285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only()\n> bfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1\n> \n> pm/linux-next branch already has Branch B merged in it (with an\n> earlier pull request).\n> \n> Branch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I\n> try to generate diff-stat with:\n> \n> git diff -M --stat pm/linux-next..7c139d3f0f99\n> \n> It shows me the diffstat between v5.0-rc1..v5.0-rc2 as well.\n> \n> If I do\n> \n> git diff -M --stat v5.0-rc2..7c139d3f0f99\n> \n> it shows only the files that have changed in patches in branch A and\n> B.\n> \n> Since pm/linux-next already has Branch B and all the rcs upto rc6, I\n> was expecting the output of first diffstat to be similar to second one\n> (without branch B stuff). Is the expectation incorrect ?\n\nYes, I think your expectation is incorrect.\n\nThe meaning of .. is different between diff and the log commands. For\ndiff it is mostly just noise, that is your first diff command is\nactually equivalent to\n\n git diff -M --stat pm/linux-next 7c139d3f0f99\n\nIt is quite obvious IMO that this is the diff between two end points.\nWhat you most likely wanted to see is\n\n git diff -M --stat pm/linux-next...7c139d3f0f99\n\nIt shows the diff between a merge base of the two revisions and the\nsecond one, i.e. between a where Branch C branched off of pm/linux-next\nand its tip.\n\n-- Hannes\n"},{"id":"369347","messageId":"CABPp-BGstumw1eyZ++0itk-AR0Bk5zd0AchgznBpMq35a6ScUg@mail.gmail.com","threadId":"50504","inReplyTo":"20190214082258.3mh3hcr2l6dl3wuf@vireshk-i7","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-14T21:23:14Z","receivedAt":"2019-02-14T21:23:29Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Thu, Feb 14, 2019 at 11:35 AM Viresh Kumar wrote:\n>\n> Hello,\n>\n> I am not sure if it is a bug or not, but the output I got wasn't what\n> I was looking for. And so looking for some help. I was looking to send\n> pull request [1] to PM maintainer and was generating the request\n> against his tree [2], which already has kernel upto v5.0-rc6 merged in\n> it.\n>\n> These are my local branches:\n>\n> Branch A:\n>\n> 55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP\n> f896d06665ec cpufreq: qcom-hw: Move to device_initcall\n> 1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2\n>\n> Branch B:\n>\n> a4f342b9607d PM / OPP: Introduce a power estimation helper\n> 285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only()\n> bfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1\n>\n> pm/linux-next branch already has Branch B merged in it (with an\n> earlier pull request).\n>\n> Branch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I\n> try to generate diff-stat with:\n>\n> git diff -M --stat pm/linux-next..7c139d3f0f99\n>\n> It shows me the diffstat between v5.0-rc1..v5.0-rc2 as well.\n>\n> If I do\n>\n> git diff -M --stat v5.0-rc2..7c139d3f0f99\n>\n> it shows only the files that have changed in patches in branch A and\n> B.\n>\n> Since pm/linux-next already has Branch B and all the rcs upto rc6, I\n> was expecting the output of first diffstat to be similar to second one\n> (without branch B stuff). Is the expectation incorrect ?\n\nI think you're getting tripped up by double-dot vs triple-dot with\ndiff being different than log:\n\n`git diff D..E` means the same thing as `git diff D E`, i.e. diff the\ntwo commits D and E.\n\n`git diff D...E` means the same thing as `git diff $(git merge-base D E) E`\n\nThere are some people for whom this state of affairs makes sense. I\nam not one of them, and I suspect you aren't either. The arguments\nmade by those who feel this makes sense seem reasonable to me in the\nmoment when they present them, but I have never been able to remember\nthese arguments longer than briefly. It just doesn't stick with me.\nThe only thing I seem to be able to retain is the following: \"git\ndiff D..E is totally useless and should be an error because (1) it\ndoesn't do what I expect and (2) for folks that want the behavior\ncurrently gotten with that syntax can instead just use a space instead\nof a double dot.\"\n\n\nHope that helps,\nElijah\n"},{"id":"369352","messageId":"xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"CABPp-BGstumw1eyZ++0itk-AR0Bk5zd0AchgznBpMq35a6ScUg@mail.gmail.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-14T22:10:53Z","receivedAt":"2019-02-14T22:10:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> The only thing I seem to be able to retain is the following: \"git\n> diff D..E is totally useless and should be an error because (1) it\n> doesn't do what I expect and (2) for folks that want the behavior\n> currently gotten with that syntax can instead just use a space instead\n> of a double dot.\"\n\nThat sums up pretty nicely. diff is fundamentally an operation\nbetween two endpoints, so the range notation a..b does not work\nnicely with it at the conceptual level.\n\nWhen we realized that we can take advantage of the above fact, and\nreuse a range notation to mean something that is generally useful in\nthe context of diff, such as 'one end of the comparison is the merge\nbase between a and b, and the other end is b', it was too late to\nuse \"a..b\", as an early adopters of Git was already used to the fact\nthat \"a..b\" happened to mean the same thing as \"comparison of one\nend is a, the other end is b\", pretty much implemented without much\nthought.\n\nIt might be _possible_ to spend a year (i.e. 4 cycles) to start\nwarning when two-dot notation is used for \"git diff\" (only, not any\nplumbing like \"git diff-files\") and tell the user to use the more\nlogical two-end notation \"git diff A B\" and then eventually error\nout when two-dot notation is used, while retaining the three-dot\nnotation throughout and to the eternity. I am not sure if it is\nworth the deprecation cost, though.\n\n\n\n"},{"id":"369374","messageId":"20190215064013.s7yfkmfvfmwfmepc@vireshk-i7","threadId":"50504","inReplyTo":"CABPp-BGstumw1eyZ++0itk-AR0Bk5zd0AchgznBpMq35a6ScUg@mail.gmail.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Viresh Kumar","fromEmail":"viresh.kumar@linaro.org","sentAt":"2019-02-15T06:40:13Z","receivedAt":"2019-02-15T06:40:19Z","isPatch":false,"sender":{"key":"viresh.kumar@linaro.org","avatar":"https://gravatar.com/avatar/938b1abce576d6c6d2734c01fce1c75040618c8fc0d1b7e836277ecd9f35fb19?d=mp&s=160"},"body":"On 14-02-19, 13:23, Elijah Newren wrote:\n> I think you're getting tripped up by double-dot vs triple-dot with\n> diff being different than log:\n> \n> `git diff D..E` means the same thing as `git diff D E`, i.e. diff the\n> two commits D and E.\n\nRight, so both the branches have at least until rc2 (though\npm/linux-next had until rc6), why will the diff D..E show the diff\nbetween rc1 and rc2 ?\n\n> `git diff D...E` means the same thing as `git diff $(git merge-base D E) E`\n\nI get exactly the same result with both .. and ... in this particular\ncase and that's why I wonder if everything is okay or not.\n\nThe problem in this case is:\n- PM tree had rc1,2,3,4,5,6 merged earlier into it.\n- Then I got merged one of my branches which was based of rc1 into\n pm/linux-next.\n- And now I am trying to send pull request for another branch which is\n a merge of the earlier branch (which got merged, based of rc1) and\n second branch that has more stuff over rc2.\n- The most recent merge commit common between my branch and\n pm/linux-next becomes the earlier branch which was based of rc1.\n- Now I expect ... to return the diff between rc1 and rc2 as it will\n diff against the most recent merge.\n- But I expected ... to not include rc1..rc2 diff.\n\n> There are some people for whom this state of affairs makes sense. I\n> am not one of them, and I suspect you aren't either. The arguments\n> made by those who feel this makes sense seem reasonable to me in the\n> moment when they present them, but I have never been able to remember\n> these arguments longer than briefly. It just doesn't stick with me.\n> The only thing I seem to be able to retain is the following: \"git\n> diff D..E is totally useless and should be an error because (1) it\n> doesn't do what I expect and (2) for folks that want the behavior\n> currently gotten with that syntax can instead just use a space instead\n> of a double dot.\"\n\nOkay but git request-pull uses .. and not ... and that's where I saw\nthe issue in the first place.\n\n-- \nviresh\n"},{"id":"369390","messageId":"CABPp-BFpj8DBxgsqe9Rqnzb4vx5fPqNc+sUj8LzrFMj2bacoBQ@mail.gmail.com","threadId":"50504","inReplyTo":"20190215064013.s7yfkmfvfmwfmepc@vireshk-i7","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-15T16:09:33Z","receivedAt":"2019-02-15T16:09:52Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Viresh,\n\nOn Thu, Feb 14, 2019 at 10:40 PM Viresh Kumar wrote:\n>\n> On 14-02-19, 13:23, Elijah Newren wrote:\n> > I think you're getting tripped up by double-dot vs triple-dot with\n> > diff being different than log:\n> >\n> > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the\n> > two commits D and E.\n>\n> Right, so both the branches have at least until rc2 (though\n> pm/linux-next had until rc6), why will the diff D..E show the diff\n> between rc1 and rc2 ?\n\nI don't think it does; it includes the diff between rc2 and rc6.\nFirst, `git diff D..E` will show you roughly the same thing as if you\nhave two clones of linux, one with D checked out, one with E checked\nout, and then you run 'diff -ru linux-copy1/ linux-copy2/'.\n\nBut let's go back to your example, you had\n\nBranch A: commit 55538fbc79e9\nBranch B: commit a4f342b9607d\nBranch C: commit 7c139d3f0f99\nBranch pm/linux-next: unspecified, but I'll guess commit a06639e89e4\n\nNow, using git describe on each of those commits in order shows us\nwhere in history they are:\n\nA: v5.0-rc2-2-g55538fbc79e9\nB: v5.0-rc1-2-ga4f342b9607d\nC: v5.0-rc2-5-g7c139d3f0f99\nD: v5.0-rc6-84-ga06639e89e47\n\nNow, you said you ran\n git diff -M --stat pm/linux-next..7c139d3f0f99\nand that it included the diff between rc1 and rc2. I think you\nactually saw pieces of the diff from rc2 to rc6 and assumed it was the\ndiff from rc1 to rc2. Besides the fact that pm/linux-next is based on\nrc6 and 7c139d3f0f99 is based on rc2, here's another way to verify\nthat:\n\n$ git diff --name-only v5.0-rc1 v5.0-rc2 | sort > early\n$ git diff --name-only v5.0-rc2 v5.0-rc6 | sort > late\n$ comm -23 early late > early-only\n$ comm -13 early late > late-only\n$ wc -l *-only\n 407 early-only\n 1149 late-only\n 1556 total\n\nSo, here \"early-only\" is a list of files that changed between rc1 and\nrc2 that did not change between rc2 and rc6. \"late-only\" is a list of\nfiles that did not change between rc1 and rc2, but did change between\nrc2 and rc6. No, let's compare that to your diff:\n\n$ git diff --name-only a06639e89e4..7c139d3f0f99 | sort > changes\n$ comm -12 changes early-only | wc -l\n6\n$ comm -12 changes late-only | wc -l\n1148\n\nSo, the files listed in your diff only included 6 files that were\nunique to early-only, and included 1148 files that were unique to\nlate-only. So, your diff looks an awful lot like the diff between rc2\nand rc6, and not much at all like the diff between rc1 and rc2.\n\n> > `git diff D...E` means the same thing as `git diff $(git merge-base D E) E`\n>\n> I get exactly the same result with both .. and ... in this particular\n> case and that's why I wonder if everything is okay or not.\n\nWith `git diff D..E` it doesn't matter much which order you put D and\nE in, other than flipping '-' lines for '+' lines. With `git diff\nD...E` it makes a huge difference. Compare:\n\n$ git diff --shortstat 7c139d3f0f99..a06639e89e4\n 1466 files changed, 15417 insertions(+), 7313 deletions(-)\n$ git diff --shortstat a06639e89e4..7c139d3f0f99\n 1466 files changed, 7313 insertions(+), 15417 deletions(-)\n\nSo, as I said, swapping with double-dot only changes '-' and '+'\nlines. In contrast:\n\n$ git diff --shortstat 7c139d3f0f99...a06639e89e4\n 1463 files changed, 15401 insertions(+), 7165 deletions(-)\n$ git diff --shortstat a06639e89e4...7c139d3f0f99\n 4 files changed, 148 insertions(+), 16 deletions(-)\n\nYou get a really small diff in one direction, but huge in the other.\nThe reason for this is that commit C (7c139d3f0f99) is really close to\nthe merge base, but pm/linux-next (a06639e89e4) is really far from it\n-- it includes rc6.\n\n> The problem in this case is:\n> - PM tree had rc1,2,3,4,5,6 merged earlier into it.\n> - Then I got merged one of my branches which was based of rc1 into\n> pm/linux-next.\n> - And now I am trying to send pull request for another branch which is\n> a merge of the earlier branch (which got merged, based of rc1) and\n> second branch that has more stuff over rc2.\n> - The most recent merge commit common between my branch and\n> pm/linux-next becomes the earlier branch which was based of rc1.\n> - Now I expect ... to return the diff between rc1 and rc2 as it will\n> diff against the most recent merge.\n> - But I expected ... to not include rc1..rc2 diff.\n>\n> > There are some people for whom this state of affairs makes sense. I\n> > am not one of them, and I suspect you aren't either. The arguments\n> > made by those who feel this makes sense seem reasonable to me in the\n> > moment when they present them, but I have never been able to remember\n> > these arguments longer than briefly. It just doesn't stick with me.\n> > The only thing I seem to be able to retain is the following: \"git\n> > diff D..E is totally useless and should be an error because (1) it\n> > doesn't do what I expect and (2) for folks that want the behavior\n> > currently gotten with that syntax can instead just use a space instead\n> > of a double dot.\"\n>\n> Okay but git request-pull uses .. and not ... and that's where I saw\n> the issue in the first place.\n\nI haven't used request-pull myself and I'm not familiar with the code,\nso I don't know if you might have just passed it the wrong arguments\nor if it has a bug. I'll have to leave it to someone else to comment\non that (though they may need to know what arguments you passed to\nthat command).\n\n\nHope that helps,\nElijah\n"},{"id":"369409","messageId":"20190215185202.GA28019@dev-l","threadId":"50504","inReplyTo":"xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-02-15T18:52:02Z","receivedAt":"2019-02-15T18:52:08Z","isPatch":false,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote:\n> Elijah Newren writes:\n> \n> > The only thing I seem to be able to retain is the following: \"git\n> > diff D..E is totally useless and should be an error because (1) it\n> > doesn't do what I expect and (2) for folks that want the behavior\n> > currently gotten with that syntax can instead just use a space instead\n> > of a double dot.\"\n> \n> That sums up pretty nicely. diff is fundamentally an operation\n> between two endpoints, so the range notation a..b does not work\n> nicely with it at the conceptual level.\n> \n> When we realized that we can take advantage of the above fact, and\n> reuse a range notation to mean something that is generally useful in\n> the context of diff, such as 'one end of the comparison is the merge\n> base between a and b, and the other end is b', it was too late to\n> use \"a..b\", as an early adopters of Git was already used to the fact\n> that \"a..b\" happened to mean the same thing as \"comparison of one\n> end is a, the other end is b\", pretty much implemented without much\n> thought.\n> \n> It might be _possible_ to spend a year (i.e. 4 cycles) to start\n> warning when two-dot notation is used for \"git diff\" (only, not any\n> plumbing like \"git diff-files\") and tell the user to use the more\n> logical two-end notation \"git diff A B\" and then eventually error\n> out when two-dot notation is used, while retaining the three-dot\n> notation throughout and to the eternity. I am not sure if it is\n> worth the deprecation cost, though.\n> \nInstead of outright deprecating it, would it make sense to include a\nconfiguration option, say \"diff.sensibleDots\", that would enable a user\nto set the dots to the other form if they desire?\n\nThis has bugged me for long enough that if there's a desire for this\nfrom others, I'm willing take on the effort of making this change.\n\nThanks,\n\nDenton\n"},{"id":"369413","messageId":"CABPp-BEmPQb4Q_38S2A_68m+Cu75VDDD2UV0qWBDjL1OUAug9Q@mail.gmail.com","threadId":"50504","inReplyTo":"20190215185202.GA28019@dev-l","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-02-15T19:25:43Z","receivedAt":"2019-02-15T19:25:58Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Feb 15, 2019 at 10:52 AM Denton Liu wrote:\n>\n> On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote:\n> > Elijah Newren writes:\n> >\n> > > The only thing I seem to be able to retain is the following: \"git\n> > > diff D..E is totally useless and should be an error because (1) it\n> > > doesn't do what I expect and (2) for folks that want the behavior\n> > > currently gotten with that syntax can instead just use a space instead\n> > > of a double dot.\"\n> >\n> > That sums up pretty nicely. diff is fundamentally an operation\n> > between two endpoints, so the range notation a..b does not work\n> > nicely with it at the conceptual level.\n> >\n> > When we realized that we can take advantage of the above fact, and\n> > reuse a range notation to mean something that is generally useful in\n> > the context of diff, such as 'one end of the comparison is the merge\n> > base between a and b, and the other end is b', it was too late to\n> > use \"a..b\", as an early adopters of Git was already used to the fact\n> > that \"a..b\" happened to mean the same thing as \"comparison of one\n> > end is a, the other end is b\", pretty much implemented without much\n> > thought.\n> >\n> > It might be _possible_ to spend a year (i.e. 4 cycles) to start\n> > warning when two-dot notation is used for \"git diff\" (only, not any\n> > plumbing like \"git diff-files\") and tell the user to use the more\n> > logical two-end notation \"git diff A B\" and then eventually error\n> > out when two-dot notation is used, while retaining the three-dot\n> > notation throughout and to the eternity. I am not sure if it is\n> > worth the deprecation cost, though.\n> >\n> Instead of outright deprecating it, would it make sense to include a\n> configuration option, say \"diff.sensibleDots\", that would enable a user\n> to set the dots to the other form if they desire?\n\nI think Junio's suggested steps would still have to come first, and\nthere may also need to be a time period after two-dot notation is an\nerror before we were to try to repurpose it for something else (e.g.\nto make it behave the same as triple-dot). Adding a config to change\nthings now without both a deprecation and error period would invite\nhorrible surprises and bug reports; people need time and warning to\nchange workflows and fix existing scripts that might be out there.\n\n> This has bugged me for long enough that if there's a desire for this\n> from others, I'm willing take on the effort of making this change.\n\nI was going to put the deprecation and erroring steps on my (always\ngrowing) TODO list to eventually get to it, but if you're motivated,\ngo for it; I wouldn't get to it soon anyway.\n"},{"id":"369414","messageId":"xmqq4l946ebw.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"20190215185202.GA28019@dev-l","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-15T19:28:51Z","receivedAt":"2019-02-15T19:28:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote:\n>\n>> It might be _possible_ to spend a year (i.e. 4 cycles) to start\n>> warning when two-dot notation is used for \"git diff\" (only, not any\n>> plumbing like \"git diff-files\") and tell the user to use the more\n>> logical two-end notation \"git diff A B\" and then eventually error\n>> out when two-dot notation is used, while retaining the three-dot\n>> notation throughout and to the eternity. I am not sure if it is\n>> worth the deprecation cost, though.\n>> \n> Instead of outright deprecating it, would it make sense to include a\n> configuration option, say \"diff.sensibleDots\", that would enable a user\n> to set the dots to the other form if they desire?\n\nI doubt that such a change helps anybody more than it hurts.\nDefining A..B to mean something totally different based on a hidden\nconfig will hurt those who want to help others. It would even hurt\nthose who are new to Git and blindly set such configuration because\ntheir local guru tells them to and then goes on to read code\nsnippets and docs on the web outside our control that have been\nwritten with the established meaning of what A..B and A...B mean to\n\"git diff\".\n\nThere is nothing sensible in using dots, i.e. range notation,\nbetween A and B _if_ you are trying to compare A and B, i.e. two\nendpoints. Just use \"diff A B\" and you are OK.\n"},{"id":"369415","messageId":"xmqqzhqw4xqe.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"CABPp-BEmPQb4Q_38S2A_68m+Cu75VDDD2UV0qWBDjL1OUAug9Q@mail.gmail.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-15T20:12:41Z","receivedAt":"2019-02-15T20:12:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> Instead of outright deprecating it, would it make sense to include a\n>> configuration option, say \"diff.sensibleDots\", that would enable a user\n>> to set the dots to the other form if they desire?\n>\n> I think Junio's suggested steps would still have to come first, and\n> there may also need to be a time period after two-dot notation is an\n> error before we were to try to repurpose it for something else (e.g.\n> to make it behave the same as triple-dot). Adding a config to change\n> things now without both a deprecation and error period would invite\n> horrible surprises and bug reports; people need time and warning to\n> change workflows and fix existing scripts that might be out there.\n\nHistorically, it was a mistake to allow A..B to be used for two\nendpoints, which was made back when we haven't thought things\nthrough. That is why I stopped \"warn to deprecate and then\ncompletely remove\", as I do not think it would help people very much\nif \"git diff A B\" can be spelled with two-dots.\n\nBut in a distant future long after that happens, by the time nobody\nremembers what A..B meant for \"git diff\", I do not think I'd\nstrongly be opposed to reusing it to mean something different.\n"},{"id":"369421","messageId":"ad85402e-0ad2-4f4d-3b66-9250115072c0@iee.org","threadId":"50504","inReplyTo":"xmqqzhqw4xqe.fsf@gitster-ct.c.googlers.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-02-15T22:48:40Z","receivedAt":"2019-02-15T22:48:43Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 15/02/2019 20:12, Junio C Hamano wrote:\n> Elijah Newren writes:\n>\n>>> Instead of outright deprecating it, would it make sense to include a\n>>> configuration option, say \"diff.sensibleDots\", that would enable a user\n>>> to set the dots to the other form if they desire?\n>> I think Junio's suggested steps would still have to come first, and\n>> there may also need to be a time period after two-dot notation is an\n>> error before we were to try to repurpose it for something else (e.g.\n>> to make it behave the same as triple-dot). Adding a config to change\n>> things now without both a deprecation and error period would invite\n>> horrible surprises and bug reports; people need time and warning to\n>> change workflows and fix existing scripts that might be out there.\n> Historically, it was a mistake to allow A..B to be used for two\n> endpoints, which was made back when we haven't thought things\n> through. That is why I stopped \"warn to deprecate and then\n> completely remove\", as I do not think it would help people very much\n> if \"git diff A B\" can be spelled with two-dots.\n>\n> But in a distant future long after that happens, by the time nobody\n> remembers what A..B meant for \"git diff\", I do not think I'd\n> strongly be opposed to reusing it to mean something different.\n\nWould an option be to add a opt-in config to do the warning, rather than \nstart immediately at a deprecation warning?\n\nIt would give users the chance to test out their usage early should the \nso wish/desire/notice.\n\nI'd agree that the deprecation period would need to be quite a few \ncycles before the default (unset) would be flipped to warn if not set, \nyet still allow the warnings to be switched off for this case.\n\n(obviously after the current release..)\n\n"},{"id":"369422","messageId":"xmqqr2c84ohn.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"ad85402e-0ad2-4f4d-3b66-9250115072c0@iee.org","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-15T23:32:20Z","receivedAt":"2019-02-15T23:32:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> On 15/02/2019 20:12, Junio C Hamano wrote:\n>>\n>> Historically, it was a mistake to allow A..B to be used for two\n>> endpoints, which was made back when we haven't thought things\n>> through. That is why I stopped \"warn to deprecate and then\n>> completely remove\", as I do not think it would help people very much\n>> if \"git diff A B\" can be spelled with two-dots.\n>>\n>> But in a distant future long after that happens, by the time nobody\n>> remembers what A..B meant for \"git diff\", I do not think I'd\n>> strongly be opposed to reusing it to mean something different.\n>\n> Would an option be to add a opt-in config to do the warning, rather\n> than start immediately at a deprecation warning?\n\nWell, anything would be \"an option\". I am not sure it would be\nparticularly a good option to allow people to \"opt\" into getting\nwarned, only to get a chance to train their fingers not to type\ndouble-dot instead of a SP, earlier than other people, though.\n\n> It would give users the chance to test out their usage early should\n> the so wish/desire/notice.\n\nI am somewhat puzzled. What are you trying to achieve by that?\n\nThose who do *not* opt into that \"early warning\" configuration dance\nwould eventually be warned whenever they type \"diff A..B\", and the\ntiming for that eventuality is not under their control, so quite\nhonestly, I do not see much point in \"giving users the chance\".\n"},{"id":"369467","messageId":"dca9622d-c8e7-abcb-2e13-73bd8bb9dd84@iee.org","threadId":"50504","inReplyTo":"xmqqr2c84ohn.fsf@gitster-ct.c.googlers.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-02-16T12:47:52Z","receivedAt":"2019-02-16T12:47:57Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 15/02/2019 23:32, Junio C Hamano wrote:\n> Philip Oakley writes:\n>\n>> On 15/02/2019 20:12, Junio C Hamano wrote:\n>>> Historically, it was a mistake to allow A..B to be used for two\n>>> endpoints, which was made back when we haven't thought things\n>>> through. That is why I stopped \"warn to deprecate and then\n>>> completely remove\", as I do not think it would help people very much\n>>> if \"git diff A B\" can be spelled with two-dots.\n>>>\n>>> But in a distant future long after that happens, by the time nobody\n>>> remembers what A..B meant for \"git diff\", I do not think I'd\n>>> strongly be opposed to reusing it to mean something different.\n>> Would an option be to add a opt-in config to do the warning, rather\n>> than start immediately at a deprecation warning?\n> Well, anything would be \"an option\". I am not sure it would be\n> particularly a good option to allow people to \"opt\" into getting\n> warned, only to get a chance to train their fingers not to type\n> double-dot instead of a SP, earlier than other people, though.\nThe point of the suggestion was very much to provide users with the \nchoice of finger training at their own time and pace, rather than being \nforced by the deprecation sequence to be surprised at an inconvenient \ntime into having to respond.\n>\n>> It would give users the chance to test out their usage early should\n>> the so wish/desire/notice.\n> I am somewhat puzzled. What are you trying to achieve by that?\n>\n> Those who do *not* opt into that \"early warning\" configuration dance\n> would eventually be warned whenever they type \"diff A..B\", and the\n> timing for that eventuality is not under their control, so quite\n> honestly, I do not see much point in \"giving users the chance\".\n\nWith the opposite hat on, not giving users the choice does seem unfair \nto those that are trying to keep up. If we are warning (in the release \nnotes) of an upcoming deprecation (in the code) then it does seem \nhelpful that users could buy into the deprecation early, and at their \nconvenience, to assist in the unlearning of an old habit, which can be \nmuch harder than learning a new habit, hence my comment.\n\nYou are right that those who neither notice nor care will be surprised \nlater, but we shouldn't let that limit others.\n\n--\n\nPhilip\n\n"},{"id":"369475","messageId":"xmqq1s473x63.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"dca9622d-c8e7-abcb-2e13-73bd8bb9dd84@iee.org","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-17T03:34:44Z","receivedAt":"2019-02-17T03:34:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n>> Those who do *not* opt into that \"early warning\" configuration dance\n>> would eventually be warned whenever they type \"diff A..B\", and the\n>> timing for that eventuality is not under their control, so quite\n>> honestly, I do not see much point in \"giving users the chance\".\n>\n> With the opposite hat on, not giving users the choice does seem unfair\n> to those that are trying to keep up. If we are warning (in the release\n> notes) of an upcoming deprecation (in the code) then it does seem\n> helpful that users could buy into the deprecation early, and at their\n> convenience, to assist in the unlearning of an old habit, which can be\n> much harder than learning a new habit, hence my comment.\n>\n> You are right that those who neither notice nor care will be surprised\n> later, but we shouldn't let that limit others.\n\nI still do not quite get where you are coming from. Are you saying\nthat those who do not opt into the early warning may get 2 cycles\n(just picked out of thin-air) of deprecation period, and with an\noptional early warning feature, those who feel that 2 cycles is not\nlong enough to train their fingers would spend 3 cycles and they\nwill be helped than without?\n\nThat line of thinking sounds somewhat ridiculous---where does it\nend? If those who opt into would find it sufficient to have 3\ncycles to train, there may still be people who feel 3 is not enough\nand want to have 4. As we make it longer, we'd cover more people\nand at some point we'd reach the point of diminishing returns.\n\nWouldn't it be even better, and far simper, to just extend the\ndeprecation period to that many cycles to make it long enough for\nmajority of users' needs, without any early warning option? \n\nThe thing is, once you train your fingers, it does not matter to you\nif the deprecation warning is still there, as you'd not be typing\n\"diff A..B\" at that point. So I am not sure who you are trying to\nhelp by the early warning option.\n\nThanks.\n\n"},{"id":"369518","messageId":"d2127f77-9124-0e01-e96b-b6a1650ebc59@iee.org","threadId":"50504","inReplyTo":"xmqq1s473x63.fsf@gitster-ct.c.googlers.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-02-17T23:34:53Z","receivedAt":"2019-02-17T23:34:58Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Junio,\n\nOn 17/02/2019 03:34, Junio C Hamano wrote:\n> Philip Oakley writes:\n>\n>>> Those who do *not* opt into that \"early warning\" configuration dance\n>>> would eventually be warned whenever they type \"diff A..B\", and the\n>>> timing for that eventuality is not under their control, so quite\n>>> honestly, I do not see much point in \"giving users the chance\".\n>> With the opposite hat on, not giving users the choice does seem unfair\n>> to those that are trying to keep up. If we are warning (in the release\n>> notes) of an upcoming deprecation (in the code) then it does seem\n>> helpful that users could buy into the deprecation early, and at their\n>> convenience, to assist in the unlearning of an old habit, which can be\n>> much harder than learning a new habit, hence my comment.\n>>\n>> You are right that those who neither notice nor care will be surprised\n>> later, but we shouldn't let that limit others.\n> I still do not quite get where you are coming from. Are you saying\n> that those who do not opt into the early warning may get 2 cycles\n> (just picked out of thin-air) of deprecation period, and with an\n> optional early warning feature, those who feel that 2 cycles is not\n> long enough to train their fingers would spend 3 cycles and they\n> will be helped than without?\n\nIt was my understanding that the end point would be total removal of any \noptions and the typing of the double dot would be an error. Given that \nhard end point I was looking to ensure that users of double dots have a \nmanageable route to unlearning old bad habits. Thus the first phase \nwould be opt-in, the second phase opt-out, and on the third final phase \nit would be a non-optional error (assuming your first comment in [1]).\n\n>\n> That line of thinking sounds somewhat ridiculous---where does it\n> end? If those who opt into would find it sufficient to have 3\n> cycles to train, there may still be people who feel 3 is not enough\n> and want to have 4. As we make it longer, we'd cover more people\n> and at some point we'd reach the point of diminishing returns.\nThe length of the phase 1 is your choice, but having a zero length (as \nsome discussions implied) felt too short. For me, one cycle of users \n'opting-in' to do their testing and training given a deprecation \nnotification would be sufficient.\n>\n> Wouldn't it be even better, and far simper, to just extend the\n> deprecation period to that many cycles to make it long enough for\n> majority of users' needs, without any early warning option?\n>\n> The thing is, once you train your fingers,\nTo train the fingers, and to check local scripts and aliases, the user \nneeds feedback, preferably at a time of their convenience (as opposed to \nbeing a time of inconvenience), so assuming they have been paying \nmoderate attention to the release notes, providing the opt-in phase \ngives them that.\n> it does not matter to you\n> if the deprecation warning is still there, as you'd not be typing\n> \"diff A..B\" at that point. So I am not sure who you are trying to\n> help by the early warning option.\n>\n> Thanks.\nI do note that you had indicated at the end of [1]: \"I am not sure if it \nis worth the deprecation cost, though.\", so this may be a bit of a mute \npoint anyway.\n\nPhilip\n\n[1] \"then eventually error \nout when two-dot notation is used\"\n\n"},{"id":"369519","messageId":"xmqqpnrq2bfg.fsf@gitster-ct.c.googlers.com","threadId":"50504","inReplyTo":"d2127f77-9124-0e01-e96b-b6a1650ebc59@iee.org","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-18T00:21:55Z","receivedAt":"2019-02-18T00:22:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> It was my understanding that the end point would be total removal of\n> any options and the typing of the double dot would be an error. Given\n> that hard end point I was looking to ensure that users of double dots\n> have a manageable route to unlearning old bad habits. Thus the first\n> phase would be opt-in,\n\nSorry, but I do not see any logical connection in this \"Thus\". If\nwe are still undecided if deprecating double-dot is a good idea and\ntrying to gauge the impact, then perhaps an early \"opt-in\" to leave\nthe door open for aborting the transition plan might make sense (as\nan escape hatch for _us_ the project developers to make excuse to\nthe end users). But I am getting an impression that it is not the\nplan you have in mind.\n\n> To train the fingers, and to check local scripts and aliases, the user\n> needs feedback, preferably at a time of their convenience (as opposed\n> to being a time of inconvenience), so assuming they have been paying\n> moderate attention to the release notes, providing the opt-in phase\n> gives them that.\n\nAnd to those who haven't been paying attention, what happens when\nyour \"first phase\" period expires?\n\nI would be a lot more sympathetic if your argument were \"some people\nwill not be ready to start training, and they will be helped if we\nhad an opt-out knob early in the long deprecation period\". Even\nthose who have not been paying attention at all _will_ be hit by\ndeprecation warning, and that is when they can decide if they want\nto start training or they are not ready and want to postpone, so in\nthat sense, \"initial opt-out\" may make sense, but I do not see how\n\"initial opt-in\" can be a viable thing.\n\n"},{"id":"369520","messageId":"20190218043453.b3jkt365bvg2deox@vireshk-i7","threadId":"50504","inReplyTo":"CABPp-BFpj8DBxgsqe9Rqnzb4vx5fPqNc+sUj8LzrFMj2bacoBQ@mail.gmail.com","subject":"Re: [Bug report] git diff stat shows unrelated diff","fromName":"Viresh Kumar","fromEmail":"viresh.kumar@linaro.org","sentAt":"2019-02-18T04:34:53Z","receivedAt":"2019-02-18T04:34:59Z","isPatch":false,"sender":{"key":"viresh.kumar@linaro.org","avatar":"https://gravatar.com/avatar/938b1abce576d6c6d2734c01fce1c75040618c8fc0d1b7e836277ecd9f35fb19?d=mp&s=160"},"body":"On 15-02-19, 08:09, Elijah Newren wrote:\n> Hi Viresh,\n> \n> On Thu, Feb 14, 2019 at 10:40 PM Viresh Kumar wrote:\n> >\n> > On 14-02-19, 13:23, Elijah Newren wrote:\n> > > I think you're getting tripped up by double-dot vs triple-dot with\n> > > diff being different than log:\n> > >\n> > > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the\n> > > two commits D and E.\n> >\n> > Right, so both the branches have at least until rc2 (though\n> > pm/linux-next had until rc6), why will the diff D..E show the diff\n> > between rc1 and rc2 ?\n> \n> I don't think it does; it includes the diff between rc2 and rc6.\n> First, `git diff D..E` will show you roughly the same thing as if you\n> have two clones of linux, one with D checked out, one with E checked\n> out, and then you run 'diff -ru linux-copy1/ linux-copy2/'.\n> \n> But let's go back to your example, you had\n> \n> Branch A: commit 55538fbc79e9\n> Branch B: commit a4f342b9607d\n> Branch C: commit 7c139d3f0f99\n> Branch pm/linux-next: unspecified, but I'll guess commit a06639e89e4\n\nI mentioned that at the bottom of the email, d26ff2405272 :)\n\nAnd yes I got confused somehow earlier, ran all the commands again to\nverify the diffs, sorry about that.\n\n-- \nviresh\n"}]}