git pull --rebase and losing commits

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 Krefting wrote:

[...]

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.

Quoting Thomas Rast <trast@student.ethz.ch>

This was implemented and posted here quite some time ago.

  http://thread.gmane.org/gmane.comp.version-control.git/85163/focus=85602

On 2009.11.02 13:26:37 +0100, Peter Krefting wrote:

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

Thomas Rast:

Sounds like it should be called "theirs", then. Or the documentation should be clarify.

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:

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.

Hi,

On Tue, 3 Nov 2009, Peter Krefting wrote:

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

Johannes Schindelin:

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.

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?

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.

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(-)

Indeed. Perhaps this clarification will help the next poor soul that tries doing what I tried?

2009/11/11 Peter Krefting <peter@softwolves.pp.se>:

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

Baz wrote:

[...]

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:

2009/11/11 Thomas Rast <trast@student.ethz.ch>:

Yes, this (with Peter's patch) makes the danger nice & clear.

Thanks!

-Baz

Thomas Rast <trast@student.ethz.ch> writes:

Looking very good.

The 11/11/09, Junio C Hamano wrote:

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 wrote:

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,

Thomas Rast <trast@student.ethz.ch> writes:

 ...

"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.

Junio C Hamano:

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).

Yes, I am apparently looking for something that is not available in the Git codebase yet. :-)

Quoting Peter Krefting <peter@softwolves.pp.se>

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 <nanako3@lavabit.com> writes:

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.

Junio C Hamano:

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.

Indeed. If it that was the case, I deserve whatever problems I get :-)

On 2009.11.11 23:55:09 -0800, Junio C Hamano wrote:

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.

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

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(-)

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(-)

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(-)

Thomas Rast <trast@student.ethz.ch> writes:

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(-)

Junio C Hamano wrote:

This is much nicer than mine!

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.

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(-)

Heya,

On Sun, Nov 15, 2009 at 19:25, Thomas Rast <trast@student.ethz.ch> wrote:

Is this solid? Would "-s  ours" (two spaces) work?

Sverre Rabbelier wrote:

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.

Hi,

On Sun, 15 Nov 2009, Thomas Rast wrote:

"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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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).

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?

Hi,

On Mon, 16 Nov 2009, Junio C Hamano wrote:

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".

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Why should it?

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.

Heya,

On Mon, Nov 16, 2009 at 22:45, Junio C Hamano <gitster@pobox.com> wrote:

I would hazard a way for the merge strategy to indicate whether it is fit to be used as rebase strategy?

Johannes Schindelin wrote: [...]

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.

Thomas Rast <trast@student.ethz.ch> writes:

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(-)

Junio C Hamano wrote:

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?

Peter Krefting wrote:

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.

Peter>  git pull --rebase --strategy=ours origin master

"No good can come of this."