patch, 3 partsUpdate to stgit parent tracking

The following series brings a couple of improvements to parent- and remote-handling stuff, including addressing the FIXME you pointed to in your reply.

The only use I know of for this parameter is 'stg pull . <branch>', for which 'stg rebase <branch>' should be used instead.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/pull.py |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

Previously we were just assuming that the remote from which we just failed defined a local branch whose name was the same as the remote def, and that this branch was the parent. While this is true for the most common case (branch "origin" from remote "origin"), it is quite an unflexible assumption.

Also move the rebasing from git.pull to commands.pull.func since that is clearly stgit-level stuff, and rename git.pull() to git.fetch() to match default behaviour.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/pull.py |   20 +++++++++++---------
 stgit/git.py           |   10 +++-------
 2 files changed, 14 insertions(+), 16 deletions(-)

On 01/02/07, Yann Dirson <ydirson@altern.org> wrote:

The t1200-push-modified.sh test fails after applying this patch. It looks like the 3rd test fails to pull the changes from 'foo' into 'bar'.

Thanks.

On Fri, Feb 02, 2007 at 09:58:06AM +0000, Catalin Marinas wrote:

Strange, it passes here, even after pulling you latest commits (HEAD=fba895f5f12a652c248b0fd02b7c41f8342cfbdc).

Best regards,

On Fri, Feb 02, 2007 at 07:07:06PM +0100, Yann Dirson wrote:

Wondering which GIT version you are using, I tried to run the testsuite with current GIT HEAD, and it fails much earlier (t1001). We'll have some work here :)

Best regards,

On 02/02/07, Yann Dirson <ydirson@altern.org> wrote:

I tried the latest GIT and it only fails at t1200. I think I got a t1001 failure but I fixed it. Do you want me to push the snapshot I have and you can see whether it fails on your system? I tried it with both Python 2.4 and 2.3 but what's strange is that 'pull' run manually (not from the test script) seems to work.

On 02/02/07, Catalin Marinas <catalin.marinas@gmail.com> wrote:

OK, tried again and 'stg pull' doesn't update the base with the last patch in the series ('stg rebase origin' updates it). I don't have anything set up in my gitconfig in this area.

I already committed the patches I have (and pushed them to the public repo) so that we know whether you have additional changes.

Thanks.

On Fri, Feb 02, 2007 at 11:25:28PM +0000, Catalin Marinas wrote:

So now that this particular problem has a fix, let's fix everything related.

Since branch.*.merge accept local ref names, there is probably far more to change that what I initially feared.

1. I guess we can continue to store local refs for those branches when
they mirror a remote one, but if we store something there without
storing a branch.*.remote to '.', git-fetch could confuse this local
ref with a ref from remote 'origin', since that's still the default.

2. branch.*.merge is not where we should read a parent branch from.
Even if we store that info there, the user could have put a remote ref
name there.  So if we want to keep a get_parent_branch() method we'll
have to store that info somewhere else, eg. branch.*.stgit.parentbranch.

3. we need get_parent_branch() to implement parent branch as default
arg to "stg rebase"

So (please correct me if necessary):

- always store local parent branch to branch.*.stgit.parentbranch
- never set branch.*.merge without setting branch.*.remote
- set them, with remote as '.' when dealing with local branch

Best regards,

Yann Dirson <ydirson@altern.org> writes:

If that is the case maybe we should fix branch.*.merge not to match the local tracking branch name. Matching it with local tracking branch name when the remote name does not match was werely a fallback to help broken configurations confused people might create by hand, and has never been the part of sane practice at all.

Junio C Hamano <junkio@cox.net> writes:

That is, I suspect that reverting 80c79776 would make things a lot more consistent.

-- >8 -- [PATCH] Revert "Allow branch.*.merge to talk about remote tracking branches."

This reverts commit 80c797764a6b6a373f0f1f47d7f56b0d950418a9.

Back when I committed this, it seemed to be a good idea. People who always use remote tracking branches can optionally use the local name they happen to use to specify what to merge, which meant that I did not have to teach them why we use the name at the remote side every time they are confused.

But allowing it seems to break other people's scripts. The real solution is not to allow more ways to express the same thing, but to educate people to use the right syntax.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-parse-remote.sh |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

On Sat, Feb 03, 2007 at 03:01:59PM -0800, Junio C Hamano wrote:

Oh, sorry, I indeed meant "less to change" :>

Since git-fetch alone makes the decisions (so no algorithm has to get duplicated into stgit), and since the only remote refs possibly confused for local ones are the old non-separate ones, and since in that case usually the remote branches are mapped to the same name, the odds for a problem are low, and will get lower.

Making branch.*.merge not understand local refs, OTOH, would require stgit to resolve a local branch to its remote name. Note that I already had to do the job to guess which remote a branch was pulled from (and thus, to decide whether a branch was local or remote).

Fortunately, all of this is going to be unnecessary on newer repos since git-clone now fills all the fields - we only have to deal with the upgrade of older repos.

Best regards,

On Sat, Feb 03, 2007 at 03:01:59PM -0800, Junio C Hamano wrote:

I was one of the major pushers of this feature in December; however, I
think it actually can create some confusing results. See:
  http://article.gmane.org/gmane.comp.version-control.git/35789

I still think it is worth pursuing, but I have been waiting for v1.5.0 to happen before stirring up more trouble. Regardless, I think this implementation is not the right way to go about it, and you should revert 80c79776.

-Peff

Jeff King <peff@peff.net> writes:

I actually was re-reading that exact post (and more importantly, messages 35810 and 36374 in the thread) before I sent the message you are replying to.

While I think branch.*.merge, which was invented for the use of git-pull -> git-merge callchain, should consistently use the remote branch name, after re-reading your messages, especially 36374, I see why you wanted a mechanism to talk about relationships between two (or more) local branches.

I do not think it is wrong to have a separate mechanism (and it might end up being called as "mergelocal" which you said might be a terrible name) for that. I just think it is probably a wrong thing to do to use branch.*.merge for that. As you said, operations you are interested in like format-patch (and perhaps rebase) of a local branch relative to another locally available ref (be it another branch or a tracking branch) do not involve fetching (hence pulling) necessarily.

On Sat, Feb 03, 2007 at 06:10:59PM -0800, Junio C Hamano wrote:

Good, it looks like we are mostly agreeing on this then (the feature might have some use, but it should _not_ be tacked onto branch.*.merge). I will give it some thought until v1.5.0 has settled, and then hopefully submit some patches.

-Peff

On Fri, Feb 02, 2007 at 07:07:06PM +0100, Yann Dirson wrote:

With current GIT HEAD, plain git-clone creates the following config (when cloning a repo with HEAD pointing to branch "downstream":

[remote "origin"]
        url = /export/work/yann/git/stgit/tmp/.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "downstream"]
        remote = origin
        merge = refs/heads/downstream
                ^^^^^^^^^^^^^^^^^^^^^

I would have expected "merge = remotes/origin/downstream" instead, and this setting confuses the rebase-after-pull logic, causing it to rebase the stack onto its own base (no-op).

Isn't that a git-clone bug ?

Now, it looks like without this patch, the post-fetch rebasing only worked by chance, even more than I thought.

Best regards, 

On Sat, Feb 03, 2007 at 12:56:55AM +0100, Yann Dirson wrote:

Hm, I see I was confused about the branch.*.merge meaning. The config doc does document it to be the remote part of the refspec.

But what does that mean - why using the remote part here ? I cannot see that, although I can see arguments why using the local part here:

- When we come to finding the branch to merge, we have already fetched
all remote branches into local ones, so why would we need to find a
local branch matching this remote name ?

- How are we supposed to handle the case where a single remote ref is
mapped to several local ones ?  Although I cannot see a use for this
situation, it would seem to be a valid one, whereas having several
fetch lines for a single local ref in a single remote would clearly
be invalid: using a local ref would avoid a possible ambiguity.

Best regards,

Yann Dirson wrote:

No, it is a feature. branch.<name>.merge is usually _remote_ head, to allow pulling without local tracking branch. IIRC there was patch to allow to use local head, if it is ambiguous.

Yann,

On 02/02/07, Yann Dirson <ydirson@altern.org> wrote:

As I haven't followed the GIT latest developments in this area, I can't comment. Regarding StGIT, I'd like it to work with earlier stable versions of GIT and not just with the current HEAD. I (and probably many others) already have repositories cloned some (long) time ago and their gitconfig might not have the cloning information.

The (interim) solution I see is for StGIT pull command to default to using git-pull and people can configure it to git-fetch and the automatic rebase if they need it. I'd like to release 0.12 this weekend but 'pull' doesn't currently work. Are you OK with my solution (for backward compatibility with older repositories and GIT versions)?

Thanks.

On Sat, Feb 03, 2007 at 12:16:47PM +0000, Catalin Marinas wrote:

As noted elsewhere, I have misinterpreted the branch.*.merge parameter; I'm trying to get things right.

There are 2 issues there:

- support for running atop previous releases of Git. For this we
should select a policy like "we support the current stable branch (or
to-be-published, like 1.5.0), and the previous (ie. 1.4.4.x or
1.4.x)", and document that in the release notes so packagers can
update their dependencies.

- and continue to work with repos created with old releases.  For the
"pull" change, what's needed (or should be, modulo bugs such as this
one), is to tell people they need to "git config" the missing info
when it's not there.  I have tried to keep the old behaviour as much
as possible (ie. relying on the user to pass the info on command-line)
instead of insisting on the user providing the info in the config
file, but this work is probably not 100% finished.

Right, it is reasonable to keep the old behaviour the default for now.

I'll still try to make it work nevertheless before the release :)

Best regards,

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/stack.py |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)