Integration update, or OH GOD WHAT DID YOU DO TO MY REPO

[Kyle Machulis]

So, I just brought the following requests into master

- Pull Request 30 - ros-pkg-git - Motor/LED functions for the new API, multiple cameras

- Pull Request 34 - stephenlovegrove - 10-bit camera access

- Pull Request 2 - jpgr87 - cmake updates

- Pull Request 31 - stephenlovegrove - C# updates

Now, everyone gather around, it's git story time:

You may notice that the pull requests don't exactly seem to be closing themselves out, and that you usually get a nice little note from me talking about what commit you actually came in at relative to the pull request commit. Basically, we're trying to avoid the problem outlined at

http://blog.xebia.com/2010/09/20/git-workflow/

Where you get a VERY nasty history and because nigh unto unusable when it comes to trying to do bisects and other handy things for finding bugs and issues. The rate we're receiving pull requests at right now means actually kicking back all requests until they're pulled up to the head of master also means we'd get very little if anything in quickly, because we'd have to bring one commit in then wait for everyone else to pull up to the head. 

I could queue-ize the pull requests and warn whoever is up next that they need to rebase before I can bring their stuff in, but, for the moment, it's faster for me to do it (and yes I realize how many bad things have come out of this phrase being said), and I am doing my very damnedest not to change any code here outside of small conflict resolves. There may be a few merge spots or fixes here or there, but otherwise it should come in pretty much clean, and if not I'll make a note of it and let the merge requester fix if needed. 

FOR THOSE OF YOU COMMITTING TO YOUR MASTER BRANCH: This is probably going to turn violent for you very quickly as you try to pull from our master back into yours. If you find yourself having problems, please let me know on this thread and we'll try to get a workflow together ASAP. Or, check out that link above and their recommended workflow. If you don't understand it, ask in this thread or the IRC channel.

FOR THOSE OF YOU THAT ALREADY KNOW HOW TO USE GIT, PLEASE SPEAK UP AND HELP OUT. Seriously, this is WAY better than having to do mass integration via SVN, but it's still tough for a lot of people who just want to get their code in, and I can only answer so many questions while also trying to integrate and work on the project myself. I would seriously appreciate any help anyone could pitch in on the support side.

If you see anything go in that's completely wrong or that I screwed up, or think this is a completely stupid way of doing integration, feel free to speak up here. I'm really just trying to make our history usable, make sure everyone gets their credit their due as commiters, and keep new stuff coming in as quick as possible without everyone having to learn git first. This is quickly turning into the largest project I've worked as integrator on though, so I'm learning here too, and am happy to take advice or criticism, either here or on the IRC channel.

Other than that, keep the changes comin' and I'll try to keep up!

Kyle

[Jason]

On 11/19/2010 02:27 AM, Kyle Machulis wrote:
> If you don't understand it, ask
> in this thread or the IRC channel.
>

grrr... Okay, so I'm trying to add a 'Signed-off-by' line to a commit I already pushed to my origin on github. (Pull request #24). Normally, I do this:

$ git remote update upstream

## rebase if necessary (new-api hasn't changed here, so not necessary)

$ git rebase -i commitish^

## make it reword the commit, add 'Signed-off-by'

$ git format-patch ...

$ git send-mail ...

However, in this situation (using github) I think it should go like this:

$ git rebase -i commitish^

## make it reword the commit

$ git push origin

## where 'origin' is my github remote.

Unfortunately, I get the following in response to my push:

To g...@github.com:jac299792458/libfreenect.git
! [rejected] local-new-api -> local-new-api (non-fast-forward)
error: failed to push some refs to 'g...@github.com:jac299792458/libfreenect.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes before pushing again. See the 'Note about
fast-forwards' section of 'git push --help' for details.

Anyone done a commit edit and push to github before?

thx,

Jason.

[Antonio Ospite]

On Fri, 19 Nov 2010 07:54:07 -0500
Jason <openk...@lakedaemon.net> wrote:

> On 11/19/2010 02:27 AM, Kyle Machulis wrote:
> > If you don't understand it, ask
> > in this thread or the IRC channel.
> >
>
> grrr... Okay, so I'm trying to add a 'Signed-off-by' line to a commit I
> already pushed to my origin on github. (Pull request #24).

[...]

> However, in this situation (using github) I think it should go like this:
>
> $ git rebase -i commitish^
>
> ## make it reword the commit
>
> $ git push origin
>
> ## where 'origin' is my github remote.
>
> Unfortunately, I get the following in response to my push:
>
> To g...@github.com:jac299792458/libfreenect.git
> ! [rejected] local-new-api -> local-new-api (non-fast-forward)
> error: failed to push some refs to 'g...@github.com:jac299792458/libfreenect.git'
> To prevent you from losing history, non-fast-forward updates were rejected
> Merge the remote changes before pushing again. See the 'Note about
> fast-forwards' section of 'git push --help' for details.
>
> Anyone done a commit edit and push to github before?
>

AFAIK rebase and push does not play well together because of how rebase
works:
http://stackoverflow.com/questions/559917/git-rebase-and-git-push-non-fast-forward-why-use
http://stackoverflow.com/questions/2219424/how-to-push-pull-git-rebase

I think the rule of thumb is: rebase only your own _local_ stuff.

If you want to rewrite the history and push an updated version of it to
the remote repository I guess you can do that with "git-filter-branch"
somehow. I don't know if that plays well with the pull request tho.

Regards,
Antonio

--
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Kelvie Wong]

Use the git push -f.

But again, listen to Antonio. It's generally bad practice to
force-push to a public repo. That said, we do have to sign off
commits before they get integrated.

--
Kelvie Wong

[Jason]

Kelvie Wong wrote:
> On Fri, Nov 19, 2010 at 4:54 AM, Jason <openk...@lakedaemon.net> wrote:
>> On 11/19/2010 02:27 AM, Kyle Machulis wrote:
>>> If you don't understand it, ask
>>> in this thread or the IRC channel.
>>>

>> Anyone done a commit edit and push to github before?
>>
>

> Use the git push -f.
>

Yep, a cup of coffee later I saw more clearly what I needed to do.

> But again, listen to Antonio. It's generally bad practice to
> force-push to a public repo.

I agree completely. The advantage of the lkml method over the github method is the ability to take critiques and edit commits _before_ merging into a public repo.

It looks like all the branches were merged into master, so I'll have to rebase against upstream/master anyway...

> That said, we do have to sign off
> commits before they get integrated.
>

Yep, usually format-patch/send-mail handled that for me. I'm still new to the collaborative aspects of git.

Thanks for the input.

Jason.

[Joshua Blake]

One of the reasons we chose to require sign-off in the commit comments is that this makes it very clear which code is from which contributor and this record travels with the repository. This way we don't depend upon a website or a mailing list which may have it's own issues. We're taking advantage of the distributed nature of DVCS/Git and everyone has a record.

There will (as always) be a learning curve as contributors get to know the project and policies. The biggest challenge is learning to use git like git instead of subversion, which would be a problem anyway. Contributors can be highly efficient in the long run since all they have to do is say "git commit -s " and it adds the sign-off.

 

We need to do better at getting new contributors up to speed with the project and with git in general. Anyone who knows git can help by documenting how to get started on the wiki. (See also https://github.com/OpenKinect/libfreenect/issues#issue/6)

 

Thanks,

Josh

---
Joshua Blake

Microsoft Surface MVP

(cell) 703-946-7176 

Twitter: http://twitter.com/joshblake
Blog: http://nui.joshland.org
Multitouch on Windows book: http://manning.com/blake

[Tully Foote]

Jason,
I had the same problem.  I worked around it by creating a new branch, call it 'signing', branched before my unsigned changes.  Then I merged with --squash from the 'unsigned' branch into the 'signing' branch.  When you commit there you can sign it at that point.  And then you can merge signed back into the 'unsigned' branch.  This worked for my repo. 

I'm not a git expert but this seemed to work for me.  If someone considers this improper please speak up.  But as far as I can tell it's a relatively painless way to do this after having pushed to a publicly available repo. 

Tully

[Kelvie Wong]

On Fri, Nov 19, 2010 at 10:22 AM, Tully Foote <tully...@gmail.com> wrote:
> Jason,
> I had the same problem.  I worked around it by creating a new branch, call
> it 'signing', branched before my unsigned changes.  Then I merged with
> --squash from the 'unsigned' branch into the 'signing' branch.  When you
> commit there you can sign it at that point.  And then you can merge signed
> back into the 'unsigned' branch.  This worked for my repo.
>
> I'm not a git expert but this seemed to work for me.  If someone considers
> this improper please speak up.  But as far as I can tell it's a relatively
> painless way to do this after having pushed to a publicly available repo.
>
> Tully
>

While this is a solution, you lose the granularity of your commits. I
assume each commit was a logical separation of work and history, and
by squashing them, you make it very difficult to separate them again.

You can no longer revert specific commits, nor bisect in-between them,
or even know which commit message refers to which change easily.

It does work around the sign-off problem though.

--
Kelvie Wong

[Jason]

This solution seems to be the best for this scenario:

git checkout -b [new-branch] [branch-to-change-commit-on]
git commit --amend
git push [remote] [new-branch]
git push --delete [remote] [branch-to-change-commit-on]

Then redo the pull request from the new branch. Thanks to Kyle for suggesting this to me...

hth,

Jason.

[Joshua Blake]

While this is a solution, you lose the granularity of your commits.  I
assume each commit was a logical separation of work and history, and
by squashing them, you make it very difficult to separate them again.

You can no longer revert specific commits, nor bisect in-between them,
or even know which commit message refers to which change easily.

It does work around the sign-off problem though.

--
Kelvie Wong

 

 

 

Kelvie, squashing isn't necessary, but if someone has a tendancy to commit partial untested work or in-progress features (which is common in non-DVCS) then it would be appropriate to squash the commits. If the commits do have logical separation, then you could just edit the commit message with the rebase -i and be good. Logically separate commits should be submitted in separate pull requests anyway.

 

Josh

[Joshua Blake]

This solution seems to be the best for this scenario:

git checkout -b [new-branch] [branch-to-change-commit-on]
git commit --amend
git push [remote] [new-branch]
git push --delete [remote] [branch-to-change-commit-on]

Then redo the pull request from the new branch.  Thanks to Kyle for suggesting this to me...

hth,

Jason.

If someone can put together a script that includes signing and pulling/rebasing to the HEAD of master, that would be a huge help and we'd point new contributors to it.

[Tully Foote]

If you want all the commits you can always merge each commit with --squash individually.  The --squash just tells git to keep the changes in the working copy w/o automatically committing.  Thus giving you the chance to sign it when you recommit. 

Tully

[Jason]

Joshua Blake wrote:
>> This solution seems to be the best for this scenario:
>>
>> git checkout -b [new-branch] [branch-to-change-commit-on]
>> git commit --amend
>> git push [remote] [new-branch]
>> git push --delete [remote] [branch-to-change-commit-on]
>>
>> Then redo the pull request from the new branch. Thanks to Kyle for
>> suggesting this to me...
>>
>

> If someone can put together a script that includes signing and
> pulling/rebasing to the HEAD of master, that would be a huge help and we'd
> point new contributors to it.
>

I believe Kyle is working on a write up right now...

thx,

Jason.

[Jason]

Jason wrote:
> On 11/19/2010 02:27 AM, Kyle Machulis wrote:
>> If you don't understand it, ask in this thread or the IRC channel.
>>
>
> grrr... Okay, so I'm trying to add a 'Signed-off-by' line to a commit
> I already pushed to my origin on github. (Pull request #24).

To facilitate things, one could do the following:

$ cd /path/to/libfreenect.git
$ git config --add format.signoff true

NOTE: understand what you are doing when you enable this.
1.) _All_ future commits to this repo are under the license of the
code already in the repo.
2.) You have the authority to place your work under that license.
3.) You're read and fully understand section 12 of
Documentation/SubmittingPatches in the linux kernel [1].

hth,

Jason.

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=689e2371095cc5dfea9927120009341f369159aa;hb=HEAD

[Kelvie Wong]

On Fri, Nov 19, 2010 at 10:38 AM, Joshua Blake <josh...@gmail.com> wrote:
> Kelvie, squashing isn't necessary, but if someone has a tendancy to commit
> partial untested work or in-progress features (which is common in non-DVCS)
> then it would be appropriate to squash the commits. If the commits do have
> logical separation, then you could just edit the commit message with the
> rebase -i and be good. Logically separate commits should be submitted in
> separate pull requests anyway.
>
> Josh

Of course, I hadn't suggested otherwise :)

I just think that using squash as a replacement for merge is a bad thing.

The people over on the Mercurial side disagree with our sentiments,
though; they employ the "take all commits, warts and all" philosphy,
or so I've heard.

--
Kelvie Wong

[Kelvie Wong]

On Fri, Nov 19, 2010 at 10:41 AM, Joshua Blake <josh...@gmail.com> wrote:
>
>> This solution seems to be the best for this scenario:
>>
>> git checkout -b [new-branch] [branch-to-change-commit-on]
>> git commit --amend
>> git push [remote] [new-branch]
>> git push --delete [remote] [branch-to-change-commit-on]
>>
>> Then redo the pull request from the new branch.  Thanks to Kyle for
>> suggesting this to me...
>>
>> hth,
>>
>> Jason.
>

I added a section on the wiki that helps with this:

http://openkinect.org/wiki/Contributing_Code#Forgetting_to_sign_off_after_committing

If anyone wants to take a look and/or add to it.

--
Kelvie Wong