Re: [PATCH] Documentation/git-rebase.txt: discuss --fork-point assumption of vanilla git rebase in DESCRIPTION.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: I think you meant to say that we may find a better source to calculate the exact set of commits to rebase,... Yes. It is debatable if we should do the same when the user tells us to rebase with respect to a specific _branch_ by giving the 'upstream' argument, but that is an entirely separate issue. We might want to do a similar command line heuristics to tell between the branch switching git checkout master (which is an operation about a branch) and head detaching git checkout refs/heads/master^0 (which is an operation about a commit) if we want to help the users by auto-enabling fork-point mode. Well, IMHO git rebase and git rebase @{u} must do exactly the same thing. That is not part of the current discussion is what I meant by It is debatable... We might want to. There is no such patch to git rebase itself in this topic ;-). Yes, but to suggest better documentation I figure I need to understand all the related issues, so it is still somewhat relevant. With the We might want to, I mean git rebase, git rebase @{u} and git rebase origin/master (if your @{u} happens to be that branch) may want to do exactly the same thing. The last one however is very questionable, as sometimes you would want the --fork-point heuristics, and some other times you would want no digging of the reflogs involved (i.e. I want everything not in this _exact_ commit to be rebased). On the other hand, I'm afraid different defaults were chosen for backward compatibility? There is no backward compatibility issue involved with the current behaviour. Changing it _will_ break compatibility, of course. It is more like the command used not to guess with fork-point at all, i.e. we liked its exactness, but git rebase (no argument) case is so obviously not about an exact commit but is about branch that it is safe to use --fork-point guess without being confusing. Well, that's exactly what ended-up being /extremely/ confusing in my case. Once you start giving the commit/branch with respect to which you conduct your rebase, it no longer is so cut-and-dried obvious that by git rebase @{u} if the user wants us to guess by digging the reflog of @{u} to find a better fork point, or if the user wants to do an exact rebase with respect to the commit at the tip of that branch. Whatever excuses are, to me it still looks entirely unnatural that 'git rebase' and 'git rebase @{u}' mean almost the same /except/ the default value of --fork-point is different, sorry. P.S. I'll prepare improved patch for the documentation shortly. -- Segey. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-rebase.txt: discuss --fork-point assumption of vanilla git rebase in DESCRIPTION.
Sergey Organov sorga...@gmail.com writes: I think you meant to say that we may find a better source to calculate the exact set of commits to rebase,... Yes. It is debatable if we should do the same when the user tells us to rebase with respect to a specific _branch_ by giving the 'upstream' argument, but that is an entirely separate issue. We might want to do a similar command line heuristics to tell between the branch switching git checkout master (which is an operation about a branch) and head detaching git checkout refs/heads/master^0 (which is an operation about a commit) if we want to help the users by auto-enabling fork-point mode. Well, IMHO git rebase and git rebase @{u} must do exactly the same thing. That is not part of the current discussion is what I meant by It is debatable... We might want to. There is no such patch to git rebase itself in this topic ;-). With the We might want to, I mean git rebase, git rebase @{u} and git rebase origin/master (if your @{u} happens to be that branch) may want to do exactly the same thing. The last one however is very questionable, as sometimes you would want the --fork-point heuristics, and some other times you would want no digging of the reflogs involved (i.e. I want everything not in this _exact_ commit to be rebased). On the other hand, I'm afraid different defaults were chosen for backward compatibility? There is no backward compatibility issue involved with the current behaviour. Changing it _will_ break compatibility, of course. It is more like the command used not to guess with fork-point at all, i.e. we liked its exactness, but git rebase (no argument) case is so obviously not about an exact commit but is about branch that it is safe to use --fork-point guess without being confusing. Once you start giving the commit/branch with respect to which you conduct your rebase, it no longer is so cut-and-dried obvious that by git rebase @{u} if the user wants us to guess by digging the reflog of @{u} to find a better fork point, or if the user wants to do an exact rebase with respect to the commit at the tip of that branch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-rebase.txt: discuss --fork-point assumption of vanilla git rebase in DESCRIPTION.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Vanilla git rebase defaults to --fork-point that in some cases makes behavior very different from git rebase upstream, where --no-fork-point is assumed. This fact was not mentioned in the DESCRIPTION section of the manual page, even though the case of omitted upstream was otherwise discussed. That in turn made actual behavior of vanilla git rebase hardly discoverable. While we are at it, clarify the --fork-point description itself as well. Signed-off-by: Sergey Organov sorga...@gmail.com --- Documentation/git-rebase.txt | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 4138554..73e1e1c 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -21,15 +21,16 @@ If branch is specified, 'git rebase' will perform an automatic it remains on the current branch. If upstream is not specified, the upstream configured in -branch.name.remote and branch.name.merge options will be used; see -linkgit:git-config[1] for details. If you are currently not on any -branch or if the current branch does not have a configured upstream, -the rebase will abort. +branch.name.remote and branch.name.merge options will be used (see +linkgit:git-config[1] for details) and the `--fork-point` option is +assumed. If you are currently not on any branch or if the current +branch does not have a configured upstream, the rebase will abort. OK. When you do not tell rebase with respect to what exact _commit_ the operation is to be done, then we will enable --fork-point, which makes perfect sense because it is clear that the user is rebasing with respect to a _branch_, for which we may find a place better than its current tip to rebase onto if we look at its reflog. I think you meant to say that we may find a better source to calculate the exact set of commits to rebase, as we still rebase onto the current tip. I.e., with this we select what to rebase, not where (the latter being handled by the --onto switch.) It is debatable if we should do the same when the user tells us to rebase with respect to a specific _branch_ by giving the 'upstream' argument, but that is an entirely separate issue. We might want to do a similar command line heuristics to tell between the branch switching git checkout master (which is an operation about a branch) and head detaching git checkout refs/heads/master^0 (which is an operation about a commit) if we want to help the users by auto-enabling fork-point mode. Well, IMHO git rebase and git rebase @{u} must do exactly the same thing. In its current state, when they have different default for fork-point, it's too surprising. From this POV I do like suggested heuristics to activate --fork-point when upstream (either specified or figured from configuration) is a branch. However, it seems that this would be functionally equivalent to just making the --fork-point the default, unconditionally, as trying to find better fork-point in a reflog for a non-reference will bring nothing anyway. The heuristics could be considered an optimization, but it would optimize very rare case. On the other hand, I'm afraid different defaults were chosen for backward compatibility? All changes made by commits in the current branch but that are not in upstream are saved to a temporary area. This is the same set -of commits that would be shown by `git log upstream..HEAD` (or -`git log HEAD`, if --root is specified). +of commits that would be shown by `git log upstream..HEAD`; or by +`git log fork_point..HEAD`, if --fork-point is either specified or +assumed; or by `git log HEAD`, if --root is specified. OK. fork_point is a new term this patch introduces to this document. Do we define what it is anywhere in this document, or would it help the readers to add something like ... where fork_point is computed in such and such way (see ... for details)? Yes, it's new and is not defined, but first I didn't want to overload the DESCRIPTION with details, and second I don't know how it's actually done, as it seems that fork_point=$(git merge-base upstream branch) sometimes returns nothing, in which case fork_point is set back to upstream? If we are going to describe it, I think it should go to --fork-point option description. Could you please suggest the wording? @@ -331,9 +332,8 @@ link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for details) between `upstream` and `branch` when calculating which commits have have been introduced by `branch` (see linkgit:git-merge-base[1]). + -If no non-option arguments are given on the command line, then the default is -`--fork-point @{u}` otherwise the `upstream` argument is interpreted literally -unless the `--fork-point` option is specified. +If either upstream or --root is given on the
Re: [PATCH] Documentation/git-rebase.txt: discuss --fork-point assumption of vanilla git rebase in DESCRIPTION.
Sergey Organov sorga...@gmail.com writes: Vanilla git rebase defaults to --fork-point that in some cases makes behavior very different from git rebase upstream, where --no-fork-point is assumed. This fact was not mentioned in the DESCRIPTION section of the manual page, even though the case of omitted upstream was otherwise discussed. That in turn made actual behavior of vanilla git rebase hardly discoverable. While we are at it, clarify the --fork-point description itself as well. Signed-off-by: Sergey Organov sorga...@gmail.com --- Documentation/git-rebase.txt | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 4138554..73e1e1c 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -21,15 +21,16 @@ If branch is specified, 'git rebase' will perform an automatic it remains on the current branch. If upstream is not specified, the upstream configured in -branch.name.remote and branch.name.merge options will be used; see -linkgit:git-config[1] for details. If you are currently not on any -branch or if the current branch does not have a configured upstream, -the rebase will abort. +branch.name.remote and branch.name.merge options will be used (see +linkgit:git-config[1] for details) and the `--fork-point` option is +assumed. If you are currently not on any branch or if the current +branch does not have a configured upstream, the rebase will abort. OK. When you do not tell rebase with respect to what exact _commit_ the operation is to be done, then we will enable --fork-point, which makes perfect sense because it is clear that the user is rebasing with respect to a _branch_, for which we may find a place better than its current tip to rebase onto if we look at its reflog. It is debatable if we should do the same when the user tells us to rebase with respect to a specific _branch_ by giving the 'upstream' argument, but that is an entirely separate issue. We might want to do a similar command line heuristics to tell between the branch switching git checkout master (which is an operation about a branch) and head detaching git checkout refs/heads/master^0 (which is an operation about a commit) if we want to help the users by auto-enabling fork-point mode. All changes made by commits in the current branch but that are not in upstream are saved to a temporary area. This is the same set -of commits that would be shown by `git log upstream..HEAD` (or -`git log HEAD`, if --root is specified). +of commits that would be shown by `git log upstream..HEAD`; or by +`git log fork_point..HEAD`, if --fork-point is either specified or +assumed; or by `git log HEAD`, if --root is specified. OK. fork_point is a new term this patch introduces to this document. Do we define what it is anywhere in this document, or would it help the readers to add something like ... where fork_point is computed in such and such way (see ... for details)? @@ -331,9 +332,8 @@ link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for details) between `upstream` and `branch` when calculating which commits have have been introduced by `branch` (see linkgit:git-merge-base[1]). + -If no non-option arguments are given on the command line, then the default is -`--fork-point @{u}` otherwise the `upstream` argument is interpreted literally -unless the `--fork-point` option is specified. +If either upstream or --root is given on the command line, then the +default is `--no-fork-point`, otherwise the default is `--fork-point`. Good. The original, especially the mention of @{u}, does not make any sense and updated text reads more clearly. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html