* git pull --rebase and losing commits @ 2009-11-02 12:26 Peter Krefting 2009-11-02 15:04 ` Thomas Rast ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Peter Krefting @ 2009-11-02 12:26 UTC (permalink / raw) To: Git Mailing List Hi! I have put my web site under Git control, and am running into some problems. Whenever I push changes, I go via a bare repository, which then is pulled into a checked out tree in my "public_html" directory. However, some scripts I have does create files directly under the "public_html", and some of them I want to push into the Git history. I am trying to use --rebase everywhere to get a linear history in the cases where I have pushed changes to the bare repository while there were uncommited changes to the public_html directory. I have come up with a script that does this (I have removed the uninteresting non-git commands): # Commit local changes git add path/to/script/output/* for file in $(git diff-index --cached --name-only HEAD); do havenew=1 done if [ $havenew = 1 ]; then git commit --quiet -m 'Automatic' path/to/script/output/* fi # Update tree (--strategy=ours avoids merge conflicts) git pull --rebase --strategy=ours origin master # Push rebased local changes git push origin master # Update all references git fetch origin master:remotes/origin/master However, this seems to lose commits. When I ran it today, it commited an automatic change, and then pulled a tree that did not contain that change, making the changed file just disappear. I had to dig through the reflog to find it: - This is the auto-commit: 608b7eda553552841f4a16167c680fc74ed3c55a 509926edd306bb2f09f563a7cfda800a4f0fdaed Peter Krefting <peter@softwolves.pp.se> 1257162580 +0100 commit: Automatisk bloggkommentarsuppdatering - This is the "git pull": 509926edd306bb2f09f563a7cfda800a4f0fdaed 9088bd4801a9008fe3fca0d351f97544cee014f1 Peter Krefting <peter@softwolves.pp.se> 1257162583 +0100 rebase finished: refs/heads/master onto 9088bd4801a9008fe3fca0d351f97544cee014f1 The history of 9088bd... does not contain the rebased version of 509926..., it just went missing. I guess I am missing something vital here. $ git --version git version 1.5.6.5 -- \\// Peter - http://www.softwolves.pp.se/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-02 12:26 git pull --rebase and losing commits Peter Krefting @ 2009-11-02 15:04 ` Thomas Rast 2009-11-02 21:34 ` Nanako Shiraishi 2009-11-02 15:10 ` Björn Steinbrink 2009-11-03 4:27 ` Randal L. Schwartz 2 siblings, 1 reply; 38+ messages in thread From: Thomas Rast @ 2009-11-02 15:04 UTC (permalink / raw) To: Peter Krefting; +Cc: Git Mailing List Peter Krefting wrote: > # Update tree (--strategy=ours avoids merge conflicts) > git pull --rebase --strategy=ours origin master [...] > However, this seems to lose commits. When I ran it today, it commited an > automatic change, and then pulled a tree that did not contain that change, > making the changed file just disappear. Not very surprising if you use the 'ours' strategy, which doesn't merge at all but instead takes the 'ours' side (IIRC that's the upstream for a rebase, but I always have these mixed up). It is *not* the often requested (but ill-defined and hence never implemented) "resolve all conflict hunks in favour of ours" strategy. So what happens is that git-rebase rebuilds some commit C from your side on some base B from the remote, but the 'ours' strategy turns the *tree* for C' into that of B. Then git-rebase sees that the trees haven't changed, and concludes that C has already been applied and drops it. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-02 15:04 ` Thomas Rast @ 2009-11-02 21:34 ` Nanako Shiraishi 0 siblings, 0 replies; 38+ messages in thread From: Nanako Shiraishi @ 2009-11-02 21:34 UTC (permalink / raw) To: Thomas Rast; +Cc: Peter Krefting, Git Mailing List Quoting Thomas Rast <trast@student.ethz.ch> > It is *not* > the often requested (but ill-defined and hence never implemented) > "resolve all conflict hunks in favour of ours" strategy. This was implemented and posted here quite some time ago. http://thread.gmane.org/gmane.comp.version-control.git/85163/focus=85602 -- Nanako Shiraishi http://ivory.ap.teacup.com/nanako3/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-02 12:26 git pull --rebase and losing commits Peter Krefting 2009-11-02 15:04 ` Thomas Rast @ 2009-11-02 15:10 ` Björn Steinbrink 2009-11-03 7:01 ` Peter Krefting 2009-11-03 4:27 ` Randal L. Schwartz 2 siblings, 1 reply; 38+ messages in thread From: Björn Steinbrink @ 2009-11-02 15:10 UTC (permalink / raw) To: Peter Krefting; +Cc: Git Mailing List On 2009.11.02 13:26:37 +0100, Peter Krefting wrote: > # Update tree (--strategy=ours avoids merge conflicts) > git pull --rebase --strategy=ours origin master The "ours" strategy doesn't just avoid merge conflicts, it avoids making any changes at all. The ours strategy means "just keep our state, just pretend that we've merged". And rebase will see that there were no changes and conclude: Already applied: 0001 test commit And thus it will drop the commit. Björn ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-02 15:10 ` Björn Steinbrink @ 2009-11-03 7:01 ` Peter Krefting 2009-11-03 9:52 ` Johannes Schindelin 2009-11-03 10:12 ` git pull --rebase and losing commits Thomas Rast 0 siblings, 2 replies; 38+ messages in thread From: Peter Krefting @ 2009-11-03 7:01 UTC (permalink / raw) To: Thomas Rast, Björn Steinbrink; +Cc: Git Mailing List Thomas Rast: > Not very surprising if you use the 'ours' strategy, which doesn't merge at > all but instead takes the 'ours' side (IIRC that's the upstream for a > rebase, but I always have these mixed up). Sounds like it should be called "theirs", then. Or the documentation should be clarify. > So what happens is that git-rebase rebuilds some commit C from your side > on some base B from the remote, but the 'ours' strategy turns the *tree* > for C' into that of B. Right. I thought it was working on the individual blobs (I want it to automatically resolve conflicts by applying the version that is in the repository I am running the rebase from, no matter what). Björn Steinbrink: > The "ours" strategy doesn't just avoid merge conflicts, it avoids making > any changes at all. The ours strategy means "just keep our state, just > pretend that we've merged". And rebase will see that there were no > changes and conclude: > > Already applied: 0001 test commit > > And thus it will drop the commit. I've seen that message show up in my logs a couple of times. I'd better drop the --strategy=ours, then. :-/ Now to figure out if it is possible to get a setup like this working at all. Maybe dropping rebase in favour of regular merge may help a bit, but I still want it to auto-resolve any conflicts for me. -- \\// Peter - http://www.softwolves.pp.se/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-03 7:01 ` Peter Krefting @ 2009-11-03 9:52 ` Johannes Schindelin 2009-11-03 10:12 ` Peter Krefting 2009-11-11 14:03 ` [PATCH] Clarify documentation on the "ours" merge strategy Peter Krefting 2009-11-03 10:12 ` git pull --rebase and losing commits Thomas Rast 1 sibling, 2 replies; 38+ messages in thread From: Johannes Schindelin @ 2009-11-03 9:52 UTC (permalink / raw) To: Peter Krefting; +Cc: Thomas Rast, Björn Steinbrink, Git Mailing List Hi, On Tue, 3 Nov 2009, Peter Krefting wrote: > Thomas Rast: > > > Not very surprising if you use the 'ours' strategy, which doesn't merge at > > all but instead takes the 'ours' side (IIRC that's the upstream for a > > rebase, but I always have these mixed up). > > Sounds like it should be called "theirs", then. Why should it be called "theirs" when it takes "ours"? Note: the thing I think Thomas wanted to clarify is that this strategy does not _resolve conflicts_ to "our" version, but it just outright ignores "theirs". IOW, after a merge with the "ours" strategy, "HEAD^{tree}" and "HEAD^^{tree}" will point to _exactly the same object_. If you want to use any merge strategy, you must understand what it does first. There is no way around that. No change in UI, or in the core code of Git, can relieve you of this obligation. Ciao, Dscho ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-03 9:52 ` Johannes Schindelin @ 2009-11-03 10:12 ` Peter Krefting 2009-11-11 14:03 ` [PATCH] Clarify documentation on the "ours" merge strategy Peter Krefting 1 sibling, 0 replies; 38+ messages in thread From: Peter Krefting @ 2009-11-03 10:12 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Thomas Rast, Björn Steinbrink, Git Mailing List Johannes Schindelin: >> Sounds like it should be called "theirs", then. > Why should it be called "theirs" when it takes "ours"? Because it took "their" (= upstream) tree, not "our" (= local branch) tree. Seems to me the name is a bit confusing in the case of a rebase, as I am "merging" my changes *onto* the upstream, not the other way round as would be the case with a regular merge. > Note: the thing I think Thomas wanted to clarify is that this strategy > does not _resolve conflicts_ to "our" version, but it just outright > ignores "theirs". IOW, after a merge with the "ours" strategy, > "HEAD^{tree}" and "HEAD^^{tree}" will point to _exactly the same object_. And in the case of a rebase, the other way around: With --rebase --strategy=ours, I am basically asking to throw all my local commits away? > If you want to use any merge strategy, you must understand what it does > first. There is no way around that. No change in UI, or in the core code > of Git, can relieve you of this obligation. No, that is why I recommended that what needed clarification was the documentation. I read the documentation of "ours": "This resolves any number of heads, but the result of the merge is always the current branch head. It is meant to be used to supersede old development history of side branches." and thought that it meant that it a) could resolve a merge conflict, no matter the number of branches involved ("resolves any number of heads"). b) would replace any merge conflict with the contents in the current repository's branch ("result of the merge is always the current branch head"). Apparently, the "used to supersede old development history" means that it actually throws the entire contents of one of the branches out, which is not what I wanted. I didn't understand that from the documentation, however. -- \\// Peter - http://www.softwolves.pp.se/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH] Clarify documentation on the "ours" merge strategy. 2009-11-03 9:52 ` Johannes Schindelin 2009-11-03 10:12 ` Peter Krefting @ 2009-11-11 14:03 ` Peter Krefting 2009-11-11 15:13 ` Baz 1 sibling, 1 reply; 38+ messages in thread From: Peter Krefting @ 2009-11-11 14:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Rast, Björn Steinbrink Make it clear that the merge strategy will discard all changes made to the branch being merged, and not just avoid creating merge conflicts. --- Documentation/merge-strategies.txt | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) > If you want to use any merge strategy, you must understand what it does > first. Indeed. Perhaps this clarification will help the next poor soul that tries doing what I tried? diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 4365b7e..a340dc9 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -30,7 +30,8 @@ octopus:: ours:: This resolves any number of heads, but the result of the - merge is always the current branch head. It is meant to + merge is always the current branch head, discarding any + changes on the merged branch. It is meant to be used to supersede old development history of side branches. -- 1.6.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] Clarify documentation on the "ours" merge strategy. 2009-11-11 14:03 ` [PATCH] Clarify documentation on the "ours" merge strategy Peter Krefting @ 2009-11-11 15:13 ` Baz 2009-11-11 20:35 ` Thomas Rast 0 siblings, 1 reply; 38+ messages in thread From: Baz @ 2009-11-11 15:13 UTC (permalink / raw) To: Peter Krefting Cc: Git Mailing List, Johannes Schindelin, Thomas Rast, Björn Steinbrink 2009/11/11 Peter Krefting <peter@softwolves.pp.se>: > Make it clear that the merge strategy will discard all changes made to > the branch being merged, and not just avoid creating merge conflicts. > --- > Documentation/merge-strategies.txt | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > >> If you want to use any merge strategy, you must understand what it does >> first. > > Indeed. Perhaps this clarification will help the next poor soul that tries > doing what I tried? > > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt > index 4365b7e..a340dc9 100644 > --- a/Documentation/merge-strategies.txt > +++ b/Documentation/merge-strategies.txt > @@ -30,7 +30,8 @@ octopus:: > > ours:: > This resolves any number of heads, but the result of the > - merge is always the current branch head. It is meant to > + merge is always the current branch head, discarding any > + changes on the merged branch. It is meant to I think part of the problem is that it is unclear what the "current branch head" means when used in a rebase, and hence when this text is included in the help for git-rebase and git-pull. This flipped behaviour is surprising given the natural meaning of 'ours', or 'current branch', particularly for git pull: git pull -s ours - discards changes in remote branch, keeps changes in current branch git pull --rebase -s ours - discards changes in current branch, keeps changes in remote branch Perhaps something more in the way of an explicit warning? ours:: This resolves any number of heads, but the result of the merge is always the current branch head, discarding any changes on the merged branch. It is meant to be used to supersede old development history of side branches. Note that when rebasing, the branch you are rebasing onto is the "current branch head", and using this strategy will lose all of your changes - unlikely to be what you wanted to do. -Baz > be used to supersede old development history of side > branches. > > -- > 1.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Clarify documentation on the "ours" merge strategy. 2009-11-11 15:13 ` Baz @ 2009-11-11 20:35 ` Thomas Rast 2009-11-11 20:54 ` Baz 2009-11-11 21:02 ` Junio C Hamano 0 siblings, 2 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-11 20:35 UTC (permalink / raw) To: Baz Cc: Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink Baz wrote: > 2009/11/11 Peter Krefting <peter@softwolves.pp.se>: > > ours:: > > This resolves any number of heads, but the result of the > > - merge is always the current branch head. It is meant to > > + merge is always the current branch head, discarding any > > + changes on the merged branch. It is meant to > > I think part of the problem is that it is unclear what the "current > branch head" means when used in a rebase, and hence when this text is > included in the help for git-rebase and git-pull. [...] > Perhaps something more in the way of an explicit warning? > > ours:: > This resolves any number of heads, but the result of the > merge is always the current branch head, discarding any > changes on the merged branch. It is meant to > be used to supersede old development history of side > branches. Note that when rebasing, the branch you are > rebasing onto is the "current branch head", and using this > strategy will lose all of your changes - unlikely to be what > you wanted to do. I'd much rather see this explained in the description of the rebase -m/-s options since it (the swap) applies to all uses of 'git rebase -m'. Perhaps with an extra (but short) note in the "ours" description, like so: diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt index 33e0ef1..181947c 100644 --- i/Documentation/git-rebase.txt +++ w/Documentation/git-rebase.txt @@ -228,6 +228,10 @@ OPTIONS Use merging strategies to rebase. When the recursive (default) merge strategy is used, this allows rebase to be aware of renames on the upstream side. ++ +Note that in a rebase merge (hence merge conflict), the sides are +swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far +rebased series, starting with <upstream>. -s <strategy>:: --strategy=<strategy>:: diff --git i/Documentation/merge-strategies.txt w/Documentation/merge-strategies.txt index 4365b7e..0cae1be 100644 --- i/Documentation/merge-strategies.txt +++ w/Documentation/merge-strategies.txt @@ -33,6 +33,9 @@ ours:: merge is always the current branch head. It is meant to be used to supersede old development history of side branches. ++ +Because the sides in a rebase are swapped, using this strategy with +git-rebase is never a good idea. subtree:: This is a modified recursive strategy. When merging trees A and -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] Clarify documentation on the "ours" merge strategy. 2009-11-11 20:35 ` Thomas Rast @ 2009-11-11 20:54 ` Baz 2009-11-11 21:02 ` Junio C Hamano 1 sibling, 0 replies; 38+ messages in thread From: Baz @ 2009-11-11 20:54 UTC (permalink / raw) To: Thomas Rast Cc: Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink 2009/11/11 Thomas Rast <trast@student.ethz.ch>: > Baz wrote: >> 2009/11/11 Peter Krefting <peter@softwolves.pp.se>: >> > ours:: >> > This resolves any number of heads, but the result of the >> > - merge is always the current branch head. It is meant to >> > + merge is always the current branch head, discarding any >> > + changes on the merged branch. It is meant to >> >> I think part of the problem is that it is unclear what the "current >> branch head" means when used in a rebase, and hence when this text is >> included in the help for git-rebase and git-pull. > [...] >> Perhaps something more in the way of an explicit warning? >> >> ours:: >> This resolves any number of heads, but the result of the >> merge is always the current branch head, discarding any >> changes on the merged branch. It is meant to >> be used to supersede old development history of side >> branches. Note that when rebasing, the branch you are >> rebasing onto is the "current branch head", and using this >> strategy will lose all of your changes - unlikely to be what >> you wanted to do. > > I'd much rather see this explained in the description of the rebase > -m/-s options since it (the swap) applies to all uses of 'git rebase > -m'. Perhaps with an extra (but short) note in the "ours" > description, like so: > > diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt > index 33e0ef1..181947c 100644 > --- i/Documentation/git-rebase.txt > +++ w/Documentation/git-rebase.txt > @@ -228,6 +228,10 @@ OPTIONS > Use merging strategies to rebase. When the recursive (default) merge > strategy is used, this allows rebase to be aware of renames on the > upstream side. > ++ > +Note that in a rebase merge (hence merge conflict), the sides are > +swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far > +rebased series, starting with <upstream>. > > -s <strategy>:: > --strategy=<strategy>:: > diff --git i/Documentation/merge-strategies.txt w/Documentation/merge-strategies.txt > index 4365b7e..0cae1be 100644 > --- i/Documentation/merge-strategies.txt > +++ w/Documentation/merge-strategies.txt > @@ -33,6 +33,9 @@ ours:: > merge is always the current branch head. It is meant to > be used to supersede old development history of side > branches. > ++ > +Because the sides in a rebase are swapped, using this strategy with > +git-rebase is never a good idea. Yes, this (with Peter's patch) makes the danger nice & clear. Thanks! -Baz > > subtree:: > This is a modified recursive strategy. When merging trees A and > > -- > Thomas Rast > trast@{inf,student}.ethz.ch > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Clarify documentation on the "ours" merge strategy. 2009-11-11 20:35 ` Thomas Rast 2009-11-11 20:54 ` Baz @ 2009-11-11 21:02 ` Junio C Hamano 2009-11-11 21:30 ` [PATCH] " Nicolas Sebrecht 1 sibling, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-11 21:02 UTC (permalink / raw) To: Thomas Rast Cc: Baz, Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink Thomas Rast <trast@student.ethz.ch> writes: > I'd much rather see this explained in the description of the rebase > -m/-s options since it (the swap) applies to all uses of 'git rebase > -m'. Perhaps with an extra (but short) note in the "ours" > description, like so: > > diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt > index 33e0ef1..181947c 100644 > --- i/Documentation/git-rebase.txt > +++ w/Documentation/git-rebase.txt > @@ -228,6 +228,10 @@ OPTIONS > Use merging strategies to rebase. When the recursive (default) merge > strategy is used, this allows rebase to be aware of renames on the > upstream side. > ++ > +Note that in a rebase merge (hence merge conflict), the sides are > +swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far > +rebased series, starting with <upstream>. > > -s <strategy>:: > --strategy=<strategy>:: > diff --git i/Documentation/merge-strategies.txt w/Documentation/merge-strategies.txt > index 4365b7e..0cae1be 100644 > --- i/Documentation/merge-strategies.txt > +++ w/Documentation/merge-strategies.txt > @@ -33,6 +33,9 @@ ours:: > merge is always the current branch head. It is meant to > be used to supersede old development history of side > branches. > ++ > +Because the sides in a rebase are swapped, using this strategy with > +git-rebase is never a good idea. > > subtree:: > This is a modified recursive strategy. When merging trees A and Looking very good. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-11 21:02 ` Junio C Hamano @ 2009-11-11 21:30 ` Nicolas Sebrecht 2009-11-11 23:37 ` Thomas Rast 0 siblings, 1 reply; 38+ messages in thread From: Nicolas Sebrecht @ 2009-11-11 21:30 UTC (permalink / raw) To: Junio C Hamano Cc: Thomas Rast, Baz, Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink, Nicolas Sebrecht The 11/11/09, Junio C Hamano wrote: > Thomas Rast <trast@student.ethz.ch> writes: > > > ++ > > +Because the sides in a rebase are swapped, using this strategy with > > +git-rebase is never a good idea. > > Looking very good. If this strategy is _never_ a good idea in this case, I tend to think that git should forbid this option, or at least, warn and refer to the documentation. -- Nicolas Sebrecht ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-11 21:30 ` [PATCH] " Nicolas Sebrecht @ 2009-11-11 23:37 ` Thomas Rast 2009-11-12 7:55 ` Junio C Hamano 0 siblings, 1 reply; 38+ messages in thread From: Thomas Rast @ 2009-11-11 23:37 UTC (permalink / raw) To: Nicolas Sebrecht Cc: Junio C Hamano, Baz, Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink Nicolas Sebrecht wrote: > The 11/11/09, Junio C Hamano wrote: > > Thomas Rast <trast@student.ethz.ch> writes: > > > > > ++ > > > +Because the sides in a rebase are swapped, using this strategy with > > > +git-rebase is never a good idea. > > > > Looking very good. > > If this strategy is _never_ a good idea in this case, I tend to think > that git should forbid this option, or at least, warn and refer to the > documentation. Then again, I'm not sure if resolve vs. recursive makes a difference in a rebase. Octopus is weird for a two-head merge, I'm not sure why the docs even talk about it. That would leave only subtree, which indeed has its uses. Should we add a note to that effect to git-rebase.txt? Like, say, diff --git i/Documentation/git-rebase.txt w/Documentation/git-rebase.txt index 33e0ef1..6e54a57 100644 --- i/Documentation/git-rebase.txt +++ w/Documentation/git-rebase.txt @@ -228,13 +228,19 @@ OPTIONS Use merging strategies to rebase. When the recursive (default) merge strategy is used, this allows rebase to be aware of renames on the upstream side. ++ +Note that in a rebase merge (hence merge conflict), the sides are +swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far +rebased series, starting with <upstream>. -s <strategy>:: --strategy=<strategy>:: Use the given merge strategy. - If there is no `-s` option, a built-in list of strategies - is used instead ('git-merge-recursive' when merging a single - head, 'git-merge-octopus' otherwise). This implies --merge. + If there is no `-s` option 'git-merge-recursive' is used + instead. This implies --merge. ++ +Due to the peculiarities of 'git-rebase' (see \--merge above) the only +built-in strategy that is actually useful is 'subtree'. -q:: --quiet:: diff --git i/Documentation/merge-strategies.txt w/Documentation/merge-strategies.txt index 4365b7e..c1c3add 100644 --- i/Documentation/merge-strategies.txt +++ w/Documentation/merge-strategies.txt @@ -33,6 +33,9 @@ ours:: merge is always the current branch head. It is meant to be used to supersede old development history of side branches. ++ +Because the sides in a rebase are swapped, using this strategy with +'git-rebase' is never a good idea. subtree:: This is a modified recursive strategy. When merging trees A and -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-11 23:37 ` Thomas Rast @ 2009-11-12 7:55 ` Junio C Hamano 2009-11-12 9:41 ` Peter Krefting ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Junio C Hamano @ 2009-11-12 7:55 UTC (permalink / raw) To: Thomas Rast Cc: Nicolas Sebrecht, Baz, Peter Krefting, Git Mailing List, Johannes Schindelin, Björn Steinbrink Thomas Rast <trast@student.ethz.ch> writes: > Then again, I'm not sure if resolve vs. recursive makes a difference > in a rebase. Octopus is weird for a two-head merge, I'm not sure why ... > + If there is no `-s` option 'git-merge-recursive' is used > + instead. This implies --merge. > ++ > +Due to the peculiarities of 'git-rebase' (see \--merge above) the only > +built-in strategy that is actually useful is 'subtree'. "recursive is just a slower and sometimes buggier alternative to resolve but can handle renames" may mean "people do not have much reason to choose resolve over recursive". But that is quite different from "resolve is not useful here _due to_ the peculiarities of rebase". Wouldn't anybody who thinks "resolve vs. recursive would not make a difference in a rebase" also think "resolve vs. recursive would not make a difference anywhere"? 58634db (rebase: Allow merge strategies to be used when rebasing, 2006-06-21) added "-m" and "-s" to rebase to solve the problem of rebasing against an upstream that has moved files. What the commit actually did was to use recursive (by default) while giving longer rope to the users by choosing other strategies with "-s", without making any judgement as to why other strategies may possibly be useful. Perhaps there is some different issue at the root of this one. Why would anybody be tempted to say "-s ours" while running a rebase? What did the user want to see it do (instead of being a no-op because "ours" by definition ignores the tree the change is replayed from)? It is easy to dismiss it as a user misconception and it also is tempting to think that it would be helped with updated description of "ours" to dispel that misconception, but there may be some user wish that is totally different from "ours merge" strategy but still can be validly labelled using a word "ours" by somebody who does not know the way the word "ours" is used in the git land, and satisfying that unknown user wish might be the real solution to this issue. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-12 7:55 ` Junio C Hamano @ 2009-11-12 9:41 ` Peter Krefting 2009-11-14 2:12 ` Nanako Shiraishi 2009-11-12 9:55 ` Björn Steinbrink 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast 2 siblings, 1 reply; 38+ messages in thread From: Peter Krefting @ 2009-11-12 9:41 UTC (permalink / raw) To: Junio C Hamano Cc: Thomas Rast, Nicolas Sebrecht, Baz, Git Mailing List, Johannes Schindelin, Björn Steinbrink Junio C Hamano: > Perhaps there is some different issue at the root of this one. Why would > anybody be tempted to say "-s ours" while running a rebase? What did the > user want to see it do (instead of being a no-op because "ours" by > definition ignores the tree the change is replayed from)? The reason why I wanted it in my initial example was due to me misreading the documentation of "ours". My scenario is like this: I have my web site under Git control (used to be CVS). Some parts of the web site is updated in-place (blog comments being saved as HTML directly in the web tree), whereas all other edits are done in clones of the repsository. These changes are then added and committed to the checked out web tree and pushed to the central repo. In some cases, I wish to edit the comments in one of my clones (to remove spam not stopped by my spam filters, for instance), but editing these risks creating a conflict if there has been other changes in the mean time. The web tree checkout script uses rebase to avoid introducing merge commits every time the blog comment is updated, as it in 99 % of cases is unrelated to any other changes found in the central repo. In the few cases where the blog comment update from the web tree conflicts with a change in the central repo, I want the "git pull --rebase" call to overwrite any changes in the central repo with my changes in the web tree (meaning that I would later have to manually re-delete the spam comments, but I can live with that). > It is easy to dismiss it as a user misconception and it also is tempting > to think that it would be helped with updated description of "ours" to > dispel that misconception, but there may be some user wish that is totally > different from "ours merge" strategy but still can be validly labelled > using a word "ours" by somebody who does not know the way the word "ours" > is used in the git land, and satisfying that unknown user wish might be > the real solution to this issue. Yes, I am apparently looking for something that is not available in the Git codebase yet. :-) -- \\// Peter - http://www.softwolves.pp.se/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-12 9:41 ` Peter Krefting @ 2009-11-14 2:12 ` Nanako Shiraishi 2009-11-15 9:10 ` Junio C Hamano 0 siblings, 1 reply; 38+ messages in thread From: Nanako Shiraishi @ 2009-11-14 2:12 UTC (permalink / raw) To: Peter Krefting Cc: Junio C Hamano, Thomas Rast, Nicolas Sebrecht, Baz, Git Mailing List, Johannes.Schindelin, B.Steinbrink Quoting Peter Krefting <peter@softwolves.pp.se> > The web tree checkout script uses rebase to avoid introducing merge > commits every time the blog comment is updated, as it in 99 % of cases > is unrelated to any other changes found in the central repo. > > In the few cases where the blog comment update from the web tree > conflicts with a change in the central repo, I want the "git pull > --rebase" call to overwrite any changes in the central repo with my > changes in the web tree (meaning that I would later have to manually > re-delete the spam comments, but I can live with that). That sounds like "-Xours" merge option that was discussed some time ago. See http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=89021 I remember that Junio and Petr were against it because it would encourage a bad workflow. Dscho was against the syntax used to pass the options also. -- Nanako Shiraishi http://ivory.ap.teacup.com/nanako3/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-14 2:12 ` Nanako Shiraishi @ 2009-11-15 9:10 ` Junio C Hamano 2009-11-16 8:20 ` Peter Krefting 0 siblings, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-15 9:10 UTC (permalink / raw) To: Nanako Shiraishi Cc: Peter Krefting, Junio C Hamano, Thomas Rast, Nicolas Sebrecht, Baz, Git Mailing List, Johannes.Schindelin, B.Steinbrink Nanako Shiraishi <nanako3@lavabit.com> writes: > Quoting Peter Krefting <peter@softwolves.pp.se> > >> The web tree checkout script uses rebase to avoid introducing merge >> commits every time the blog comment is updated, as it in 99 % of cases >> is unrelated to any other changes found in the central repo. >> >> In the few cases where the blog comment update from the web tree >> conflicts with a change in the central repo, I want the "git pull >> --rebase" call to overwrite any changes in the central repo with my >> changes in the web tree (meaning that I would later have to manually >> re-delete the spam comments, but I can live with that). > > That sounds like "-Xours" merge option that was discussed some time > ago. See > > http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=89021 > > I remember that Junio and Petr were against it because it would > encourage a bad workflow. Dscho was against the syntax used to > pass the options also. Yeah, Björn seems to speculate the same. Even though I still think -Xours/-Xtheirs are nonsense options in the context of source code management, I suspect that they might be exactly what Peter needs in this situation. As long as the changes made on the "web tree" side only consist of user-generated blog contents and never touch framework code that is controlled by his "central repo" side (and that condition should hold true; otherwise Peter's web site is seriously broken from the security point of view and no SCM can fix that), running a merge with the fabled -Xours option in the "web tree" to slurp in the changes made on the "central repo" side does not sound like an unreasonable thing to do. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-15 9:10 ` Junio C Hamano @ 2009-11-16 8:20 ` Peter Krefting 0 siblings, 0 replies; 38+ messages in thread From: Peter Krefting @ 2009-11-16 8:20 UTC (permalink / raw) To: Junio C Hamano Cc: Nanako Shiraishi, Thomas Rast, Nicolas Sebrecht, Baz, Git Mailing List, Johannes.Schindelin, B.Steinbrink Junio C Hamano: > Even though I still think -Xours/-Xtheirs are nonsense options in the > context of source code management, I suspect that they might be exactly > what Peter needs in this situation. Yes, it sounds like it would. That is not something I would use for source code management, but it would fit this, and some other use-cases I have, quite nicely. I tend to use Git not only for source code management, but also for document synchronisation across machines which may, or may not, be connected to a network at any given time. Git is very nice for that sort of work. > ; otherwise Peter's web site is seriously broken from the security point > of view and no SCM can fix that), Indeed. If it that was the case, I deserve whatever problems I get :-) -- \\// Peter - http://www.softwolves.pp.se/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH] Re: Clarify documentation on the "ours" merge strategy. 2009-11-12 7:55 ` Junio C Hamano 2009-11-12 9:41 ` Peter Krefting @ 2009-11-12 9:55 ` Björn Steinbrink 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast 2 siblings, 0 replies; 38+ messages in thread From: Björn Steinbrink @ 2009-11-12 9:55 UTC (permalink / raw) To: Junio C Hamano Cc: Thomas Rast, Nicolas Sebrecht, Baz, Peter Krefting, Git Mailing List, Johannes Schindelin On 2009.11.11 23:55:09 -0800, Junio C Hamano wrote: > 58634db (rebase: Allow merge strategies to be used when rebasing, > 2006-06-21) added "-m" and "-s" to rebase to solve the problem of rebasing > against an upstream that has moved files. What the commit actually did > was to use recursive (by default) while giving longer rope to the users by > choosing other strategies with "-s", without making any judgement as to > why other strategies may possibly be useful. At least the original reason for 58634db became (partially?) moot half a year later, thanks to 579c9bb19 "Use merge-recursive in git-am -3". Rebase already falls back to recursive merging in am, so using rebase -m with the recursive strategy just stops it from trying the fast path, right? That should probably be reflected in the man page, but honestly I have no idea what to write there now. The note about recursive should go, but keeping only "Use merging strategies to rebase" doesn't actually look like it's going to be helpful in any way. > Perhaps there is some different issue at the root of this one. Why would > anybody be tempted to say "-s ours" while running a rebase? What did the > user want to see it do (instead of being a no-op because "ours" by > definition ignores the tree the change is replayed from)? Given the few requests I've seen of it (here + #git), I'd guess that the user wants "git rebase -s ours $up" to do either: MB=$(git merge-base $up HEAD) git filter-branch --parent-filter "sed -e s/$MB/$up/" -- HEAD --not $up i.e. just re-attach things to upstream, ignoring whatever upstream did (git-svn users seem to want something like that sometimes to be able to dcommit. Dunno if they have some hatred against the other users of their svn repo ;-)) Or the user wants the infamous "resolve conflicts to want I did", often enough without thinking about what that actually means and how it can easily lead to total crap. (Yes, I'm biased...) Björn ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 0/3] Document and refuse rebase -s ours 2009-11-12 7:55 ` Junio C Hamano 2009-11-12 9:41 ` Peter Krefting 2009-11-12 9:55 ` Björn Steinbrink @ 2009-11-15 18:25 ` Thomas Rast 2009-11-15 18:25 ` [PATCH 1/3] Documentation: clarify 'ours' merge strategy Thomas Rast ` (3 more replies) 2 siblings, 4 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 18:25 UTC (permalink / raw) To: git Cc: Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Junio convinced me that it is not possible to limit the choice to only 'subtree', so here's a short series that implements the other changes I had already posted in diff form. I also implemented Nicolas's suggestion to reject -s ours outright; I'm not really happy with starting a blacklist there, but maybe it helps the next unwary user. I split it because even if you reject 3/3, I think the first two should go in as extra documentation. Thomas Rast (3): Documentation: clarify 'ours' merge strategy rebase docs: clarify --merge and --strategy rebase: refuse to rebase with -s ours Documentation/git-rebase.txt | 14 +++++++++++--- Documentation/merge-strategies.txt | 5 +++-- git-rebase--interactive.sh | 4 ++++ git-rebase.sh | 4 ++++ 4 files changed, 22 insertions(+), 5 deletions(-) ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 1/3] Documentation: clarify 'ours' merge strategy 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast @ 2009-11-15 18:25 ` Thomas Rast 2009-11-15 18:25 ` [PATCH 2/3] rebase docs: clarify --merge and --strategy Thomas Rast ` (2 subsequent siblings) 3 siblings, 0 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 18:25 UTC (permalink / raw) To: git Cc: Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Make it clear in the docs that the merge takes the tree of HEAD and ignores everything in the other branches. This should hopefully clear up confusion, usually caused by the user looking for a strategy that resolves all conflict hunks in favour of HEAD (which is completely different and currently not supported). Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/merge-strategies.txt | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 4365b7e..42910a3 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -29,8 +29,9 @@ octopus:: pulling or merging more than one branch. ours:: - This resolves any number of heads, but the result of the - merge is always the current branch head. It is meant to + This resolves any number of heads, but the resulting tree of the + merge is always that of the current branch head, effectively + ignoring all changes from all other branches. It is meant to be used to supersede old development history of side branches. -- 1.6.5.2.420.gf6c057.dirty ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 2/3] rebase docs: clarify --merge and --strategy 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast 2009-11-15 18:25 ` [PATCH 1/3] Documentation: clarify 'ours' merge strategy Thomas Rast @ 2009-11-15 18:25 ` Thomas Rast 2009-11-15 21:05 ` Junio C Hamano 2009-11-15 18:25 ` [PATCH 3/3] rebase: refuse to rebase with -s ours Thomas Rast 2009-11-15 21:04 ` [PATCH 0/3] Document and refuse rebase " Junio C Hamano 3 siblings, 1 reply; 38+ messages in thread From: Thomas Rast @ 2009-11-15 18:25 UTC (permalink / raw) To: git Cc: Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Add a paragraph about the swapped sides in a --merge rebase, which was otherwise only documented in the sources. Add a paragraph about the effects of the 'ours' strategy to the -s description. Also remove the mention of the 'octopus' strategy, which was copied from the git-merge description but is pointless in a rebase. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/git-rebase.txt | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 33e0ef1..5fa9100 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -228,13 +228,20 @@ OPTIONS Use merging strategies to rebase. When the recursive (default) merge strategy is used, this allows rebase to be aware of renames on the upstream side. ++ +Note that in a rebase merge (hence merge conflict), the sides are +swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far +rebased series, starting with <upstream>. -s <strategy>:: --strategy=<strategy>:: Use the given merge strategy. - If there is no `-s` option, a built-in list of strategies - is used instead ('git-merge-recursive' when merging a single - head, 'git-merge-octopus' otherwise). This implies --merge. + If there is no `-s` option 'git-merge-recursive' is used + instead. This implies --merge. ++ +Due to the peculiarities of 'git-rebase' (see \--merge above), using +the 'ours' strategy simply discards all patches from the <branch>, +which makes little sense. -q:: --quiet:: -- 1.6.5.2.420.gf6c057.dirty ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 2/3] rebase docs: clarify --merge and --strategy 2009-11-15 18:25 ` [PATCH 2/3] rebase docs: clarify --merge and --strategy Thomas Rast @ 2009-11-15 21:05 ` Junio C Hamano 2009-11-15 21:11 ` Thomas Rast 0 siblings, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-15 21:05 UTC (permalink / raw) To: Thomas Rast Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Thomas Rast <trast@student.ethz.ch> writes: > Add a paragraph about the swapped sides in a --merge rebase, which was > otherwise only documented in the sources. > > Add a paragraph about the effects of the 'ours' strategy to the -s > description. Also remove the mention of the 'octopus' strategy, which > was copied from the git-merge description but is pointless in a > rebase. Instead of saying "peculiarities" without saying what is peculiar about it, it might be better to give an explanation that would help the reader understand why they appear "swapped". Here is my attempt. Thoughts? Documentation/git-rebase.txt | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index e802421..a6f8182 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -229,9 +229,11 @@ OPTIONS strategy is used, this allows rebase to be aware of renames on the upstream side. + -Note that in a rebase merge (hence merge conflict), the sides are -swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far -rebased series, starting with <upstream>. +Note that a rebase merge works by replaying each commit from the working +branch on top of the <upstream> branch. Because of this, when a merge +conflict happens, the side reported as 'ours' is the so-far rebased +series, starting with <upstream>, and 'theirs' is the working branch. In +other words, the sides are swapped. -s <strategy>:: --strategy=<strategy>:: @@ -239,7 +241,9 @@ rebased series, starting with <upstream>. If there is no `-s` option 'git-merge-recursive' is used instead. This implies --merge. + -Due to the peculiarities of 'git-rebase' (see \--merge above), using +Because 'git-rebase' replays each commit from the working branch +on top of the <upstream> branch using the given strategy, +(see \--merge above), using the 'ours' strategy simply discards all patches from the <branch>, which makes little sense. Thus 'git-rebase' does not accept this strategy. ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 2/3] rebase docs: clarify --merge and --strategy 2009-11-15 21:05 ` Junio C Hamano @ 2009-11-15 21:11 ` Thomas Rast 0 siblings, 0 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 21:11 UTC (permalink / raw) To: Junio C Hamano Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Junio C Hamano wrote: > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index e802421..a6f8182 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -229,9 +229,11 @@ OPTIONS > strategy is used, this allows rebase to be aware of renames on the > upstream side. > + > -Note that in a rebase merge (hence merge conflict), the sides are > -swapped: "theirs" is the to-be-applied patch, and "ours" is the so-far > -rebased series, starting with <upstream>. > +Note that a rebase merge works by replaying each commit from the working > +branch on top of the <upstream> branch. Because of this, when a merge > +conflict happens, the side reported as 'ours' is the so-far rebased > +series, starting with <upstream>, and 'theirs' is the working branch. In > +other words, the sides are swapped. This is much nicer than mine! > -Due to the peculiarities of 'git-rebase' (see \--merge above), using > +Because 'git-rebase' replays each commit from the working branch > +on top of the <upstream> branch using the given strategy, > +(see \--merge above), using > the 'ours' strategy simply discards all patches from the <branch>, > which makes little sense. Thus 'git-rebase' does not accept this > strategy. Here I'm not sure if it makes such a big difference, since we already explained the problem in --merge (and point to it). But yours is fine too. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast 2009-11-15 18:25 ` [PATCH 1/3] Documentation: clarify 'ours' merge strategy Thomas Rast 2009-11-15 18:25 ` [PATCH 2/3] rebase docs: clarify --merge and --strategy Thomas Rast @ 2009-11-15 18:25 ` Thomas Rast 2009-11-15 18:39 ` Sverre Rabbelier 2009-11-16 12:35 ` Johannes Schindelin 2009-11-15 21:04 ` [PATCH 0/3] Document and refuse rebase " Junio C Hamano 3 siblings, 2 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 18:25 UTC (permalink / raw) To: git Cc: Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Using the "ours" strategy with rebase just discards all changes, turning <branch> into <upstream> (or <newbase> if given). This is unlikely to be what the user wants, so simply refuse to do it. Also document what would happen near the -s option, and point the user at it with the error message. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/git-rebase.txt | 3 ++- git-rebase--interactive.sh | 4 ++++ git-rebase.sh | 4 ++++ 3 files changed, 10 insertions(+), 1 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 5fa9100..2203e63 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -241,7 +241,8 @@ rebased series, starting with <upstream>. + Due to the peculiarities of 'git-rebase' (see \--merge above), using the 'ours' strategy simply discards all patches from the <branch>, -which makes little sense. +which makes little sense. Thus 'git-rebase' does not accept this +strategy. -q:: --quiet:: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 53ad248..c6bc156 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -584,6 +584,10 @@ first and then run 'git rebase --continue' again." STRATEGY="-s $2" shift ;; esac + if test "$STRATEGY" = "-s ours" + then + die "Refusing to rebase with 'ours' strategy; see git help rebase." + fi ;; -m) # we use merge anyway diff --git a/git-rebase.sh b/git-rebase.sh index 6ec155c..2d7d566 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -306,6 +306,10 @@ do strategy="$2" shift ;; esac + if test $strategy = ours + then + die "Refusing to rebase with 'ours' strategy; see git help rebase." + fi do_merge=t ;; -n|--no-stat) -- 1.6.5.2.420.gf6c057.dirty ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-15 18:25 ` [PATCH 3/3] rebase: refuse to rebase with -s ours Thomas Rast @ 2009-11-15 18:39 ` Sverre Rabbelier 2009-11-15 18:44 ` Thomas Rast 2009-11-16 12:35 ` Johannes Schindelin 1 sibling, 1 reply; 38+ messages in thread From: Sverre Rabbelier @ 2009-11-15 18:39 UTC (permalink / raw) To: Thomas Rast Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Heya, On Sun, Nov 15, 2009 at 19:25, Thomas Rast <trast@student.ethz.ch> wrote: > + if test "$STRATEGY" = "-s ours" Is this solid? Would "-s ours" (two spaces) work? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-15 18:39 ` Sverre Rabbelier @ 2009-11-15 18:44 ` Thomas Rast 0 siblings, 0 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 18:44 UTC (permalink / raw) To: Sverre Rabbelier Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Sverre Rabbelier wrote: > Heya, > > On Sun, Nov 15, 2009 at 19:25, Thomas Rast <trast@student.ethz.ch> wrote: > > + if test "$STRATEGY" = "-s ours" > > Is this solid? Would "-s ours" (two spaces) work? Well, the variable is set by the case immediately before the new test: case "$#,$1" in *,*=*) STRATEGY="-s "$(expr "z$1" : 'z-[^=]*=\(.*\)') ;; 1,*) usage ;; *) STRATEGY="-s $2" shift ;; esac I didn't want to split that for a direct comparison with the second half of the value, but unless I'm missing something, you'd have to say -s ' ours' to make the test fail. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-15 18:25 ` [PATCH 3/3] rebase: refuse to rebase with -s ours Thomas Rast 2009-11-15 18:39 ` Sverre Rabbelier @ 2009-11-16 12:35 ` Johannes Schindelin 2009-11-16 19:57 ` Junio C Hamano 1 sibling, 1 reply; 38+ messages in thread From: Johannes Schindelin @ 2009-11-16 12:35 UTC (permalink / raw) To: Thomas Rast Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Hi, On Sun, 15 Nov 2009, Thomas Rast wrote: > Using the "ours" strategy with rebase just discards all changes, turning > <branch> into <upstream> (or <newbase> if given). This is unlikely to > be what the user wants, so simply refuse to do it. "Unlikely" or "impossible"? Besides, I find it rather arbitrary that the "ours" strategy is refused, but none of the user-provided merge strategies. IOW disallowing "ours" may very well foster unreasonable expectations. Ciao, Dscho ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-16 12:35 ` Johannes Schindelin @ 2009-11-16 19:57 ` Junio C Hamano 2009-11-16 21:25 ` Johannes Schindelin 0 siblings, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-16 19:57 UTC (permalink / raw) To: Johannes Schindelin Cc: Thomas Rast, git, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Sun, 15 Nov 2009, Thomas Rast wrote: > >> Using the "ours" strategy with rebase just discards all changes, turning >> <branch> into <upstream> (or <newbase> if given). This is unlikely to >> be what the user wants, so simply refuse to do it. > > "Unlikely" or "impossible"? It is more like "very likely to be a mistake". Our tradition has been to give them long enough rope, but the recent trend is to consider ourselves experienced enough with various git workflows to be capable of identifying not just "cannot possibly a meaningful request" but also "almost always a mistake" cases, and tighten the rope to help people from stumbling, I think. But it needs more careful thought to avoid forbidding useful use cases, and your input is hugely appreciated if you have doubts (even better, an example of useful use case that will become impossible). > Besides, I find it rather arbitrary that the "ours" strategy is refused, > but none of the user-provided merge strategies. IOW disallowing "ours" > may very well foster unreasonable expectations. I cannot read this quite clearly. Unreasonable expectations being...? * "ours" is disallowed but anything else including user-provided ones are Ok, so we are allowed to circumvent this restriction by adding a synonym for "ours" as a user-defined one, and are encouraged to do so. ---that is a wrong message to send. Is that what you mean? * strategy X, unlike "ours", is allowed, so users will have rights to expect use of X as a rebase strategy would yield useful result, but that is wrong---Dscho knows that merge strategy X (I cannot read which one you had in mind if this is what you are talking about) does not work well in this and that cases. Is this what you mean, and if so what is X? Perhaps you had something other than the above two in mind? ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-16 19:57 ` Junio C Hamano @ 2009-11-16 21:25 ` Johannes Schindelin 2009-11-16 21:45 ` Junio C Hamano 2009-11-16 23:04 ` A Large Angry SCM 0 siblings, 2 replies; 38+ messages in thread From: Johannes Schindelin @ 2009-11-16 21:25 UTC (permalink / raw) To: Junio C Hamano Cc: Thomas Rast, git, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Hi, On Mon, 16 Nov 2009, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Sun, 15 Nov 2009, Thomas Rast wrote: > > > >> Using the "ours" strategy with rebase just discards all changes, > >> turning <branch> into <upstream> (or <newbase> if given). This is > >> unlikely to be what the user wants, so simply refuse to do it. > > > Besides, I find it rather arbitrary that the "ours" strategy is > > refused, but none of the user-provided merge strategies. IOW > > disallowing "ours" may very well foster unreasonable expectations. > > I cannot read this quite clearly. I meant the following: if "rebase -s ours" refuses to run, but my boss has written this cunning merge strategy "superduper" which is equally unlikely to yield a sensible result, "rebase -s superduper" should still refuse to run, no? Now, this scenario might be too rare to take care of, but maybe it shows that we have a design flaw here? Ciao, Dscho P.S.: Please note that I do not make a case against Thomas' patch series. As gitzilla once said "I cannot provide alternative patches, so that's that". ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-16 21:25 ` Johannes Schindelin @ 2009-11-16 21:45 ` Junio C Hamano 2009-11-16 22:04 ` Sverre Rabbelier 2009-11-16 23:04 ` A Large Angry SCM 1 sibling, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-16 21:45 UTC (permalink / raw) To: Johannes Schindelin Cc: Thomas Rast, git, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > I meant the following: if "rebase -s ours" refuses to run, but my boss has > written this cunning merge strategy "superduper" which is equally unlikely > to yield a sensible result, "rebase -s superduper" should still refuse to > run, no? Why should it? > Now, this scenario might be too rare to take care of, but maybe it shows > that we have a design flaw here? The decision is up to the user who is much more familiar with such a custom 'superduper' strategy, and git itself is in no position to make that decision for the user. It is none of our business to forbid users from using what he wrote, when we do not know what it is. I do not think the "rarity" is relevant. What do you mean by a design flaw? In other words, how should things look like in your ideal design? Certainly you are not talking about a design that enforces users who want to use custom strategy to first submit the strategy implementation to us for a review and have our blessings (perhaps we digitally sign approved strategy implementations) before being able to use it in "merge -s" and "rebase -s". I can _guess_ what you are _not_ talking about but I cannot tell what you _are_ talking about; sorry. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-16 21:45 ` Junio C Hamano @ 2009-11-16 22:04 ` Sverre Rabbelier 0 siblings, 0 replies; 38+ messages in thread From: Sverre Rabbelier @ 2009-11-16 22:04 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin, Thomas Rast, git, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Heya, On Mon, Nov 16, 2009 at 22:45, Junio C Hamano <gitster@pobox.com> wrote: > I can _guess_ what you are _not_ talking about but I cannot tell what you > _are_ talking about; sorry. I would hazard a way for the merge strategy to indicate whether it is fit to be used as rebase strategy? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 3/3] rebase: refuse to rebase with -s ours 2009-11-16 21:25 ` Johannes Schindelin 2009-11-16 21:45 ` Junio C Hamano @ 2009-11-16 23:04 ` A Large Angry SCM 1 sibling, 0 replies; 38+ messages in thread From: A Large Angry SCM @ 2009-11-16 23:04 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Junio C Hamano, Thomas Rast, Nicolas Sebrecht, Baz, Peter Krefting, Björn Steinbrink Johannes Schindelin wrote: [...] > As gitzilla once said "I cannot provide alternative patches, so that's > that". I'm not sure I actually said that [*1*] but I did point out that when there is an active discussion about which of multiple ways a feature can be implemented, the party that produces code usually gets their way. [*1*] There was beer involved and I was jet-lagged so maybe I did say that when I meant what I wrote above. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/3] Document and refuse rebase -s ours 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast ` (2 preceding siblings ...) 2009-11-15 18:25 ` [PATCH 3/3] rebase: refuse to rebase with -s ours Thomas Rast @ 2009-11-15 21:04 ` Junio C Hamano 2009-11-15 21:13 ` Thomas Rast 3 siblings, 1 reply; 38+ messages in thread From: Junio C Hamano @ 2009-11-15 21:04 UTC (permalink / raw) To: Thomas Rast Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Thomas Rast <trast@student.ethz.ch> writes: > I also implemented Nicolas's suggestion to reject -s ours outright; > I'm not really happy with starting a blacklist there, but maybe it > helps the next unwary user. I am inclined to agree with you and Nicolas on this, but I'll let the list decide if [3/3] is a good idea. I'd rewrite [3/3] in the following way to keep it easier to maintain the blacklist, like this. case "$1" in - ours) + ours | theirs | octopus | subtree) die "Refusing to rebase with $1; see git help rebase." esac It would also make it easier to turn this into a whitelist if we choose to, git-rebase--interactive.sh | 5 +---- git-rebase.sh | 5 +---- git-sh-setup.sh | 7 +++++++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 53d35f3..de7448b 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -571,10 +571,7 @@ first and then run 'git rebase --continue' again." STRATEGY="-s $2" shift ;; esac - if test "$STRATEGY" = "-s ours" - then - die "Refusing to rebase with 'ours' strategy; see git help rebase." - fi + git_check_merge_strategy_used_in_rebase "${STRATEGY#-s }" ;; -m) # we use merge anyway diff --git a/git-rebase.sh b/git-rebase.sh index 2d7d566..dd9ec63 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -306,10 +306,7 @@ do strategy="$2" shift ;; esac - if test $strategy = ours - then - die "Refusing to rebase with 'ours' strategy; see git help rebase." - fi + git_check_merge_strategy_used_in_rebase "$strategy" do_merge=t ;; -n|--no-stat) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index c41c2f7..724955f 100755 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -199,3 +199,10 @@ case $(uname -s) in } ;; esac + +git_check_merge_strategy_used_in_rebase () { + case "$1" in + ours) + die "Refusing to rebase with $1; see git help rebase." + esac +} ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 0/3] Document and refuse rebase -s ours 2009-11-15 21:04 ` [PATCH 0/3] Document and refuse rebase " Junio C Hamano @ 2009-11-15 21:13 ` Thomas Rast 0 siblings, 0 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-15 21:13 UTC (permalink / raw) To: Junio C Hamano Cc: git, Nicolas Sebrecht, Baz, Peter Krefting, Johannes Schindelin, Björn Steinbrink Junio C Hamano wrote: > > I'd rewrite [3/3] in the following way to keep it easier to maintain the > blacklist, like this. > > case "$1" in > - ours) > + ours | theirs | octopus | subtree) I agree with the rewrite; it's easier to maintain, even though it's now in a quite strange place. However, I think the example you gave is misleading: 'subtree' is a useful strategy if you want to rebase across a subtree merge boundary, isn't it? -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-03 7:01 ` Peter Krefting 2009-11-03 9:52 ` Johannes Schindelin @ 2009-11-03 10:12 ` Thomas Rast 1 sibling, 0 replies; 38+ messages in thread From: Thomas Rast @ 2009-11-03 10:12 UTC (permalink / raw) To: Peter Krefting; +Cc: Björn Steinbrink, Git Mailing List Peter Krefting wrote: > Thomas Rast: > > > Not very surprising if you use the 'ours' strategy, which doesn't merge at > > all but instead takes the 'ours' side (IIRC that's the upstream for a > > rebase, but I always have these mixed up). > > Sounds like it should be called "theirs", then. Or the documentation should > be clarify. The problem isn't that ours and theirs are swapped, it's that in a rebase, the 'ours' side is the upstream and 'theirs' is the commit you are currently rebasing. This makes sort of sense, because you are rebuilding your commit on top of the upstream (or actually, the so-far rebuilt commits, starting with the upstream), so the merge happens "on" the upstream. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: git pull --rebase and losing commits 2009-11-02 12:26 git pull --rebase and losing commits Peter Krefting 2009-11-02 15:04 ` Thomas Rast 2009-11-02 15:10 ` Björn Steinbrink @ 2009-11-03 4:27 ` Randal L. Schwartz 2 siblings, 0 replies; 38+ messages in thread From: Randal L. Schwartz @ 2009-11-03 4:27 UTC (permalink / raw) To: Peter Krefting; +Cc: Git Mailing List >>>>> "Peter" == Peter Krefting <peter@softwolves.pp.se> writes: Peter> git pull --rebase --strategy=ours origin master "No good can come of this." -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 <merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/> Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc. See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2009-11-16 23:06 UTC | newest] Thread overview: 38+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-02 12:26 git pull --rebase and losing commits Peter Krefting 2009-11-02 15:04 ` Thomas Rast 2009-11-02 21:34 ` Nanako Shiraishi 2009-11-02 15:10 ` Björn Steinbrink 2009-11-03 7:01 ` Peter Krefting 2009-11-03 9:52 ` Johannes Schindelin 2009-11-03 10:12 ` Peter Krefting 2009-11-11 14:03 ` [PATCH] Clarify documentation on the "ours" merge strategy Peter Krefting 2009-11-11 15:13 ` Baz 2009-11-11 20:35 ` Thomas Rast 2009-11-11 20:54 ` Baz 2009-11-11 21:02 ` Junio C Hamano 2009-11-11 21:30 ` [PATCH] " Nicolas Sebrecht 2009-11-11 23:37 ` Thomas Rast 2009-11-12 7:55 ` Junio C Hamano 2009-11-12 9:41 ` Peter Krefting 2009-11-14 2:12 ` Nanako Shiraishi 2009-11-15 9:10 ` Junio C Hamano 2009-11-16 8:20 ` Peter Krefting 2009-11-12 9:55 ` Björn Steinbrink 2009-11-15 18:25 ` [PATCH 0/3] Document and refuse rebase -s ours Thomas Rast 2009-11-15 18:25 ` [PATCH 1/3] Documentation: clarify 'ours' merge strategy Thomas Rast 2009-11-15 18:25 ` [PATCH 2/3] rebase docs: clarify --merge and --strategy Thomas Rast 2009-11-15 21:05 ` Junio C Hamano 2009-11-15 21:11 ` Thomas Rast 2009-11-15 18:25 ` [PATCH 3/3] rebase: refuse to rebase with -s ours Thomas Rast 2009-11-15 18:39 ` Sverre Rabbelier 2009-11-15 18:44 ` Thomas Rast 2009-11-16 12:35 ` Johannes Schindelin 2009-11-16 19:57 ` Junio C Hamano 2009-11-16 21:25 ` Johannes Schindelin 2009-11-16 21:45 ` Junio C Hamano 2009-11-16 22:04 ` Sverre Rabbelier 2009-11-16 23:04 ` A Large Angry SCM 2009-11-15 21:04 ` [PATCH 0/3] Document and refuse rebase " Junio C Hamano 2009-11-15 21:13 ` Thomas Rast 2009-11-03 10:12 ` git pull --rebase and losing commits Thomas Rast 2009-11-03 4:27 ` Randal L. Schwartz
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 NNTP newsgroup(s).