Re: StGIT discards local commits on "stg pull"

Previous thread: git-svn test suite failures due to Subversion race by Michael Spang on Sunday, February 11, 2007 - 8:32 pm. (7 messages)

Next thread: Why a _full_ diff? (was: [PATCH] Char: serial167, cleanup (fwd)) by Geert Uytterhoeven on Monday, February 12, 2007 - 1:39 am. (5 messages)
From: Pavel Roskin
Date: Monday, February 12, 2007 - 12:26 am

Hello!

I have been bitten by a strange bug/feature of StGIT, and it looks like it's not
only counterintuitive, but also inconsistent with git.

I have a repository available over ssh and I push to it from several places.  
Sometimes I make a commit and forget to push it.  Then I run "stg pull" to make
sure my repository is up to date.

The result is that the repository is rebased back to the last remote commit. 
It's very easy to miss.  There is no warning.  Everything looks just like an
update from the remote.

The example below shows that git-pull keeps my commit, but "stg pull" discards
it by rebasing back to the remote ID.

My .gitconfig doesn't override "stg pull" behavior; it merely sets my name and
mail aliases.

[proski@dl stgit-test]$ stg id
ebc429e7b7e596a12e8255fadc397123893cec73
[proski@dl stgit-test]$ echo "test change" >README
[proski@dl stgit-test]$ git-commit -m "test commit" README
Created commit 468861a2a1530f3bf98108e69632b3059e4ca0ce
 1 files changed, 1 insertions(+), 14 deletions(-)
[proski@dl stgit-test]$ git-pull
Fetching refs/heads/master from http://homepage.ntlworld.com/cmarinas/stgit.git
using http
Fetching refs/heads/stable from http://homepage.ntlworld.com/cmarinas/stgit.git
using http
Already up-to-date.
[proski@dl stgit-test]$ stg id
468861a2a1530f3bf98108e69632b3059e4ca0ce
[proski@dl stgit-test]$ stg pull
Checking for changes in the working directory... done
Pulling from "origin"...
Fetching refs/heads/master from http://homepage.ntlworld.com/cmarinas/stgit.git
using http
Fetching refs/heads/stable from http://homepage.ntlworld.com/cmarinas/stgit.git
using http
rebasing to "ebc429e7b7e596a12e8255fadc397123893cec73"...
done
No patches applied
[proski@dl stgit-test]$ stg id
ebc429e7b7e596a12e8255fadc397123893cec73
[proski@dl stgit-test]$

--
Regards,
Pavel Roskin
-

From: Catalin Marinas
Date: Monday, February 12, 2007 - 2:31 am

I think this is a "feature" but we should've probably leave the
original behaviour as the default. Maybe we should also have this
per-branch rather than per-repository.

In StGIT 0.12, git-fetch is used by default rather than git-pull and
StGIT performs the rebasing. We had some discussions on whether this
would break existing workflows and we thought it wouldn't (I don't
usually mix git-commit with stg commands).

The solution would be to define the following in your gitconfig file
(either ~/.gitconfig or .git/config; a full example in StGIT's
examples/gitconfig):

[stgit]
	pullcmd = git-pull
	pull-does-rebase = no

The last line would tell StGIT not to do the rebasing and let git-pull
handle it.

I agree that for the rebasing case, we should have some warning if
fast-forwarding of the stack's base is not possible so that you could
run 'stg uncommit'.

-- 
Catalin
-

From: Yann Dirson
Date: Monday, February 12, 2007 - 1:26 pm

No, I agree it's a bug.  Rebasing after a fetch should allow this
workflow to work as well.  If the parent branch is not a rewinding
one, we should ensure there is nothing lost.  And even for rewinding
branches, we should probably keep track of the existence of commits,
so we can warn and nothing gets lost without knowing.

That makes me think that in most cases, an "stg commit" is done to be
pushed to a remote repo, as symetric operation to "stg pull".
Probably we could add support for this sort of operation (similar to

Right, having it per-branch would allow, eg. to declare something like
git-svn as the way to pull from a given branch, while using different

Right.  "rebase --undo" could help too.

best regards,
-- 
Yann.
-

From: Yann Dirson
Date: Monday, February 12, 2007 - 2:47 pm

Thinking about it, detecting whether we're going to lose a commit is
just checking *before pulling* whether the current base is reachable
from the parent's current head.

In git-fetch-based workflows, proceeding further should be simply
refused.  I'm not sure about the git-pull-based workflows; here is at
least one problem with git-pull I can think of:

AFAICT, someone using git-pull would in this case get a merge commit,
so under 0.11 (or with the compat settings outlined by Catalin) you
should now have your stack based on a merge commit, with as parents
the new parent-branch head and the commits you did not push yet, right ?
So how did you proceed from there using 0.11 ?

I'd think you still want to push your patch into the parent repo, but
the situtation is cumbersome: it would have been far easier IMHO to
"stg pull" first, in which case the fetch-and-rebase model is what you
wanted, and then do a "stg commit && git push".

That makes me think that indeed we should have an stgit command doing
precisely this "stg commit && git push" (suggested in previous mail)
as an atomic operation, rolling back the commit if the push failed
because you were out of date.


Since that looks like a pathological case, I suppose this may not be
what you were trying to do.  Can you please give more information if
that is the case ?

Best regards,
-- 
Yann.
-

From: Catalin Marinas
Date: Monday, February 19, 2007 - 4:07 pm

There is a potential problem with this approach - pulling/fetching
from a tree which is always rebased (either managed with StGIT or
simply running git-rebase before publishing it) would report an error
since the old base is no longer reachable from the current head. In
this case, the current fetch+rebase behaviour would be desirable.

I think the fail-safe solution would be to leave the old behaviour
(i.e. git-pull and pull-does-rebase=no) and people that need to pull
from branches like that described above would use the fetch+rebase
approach. Ideally, we'll have this configurable per-branch (and could
leave the global one as well if the most specific is not available,
but should default to git-pull).

Let me know what you think so that I'll try to release a 0.12.1 update
(I already have the simple patch for using git-pull by default if you
are OK with this scenario).

Thanks.

-- 
Catalin
-

From: Pavel Roskin
Date: Monday, February 19, 2007 - 4:28 pm

One possible workaround would be to report an error and do nothing if
the old head would become unreachable (it's possible that I'm missing

By the way, it would be great to reduce all complexity to "one bit" per
branch.  If stgit.internal-pull (the name is subject to improvement) is
on, "stgit pull" calls git-fetch and does rebase.  Otherwise, it calls

Any fix for the current behavior would be fine to me.  Either restore
the old default or don't rebase if the old head becomes unreachable.

-- 
Regards,
Pavel Roskin

-

From: Yann Dirson
Date: Monday, February 19, 2007 - 5:00 pm

Being able to specify the command to use makes it possible to hook to
other SCM's (eg. using git-cvsimport, git-svn or similar).

I'll try to adapt by stg-cvs script to fit into this model some day
(indeed, "pullcmd = stg-cvs fetch" may indeed already work).

Best regards,
-- 
Yann.
-

From: Yann Dirson
Date: Tuesday, February 20, 2007 - 11:55 am

Thinking twice, we could make things even better :)

Let's acknowledge that we have several ways of pulling, by getting rid
of the awkward 'stgit.pull-does-rebase'.  Instead we could use a new
"pull-policy" setting, with possible values "pull" and "fetch-rebase"
to match the 2 currently-available behaviours.  Then according to the
pull-policy, "stg pull" would use "pullcmd" or a new "fetchcmd"
setting, thus removing the requirement to mess with pullcmd in the
common case.

All of "pull-policy", "pullcmd", and "fetchcmd" would have a
per-branch and a global setting (stgit.<foo> and
branch.*.stgit.<foo>).

We could also introduce a 3rd "rebase" policy, to support local parent
branches better.

Best regards,
-- 
Yann.
-

From: Yann Dirson
Date: Monday, February 19, 2007 - 4:44 pm

Right - a better solution is highlighted in today's proposal (which I

We could do this, but I only see this as a temporary measure to avoid
unleashing the new behaviour onto unsuspicious users before we've
ironed out the final behaviour.  I still think that the current
behaviour belongs to a very specific workflow which is not that


Since it is important that users don't unknowingly loose commits, yes
it is preferable to release 0.12.1 with the old behaviour.  Let's take
the time to shake things more for 0.13, and we can reconsider this
choice by then.

Best regards,
-- 
Yann.
-

From: Pavel Roskin
Date: Monday, February 12, 2007 - 5:20 pm

I don't know the original motivation behind effectively reimplementing
"git pull" in StGIT, but it's clear that the StGIT's own implementation
needs some polish.

I think it's always wrong to lose local commits.  I think StGIT should
refuse to rebase if a merge would be needed or the rebase would go back
in history (in other words, if git-pull would not go to the remote

Maybe such assumptions could be enforced?  Perhaps we could consider
branch specialization.  As it stands now, we can have branches where
fast-forward is OK and branches where it's not OK.

If we look at it from the user standpoint, the branches could be
distinguished by the use model:

1) Tracking branch: pull is OK, commit is not OK, push is not OK.  All
development is done in StGIT patches and sent to others.

2) Development branch: commit is OK, push is OK, pull is OK but no
merges by default.

3) Merge branch: pull is OK, even with automatic merge, commit is OK,
merge is OK.


It's actually my deliberate choice to subject myself to the pains of the
default configuration.  I don't want to live in backwards compatible
environment until it rots away.  I'll rather eat the dogfood we are

Sounds good to me.

-- 
Regards,
Pavel Roskin

-

From: Catalin Marinas
Date: Tuesday, February 13, 2007 - 3:48 pm

Yann can describe the motivation better as he was the one pushing for
it. It was mainly to allow different local branches to be based on
different remote branches. In the normal case, i.e. using only StGIT
commands, there shouldn't be any difference. As I said, the old
behaviour is still present and maybe we should do the new one


I probably have another situation - a branch managed partially with
StGIT but GIT commits (or 'stg commit') used and pulling would lead to
a merge of the base, followed by patch pushing. This would work if we

I don't consider this as a backward-compatibility feature. It simply
targets a different workflow and it would be even better if we have it
per-branch. The default should be the current fetch+rebase (as the
most common case would be to use StGIT commands only) but with a
warning if stack base fast-forwarding is not possible.

-- 
Catalin
-

From: Yann Dirson
Date: Monday, February 19, 2007 - 1:47 pm

The primary motivation was to allow some sort of distributed handling
of a stack.  That is, I publish an stgit-managed branch, you "stg
clone" it, you create your own patches on top of that, and when I
refresh my stack, you can just "stg pull" it as you would do if my

Right, we should make it so losing local commits cannot be done by
error.  However, we cannot rely on the branch topology at the time
we're rebasing.  Consider those 2 use cases:

- my stack forks off a non-rewinding branch, and I "stg commit" part
of it, then "stg pull"

- my stack forks off a rewinding branch, and I use "git fetch" to look
at what's new upstream, then I decide it's a good time to rebase, and
run "stg pull" (or preferably "stg rebase" to avoid fetching more
recent commits by error)

In the 2 cases "git pull" would do a merge.  One crucial thing
distinguishes the 2 cases, is that in the 1st case we have "manually"
messed with the stack base.  By "manually", I mean we changed the
stack base by other means than pulling/rebasing, but this notion
possibly needs some tuning.  "stg commit" is an example, "stg
uncommit" is another, and both would cause "git pull" to do a merge
*because rebasing would not be what you want*, whereas in the
rewinding-branch use-case *what you is precisely rebasing*.

What we could do then, is having "rebase" and "pull" record an
orig-base ref as a "backup copy" of the the new base, and first refuse
to do the job (unless --force'd) if the base does not match its backup

I'm less sure about the use-case you're trying to address here.  Could
you please give more details ?  The only case I can think of would be
that of a rewinding branch that would have been just that, rewinded,
so it should be handled differently than when any new commit would


This one I'm not sure to understand well.  Could you please describe
it in more details, perhaps in terms of the relation with other repos,

You mean, a branch to which possibly several devs would commit (or to
which you ...
Previous thread: git-svn test suite failures due to Subversion race by Michael Spang on Sunday, February 11, 2007 - 8:32 pm. (7 messages)

Next thread: Why a _full_ diff? (was: [PATCH] Char: serial167, cleanup (fwd)) by Geert Uytterhoeven on Monday, February 12, 2007 - 1:39 am. (5 messages)