* Possible git bug @ 2021-09-16 3:29 Wesley Schwengle 2021-09-16 5:37 ` Taylor Blau 0 siblings, 1 reply; 20+ messages in thread From: Wesley Schwengle @ 2021-09-16 3:29 UTC (permalink / raw) To: git Hello, I'm running into a weird issue with git at the moment and I'm wondering if this is a bug. I have a small reproduction path: $ mkdir reproduction $ cd reproduction $ git init . $ echo "commit a" > file.txt $ git commit -m "First commit" file.txt $ echo "commit b" >> file.txt $ git commit -m "Second commit" file.txt $ git switch -c foo $ echo "commit c" >> file.txt" $ git commit -m "Third commit" file.txt $ git branch --set-upstream-to=master $ git status On branch foo Your branch is ahead of 'master' by 1 commit. $ git switch master $ git merge foo $ git reset --hard HEAD^ $ git switch foo Switched to branch 'foo' Your branch is ahead of 'master' by 1 commit. $ git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset' 5f427e3 Third commit 03ad791 Second commit 411e6d4 First commit $ git rebase master $ git status On branch foo Your branch is up to date with 'master'. $ git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset' 03ad791 Second commit 411e6d4 First commit I do not expect to lose the commits from foo, it seems the ref of master doesn't get updated after a reset. I didn't lose any work because I pushed my work to a remote and can still get it from there (and git reflog would also reach my code). It is rather surprising to see this kind of behavior. I'm running: $ git --version git version 2.33.0.363.g4c719308ce But I also got this behavior with the git shipped with Debian: $ /usr/bin/git --version git version 2.32.0 Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 3:29 Possible git bug Wesley Schwengle @ 2021-09-16 5:37 ` Taylor Blau 2021-09-16 12:07 ` Wesley Schwengle 0 siblings, 1 reply; 20+ messages in thread From: Taylor Blau @ 2021-09-16 5:37 UTC (permalink / raw) To: Wesley Schwengle; +Cc: git On Wed, Sep 15, 2021 at 11:29:14PM -0400, Wesley Schwengle wrote: > > Hello, > > I'm running into a weird issue with git at the moment and I'm wondering if > this is a bug. I have a small reproduction path: > $ git commit -m "First commit" file.txt FWIW, I had to tweak this script a little, since file.txt is untracked before it is added initially. (So a "git add file.txt" before this first commit is required.) But even after this, I got exactly what I expected from this script (which was that your "foo" branch had three commits before and after). Is there something else interesting going on with your setup that might explain why I can't reproduce this? Thanks, Taylor ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 5:37 ` Taylor Blau @ 2021-09-16 12:07 ` Wesley Schwengle 2021-09-16 12:47 ` wesley 0 siblings, 1 reply; 20+ messages in thread From: Wesley Schwengle @ 2021-09-16 12:07 UTC (permalink / raw) To: Taylor Blau; +Cc: git On 9/16/21 1:37 AM, Taylor Blau wrote: > FWIW, I had to tweak this script a little, since file.txt is untracked > before it is added initially. (So a "git add file.txt" before this first > commit is required.) Oh sorry! I overlooked that part. > But even after this, I got exactly what I expected from this script > (which was that your "foo" branch had three commits before and after). > Is there something else interesting going on with your setup that might > explain why I can't reproduce this? Oh, I found it.. Replace `git rebase master' with `git rebase' in the reproduction path. Disregard my post, it seems this is documented behavior in the rebase man-page. When you have an upstream configured and you don't specify it on the command line, --fork-point is used, while if you specify the upstream --no-fork-point is used. `git rebase master --fork-point' exhibits the same as I was seeing. Although I'm now completely confused by this behavior. It doesn't make sense to me. This happens: We are on a branch, we merge it into another branch. We undo the merge because reasons. Now we git rebase, without the upstream, because we've set it. Fork-point is used now, because we haven't specified an upstream, but we did set it and git merge-base decides, oh, we had those commits in master but these where dropped so we drop them in this branch as well. New question, is there a way to tell rebase to NOT use fork-point via git-config in this situation? Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 12:07 ` Wesley Schwengle @ 2021-09-16 12:47 ` wesley 2021-09-16 12:47 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 2021-09-16 15:33 ` Possible git bug Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: wesley @ 2021-09-16 12:47 UTC (permalink / raw) To: git; +Cc: me On 9/16/21 8:07 AM, Wesley Schwengle wrote: > New question, is there a way to tell rebase to NOT use fork-point via > git-config in this situation? I seem to have found the answer in the source code: rebase.forkpoint exists. Would you accept the following patch that adds the following text to the documentation? Cheers, Wesley ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 12:47 ` wesley @ 2021-09-16 12:47 ` wesley 2021-09-16 15:43 ` Junio C Hamano 2021-09-16 15:33 ` Possible git bug Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: wesley @ 2021-09-16 12:47 UTC (permalink / raw) To: git; +Cc: me From: Wesley Schwengle <wesley@opperschaap.net> The option exists and the rebase behaviour tricked me into thinking there was a bug with git. This will tell people how they can tweak the default behavior. Signed-off-by: Wesley Schwengle <wesley@opperschaap.net> --- Documentation/git-rebase.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 506345cb0e..8d2bee3365 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of ends up being empty, the <upstream> will be used as a fallback. + If <upstream> is given on the command line, then the default is -`--no-fork-point`, otherwise the default is `--fork-point`. +`--no-fork-point`, otherwise the default is `--fork-point`. You can override +this default by setting the configuration option `rebase.forkpoint` to false. + If your branch was based on <upstream> but <upstream> was rewound and your branch contains commits which were dropped, this option can be used -- 2.33.0.364.gff7047fb76 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 12:47 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley @ 2021-09-16 15:43 ` Junio C Hamano 2021-09-16 21:21 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 15:43 UTC (permalink / raw) To: wesley; +Cc: git, me wesley@schwengle.net writes: > From: Wesley Schwengle <wesley@opperschaap.net> > > The option exists and the rebase behaviour tricked me into thinking > there was a bug with git. This will tell people how they can tweak the > default behavior. This tells readers about almost nothing but your frustration. We, or anybody who will be reading "git log" in 6 months to improve the system, will not need to hear it. Instead we need to understand what the real problem is, what was wrong in the behaviour, or what the expected behaviour was and why the use of the feature was inappropriate in the particular case, without which it is impossible to understand why this sentence was added when a future developer and documenter tries to improve upon this text. > Signed-off-by: Wesley Schwengle <wesley@opperschaap.net> > --- > Documentation/git-rebase.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 506345cb0e..8d2bee3365 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of > ends up being empty, the <upstream> will be used as a fallback. > + > If <upstream> is given on the command line, then the default is > -`--no-fork-point`, otherwise the default is `--fork-point`. > +`--no-fork-point`, otherwise the default is `--fork-point`. You can override > +this default by setting the configuration option `rebase.forkpoint` to false. We often do: "See also `rebase.forkpoint` in linkgit:git-config[1]." (for example, there is a reference to linkgit:githooks[5] in the same page). Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 15:43 ` Junio C Hamano @ 2021-09-16 21:21 ` Junio C Hamano 2021-09-16 22:35 ` Possible git bug wesley 2021-09-16 22:46 ` Possible git bug wesley 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 21:21 UTC (permalink / raw) To: wesley; +Cc: git, me Junio C Hamano <gitster@pobox.com> writes: > wesley@schwengle.net writes: > >> From: Wesley Schwengle <wesley@opperschaap.net> >> >> The option exists and the rebase behaviour tricked me into thinking >> there was a bug with git. This will tell people how they can tweak the >> default behavior. > > This tells readers about almost nothing but your frustration. > > We, or anybody who will be reading "git log" in 6 months to improve > the system, will not need to hear it. Instead we need to understand > > what the real problem is, what was wrong in the behaviour, or what > the expected behaviour was and why the use of the feature was > inappropriate in the particular case, without which it is impossible > to understand why this sentence was added when a future developer > and documenter tries to improve upon this text. I misspke a bit here. While hearing only your frustration and nothing else won't help us much, we do need to understand what caused your frustration, to avoid frustrating the next user the same way. All of the "Instead we need to understand ..." are about that. > We often do: > > "See also `rebase.forkpoint` in linkgit:git-config[1]." > > (for example, there is a reference to linkgit:githooks[5] in the > same page). One reason why you didn't find how to tweak the forkpoint feature, other than giving a command line option to countermand it, is because this link pointing at the list of configurations, where the variable is already described, was missing in the doucmentation for the "rebase" command. Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 21:21 ` Junio C Hamano @ 2021-09-16 22:35 ` wesley 2021-09-16 22:35 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 2021-09-16 22:46 ` Possible git bug wesley 1 sibling, 1 reply; 20+ messages in thread From: wesley @ 2021-09-16 22:35 UTC (permalink / raw) To: gitster; +Cc: git, me I've updated the patch and the commit message. I hope this is sufficient to be included. Cheers, Wesley ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:35 ` Possible git bug wesley @ 2021-09-16 22:35 ` wesley 2021-09-16 22:47 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: wesley @ 2021-09-16 22:35 UTC (permalink / raw) To: gitster; +Cc: git, me From: Wesley Schwengle <wesley@opperschaap.net> The option exists and the rebase behaviour tricked me into thinking there was a bug with git. This will tell people how they can tweak the default behavior. Signed-off-by: Wesley Schwengle <wesley@opperschaap.net> --- Documentation/git-rebase.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 506345cb0e..8d2bee3365 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of ends up being empty, the <upstream> will be used as a fallback. + If <upstream> is given on the command line, then the default is -`--no-fork-point`, otherwise the default is `--fork-point`. +`--no-fork-point`, otherwise the default is `--fork-point`. You can override +this default by setting the configuration option `rebase.forkpoint` to false. + If your branch was based on <upstream> but <upstream> was rewound and your branch contains commits which were dropped, this option can be used -- 2.33.0.364.gff7047fb76 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:35 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley @ 2021-09-16 22:47 ` Junio C Hamano 2021-09-16 22:50 ` Wesley Schwengle 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 22:47 UTC (permalink / raw) To: wesley; +Cc: git, me wesley@schwengle.net writes: > From: Wesley Schwengle <wesley@opperschaap.net> > > The option exists and the rebase behaviour tricked me into thinking > there was a bug with git. This will tell people how they can tweak the > default behavior. So this still does not explain what rebase behaviour tricked you into thinking so. That leaves the readers of "git log" frustrated, much more than a log message based only on a simple statement of fact, e.g. "git config --help" describes rebase.forkpoint as a way to give the default value for --[no-]forkpoint option, but "git rebase --help" does not mention it. Help people who visits the documentation of "rebase" to find the variable. or something like that. > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 506345cb0e..8d2bee3365 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of > ends up being empty, the <upstream> will be used as a fallback. > + > If <upstream> is given on the command line, then the default is > -`--no-fork-point`, otherwise the default is `--fork-point`. > +`--no-fork-point`, otherwise the default is `--fork-point`. You can override > +this default by setting the configuration option `rebase.forkpoint` to false. It is not wrong per-se, but sounds overly verbose and seems to give only half an advice. You can also set it to 'true' to override the default --no-fork-point given when you give <upstream> on the command line, no? So perhaps only a single line addition (plus downcasing "If" to "if"), i.e. + If `rebase.forkpoint` is set, that gives the default. Otherwise, if <upstream is given on the command line, the default is ... would be a better rewrite? Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:47 ` Junio C Hamano @ 2021-09-16 22:50 ` Wesley Schwengle 2021-09-16 22:53 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Wesley Schwengle @ 2021-09-16 22:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, me On 9/16/21 6:47 PM, Junio C Hamano wrote: > wesley@schwengle.net writes: > >> From: Wesley Schwengle <wesley@opperschaap.net> >> >> The option exists and the rebase behaviour tricked me into thinking >> there was a bug with git. This will tell people how they can tweak the >> default behavior. > > So this still does not explain what rebase behaviour tricked you > into thinking so. That leaves the readers of "git log" frustrated, > much more than a log message based only on a simple statement of > fact, e.g. > > "git config --help" describes rebase.forkpoint as a way to give > the default value for --[no-]forkpoint option, but "git rebase > --help" does not mention it. > > Help people who visits the documentation of "rebase" to find the > variable. > > or something like that. I made a booboo. I did not run format-patch with the updated commit message and commit. I saw it too late. You should have a new one by the time you read this :) Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:50 ` Wesley Schwengle @ 2021-09-16 22:53 ` Junio C Hamano 2021-09-20 14:34 ` Wesley Schwengle 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 22:53 UTC (permalink / raw) To: Wesley Schwengle; +Cc: git, me Wesley Schwengle <wesley@schwengle.net> writes: > I made a booboo. I did not run format-patch with the updated commit > message and commit. I saw it too late. You should have a new one by That is OK. Mistakes happen. I'll slow down to avoid further confusion from mails crossing, but I have a feeling that you either forgot to read, or sent an updated patch before reading, the latter half of the message you are responding to. Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:53 ` Junio C Hamano @ 2021-09-20 14:34 ` Wesley Schwengle 0 siblings, 0 replies; 20+ messages in thread From: Wesley Schwengle @ 2021-09-20 14:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, me On 9/16/21 6:53 PM, Junio C Hamano wrote: > Wesley Schwengle <wesley@schwengle.net> writes: > >> I made a booboo. I did not run format-patch with the updated commit >> message and commit. I saw it too late. You should have a new one by > > That is OK. Mistakes happen. > > I'll slow down to avoid further confusion from mails crossing, but I > have a feeling that you either forgot to read, or sent an updated > patch before reading, the latter half of the message you are > responding to. I did the same thing, the patch is present in the thread at https://public-inbox.org/git/20210916224603.2912887-1-wesley@schwengle.net/ Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 21:21 ` Junio C Hamano 2021-09-16 22:35 ` Possible git bug wesley @ 2021-09-16 22:46 ` wesley 2021-09-16 22:46 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 1 sibling, 1 reply; 20+ messages in thread From: wesley @ 2021-09-16 22:46 UTC (permalink / raw) To: gitster; +Cc: git, me I failed to include the correct format-patch, 3rd time is a charm :) Cheers, Wesley ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:46 ` Possible git bug wesley @ 2021-09-16 22:46 ` wesley 2021-09-20 16:07 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: wesley @ 2021-09-16 22:46 UTC (permalink / raw) To: gitster; +Cc: git, me From: Wesley Schwengle <wesley@opperschaap.net> The configuration option `rebase.forkpoint' is only mentioned in the man page of git-config(1). Since it is a configuration for rebase, mention it in the documentation of rebase at the --fork-point/--no-fork-point section. This will help users set a preferred default for their workflow. Signed-off-by: Wesley Schwengle <wesley@opperschaap.net> --- Documentation/git-rebase.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 506345cb0e..c116dbf4bb 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of ends up being empty, the <upstream> will be used as a fallback. + If <upstream> is given on the command line, then the default is -`--no-fork-point`, otherwise the default is `--fork-point`. +`--no-fork-point`, otherwise the default is `--fork-point`. See also +`rebase.forkpoint` in linkgit:git-config[1]. + If your branch was based on <upstream> but <upstream> was rewound and your branch contains commits which were dropped, this option can be used -- 2.33.0.364.gff7047fb76 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] Document `rebase.forkpoint` in rebase man page 2021-09-16 22:46 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley @ 2021-09-20 16:07 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2021-09-20 16:07 UTC (permalink / raw) To: wesley; +Cc: git, me wesley@schwengle.net writes: > From: Wesley Schwengle <wesley@opperschaap.net> > > The configuration option `rebase.forkpoint' is only mentioned in the man > page of git-config(1). Since it is a configuration for rebase, mention > it in the documentation of rebase at the --fork-point/--no-fork-point > section. This will help users set a preferred default for their > workflow. > > Signed-off-by: Wesley Schwengle <wesley@opperschaap.net> > --- > Documentation/git-rebase.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 506345cb0e..c116dbf4bb 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -446,7 +446,8 @@ When --fork-point is active, 'fork_point' will be used instead of > ends up being empty, the <upstream> will be used as a fallback. > + > If <upstream> is given on the command line, then the default is > -`--no-fork-point`, otherwise the default is `--fork-point`. > +`--no-fork-point`, otherwise the default is `--fork-point`. See also > +`rebase.forkpoint` in linkgit:git-config[1]. > + > If your branch was based on <upstream> but <upstream> was rewound and > your branch contains commits which were dropped, this option can be used I still think "the variable gives the default, otherwise the presence of <upstream> on the command line affects which one is used as the default" presentation order is better, but the above is a strict improvement over the current message, so let's take it. Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 12:47 ` wesley 2021-09-16 12:47 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley @ 2021-09-16 15:33 ` Junio C Hamano 2021-09-16 19:39 ` Wesley Schwengle 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 15:33 UTC (permalink / raw) To: wesley; +Cc: git, me wesley@schwengle.net writes: > On 9/16/21 8:07 AM, Wesley Schwengle wrote: > >> New question, is there a way to tell rebase to NOT use fork-point via >> git-config in this situation? > > I seem to have found the answer in the source code: rebase.forkpoint exists. > > Would you accept the following patch that adds the following text to the > documentation? Not so fast. Earlier you said: > ... When you have an upstream configured and you don't specify > it on the command line, --fork-point is used, while if you specify the > upstream --no-fork-point is used. `git rebase master --fork-point' > exhibits the same as I was seeing. Although I'm now completely > confused by this behavior. It doesn't make sense to me. > > This happens: > > We are on a branch, we merge it into another branch. > We undo the merge because reasons. > Now we git rebase, without the upstream, because we've set it. > Fork-point is used now, because we haven't specified an upstream, but > we did set it and git merge-base decides, oh, we had those commits in > master but these where dropped so we drop them in this branch as well. If you feel "It doesn't make sense to me", either - the behaviour does not make sense because it is simply buggy, in which case, adding a sentence to the documentation and explaining how not to use it is missing the point---don't you rather want it to behave in a way that makes sense to you instead? or - it appears as nonsense to you only because your understanding of the behaviour is faulty but the feature is working correctly and is not a bug, in which case, adding a sentence to the documentation and explaining how not to use it is missing the point---don't you rather want the existing documentation extended to help you and other users to understand the behaviour better first? Between "buggy behaviour" and "bad documentation of a well-designed behaviour", I offhand do not know which side "--fork-point" is for this particular case, but I've always felt that it is a bad heuristic that should be used with care, and my gut feeling is it might be the third possibility: "bad heuristic that sometimes misbehave badly and that is unfixable". If that is the case, perhaps the documentation should tell readers the unreliable nature of the option and warn them to double check the result before teaching them how to turn it off permanently. Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 15:33 ` Possible git bug Junio C Hamano @ 2021-09-16 19:39 ` Wesley Schwengle 2021-09-16 21:52 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Wesley Schwengle @ 2021-09-16 19:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, me On 9/16/21 11:33 AM, Junio C Hamano wrote: >> We are on a branch, we merge it into another branch. >> We undo the merge because reasons. >> Now we git rebase, without the upstream, because we've set it. >> Fork-point is used now, because we haven't specified an upstream, but >> we did set it and git merge-base decides, oh, we had those commits in >> master but these where dropped so we drop them in this branch as well. > > If you feel "It doesn't make sense to me", either > > - the behaviour does not make sense because it is simply buggy, in > which case, adding a sentence to the documentation and explaining > how not to use it is missing the point---don't you rather want it > to behave in a way that makes sense to you instead? > > or > > - it appears as nonsense to you only because your understanding of > the behaviour is faulty but the feature is working correctly and > is not a bug, in which case, adding a sentence to the > documentation and explaining how not to use it is missing the > point---don't you rather want the existing documentation extended > to help you and other users to understand the behaviour better > first? > > Between "buggy behaviour" and "bad documentation of a well-designed > behaviour", I offhand do not know which side "--fork-point" is for > this particular case, but I've always felt that it is a bad > heuristic that should be used with care, and my gut feeling is it > might be the third possibility: "bad heuristic that sometimes > misbehave badly and that is unfixable". If that is the case, > perhaps the documentation should tell readers the unreliable nature > of the option and warn them to double check the result before > teaching them how to turn it off permanently. I feel like it is a bad default, it caught me by surprise. Especially because in the reproduction path I wanted to explicit in my rebase action and this caused different behavior. After this was pointed out I read the man page because I thought `git rebase' and `git rebase master' was the same thing if that was configured as an upstream. It took me a while to figure this out, because I kept typing `git rebase' instead of `git rebase master' when quickly trying to find out why it wasn't behaving like it did earlier. I'm clueless about "buggy behavior", "bad documentation of a well designed feature" or "bad heuristic that sometimes misbehave badly and that is unfixable". To me `git rebase' with a configured upstream should behave the same as `git rebase @{u}'. Only when adding --fork-point it should behave as it does currently. I'm not sure when I would want to use it, but I'm thinking people want it, otherwise it wouldn't be a default. As for the patch. The reason why --fork-point is default I do not know, but how to disable it isn't documented and I think it should. It is hidden in the source code and the release notes of 2.31.0. It should be more visible. Which is the reason I submitted the patch. Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 19:39 ` Wesley Schwengle @ 2021-09-16 21:52 ` Junio C Hamano 2021-09-16 22:30 ` Wesley Schwengle 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2021-09-16 21:52 UTC (permalink / raw) To: Wesley Schwengle; +Cc: git, me Wesley Schwengle <wesley@schwengle.net> writes: > I feel like it is a bad default, it caught me by surprise. I tend to agree. It seems that d44e7126 (pull: support rebased upstream + fetch + pull --rebase, 2009-07-19) started it, probably by mistake, which was partially corrected by ad8261d2 (rebase: use reflog to find common base with upstream, 2013-12-09). The thread that contains https://lore.kernel.org/git/xmqq7gbdzsvt.fsf@gitster.dls.corp.google.com/ seems to have resulted in the design of the current behaviour, where the discussion refers to an even older discussion thread: https://lore.kernel.org/git/d8e9f102609ee4502f579cb4ce872e0a40756204.1381949622.git.john@keeping.me.uk/ Side note: I am kind-of surprised that I contributed the core computation of the fork-point logic, even though I wasn't buying it is a good feature back then. In any case, updating the documentation to refer to the configuraion variable that tweaks the default for --fork-point would be a good near-term thing to do, but in the longer term, I think it may make sense to fix this "surprise" and transition the default over time, i.e. (1) when "git rebase" is run without --[no-]fork-point from the command line, and without rebase.forkpoint configuration variable in effect, give a warning that says we'll change the default to 'false' and the users who want to can use the configuration variable to force it to 'true'. Update the documentation to say the special casing of "If <upstream> is not specified, --fork-point option is assumed" will be changed in the future. Ship such a version of Git and wait for several development cycles. (2) flip the default and remove the warning. Update the documentation. > As for the patch. The reason why --fork-point is default I do not > know, but how to disable it isn't documented and I think it should. It > is hidden in the source code and the release notes of 2.31.0. It > should be more visible. Which is the reason I submitted the patch. Certainly. "git config --help" is the only end-user facing place the reference from the configuration variable to the command line option is found. We should also have a backreference from the command line option to the configuration variable. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Possible git bug 2021-09-16 21:52 ` Junio C Hamano @ 2021-09-16 22:30 ` Wesley Schwengle 0 siblings, 0 replies; 20+ messages in thread From: Wesley Schwengle @ 2021-09-16 22:30 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, me On 9/16/21 5:52 PM, Junio C Hamano wrote: > Wesley Schwengle <wesley@schwengle.net> writes: >> As for the patch. The reason why --fork-point is default I do not >> know, but how to disable it isn't documented and I think it should. It >> is hidden in the source code and the release notes of 2.31.0. It >> should be more visible. Which is the reason I submitted the patch. > > Certainly. > > In any case, updating the documentation to refer to the configuraion > variable that tweaks the default for --fork-point would be a good > near-term thing to do, but in the longer term, I think it may make > sense to fix this "surprise" and transition the default over time, > i.e. > > "git config --help" is the only end-user facing place the reference > from the configuration variable to the command line option is found. > We should also have a backreference from the command line option to > the configuration variable. A simple reference in the documentation would be a good first step and the change of the default can than happen over multiple iterations/versions. Cheers, Wesley -- Wesley Schwengle E: wesley@schwengle.net ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-09-20 16:07 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-16 3:29 Possible git bug Wesley Schwengle 2021-09-16 5:37 ` Taylor Blau 2021-09-16 12:07 ` Wesley Schwengle 2021-09-16 12:47 ` wesley 2021-09-16 12:47 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 2021-09-16 15:43 ` Junio C Hamano 2021-09-16 21:21 ` Junio C Hamano 2021-09-16 22:35 ` Possible git bug wesley 2021-09-16 22:35 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 2021-09-16 22:47 ` Junio C Hamano 2021-09-16 22:50 ` Wesley Schwengle 2021-09-16 22:53 ` Junio C Hamano 2021-09-20 14:34 ` Wesley Schwengle 2021-09-16 22:46 ` Possible git bug wesley 2021-09-16 22:46 ` [PATCH] Document `rebase.forkpoint` in rebase man page wesley 2021-09-20 16:07 ` Junio C Hamano 2021-09-16 15:33 ` Possible git bug Junio C Hamano 2021-09-16 19:39 ` Wesley Schwengle 2021-09-16 21:52 ` Junio C Hamano 2021-09-16 22:30 ` Wesley Schwengle
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.