search

mloskot / qt-creator-plugin-boostbuild

Boost.Build Project Manager Plugin for Qt Creator
GNU Lesser General Public License v2.1
14 stars 8 forks source link

Preparing contribution of plug-in to Qt Creator #13

Closed mloskot closed 9 years ago

mloskot commented 9 years ago

As it is explained in the README/FAQ, originally I wasn't going to contribute the plug-in to Qt Creator, but @jhunold suggested (#8 and #12) it would be beneficial for the plug-in if it is maintained as part of Qt Creator.

Let's discuss what it takes, tasks and steps needed, to submit the plug-in to Qt Creator. I have also announced this issue on Qt Creator ML: Preparing to contribute Boost.Build Plugin to Qt Creator

AFAIU, according to the guidelines I've found, here is the procedure and related issues:

  1. Basically, follow Qt Contribution Guidelines
  2. Build Qt Creator using the repository from Gerrit, then build the plug-in against such build.
  3. Setting up Gerrit
  4. Follow the Qt Creator Plug-in Contribution Guide, where I see a few issues:
    • What does merge request on gitorious.org mean? Andre Poenitz from Qt clarified that this is completely outdated nonsense. He'll try to correct it. All Qt and Qt Creator code submissions need go through gerrit (codereview.qt-project.org) for quite some time.
    • Presumably, coding standard conformance is deal breaker, becasue I did neither follow Qt Coding Qt Creator Coding Rules. If I'm asked to restructure and reformat the plug-in codebase, it's going to be a time issue.
    • There is tangible commitment by the submitter to help maintain the code, as the Qt Creator Plug-in Contribution Guide suggests, but I can't make hard promises. @jhunold would you be willing to join me?

The points 2 and 3 are not a mystery, so I will proceed, but it is not clear to me how to perform the actual code submission to Qt Creator for review. Would you be available to help and clarify, @jhunold?

jhunold commented 9 years ago

Hi Mateusz, of course I can help out with both maintenance and first getting the plugin into creator. So we can share the maintenance load. On point 3: I think you only have to put the module inside Qt-Creators plugin structure and submit a Gerrit review request containing the added files. That would be:

  1. Move the code to src/plugins/boostbuild
  2. git add -A src/plugins/boostbuild
  3. push to Gerrit
  4. wait for comments
  5. fix issues
  6. goto 3 until accepted.

We can probably clone the (in-)official mirror at https://github.com/qtproject/qt-creator and prepare everything. This includes integration into QtCreator's build system as a first-class citizen, renaming the .hpp files to .h and fixing the coding style. The latter can be enforced/assistet by QtCreator itself, I hope. We can then prepare a squashed commit for submission into Gerrit. This is how I would tacklet the problem with my (limited) experience with Gerrit.

mloskot commented 9 years ago

Jürgen, the steps you outlined look sensible. I didn't know we can clone from the mirror at GitHub. I double checked and it actually is mentioned as one of possible sources indeed: http://qt-project.org/wiki/Setting-up-Gerrit#bf8aa9648651d4dab8aafb6408a51427 I'm gone for the weekend, so I'll kickstart the process next week.

Thanks for offering your help.

jhunold commented 9 years ago

Hi Mateusz, any chance you can at least fork QtCreator and import your code to "plugins/boostbuildprojectmanager/" ? Those interestet could then start from there and clenaup the code.

mloskot commented 9 years ago

@jhunold I've cleaned my desk enough to move forward with the submission. I've forked Qt Creator in https://github.com/mloskot/qt-creator, but I've also learned about some caveats of development outside the Gerrit, see section Feature development outside of Gerrit. They strongly recommend Gerrit. What we do?

jhunold commented 9 years ago

I've already signed the CLA so this should not be a problem. Let's see what happens with our ML discussion and then decide. I'm short on free time this week anyway. I hope to finish #12 next and then proceed. Some naming ideas:

mloskot commented 9 years ago

Great. Yes, let's wait for some conclusive response (there is this new thread QtCreator plugin contribution), where I also asked about the GitHub-based workflow and which branch to branch off. Qt Wiki is being still clarified.

  • the plugin directory should be name "boostbuildprojectmanager" I guess.

Agreed. (FYI, I'd prefer to keep this GitHub repository named as it is now, qt-creator-plugin-boostbuild - it makes it clear what is the project about).

  • we should name the files b2foo.h and b2foo.cpp if this is allowed.

Agreed.

mloskot commented 9 years ago

Concrete pros/cons suggestion arrived in that thread, that a wip (work in progress) branch on Qt Gerrit is the better choice.

I'm inclined to go for it as I will have to set up the damn Gerrit anyway :) What's your final call, @jhunold?

jhunold commented 9 years ago

Decision time again. I'm all for the wip branch approach. This really allows us to work with maximal feedback toward a working longtime solution. I simply did not think of this option for a 3rdparty addin. Can you please ask for a "wip/boostbuildprojectmanager" branch on the ML?

mloskot commented 9 years ago

:+1: Following the long-lived branches guidelines I've sent request to the Gerrit Administrators (CC'ed to you too).

mloskot commented 9 years ago

We've got the branch wip/boostbuildprojectmanager created.

Last night, I've finished setting up Gerrit in fresh Qt/Qt Creator clones. Still need to learn more about the Gerrit workflow. Posted some additional questions in the Preparing to contribute Boost.Build Plugin to Qt Creator thread.

mloskot commented 9 years ago

Everything that wasn't clear to me has been clarified now, see http://lists.qt-project.org/pipermail/qt-creator/2015-April/004593.html. IMO, we're ready to do initial import of the current sources in Gerrit and start cleaning up.

@jhunold Are you OK if I do the import?

jhunold commented 9 years ago

I've tracked the discussion on the QtCreator ML. You can start working in gerrit if you like. I'm just very short on time this week, thing should be getting better next week.

mloskot commented 9 years ago

No worries. I was only asking for confirmation. Turns out, I may not be able to touch the import until after the weekend.

mloskot commented 9 years ago

Finally, I've pushed initial Change to the Gerrit.

Here is the direct link generated by git gpush command: https://codereview.qt-project.org/#/c/111340/ It is also accessible from the list of my open Changes. Let's get the party started :)

The initial TODO items, also outlined in the commit message, are:

Before I rename the files and do any further changes, I'll wait a day or two in case Qt Creator devs would have any initial feedback for us.

mloskot commented 9 years ago

Meanwhile, I pushed two changes to Gerrit:

  1. Added patch from PR #12: https://codereview.qt-project.org/111415
  2. Enabled building boostbuildprojectmanager as part of Qt Creator build: https://codereview.qt-project.org/111416
  3. Renamed the source files as we discussed above: https://codereview.qt-project.org/111419

I've successfully built wip/boostbuildprojectmanager branch and loaded the plugin.

jhunold commented 9 years ago

Great news. I'll switch to the wip-Branch and Gerrit the next days. Don't expect anything before next week due to deadlines.

mloskot commented 9 years ago

:+1:

mloskot commented 9 years ago

Apparently, I messed up my local wip/boostbuildprojectmanager branch because I did:

$ git push gerrit HEAD:refs/for/wip/boostbuildprojectmanager
Enter passphrase for key '/home/mloskot/.ssh/id_rsa.codereview.qt-project':
Counting objects: 113, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (112/112), done.
Writing objects: 100% (113/113), 67.31 KiB | 0 bytes/s, done.
Total 113 (delta 70), reused 0 (delta 0)
remote: Resolving deltas: 100% (70/70)
remote: Processing changes: refs: 1, done    
remote: (W) eda6273: no files changed, was rebased
remote: (W) 8790843: no files changed, was rebased
remote: (W) bdea040: no files changed, was rebased
To ssh://codereview.qt-project.org/qt-creator/qt-creator
 ! [remote rejected] HEAD -> refs/for/wip/boostbuildprojectmanager (no changes made)

    error: failed to push some refs to 'ssh://codereview.qt-project.org/qt-creator/qt-creator'

Perhaps, Qt Creator devs will be able to help me to clean my mess.

<mloskot> Folks, I've messed up my gerrit-based working copy and now git gpush is refusing my changes with "no change made". I suppose, it's because I created new wip/ branch locally, then cherry picked commits from original one.
<mloskot> Anyone has an idea how to make gerrit happy again?
<andre_> mloskot: where is you branch on gerrit
<mloskot> andre_, wip/boostbuildprojectmanager
<mloskot> I'm now looking at checking out clean gerrit/wip/boostbuildprojectmanager, then pulling the last change, then applying two commits I tried, then pushing
<mloskot> andre_, all the problem started when I rebased my wip/boostbuildprojectmanager on top of master, pulling 250+ commits. That obviously made my branch non-pushable to gerrit :(
<andre_> you need to resolve conflicts before you can push again
<mloskot> andre_, the thing is, I cherry picked and there were no conflicts.
<mloskot> andre_, what I did was this:
<mloskot> git checkout -b wip/boostbuildprojectmanager  gerrit/wip/boostbuildprojectmanager
<mloskot> for each commit in changes (#/c/111340/, #/c/111415/, #/c/111416/, #/c/111419/), git cherry-pick 
<mloskot> Then, track my newly re-created wip/boostbuildprojectmanager
<mloskot> Then, try to git gpush
<mloskot> what obviously failed
<mloskot> http://pastebin.com/W8RvttvQ
<mloskot> If a co-developer wants to pull wip/boostbuildprojectmanager with all my submitted changes, so he can start at the point of last update, what is the procedure?
<mloskot> What is the equivalent of git pull to get all gerrit Changes together by someone to pick the work where it was left and continue with further updates, submitting new Chnages
<mloskot> I could use that procedure to fix my branch locally
<nikolai_> mloskot: you can choose to 'checkout' instead of 'cherry-pick' in gerrit, then you will get all the dependencies, too
<mloskot> nikolai_, yes, I realise I made mistake
<andre_> mloskot: gerrit/wip/boostbuildprojectmanager seems to be part of the master branch?
<mloskot> likely
<mloskot> yes
<mloskot> andre_, I emailed the admins to create it 
...

Regardless, these problems and undocumented steps for multi-developer collaboration on wip/ branches made me think it would be much easier to work in Qt Creator fork on GitHub, then push the plugin contribution as a single change for review.

The gerrit hassle, the Bible-size yet still incomplete, IMHO, Qt wiki on how to use Gerrit and not to mess things up is slowly sucking all my motivation to move on with this contribution.

This is a plugin, biggish contribution, so perhaps Gerrit PITA is justified, but I can't think of anyone new to Gerrit to be able to swallow it easily just to make contribution of a small patch. It's not sane, it's just not right.

Good spirits bless the idea of a Pull Request.

mloskot commented 9 years ago

Tonight, I'll be testing the following suggested solution:

<andre_> hi. your patches seem to cleanly apply on current master, it's just the wip/ branch that is a bit out ouf date
<andre_> should I try to update this (and note that I usually don't play around with branches on gerrit, so this might go wrong...)
<mloskot> there are two more commits  I was trying to push, which were refused
<mloskot> ideally if I could rename my current local wip/boostbuildprojectmanager branch, then
<mloskot> git co -b wip/boostbuildprojectmanager gerrit/wip/boostbuildprojectmanager
<mloskot> then somehow replay all the 4 changes I submitted to gerrit
<andre_> I'd try (from a clean master)  "git push gerrit master:wip/boostbuildprojectmanager"
<mloskot> then, I will be able to apply patch with my latest commits
<andre_> then git fetch ssh://codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/16/111416/1 && git checkout FETCH_HEAD
<andre_> then clean conflicts, then  "git push gerrit master:wip/boostbuildprojectmanager" again.
<mloskot> I see.
mloskot commented 9 years ago

No luck so far:

mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (master) 
$ git push gerrit master:wip/boostbuildprojectmanager
Enter passphrase for key '/home/mloskot/.ssh/id_rsa.codereview.qt-project': 
Total 0 (delta 0), reused 0 (delta 0)
remote: Branch refs/heads/wip/boostbuildprojectmanager:
remote: You are not allowed to perform this operation.
remote: To push into this reference you need 'Push' rights.
remote: User: mloskot
remote: Please read the documentation and contact an administrator
remote: if you feel the configuration is incorrect
remote: Processing changes: refs: 1, done    
To ssh://codereview.qt-project.org/qt-creator/qt-creator
 ! [remote rejected] master -> wip/boostbuildprojectmanager (prohibited by Gerrit)
error: failed to push some refs to 'ssh://codereview.qt-project.org/qt-creator/qt-creator'

mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (master) 
$ git fetch ssh://codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/16/111416/1 && git checkout FETCH_HEAD
Enter passphrase for key '/home/mloskot/.ssh/id_rsa.codereview.qt-project': 
From ssh://codereview.qt-project.org:29418/qt-creator/qt-creator
 * branch            refs/changes/16/111416/1 -> FETCH_HEAD
Note: checking out 'FETCH_HEAD'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at ca68d06... Add boostbuildprojectmanager plugin to plugins.pro.

mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (ca68d06...) 
$ git push gerrit master:wip/boostbuildprojectmanager                                                                      
Enter passphrase for key '/home/mloskot/.ssh/id_rsa.codereview.qt-project': 
Total 0 (delta 0), reused 0 (delta 0)
remote: Branch refs/heads/wip/boostbuildprojectmanager:
remote: You are not allowed to perform this operation.
remote: To push into this reference you need 'Push' rights.
remote: User: mloskot
remote: Please read the documentation and contact an administrator
remote: if you feel the configuration is incorrect
remote: Processing changes: refs: 1, done    
To ssh://codereview.qt-project.org/qt-creator/qt-creator
 ! [remote rejected] master -> wip/boostbuildprojectmanager (prohibited by Gerrit)
error: failed to push some refs to 'ssh://codereview.qt-project.org/qt-creator/qt-creator'

mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (ca68d06...)
mloskot commented 9 years ago

I've managed to fix the problem but it required Change(s) re-submission. Basically, I did:

git co -b wip/boostbuildprojectmanager-temp gerrit/wip/boostbuildprojectmanager
git pull ssh://mloskot@codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/40/111340/1
git pull ssh://mloskot@codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/15/111415/1
git pull ssh://mloskot@codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/16/111416/1
git pull ssh://mloskot@codereview.qt-project.org:29418/qt-creator/qt-creator refs/changes/19/111419/1
git st
On branch wip/boostbuildprojectmanager-temp
Your branch is ahead of 'gerrit/wip/boostbuildprojectmanager' by 9 commits.
  (use "git push" to publish your local commits)
nothing to commit, working directory clean
git co -b wip/boostbuildprojectmanager gerrit/wip/boostbuildprojectmanager
git merge --squash wip/boostbuildprojectmanager2
git commit
mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (wip/boostbuildprojectmanager) 
$ git gpush                                        
Pushing HEAD for wip/boostbuildprojectmanager on gerrit ...
Enter passphrase for key '/home/mloskot/.ssh/id_rsa.codereview.qt-project': 
Counting objects: 47, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (46/46), done.
Writing objects: 100% (47/47), 54.85 KiB | 0 bytes/s, done.
Total 47 (delta 14), reused 0 (delta 0)
remote: Resolving deltas: 100% (14/14)
remote: Processing changes: new: 1, refs: 1, done    
remote: 
remote: New Changes:
remote:   https://codereview.qt-project.org/111798
remote: 
To ssh://codereview.qt-project.org/qt-creator/qt-creator
 * [new branch]      HEAD -> refs/for/wip/boostbuildprojectmanager
mloskot at vb-arch64 in ~/dev/qt/qt-creator.git (wip/boostbuildprojectmanager) 
$ git st
On branch wip/boostbuildprojectmanager
Your branch is ahead of 'gerrit/wip/boostbuildprojectmanager' by 1 commit.
  (use "git push" to publish your local commits)
nothing to commit, working directory clean

Looks, I'm clean & happy to continue.

@jhunold if you will be going to pick the latest state of the preparation work, I have no idea how to pull all the changes from Gerrit easily, so you can edit the code and git gpush updates straight away.

In case anything goes wrong on Gerrit again, I've backed up the curent state in Qt Creator's fork on GitHub, in wip/boostbuildprojectmanager branch.

jhunold commented 9 years ago

Glad to hear that you have solved your problems. I'm still swamped with work but will get the latest changes from gerrit today. Backup on GitHub is a good idea. I think that you/we should simply request/gpush a merge from master to our wip-branch. This is what all other repositories do all day.

mloskot commented 9 years ago

@jhunold no worries, no rush. I'm just 'blogging' about my adventures :)

Good idea, I'll try to request the merge from the master.

mloskot commented 9 years ago

@jhunold Two issues to discuss:

Leena Miettinen posted two comments to the change https://codereview.qt-project.org/#/c/111798/

I've just addressed those comments by updating the Change with new Patch Set, also as a test if Gerrit works for me. I followed the guide https://wiki.qt.io/Gerrit_Introduction#Updating_a_Contribution_With_New_Code Namely this rule:

If the original commit(s) are still available, it is possible to amend them right away.

So, I simply applied modification to .cpp files, then git ci add and git ci --amend slightly changing the commit message, but preserving the Change-Id. Finally, git gpush what created Patch Set 2. I guess, if you will be trying to pull updates, you need to get this Patch Set 2.

Updating an existing Change (with new Patch Set) works for me, but I'm not sure

One of Leena's comment I addressed, was about use of --build-dir option in UI strings. I explained my motivation behind that and I've chnaged it to build directory, but I'm not sure about it. The rationale behind using explicit b2 command line options was to make users understand how the plugin works - it's a wrapper on top of b2 anyway. What's your opinion?

jhunold commented 9 years ago

I'm fine with "build directory" for now. Rationale: Most users don't want to know the gory details of the underlying build system. Even one as fine as Boost.Build :smirk: I'd rather put a detailed description into the docs, help system or a tooltip. Not sure what the QtCreator guidelines recommend.

mloskot commented 9 years ago

I admit, I assumed users are somewhat conversant with details of b2 :) Later comment from Leena includes similar suggestion, to put details into docs. Let's stick to docs.

mloskot commented 9 years ago

I'm shutting the contribution process down (http://lists.qt-project.org/pipermail/qt-creator/2015-June/004739.html) and I'm sticking to the original idea of keeping the plugin out of the Qt Creator source tree.

The plugin source code is open, so anyone willing to take the hassle over is free to do so.

Thanks to all who have provided help and support to the code and process so far.