On 05/05/2011 10:09 PM, mark florisson wrote:

On 5 May 2011 21:52, Dag Sverre Seljebotn<d.s.seljebotn@astro.uio.no> wrote:

...

There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times.

It doesn't matter, since the two openmp branches with the same changes merged OK, but we shouldn't make this a habit. For instance, the openMP commits also show up as part of vitja's pull request, which is confusing.

In Mercurial speak: The openmp branch was used like you would use a Mercurial "patch queue" in one case, and as a branch in another case. In git they are the same technically and you rely on conventions to make sure you don't treat a "queue" as a "branch".

OPTION A) Either i) only branch from master, or ii) make sure you agree with whoever you're branching from that this is a "branch", not a "patch queue", so that it isn't rebased under your feet.

We could also, say, prepend all patch queues with an underscore (its private).

OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but nobody are pulling from dagss/cython these days anyway.

Opinions?

FYI,

The workflow me and Mark is currently using is:

a) Fork off a feature branch from master (with master I'll always refer to cython/master)

b) When one gets in sync with master, do NOT merge master, but rather rebase on top of it:

git pull --rebase origin master

c) Continue rebasing, and eventually .

The advantage of this approach is that ugly merges disappear from history, since commits are rewritten. And the history graph looks very nice and is easy to follow.

BUT, if the result is duplication, we should avoid this practice, and rather always merge.

Dag Sverre

I think the rebasing is pretty elegant, so I'm +1 on that, as long as everyone agrees because those duplicated commits are nasty. I'm surprised git didn't issue an error to prevent this.

Going OT: I guess a principle of git is to be as dumb as possible, so that you can predict what it does. When you rebase, you really get entirely new versions of all your commits. There's no way to link the old commits with the new commits, except to compare the commit message. And not even that, since you can change commit messages during rebases. Git doesn't even store commits by their diff, it stores each commit by the resulting full tree contents, so even if the patch is the exact same, git couldn't really know (without going through your entire history and check for "similar changes"...ugh) What I wish they did was to add a simple "rebased-from" header field on commits, which could be trivially checked on merges to issue a warning. I guess the reason it is not there is because usually you're about to (automatically) throw away the commits, so the only purpose it would have would be sort of pedagogical. Dag Sverre

On 5 May 2011 22:22, Stefan Behnel <stefan_ml@behnel.de> wrote:

Dag Sverre Seljebotn, 05.05.2011 21:52:

...

There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times.

What (I think) happened, was that Vitja pulled in Mark's changes into his unreachable code removal branch, and they ended up in his pull request. I guess I was assuming that git wouldn't care too much about branch duplication, so I just accepted the pull request via the web interface. Apparently, it did care.

I tend to rebase my local change sets before pushing them, and I think it makes sense to continue doing that.

Stefan _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel

Whatever approach we take, it should be documented extensively on the hackerguide, at least the rebase approach, as it has apparently somewhat catastrophic results if not everyone does it. On the one hand rebasing gives you a nice history, but on the other hand it's also a bit harder to use and if you start merging at some point reverting to rebasing is not so easy. So perhaps we should just ignore the 'merge' commits (using command line git, git log --no-merges) and pull and push away.

Yes, that is the only time it happens. Do we agree on a) ask before you pull anything that is not in cython/* (ie in private repos), b) document it in hackerguide? DS -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. Robert Bradshaw <robertwb@math.washington.edu> wrote: On Thu, May 5, 2011 at 1:22 PM, Stefan Behnel <stefan_ml@behnel.de> wrote: > Dag Sverre Seljebotn, 05.05.2011 21:52: >> >> There was just a messup in git history: Mark's OpenMP pull request got >> merged twice; all commits show up two times. > > What (I think) happened, was that Vitja pulled in Mark's changes into his > unreachable code removal branch, and they ended up in his pull request. I > guess I was assuming that git wouldn't care too much about branch > duplication, so I just accepted the pull request via the web interface. > Apparently, it did care. > > I tend to rebase my local change sets before pushing them, and I think it > makes sense to continue doing that. +1, I think for as-yet-unpublished changes, it makes the most sense to rebase, but for a longer-term branch, merging isn't as disruptive to the history (in fact is probably more reflective of what's going on) and is much better than duplication. To clarify, is this only a problem when we have A cloned from master B cloned from A (or from master and then pulls in A) A rebases A+B merged into master ? If this is the case, then we could simply make the rule that you should ask before hacking a clone atop anything but master. (Multiple people can share a repeatedly-rebased branch, right.) We could also us the underscore (or another) convention to mean "this branch is being used as a queue, puller beware." Surely other projects have dealt with this. - Robert_____________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel

2011/5/6 Robert Bradshaw <robertwb@math.washington.edu>:

I don't like the default to be "don't pull from me"--I'd rather there be some convention to indicate a branch is being used as a queue. Maybe even foo-queue, or a leading underscore if people like that.

On Thu, May 5, 2011 at 2:03 PM, Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no> wrote:

...

Yes, that is the only time it happens.

Do we agree on a) ask before you pull anything that is not in cython/* (ie in private repos), b) document it in hackerguide?

DS

-- Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Robert Bradshaw <robertwb@math.washington.edu> wrote:

...

On Thu, May 5, 2011 at 1:22 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:

...

Dag Sverre Seljebotn, 05.05.2011 21:52: >> >> There was just a messup in git history: Mark's OpenMP pull request got >> merged twice; all commits show up two times. > > What (I think) happened, was that Vitja pulled in Mark's changes into his > unreachable code removal branch, and they ended up in his pull request. I > guess I was assuming that git wouldn't care too much about branch > duplication, so I just accepted the pull request via the web interface. > Apparently, it did care. > > I tend to rebase my local change sets before pushing them, and I think it > makes sense to continue doing that. +1, I think for as-yet-unpublished changes, it makes the most sense to rebase, but for a longer-term branch, merging isn't as disruptive to the history (in fact is probably more reflective of what's going on) and is much better than duplication. To clarify, is this only a problem when we have A cloned from master B cloned from A (or from master and then pulls in A) A rebases A+B merged into master ? If this is the case, then we could simply make the rule that you should ask before hacking a clone atop anything but master. (Multiple people can share a repeatedly-rebased branch, right.) We could also us the underscore (or another) convention to mean "this branch is being used as a queue, puller beware." Surely other projects have dealt with this. - Robert

About my branch: I've rebased it from upstream/master at home and made "forced push" At work I pulled it back and rebased from origin, then I tried to rebase if again from upstream/master Guess I was wrong somewhere. So I've lost two latest commits (generators related fix) Sometimes it's much easy to branch from upstream and then make cherry-pick (manual rebase). -- vitja.

2011/5/6 Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no>:

On 05/06/2011 08:20 AM, Vitja Makarov wrote:

...

2011/5/6 Robert Bradshaw<robertwb@math.washington.edu>:

...

I don't like the default to be "don't pull from me"--I'd rather there be some convention to indicate a branch is being used as a queue. Maybe even foo-queue, or a leading underscore if people like that.

On Thu, May 5, 2011 at 2:03 PM, Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no>  wrote:

...

Yes, that is the only time it happens.

Do we agree on a) ask before you pull anything that is not in cython/* (ie in private repos), b) document it in hackerguide?

DS

-- Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Robert Bradshaw<robertwb@math.washington.edu>  wrote:

...

On Thu, May 5, 2011 at 1:22 PM, Stefan Behnel<stefan_ml@behnel.de>  wrote:

...

Dag Sverre Seljebotn, 05.05.2011 21:52:>>  >>  There was just a messup in

git history: Mark's OpenMP pull request got>>  merged twice; all commits show up two times.>  >  What (I think) happened, was that Vitja pulled in Mark's changes into his>  unreachable code removal branch, and they ended up in his pull request. I>  guess I was assuming that git wouldn't care too much about branch>  duplication, so I just accepted the pull request via the web interface.>  Apparently, it did care.>  >  I tend to rebase my local change sets before pushing them, and I think it>  makes sense to continue doing that. +1, I think for as-yet-unpublished changes, it makes the most sense to rebase, but for a longer-term branch, merging isn't as disruptive to the history (in fact is probably more reflective of what's going on) and is much better than duplication. To clarify, is this only a problem when we have A cloned from master B cloned from A (or from master and then pulls in A) A rebases A+B merged into master ? If this is the case, then we could simply make the rule that you should ask before hacking a clone atop anything but master. (Multiple people can share a repeatedly-rebased branch, right.) We could also us the underscore (or another) convention to mean "this branch is being used as a queue, puller beware." Surely other projects have dealt with this. - Robert

About my branch:

I've rebased it from upstream/master at home and made "forced push" At work I pulled it back and rebased from origin, then I tried to rebase if again from upstream/master

Do I understand correctly that you:

 a) You make local changes at home  b) Rebase them on cython/master  c) Force-push to vitja/somebranch  d) Go to work, where you have other local changes  e) Rebase your work changes at work on top of vitja/somebranch

Right.

If this is correct; then this can't work. The reason is that after the force-push in c), there are no shared commits (apart from what's shared from cython/master) between your work computer and vitja/somebranch.

So the rule is: If you rebase a branch, then if you have other copies of that branch (like on a work computer), destroy them (e.g., git branch -D)!  And then fetch new copies of the branches.

(And as you say, if you do have different changes in many places then you can recover from an unfortunate rebase by cherry-picking. And you can always undo a rebase by looking at "git reflog" and manually check out the old HEAD.)

Thank you for explanation. So btw, when I do rebase and my changes were already pushed I have to use forced push. Is forced push ok? -- vitja.

On 05/05/2011 11:07 PM, Robert Bradshaw wrote:

I don't like the default to be "don't pull from me"--I'd rather there be some convention to indicate a branch is being used as a queue. Maybe even foo-queue, or a leading underscore if people like that.

I've seen leading underscore being used by other people on github, so let's settle on that for now. Of course, if you do pull from a non-master branch, you should be communicating a lot about that fact anyway; it's a bad idea for a lot of other reasons as well. I've updated http://wiki.cython.org/HackerGuide. Here's an example of prior art in git workflows, developed I think primarily for IPython: https://github.com/matthew-brett/gitwash It's essentially some Sphinx documentation with replacable names ("To contribute to PROJECTNAME, you should get an account on github...") that one can merge into ones own documentation. If anybody is interested in looking at that I'm all for it. Dag Sverre

On Thu, 2011-05-05 at 13:49 -0700, Robert Bradshaw wrote:

Surely other projects have dealt with this.

We have for instance adopted a very strict set of policies on that: 1) Never branch of anything except for master or x.y.z-stable branches 2) Never rebase master or x.y.z-stable branches, unless everybody agrees 3) Never do fast-forward merges, always rebase your stuff before merge 4) Never commit directly to master, use feature branches That is, all branches are either in the personal namespace of the developers for experimental stuff, or in the main namespace prefixed with the ticket number. I admit that this is quite restrictive, but in the end, it doesn't take so much effort to follow and the history is very nice and easy to understand, I even literally follow the development by reading the history, when I am unable to track IRC etc. Of course, you don't have to copy this blindly, but few permissive conventions, which do not impose additional load on the developers like 1) Always branch from master 2) Never rebase master would definitively not hurt. I was recently trying to bisect Cython, as I was under impression that recent commits broke something for me (it turned out not to be the case, but that's not the point) and the history is just to horribly confusing that I couldn't make any sense out of it. All these branches from branches from branches which gets merged in random order make it impossible for an outside person to make any sensible use of this information... Hope that helps, -- Sincerely yours, Yury V. Zaytsev

On 05/13/2011 12:36 AM, Ondrej Certik wrote:

Hi,

On Thu, May 5, 2011 at 12:52 PM, Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no> wrote:

...

There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times.

It doesn't matter, since the two openmp branches with the same changes merged OK, but we shouldn't make this a habit. For instance, the openMP commits also show up as part of vitja's pull request, which is confusing.

In Mercurial speak: The openmp branch was used like you would use a Mercurial "patch queue" in one case, and as a branch in another case. In git they are the same technically and you rely on conventions to make sure you don't treat a "queue" as a "branch".

OPTION A) Either i) only branch from master, or ii) make sure you agree with whoever you're branching from that this is a "branch", not a "patch queue", so that it isn't rebased under your feet.

We could also, say, prepend all patch queues with an underscore (its private).

OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but nobody are pulling from dagss/cython these days anyway.

What about:

OPTION C) The one who pushes things into the master knows master enough to see whether or not it makes sense to merge this, or if it was already in, he/she will simply comment into the pull request and close it manually

This doesn't make sense to me. Are you sure you read the scenario correctly? Dag Sverre

On 05/13/2011 09:05 AM, Ondrej Certik wrote:

On Thu, May 12, 2011 at 11:34 PM, Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no> wrote:

...

On 05/13/2011 12:36 AM, Ondrej Certik wrote:

...

Hi,

On Thu, May 5, 2011 at 12:52 PM, Dag Sverre Seljebotn <d.s.seljebotn@astro.uio.no> wrote:

...

There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times.

It doesn't matter, since the two openmp branches with the same changes merged OK, but we shouldn't make this a habit. For instance, the openMP commits also show up as part of vitja's pull request, which is confusing.

In Mercurial speak: The openmp branch was used like you would use a Mercurial "patch queue" in one case, and as a branch in another case. In git they are the same technically and you rely on conventions to make sure you don't treat a "queue" as a "branch".

OPTION A) Either i) only branch from master, or ii) make sure you agree with whoever you're branching from that this is a "branch", not a "patch queue", so that it isn't rebased under your feet.

We could also, say, prepend all patch queues with an underscore (its private).

OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but nobody are pulling from dagss/cython these days anyway.

What about:

OPTION C) The one who pushes things into the master knows master enough to see whether or not it makes sense to merge this, or if it was already in, he/she will simply comment into the pull request and close it manually

This doesn't make sense to me. Are you sure you read the scenario correctly?

You wrote:

" There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times. "

So somebody pushed in Marks' patches twice. My OPTION C) is that the one, who pushes patches in is responsible to make sure that they only get pushed in once.

That's what we do in sympy, we don't have any formal option A or B, but people with push access must prove that they are capable of using git, and not breaking (or messing up) things. Of course, everybody can make a mistake though.

It seems to be working just great, so I just wanted to share our experience. Let me know what doesn't make sense.

Ah ok. So in this case, the reviewer would have to request that the second pull request was fixed/rebased. I guess that is still the safety mechanism, but it's nice to also discuss how to not get into those situations. I'm not saying that there will be serious repercussions if one doesn't follow the rules, I was more talking guidelines for not getting into trouble without having to learn all of git. Note that a big part of this thread was to actually make sure everybody (in particular the core devs) knew about how Git rebasing works. The mistake was made in the first place because the reviewer assumed that "git will figure this out". Keep in mind that we just switched, and I think some core devs are still using hg-git, for instance. Dag Sverre