This is the second version of the patch series. Patch 1: Be able to use rebase.forkpoint and --root Patch 2: Adding the warning + tests Patch 3: Update documenation I think I have covered most of your concerns and feedback in this second version. On 8/31/23 16:57, Junio C Hamano wrote: > Wesley Schwengle <wesleys@xxxxxxxxxxxxxxx> writes: > > Here is my attempt to rewrite the above: > > When 'git rebase' is run without specifying <upstream> on the > command line, the current default is to use the fork-point > heuristics, but this is expected to change in a future version > of Git, and you will have to explicitly give "--fork-point" from > the command line if you keep using the fork-point mode. You can > run "git config rebase.forkpoint false" to adopt the new default > in advance and that will also squelch the message. I agree. I'll change the text to your version. > Note that the parsing of "rebase.forkpoint" is a bit peculiar in > that > > - By leaving it unspecified, the .fork_point = -1 in > REBASE_OPTIONS_INIT takes effect (which is unsurprising); > > - By setting it to false, .fork_point becomes 0; but > > - If you set the configuration variable to true, .fork_point > becomes -1, not 1. I changed this in patch 1. > And this is very much deliberate if I understand it correctly [*1*]. > By the time we get to this part of the code (i.e. .fork_point is > -1), the user may already have rebase.forkpoint set to true. IOW, > setting it to 'true' is not a valid way to squelch this message. So this works now with patch 2. > Another worrysome thing about rebase.forkpoint is that it will be > inevitable for folks to start complaining that it does not work the > way other configuration variables do. Setting the variable to > 'true' is not the same as passing '--fork-point=true' from the > command line. I think it is now with the current series. > I actually think it would be a lot larger behaviour change with a > huge potential to be received as a regression if we start making the > variable to mean the same thing as passing '--fork-point=true'. > People may like the current "if you are rebuilding your branch on > its usual upstream, pay attention to the rebase and rewind of the > upstream itself, but if you are giving an explicit upstream from the > command line, the tool does not second guess you with the fork-point > heuristics" behaviour and prefer to set it to true. We would be > breaking them big time if suddenly the rebase.forkpoint=true they > set previously starts triggering the fork-point heuristics when they > run "git rebase upstream". So that needs to be kept in mind when/if > we fix the "setting the variable, even to 'true', will squelch the > warning". I get what you are saying. My solution is to make the --fork-point or --no-fork-point more explicit. People could use an alias for this? It would mean a different approach to the problem and deprecating rebase.forkpoint as a boolean value. It could become one of three values: "true", "false" and "legacy". Where "legacy" can be "implicit" or "auto". Although you had some ideas on "auto" already. I'm not sure on how I would call it. "no-upstream"? -- Wesley Why not both?
