* [Bug report] git diff stat shows unrelated diff @ 2019-02-14 8:22 Viresh Kumar 2019-02-14 18:42 ` Johannes Sixt 2019-02-14 21:23 ` Elijah Newren 0 siblings, 2 replies; 17+ messages in thread From: Viresh Kumar @ 2019-02-14 8:22 UTC (permalink / raw) To: git; +Cc: vincent.guittot Hello, I am not sure if it is a bug or not, but the output I got wasn't what I was looking for. And so looking for some help. I was looking to send pull request [1] to PM maintainer and was generating the request against his tree [2], which already has kernel upto v5.0-rc6 merged in it. These are my local branches: Branch A: 55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP f896d06665ec cpufreq: qcom-hw: Move to device_initcall 1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2 Branch B: a4f342b9607d PM / OPP: Introduce a power estimation helper 285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only() bfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1 pm/linux-next branch already has Branch B merged in it (with an earlier pull request). Branch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I try to generate diff-stat with: git diff -M --stat pm/linux-next..7c139d3f0f99 It shows me the diffstat between v5.0-rc1..v5.0-rc2 as well. If I do git diff -M --stat v5.0-rc2..7c139d3f0f99 it shows only the files that have changed in patches in branch A and B. Since pm/linux-next already has Branch B and all the rcs upto rc6, I was expecting the output of first diffstat to be similar to second one (without branch B stuff). Is the expectation incorrect ? -- viresh [1] My tree: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git [2] PM tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git Top commit in PM tree at the time I was trying it: d26ff2405272 (pm/testing, pm/linux-next) Merge branch 'pm-opp' into linux-next ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-14 8:22 [Bug report] git diff stat shows unrelated diff Viresh Kumar @ 2019-02-14 18:42 ` Johannes Sixt 2019-02-14 21:23 ` Elijah Newren 1 sibling, 0 replies; 17+ messages in thread From: Johannes Sixt @ 2019-02-14 18:42 UTC (permalink / raw) To: Viresh Kumar; +Cc: git, vincent.guittot Am 14.02.19 um 09:22 schrieb Viresh Kumar: > Hello, > > I am not sure if it is a bug or not, but the output I got wasn't what > I was looking for. And so looking for some help. I was looking to send > pull request [1] to PM maintainer and was generating the request > against his tree [2], which already has kernel upto v5.0-rc6 merged in > it. > > These are my local branches: > > Branch A: > > 55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP > f896d06665ec cpufreq: qcom-hw: Move to device_initcall > 1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2 > > Branch B: > > a4f342b9607d PM / OPP: Introduce a power estimation helper > 285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only() > bfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1 > > pm/linux-next branch already has Branch B merged in it (with an > earlier pull request). > > Branch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I > try to generate diff-stat with: > > git diff -M --stat pm/linux-next..7c139d3f0f99 > > It shows me the diffstat between v5.0-rc1..v5.0-rc2 as well. > > If I do > > git diff -M --stat v5.0-rc2..7c139d3f0f99 > > it shows only the files that have changed in patches in branch A and > B. > > Since pm/linux-next already has Branch B and all the rcs upto rc6, I > was expecting the output of first diffstat to be similar to second one > (without branch B stuff). Is the expectation incorrect ? Yes, I think your expectation is incorrect. The meaning of .. is different between diff and the log commands. For diff it is mostly just noise, that is your first diff command is actually equivalent to git diff -M --stat pm/linux-next 7c139d3f0f99 It is quite obvious IMO that this is the diff between two end points. What you most likely wanted to see is git diff -M --stat pm/linux-next...7c139d3f0f99 It shows the diff between a merge base of the two revisions and the second one, i.e. between a where Branch C branched off of pm/linux-next and its tip. -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-14 8:22 [Bug report] git diff stat shows unrelated diff Viresh Kumar 2019-02-14 18:42 ` Johannes Sixt @ 2019-02-14 21:23 ` Elijah Newren 2019-02-14 22:10 ` Junio C Hamano 2019-02-15 6:40 ` Viresh Kumar 1 sibling, 2 replies; 17+ messages in thread From: Elijah Newren @ 2019-02-14 21:23 UTC (permalink / raw) To: Viresh Kumar; +Cc: Git Mailing List, vincent.guittot Hi, On Thu, Feb 14, 2019 at 11:35 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hello, > > I am not sure if it is a bug or not, but the output I got wasn't what > I was looking for. And so looking for some help. I was looking to send > pull request [1] to PM maintainer and was generating the request > against his tree [2], which already has kernel upto v5.0-rc6 merged in > it. > > These are my local branches: > > Branch A: > > 55538fbc79e9 cpufreq: qcom: Read voltage LUT and populate OPP > f896d06665ec cpufreq: qcom-hw: Move to device_initcall > 1c7fc5cbc339 (tag: v5.0-rc2) Linux 5.0-rc2 > > Branch B: > > a4f342b9607d PM / OPP: Introduce a power estimation helper > 285881b51eb5 PM / OPP: Remove unused parameter of _generic_set_opp_clk_only() > bfeffd155283 (tag: v5.0-rc1) Linux 5.0-rc1 > > pm/linux-next branch already has Branch B merged in it (with an > earlier pull request). > > Branch C (7c139d3f0f99) is a merge of Branch A and Branch B. When I > try to generate diff-stat with: > > git diff -M --stat pm/linux-next..7c139d3f0f99 > > It shows me the diffstat between v5.0-rc1..v5.0-rc2 as well. > > If I do > > git diff -M --stat v5.0-rc2..7c139d3f0f99 > > it shows only the files that have changed in patches in branch A and > B. > > Since pm/linux-next already has Branch B and all the rcs upto rc6, I > was expecting the output of first diffstat to be similar to second one > (without branch B stuff). Is the expectation incorrect ? I think you're getting tripped up by double-dot vs triple-dot with diff being different than log: `git diff D..E` means the same thing as `git diff D E`, i.e. diff the two commits D and E. `git diff D...E` means the same thing as `git diff $(git merge-base D E) E` There are some people for whom this state of affairs makes sense. I am not one of them, and I suspect you aren't either. The arguments made by those who feel this makes sense seem reasonable to me in the moment when they present them, but I have never been able to remember these arguments longer than briefly. It just doesn't stick with me. The only thing I seem to be able to retain is the following: "git diff D..E is totally useless and should be an error because (1) it doesn't do what I expect and (2) for folks that want the behavior currently gotten with that syntax can instead just use a space instead of a double dot." Hope that helps, Elijah ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-14 21:23 ` Elijah Newren @ 2019-02-14 22:10 ` Junio C Hamano 2019-02-15 18:52 ` Denton Liu 2019-02-15 6:40 ` Viresh Kumar 1 sibling, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2019-02-14 22:10 UTC (permalink / raw) To: Elijah Newren; +Cc: Viresh Kumar, Git Mailing List, vincent.guittot Elijah Newren <newren@gmail.com> writes: > The only thing I seem to be able to retain is the following: "git > diff D..E is totally useless and should be an error because (1) it > doesn't do what I expect and (2) for folks that want the behavior > currently gotten with that syntax can instead just use a space instead > of a double dot." That sums up pretty nicely. diff is fundamentally an operation between two endpoints, so the range notation a..b does not work nicely with it at the conceptual level. When we realized that we can take advantage of the above fact, and reuse a range notation to mean something that is generally useful in the context of diff, such as 'one end of the comparison is the merge base between a and b, and the other end is b', it was too late to use "a..b", as an early adopters of Git was already used to the fact that "a..b" happened to mean the same thing as "comparison of one end is a, the other end is b", pretty much implemented without much thought. It might be _possible_ to spend a year (i.e. 4 cycles) to start warning when two-dot notation is used for "git diff" (only, not any plumbing like "git diff-files") and tell the user to use the more logical two-end notation "git diff A B" and then eventually error out when two-dot notation is used, while retaining the three-dot notation throughout and to the eternity. I am not sure if it is worth the deprecation cost, though. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-14 22:10 ` Junio C Hamano @ 2019-02-15 18:52 ` Denton Liu 2019-02-15 19:25 ` Elijah Newren 2019-02-15 19:28 ` Junio C Hamano 0 siblings, 2 replies; 17+ messages in thread From: Denton Liu @ 2019-02-15 18:52 UTC (permalink / raw) To: Junio C Hamano Cc: Elijah Newren, Viresh Kumar, Git Mailing List, vincent.guittot On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote: > Elijah Newren <newren@gmail.com> writes: > > > The only thing I seem to be able to retain is the following: "git > > diff D..E is totally useless and should be an error because (1) it > > doesn't do what I expect and (2) for folks that want the behavior > > currently gotten with that syntax can instead just use a space instead > > of a double dot." > > That sums up pretty nicely. diff is fundamentally an operation > between two endpoints, so the range notation a..b does not work > nicely with it at the conceptual level. > > When we realized that we can take advantage of the above fact, and > reuse a range notation to mean something that is generally useful in > the context of diff, such as 'one end of the comparison is the merge > base between a and b, and the other end is b', it was too late to > use "a..b", as an early adopters of Git was already used to the fact > that "a..b" happened to mean the same thing as "comparison of one > end is a, the other end is b", pretty much implemented without much > thought. > > It might be _possible_ to spend a year (i.e. 4 cycles) to start > warning when two-dot notation is used for "git diff" (only, not any > plumbing like "git diff-files") and tell the user to use the more > logical two-end notation "git diff A B" and then eventually error > out when two-dot notation is used, while retaining the three-dot > notation throughout and to the eternity. I am not sure if it is > worth the deprecation cost, though. > Instead of outright deprecating it, would it make sense to include a configuration option, say "diff.sensibleDots", that would enable a user to set the dots to the other form if they desire? This has bugged me for long enough that if there's a desire for this from others, I'm willing take on the effort of making this change. Thanks, Denton ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 18:52 ` Denton Liu @ 2019-02-15 19:25 ` Elijah Newren 2019-02-15 20:12 ` Junio C Hamano 2019-02-15 19:28 ` Junio C Hamano 1 sibling, 1 reply; 17+ messages in thread From: Elijah Newren @ 2019-02-15 19:25 UTC (permalink / raw) To: Denton Liu Cc: Junio C Hamano, Viresh Kumar, Git Mailing List, vincent.guittot On Fri, Feb 15, 2019 at 10:52 AM Denton Liu <liu.denton@gmail.com> wrote: > > On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote: > > Elijah Newren <newren@gmail.com> writes: > > > > > The only thing I seem to be able to retain is the following: "git > > > diff D..E is totally useless and should be an error because (1) it > > > doesn't do what I expect and (2) for folks that want the behavior > > > currently gotten with that syntax can instead just use a space instead > > > of a double dot." > > > > That sums up pretty nicely. diff is fundamentally an operation > > between two endpoints, so the range notation a..b does not work > > nicely with it at the conceptual level. > > > > When we realized that we can take advantage of the above fact, and > > reuse a range notation to mean something that is generally useful in > > the context of diff, such as 'one end of the comparison is the merge > > base between a and b, and the other end is b', it was too late to > > use "a..b", as an early adopters of Git was already used to the fact > > that "a..b" happened to mean the same thing as "comparison of one > > end is a, the other end is b", pretty much implemented without much > > thought. > > > > It might be _possible_ to spend a year (i.e. 4 cycles) to start > > warning when two-dot notation is used for "git diff" (only, not any > > plumbing like "git diff-files") and tell the user to use the more > > logical two-end notation "git diff A B" and then eventually error > > out when two-dot notation is used, while retaining the three-dot > > notation throughout and to the eternity. I am not sure if it is > > worth the deprecation cost, though. > > > Instead of outright deprecating it, would it make sense to include a > configuration option, say "diff.sensibleDots", that would enable a user > to set the dots to the other form if they desire? I think Junio's suggested steps would still have to come first, and there may also need to be a time period after two-dot notation is an error before we were to try to repurpose it for something else (e.g. to make it behave the same as triple-dot). Adding a config to change things now without both a deprecation and error period would invite horrible surprises and bug reports; people need time and warning to change workflows and fix existing scripts that might be out there. > This has bugged me for long enough that if there's a desire for this > from others, I'm willing take on the effort of making this change. I was going to put the deprecation and erroring steps on my (always growing) TODO list to eventually get to it, but if you're motivated, go for it; I wouldn't get to it soon anyway. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 19:25 ` Elijah Newren @ 2019-02-15 20:12 ` Junio C Hamano 2019-02-15 22:48 ` Philip Oakley 0 siblings, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2019-02-15 20:12 UTC (permalink / raw) To: Elijah Newren; +Cc: Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot Elijah Newren <newren@gmail.com> writes: >> Instead of outright deprecating it, would it make sense to include a >> configuration option, say "diff.sensibleDots", that would enable a user >> to set the dots to the other form if they desire? > > I think Junio's suggested steps would still have to come first, and > there may also need to be a time period after two-dot notation is an > error before we were to try to repurpose it for something else (e.g. > to make it behave the same as triple-dot). Adding a config to change > things now without both a deprecation and error period would invite > horrible surprises and bug reports; people need time and warning to > change workflows and fix existing scripts that might be out there. Historically, it was a mistake to allow A..B to be used for two endpoints, which was made back when we haven't thought things through. That is why I stopped "warn to deprecate and then completely remove", as I do not think it would help people very much if "git diff A B" can be spelled with two-dots. But in a distant future long after that happens, by the time nobody remembers what A..B meant for "git diff", I do not think I'd strongly be opposed to reusing it to mean something different. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 20:12 ` Junio C Hamano @ 2019-02-15 22:48 ` Philip Oakley 2019-02-15 23:32 ` Junio C Hamano 0 siblings, 1 reply; 17+ messages in thread From: Philip Oakley @ 2019-02-15 22:48 UTC (permalink / raw) To: Junio C Hamano, Elijah Newren Cc: Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot On 15/02/2019 20:12, Junio C Hamano wrote: > Elijah Newren <newren@gmail.com> writes: > >>> Instead of outright deprecating it, would it make sense to include a >>> configuration option, say "diff.sensibleDots", that would enable a user >>> to set the dots to the other form if they desire? >> I think Junio's suggested steps would still have to come first, and >> there may also need to be a time period after two-dot notation is an >> error before we were to try to repurpose it for something else (e.g. >> to make it behave the same as triple-dot). Adding a config to change >> things now without both a deprecation and error period would invite >> horrible surprises and bug reports; people need time and warning to >> change workflows and fix existing scripts that might be out there. > Historically, it was a mistake to allow A..B to be used for two > endpoints, which was made back when we haven't thought things > through. That is why I stopped "warn to deprecate and then > completely remove", as I do not think it would help people very much > if "git diff A B" can be spelled with two-dots. > > But in a distant future long after that happens, by the time nobody > remembers what A..B meant for "git diff", I do not think I'd > strongly be opposed to reusing it to mean something different. Would an option be to add a opt-in config to do the warning, rather than start immediately at a deprecation warning? It would give users the chance to test out their usage early should the so wish/desire/notice. I'd agree that the deprecation period would need to be quite a few cycles before the default (unset) would be flipped to warn if not set, yet still allow the warnings to be switched off for this case. (obviously after the current release..) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 22:48 ` Philip Oakley @ 2019-02-15 23:32 ` Junio C Hamano 2019-02-16 12:47 ` Philip Oakley 0 siblings, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2019-02-15 23:32 UTC (permalink / raw) To: Philip Oakley Cc: Elijah Newren, Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot Philip Oakley <philipoakley@iee.org> writes: > On 15/02/2019 20:12, Junio C Hamano wrote: >> >> Historically, it was a mistake to allow A..B to be used for two >> endpoints, which was made back when we haven't thought things >> through. That is why I stopped "warn to deprecate and then >> completely remove", as I do not think it would help people very much >> if "git diff A B" can be spelled with two-dots. >> >> But in a distant future long after that happens, by the time nobody >> remembers what A..B meant for "git diff", I do not think I'd >> strongly be opposed to reusing it to mean something different. > > Would an option be to add a opt-in config to do the warning, rather > than start immediately at a deprecation warning? Well, anything would be "an option". I am not sure it would be particularly a good option to allow people to "opt" into getting warned, only to get a chance to train their fingers not to type double-dot instead of a SP, earlier than other people, though. > It would give users the chance to test out their usage early should > the so wish/desire/notice. I am somewhat puzzled. What are you trying to achieve by that? Those who do *not* opt into that "early warning" configuration dance would eventually be warned whenever they type "diff A..B", and the timing for that eventuality is not under their control, so quite honestly, I do not see much point in "giving users the chance". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 23:32 ` Junio C Hamano @ 2019-02-16 12:47 ` Philip Oakley 2019-02-17 3:34 ` Junio C Hamano 0 siblings, 1 reply; 17+ messages in thread From: Philip Oakley @ 2019-02-16 12:47 UTC (permalink / raw) To: Junio C Hamano Cc: Elijah Newren, Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot On 15/02/2019 23:32, Junio C Hamano wrote: > Philip Oakley <philipoakley@iee.org> writes: > >> On 15/02/2019 20:12, Junio C Hamano wrote: >>> Historically, it was a mistake to allow A..B to be used for two >>> endpoints, which was made back when we haven't thought things >>> through. That is why I stopped "warn to deprecate and then >>> completely remove", as I do not think it would help people very much >>> if "git diff A B" can be spelled with two-dots. >>> >>> But in a distant future long after that happens, by the time nobody >>> remembers what A..B meant for "git diff", I do not think I'd >>> strongly be opposed to reusing it to mean something different. >> Would an option be to add a opt-in config to do the warning, rather >> than start immediately at a deprecation warning? > Well, anything would be "an option". I am not sure it would be > particularly a good option to allow people to "opt" into getting > warned, only to get a chance to train their fingers not to type > double-dot instead of a SP, earlier than other people, though. The point of the suggestion was very much to provide users with the choice of finger training at their own time and pace, rather than being forced by the deprecation sequence to be surprised at an inconvenient time into having to respond. > >> It would give users the chance to test out their usage early should >> the so wish/desire/notice. > I am somewhat puzzled. What are you trying to achieve by that? > > Those who do *not* opt into that "early warning" configuration dance > would eventually be warned whenever they type "diff A..B", and the > timing for that eventuality is not under their control, so quite > honestly, I do not see much point in "giving users the chance". With the opposite hat on, not giving users the choice does seem unfair to those that are trying to keep up. If we are warning (in the release notes) of an upcoming deprecation (in the code) then it does seem helpful that users could buy into the deprecation early, and at their convenience, to assist in the unlearning of an old habit, which can be much harder than learning a new habit, hence my comment. You are right that those who neither notice nor care will be surprised later, but we shouldn't let that limit others. -- Philip ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-16 12:47 ` Philip Oakley @ 2019-02-17 3:34 ` Junio C Hamano 2019-02-17 23:34 ` Philip Oakley 0 siblings, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2019-02-17 3:34 UTC (permalink / raw) To: Philip Oakley Cc: Elijah Newren, Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot Philip Oakley <philipoakley@iee.org> writes: >> Those who do *not* opt into that "early warning" configuration dance >> would eventually be warned whenever they type "diff A..B", and the >> timing for that eventuality is not under their control, so quite >> honestly, I do not see much point in "giving users the chance". > > With the opposite hat on, not giving users the choice does seem unfair > to those that are trying to keep up. If we are warning (in the release > notes) of an upcoming deprecation (in the code) then it does seem > helpful that users could buy into the deprecation early, and at their > convenience, to assist in the unlearning of an old habit, which can be > much harder than learning a new habit, hence my comment. > > You are right that those who neither notice nor care will be surprised > later, but we shouldn't let that limit others. I still do not quite get where you are coming from. Are you saying that those who do not opt into the early warning may get 2 cycles (just picked out of thin-air) of deprecation period, and with an optional early warning feature, those who feel that 2 cycles is not long enough to train their fingers would spend 3 cycles and they will be helped than without? That line of thinking sounds somewhat ridiculous---where does it end? If those who opt into would find it sufficient to have 3 cycles to train, there may still be people who feel 3 is not enough and want to have 4. As we make it longer, we'd cover more people and at some point we'd reach the point of diminishing returns. Wouldn't it be even better, and far simper, to just extend the deprecation period to that many cycles to make it long enough for majority of users' needs, without any early warning option? The thing is, once you train your fingers, it does not matter to you if the deprecation warning is still there, as you'd not be typing "diff A..B" at that point. So I am not sure who you are trying to help by the early warning option. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-17 3:34 ` Junio C Hamano @ 2019-02-17 23:34 ` Philip Oakley 2019-02-18 0:21 ` Junio C Hamano 0 siblings, 1 reply; 17+ messages in thread From: Philip Oakley @ 2019-02-17 23:34 UTC (permalink / raw) To: Junio C Hamano Cc: Elijah Newren, Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot Hi Junio, On 17/02/2019 03:34, Junio C Hamano wrote: > Philip Oakley <philipoakley@iee.org> writes: > >>> Those who do *not* opt into that "early warning" configuration dance >>> would eventually be warned whenever they type "diff A..B", and the >>> timing for that eventuality is not under their control, so quite >>> honestly, I do not see much point in "giving users the chance". >> With the opposite hat on, not giving users the choice does seem unfair >> to those that are trying to keep up. If we are warning (in the release >> notes) of an upcoming deprecation (in the code) then it does seem >> helpful that users could buy into the deprecation early, and at their >> convenience, to assist in the unlearning of an old habit, which can be >> much harder than learning a new habit, hence my comment. >> >> You are right that those who neither notice nor care will be surprised >> later, but we shouldn't let that limit others. > I still do not quite get where you are coming from. Are you saying > that those who do not opt into the early warning may get 2 cycles > (just picked out of thin-air) of deprecation period, and with an > optional early warning feature, those who feel that 2 cycles is not > long enough to train their fingers would spend 3 cycles and they > will be helped than without? It was my understanding that the end point would be total removal of any options and the typing of the double dot would be an error. Given that hard end point I was looking to ensure that users of double dots have a manageable route to unlearning old bad habits. Thus the first phase would be opt-in, the second phase opt-out, and on the third final phase it would be a non-optional error (assuming your first comment in [1]). > > That line of thinking sounds somewhat ridiculous---where does it > end? If those who opt into would find it sufficient to have 3 > cycles to train, there may still be people who feel 3 is not enough > and want to have 4. As we make it longer, we'd cover more people > and at some point we'd reach the point of diminishing returns. The length of the phase 1 is your choice, but having a zero length (as some discussions implied) felt too short. For me, one cycle of users 'opting-in' to do their testing and training given a deprecation notification would be sufficient. > > Wouldn't it be even better, and far simper, to just extend the > deprecation period to that many cycles to make it long enough for > majority of users' needs, without any early warning option? > > The thing is, once you train your fingers, To train the fingers, and to check local scripts and aliases, the user needs feedback, preferably at a time of their convenience (as opposed to being a time of inconvenience), so assuming they have been paying moderate attention to the release notes, providing the opt-in phase gives them that. > it does not matter to you > if the deprecation warning is still there, as you'd not be typing > "diff A..B" at that point. So I am not sure who you are trying to > help by the early warning option. > > Thanks. I do note that you had indicated at the end of [1]: "I am not sure if it is worth the deprecation cost, though.", so this may be a bit of a mute point anyway. Philip [1] <xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com> "then eventually error out when two-dot notation is used" ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-17 23:34 ` Philip Oakley @ 2019-02-18 0:21 ` Junio C Hamano 0 siblings, 0 replies; 17+ messages in thread From: Junio C Hamano @ 2019-02-18 0:21 UTC (permalink / raw) To: Philip Oakley Cc: Elijah Newren, Denton Liu, Viresh Kumar, Git Mailing List, vincent.guittot Philip Oakley <philipoakley@iee.org> writes: > It was my understanding that the end point would be total removal of > any options and the typing of the double dot would be an error. Given > that hard end point I was looking to ensure that users of double dots > have a manageable route to unlearning old bad habits. Thus the first > phase would be opt-in, Sorry, but I do not see any logical connection in this "Thus". If we are still undecided if deprecating double-dot is a good idea and trying to gauge the impact, then perhaps an early "opt-in" to leave the door open for aborting the transition plan might make sense (as an escape hatch for _us_ the project developers to make excuse to the end users). But I am getting an impression that it is not the plan you have in mind. > To train the fingers, and to check local scripts and aliases, the user > needs feedback, preferably at a time of their convenience (as opposed > to being a time of inconvenience), so assuming they have been paying > moderate attention to the release notes, providing the opt-in phase > gives them that. And to those who haven't been paying attention, what happens when your "first phase" period expires? I would be a lot more sympathetic if your argument were "some people will not be ready to start training, and they will be helped if we had an opt-out knob early in the long deprecation period". Even those who have not been paying attention at all _will_ be hit by deprecation warning, and that is when they can decide if they want to start training or they are not ready and want to postpone, so in that sense, "initial opt-out" may make sense, but I do not see how "initial opt-in" can be a viable thing. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 18:52 ` Denton Liu 2019-02-15 19:25 ` Elijah Newren @ 2019-02-15 19:28 ` Junio C Hamano 1 sibling, 0 replies; 17+ messages in thread From: Junio C Hamano @ 2019-02-15 19:28 UTC (permalink / raw) To: Denton Liu; +Cc: Elijah Newren, Viresh Kumar, Git Mailing List, vincent.guittot Denton Liu <liu.denton@gmail.com> writes: > On Thu, Feb 14, 2019 at 02:10:53PM -0800, Junio C Hamano wrote: > >> It might be _possible_ to spend a year (i.e. 4 cycles) to start >> warning when two-dot notation is used for "git diff" (only, not any >> plumbing like "git diff-files") and tell the user to use the more >> logical two-end notation "git diff A B" and then eventually error >> out when two-dot notation is used, while retaining the three-dot >> notation throughout and to the eternity. I am not sure if it is >> worth the deprecation cost, though. >> > Instead of outright deprecating it, would it make sense to include a > configuration option, say "diff.sensibleDots", that would enable a user > to set the dots to the other form if they desire? I doubt that such a change helps anybody more than it hurts. Defining A..B to mean something totally different based on a hidden config will hurt those who want to help others. It would even hurt those who are new to Git and blindly set such configuration because their local guru tells them to and then goes on to read code snippets and docs on the web outside our control that have been written with the established meaning of what A..B and A...B mean to "git diff". There is nothing sensible in using dots, i.e. range notation, between A and B _if_ you are trying to compare A and B, i.e. two endpoints. Just use "diff A B" and you are OK. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-14 21:23 ` Elijah Newren 2019-02-14 22:10 ` Junio C Hamano @ 2019-02-15 6:40 ` Viresh Kumar 2019-02-15 16:09 ` Elijah Newren 1 sibling, 1 reply; 17+ messages in thread From: Viresh Kumar @ 2019-02-15 6:40 UTC (permalink / raw) To: Elijah Newren, gitster; +Cc: Git Mailing List, j6t, vincent.guittot On 14-02-19, 13:23, Elijah Newren wrote: > I think you're getting tripped up by double-dot vs triple-dot with > diff being different than log: > > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the > two commits D and E. Right, so both the branches have at least until rc2 (though pm/linux-next had until rc6), why will the diff D..E show the diff between rc1 and rc2 ? > `git diff D...E` means the same thing as `git diff $(git merge-base D E) E` I get exactly the same result with both .. and ... in this particular case and that's why I wonder if everything is okay or not. The problem in this case is: - PM tree had rc1,2,3,4,5,6 merged earlier into it. - Then I got merged one of my branches which was based of rc1 into pm/linux-next. - And now I am trying to send pull request for another branch which is a merge of the earlier branch (which got merged, based of rc1) and second branch that has more stuff over rc2. - The most recent merge commit common between my branch and pm/linux-next becomes the earlier branch which was based of rc1. - Now I expect ... to return the diff between rc1 and rc2 as it will diff against the most recent merge. - But I expected ... to not include rc1..rc2 diff. > There are some people for whom this state of affairs makes sense. I > am not one of them, and I suspect you aren't either. The arguments > made by those who feel this makes sense seem reasonable to me in the > moment when they present them, but I have never been able to remember > these arguments longer than briefly. It just doesn't stick with me. > The only thing I seem to be able to retain is the following: "git > diff D..E is totally useless and should be an error because (1) it > doesn't do what I expect and (2) for folks that want the behavior > currently gotten with that syntax can instead just use a space instead > of a double dot." Okay but git request-pull uses .. and not ... and that's where I saw the issue in the first place. -- viresh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 6:40 ` Viresh Kumar @ 2019-02-15 16:09 ` Elijah Newren 2019-02-18 4:34 ` Viresh Kumar 0 siblings, 1 reply; 17+ messages in thread From: Elijah Newren @ 2019-02-15 16:09 UTC (permalink / raw) To: Viresh Kumar Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, vincent.guittot Hi Viresh, On Thu, Feb 14, 2019 at 10:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 14-02-19, 13:23, Elijah Newren wrote: > > I think you're getting tripped up by double-dot vs triple-dot with > > diff being different than log: > > > > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the > > two commits D and E. > > Right, so both the branches have at least until rc2 (though > pm/linux-next had until rc6), why will the diff D..E show the diff > between rc1 and rc2 ? I don't think it does; it includes the diff between rc2 and rc6. First, `git diff D..E` will show you roughly the same thing as if you have two clones of linux, one with D checked out, one with E checked out, and then you run 'diff -ru linux-copy1/ linux-copy2/'. But let's go back to your example, you had Branch A: commit 55538fbc79e9 Branch B: commit a4f342b9607d Branch C: commit 7c139d3f0f99 Branch pm/linux-next: unspecified, but I'll guess commit a06639e89e4 Now, using git describe on each of those commits in order shows us where in history they are: A: v5.0-rc2-2-g55538fbc79e9 B: v5.0-rc1-2-ga4f342b9607d C: v5.0-rc2-5-g7c139d3f0f99 D: v5.0-rc6-84-ga06639e89e47 Now, you said you ran git diff -M --stat pm/linux-next..7c139d3f0f99 and that it included the diff between rc1 and rc2. I think you actually saw pieces of the diff from rc2 to rc6 and assumed it was the diff from rc1 to rc2. Besides the fact that pm/linux-next is based on rc6 and 7c139d3f0f99 is based on rc2, here's another way to verify that: $ git diff --name-only v5.0-rc1 v5.0-rc2 | sort > early $ git diff --name-only v5.0-rc2 v5.0-rc6 | sort > late $ comm -23 early late > early-only $ comm -13 early late > late-only $ wc -l *-only 407 early-only 1149 late-only 1556 total So, here "early-only" is a list of files that changed between rc1 and rc2 that did not change between rc2 and rc6. "late-only" is a list of files that did not change between rc1 and rc2, but did change between rc2 and rc6. No, let's compare that to your diff: $ git diff --name-only a06639e89e4..7c139d3f0f99 | sort > changes $ comm -12 changes early-only | wc -l 6 $ comm -12 changes late-only | wc -l 1148 So, the files listed in your diff only included 6 files that were unique to early-only, and included 1148 files that were unique to late-only. So, your diff looks an awful lot like the diff between rc2 and rc6, and not much at all like the diff between rc1 and rc2. > > `git diff D...E` means the same thing as `git diff $(git merge-base D E) E` > > I get exactly the same result with both .. and ... in this particular > case and that's why I wonder if everything is okay or not. With `git diff D..E` it doesn't matter much which order you put D and E in, other than flipping '-' lines for '+' lines. With `git diff D...E` it makes a huge difference. Compare: $ git diff --shortstat 7c139d3f0f99..a06639e89e4 1466 files changed, 15417 insertions(+), 7313 deletions(-) $ git diff --shortstat a06639e89e4..7c139d3f0f99 1466 files changed, 7313 insertions(+), 15417 deletions(-) So, as I said, swapping with double-dot only changes '-' and '+' lines. In contrast: $ git diff --shortstat 7c139d3f0f99...a06639e89e4 1463 files changed, 15401 insertions(+), 7165 deletions(-) $ git diff --shortstat a06639e89e4...7c139d3f0f99 4 files changed, 148 insertions(+), 16 deletions(-) You get a really small diff in one direction, but huge in the other. The reason for this is that commit C (7c139d3f0f99) is really close to the merge base, but pm/linux-next (a06639e89e4) is really far from it -- it includes rc6. > The problem in this case is: > - PM tree had rc1,2,3,4,5,6 merged earlier into it. > - Then I got merged one of my branches which was based of rc1 into > pm/linux-next. > - And now I am trying to send pull request for another branch which is > a merge of the earlier branch (which got merged, based of rc1) and > second branch that has more stuff over rc2. > - The most recent merge commit common between my branch and > pm/linux-next becomes the earlier branch which was based of rc1. > - Now I expect ... to return the diff between rc1 and rc2 as it will > diff against the most recent merge. > - But I expected ... to not include rc1..rc2 diff. > > > There are some people for whom this state of affairs makes sense. I > > am not one of them, and I suspect you aren't either. The arguments > > made by those who feel this makes sense seem reasonable to me in the > > moment when they present them, but I have never been able to remember > > these arguments longer than briefly. It just doesn't stick with me. > > The only thing I seem to be able to retain is the following: "git > > diff D..E is totally useless and should be an error because (1) it > > doesn't do what I expect and (2) for folks that want the behavior > > currently gotten with that syntax can instead just use a space instead > > of a double dot." > > Okay but git request-pull uses .. and not ... and that's where I saw > the issue in the first place. I haven't used request-pull myself and I'm not familiar with the code, so I don't know if you might have just passed it the wrong arguments or if it has a bug. I'll have to leave it to someone else to comment on that (though they may need to know what arguments you passed to that command). Hope that helps, Elijah ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Bug report] git diff stat shows unrelated diff 2019-02-15 16:09 ` Elijah Newren @ 2019-02-18 4:34 ` Viresh Kumar 0 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2019-02-18 4:34 UTC (permalink / raw) To: Elijah Newren Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, vincent.guittot On 15-02-19, 08:09, Elijah Newren wrote: > Hi Viresh, > > On Thu, Feb 14, 2019 at 10:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 14-02-19, 13:23, Elijah Newren wrote: > > > I think you're getting tripped up by double-dot vs triple-dot with > > > diff being different than log: > > > > > > `git diff D..E` means the same thing as `git diff D E`, i.e. diff the > > > two commits D and E. > > > > Right, so both the branches have at least until rc2 (though > > pm/linux-next had until rc6), why will the diff D..E show the diff > > between rc1 and rc2 ? > > I don't think it does; it includes the diff between rc2 and rc6. > First, `git diff D..E` will show you roughly the same thing as if you > have two clones of linux, one with D checked out, one with E checked > out, and then you run 'diff -ru linux-copy1/ linux-copy2/'. > > But let's go back to your example, you had > > Branch A: commit 55538fbc79e9 > Branch B: commit a4f342b9607d > Branch C: commit 7c139d3f0f99 > Branch pm/linux-next: unspecified, but I'll guess commit a06639e89e4 I mentioned that at the bottom of the email, d26ff2405272 :) And yes I got confused somehow earlier, ran all the commands again to verify the diffs, sorry about that. -- viresh ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-02-18 4:34 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-14 8:22 [Bug report] git diff stat shows unrelated diff Viresh Kumar 2019-02-14 18:42 ` Johannes Sixt 2019-02-14 21:23 ` Elijah Newren 2019-02-14 22:10 ` Junio C Hamano 2019-02-15 18:52 ` Denton Liu 2019-02-15 19:25 ` Elijah Newren 2019-02-15 20:12 ` Junio C Hamano 2019-02-15 22:48 ` Philip Oakley 2019-02-15 23:32 ` Junio C Hamano 2019-02-16 12:47 ` Philip Oakley 2019-02-17 3:34 ` Junio C Hamano 2019-02-17 23:34 ` Philip Oakley 2019-02-18 0:21 ` Junio C Hamano 2019-02-15 19:28 ` Junio C Hamano 2019-02-15 6:40 ` Viresh Kumar 2019-02-15 16:09 ` Elijah Newren 2019-02-18 4:34 ` Viresh Kumar
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).