Merging the android plugin to Qt Creator

Fri Jul 22 16:38:40 CEST 2011

WOW !
I'll provide feedback soon.
Thanks !

>________________________________
>From: Daniel Teske <daniel.teske at nokia.com>
>To: "necessitas-devel at kde.org" <necessitas-devel at kde.org>
>Sent: Friday, July 22, 2011 5:08 PM
>Subject: Merging the android plugin to Qt Creator
>
>Hi, 
>
>BogDan recently opened a merge request for the creator changes. So I had a not 
>very through look through it to figure out what needs to be done before we 
>could accept the MR.
>
>There's general agreement from the Chief Maintainer that a Android plugin fits 
>into Creator. I agree. 
>
>So, my initial issues list, which need to be fixed before we can consider 
>merging the code. I concentrated on those parts that are outside of the 
>android plugin. Don't worry about the lengh of this list, I'll help you 
>getting it sorted out. Also most of it is for Ryan. Also feel free to come to 
>the #qt-creator irc channel to discuss specifics.
>
>a) Contributor Agreement
>Everyone that has committed to the qt creator android plugin needs to be added 
>to our internal list of people that have agreed to the contributors agreement, 
>which means everyone needs to do one MR for qt-creator or qt. 
>Everyone being:
>BogDan Vatra (already done)
>mingw-android, Ray Donnelly, Thomas Zander, Marius Bugge Monsen and Espen 
>Riskedal
>
>b) License
>Some files currently have no license header at all
>Most of the other files do have the following header:
>/*
>I BogDan Vatra < bog_dan_ro at yahoo.com >, the copyright holder of this work,
>hereby release it into the public domain. This applies worldwide.
>
>In case this is not legally possible, I grant any entity the right to use
>this work for any purpose, without any conditions, unless such conditions
>are required by law.
>*/
>Considering that some files do have multiple authors, that needs to be fixed.
>You'll need to decide whether you want to have the plugin under the above 
>license, then all authors must agree to that, or if you want to go with the 
>standard LGPL. A situation where some parts are by BogDan are in the public 
>domain and other author's work is not is theoretically possible, but really 
>not a good option.
>
>c) Target branch
>2.3 is already feature frozen, so the target branch should be master. 
>Currently master merges cleanly into the MR.
>
>d) Changes I'll integrate to master
>The commit 43240a9 MinGW compilation fixes for private headers 
>Exporting Qt4ProjectManager::QMakeStep and Qt4ProjectManager::MakeStep
>
>e) Changes to the android plugin
>I have attached a few commits for stuff that was pretty easy to fix:
>
>0001-Remove-wrong-merge-resolution.patch
>The code moved in between the merges and is already fixed at the new place.
>
>0002-Remove-accidentally-added-.commit.template.patch
>The commit message didn't mention the .commit.tempalte, I assumed it was 
>commited in error. I could also add the .commit.template to master if you want 
>that.
>
>0003-Remove-unecessary-whitespace-changes.patch
>
>0004-Unecessary-change-to-import-code.patch
>The code there is only used for importing .user files that are pre 2.0, as such 
>the code change should be unecessary.
>
>0005-Simplfy-template-code.patch
>0006-Reformat-deployment.pri-to-be-more-readeable.patch
>0007-Bump-stub-version-of-template.patch
>Those 3 patches change the template to what I think is better. Also see 
>comments below.
>
>0008-Remove-settign-of-ANDROID_NDK_HOST-from-necessitas.b.patch
>See commit message.
>
>Feel free to submit them to the applicable branch.
>
>f) mingw/*
>Those files need to be removed from the MR for hopefully obvious reasons.
>
>g) macdeployqt/*
>============= TODO look at the patches
>There is unfourtanetly no maintainer for macdeployqt, but the patches should 
>be made against the macdeployqt in qt. 
>
>h) Changes in src/tools/qpatch
>After a quick discussion at the office we removed src/tools/qpatch completely 
>from master since it is neither used by the stand alone creator packages nor 
>by the sdk and is known to be buggy.
>I'd recommend instead of patching and using qpatch to use the facilities in 
>the QtSDK found in installer-framework/installerbuilder/libinstaller/qtpatch 
>and installerbuilder/libinstaller/qtpatchoperation.cpp and then backing out 
>the changes to qpatch
>
>i) build.sh
>We don't want that as part of the creator repository. As far as we are 
>concerned building creator should be done in a normal cmd shell on windows and 
>not in a msys shell.
>
>j) bin/necessitas and bin/necessitas.bat
>The setting of ANDROID_NDK_HOST seemed to be a left over from before setting 
>it in androidtoolchain.cpp, so I removed it, see attached patch.
>We used to have a similar LD_LIBRARY_PATH setting script on linux but after 
>some internal discussions we removed it. On windows
>
>k) Settings Path and Renaming to Necessitas
>We think if we merge the android plugin into creator, then there is no point 
>in renaming Qt Creator and no point in using a different settings path. The sdk 
>team also wants to produce "one" sdk that can install components from different 
>providers.
>If you do want to have a different name/settings path, then there are quite a 
>few places which need to be fixed and the default should be "Qt Creator" not 
>"Necessitas Qt Creator"
>
>l) qtcreator.rc
>I don't understand the need to change the file and the commit mesage doesn't 
>explain it either: "Fix errors in qtcreator.rc by saving it from Visual 
>Studio"
>
>We are compiling qt creator with visual studio and have no problems at all 
>with qtcreator.rc, so could you explain why you need this change?
>
>m) qmakeStep::moreArguments()
>Adding the "-win32" to the qmake command line. As far as I understand you want 
>to have qmake run in HOST_WIN_MODE on windows. But that is the default if 
>qmake is compiled for windows.
>
>n) lowerCasing in qt4buildconfiguration.cpp
>#ifdefing out the lowercasing of specs
>"Don't do the toLower stuff on lots of paths when Q_OS_WIN and __MINGW32__ is 
>defined. This is an aesthetic thing. I don't like showing all pahs in lower 
>case."
>The paths are lowercased to compare them against each other, without the 
>lowercasing the comparision will fail for paths that are the same. Thus since 
>the change is just for aesthetics, I'd back it out again.
>
>o) profileeavluator.cpp
>The profileevaluator in creator should behave the same as the one in qmake.
>And the evaluator in qmake lowercases the first device letter (even if that's 
>not correct) and uses "\" on windows. I guess the drive letter change is just 
>for asthetics? What does the path separator break? Does that have something to 
>do with msys?
>
>p) debugger plugin
>The debbuger plugin has quite a few changes, some of them seem to be hacks.
>Also I'm not that well versed in the debugger plugins code, though I discussed 
>a earlier diff with the debugger maintainer (Andre).
>I'm somewhat confussed whether you are using the python dumpers, or not.
>So, the changes:
>Adding "arm-linux-androideabi" to gnuTargets: fine
>
>soLibSearchPath: in general looks fine, andre mentioned some details he wanted 
>to see fixed
>
>Always offering all toolchains in DebuggerToolChainComboBox::init
>The commit message already classifies that as a hack, I just don't understand 
>why it was needed. The check for hostAbi.os() == abi.os() should be true for 
>msys toolchains too. Or is .os() broken for msys toolchains?
>
>Disabling setting PYTHONPATH. Why is that needed? Note that the code that is 
>disabled was rewritten a lot since you disabled it.
>
>q) abi.h abi.cpp
>The change looks good, but can't be merged without the rest of the android 
>plugin.
>
>r) templates/qtquickapp/*
>The changes looked almost okay. I didn't understand the setBaseUrl() change, 
>nor can I see how that makes sense. I reformatted some of the code for better 
>readability and added a bumping of the template version. (Projects with prior 
>template versions are automatically updated then.) See the attached patches.
>
>s) main.cpp #ifdef
>There is a change in main.cpp changing the plugin search paths to be only 
>searched on the "matching" platform. The commit message is:
>"Mac OS X: Fix for case sensitive filesystems (had mixed plugins/PlugIns, 
>always uses PlugIns now)"
>Ray can you explain what that commit fixes? That sounds like something I could 
>cherry-pick to master.
>
>t) main.cpp myMessageOutput
>The main motivation for that seems to be:
>"qInstallMsgHandler so that asserts are not fatal"
>We like Q_ASSERTS to be fatal, we do have a separate macro QTC_ASSERT for 
>cases where a non fatal handling is possible/desired.
>
>u) Settings SystemScope to UserScope
>Quite a few settings (if not all) are using the UserScope instead of the 
>SystemScope. Why is that needed?
>
>I might have missed a few changes here and there.
>
>I'm looking forward to working on that. :)
>
>daniel
>
>-- 
>Daniel Teske
>Software Engineer
>Nokia, Qt Development Frameworks
>
>Nokia gate5 GmbH
>Firmensitz: Invalidenstr. 117, 10115 Berlin, Germany
>Registergericht: Amtsgericht Charlottenburg, Berlin: HRB 106443 B
>Umsatzsteueridentifikationsnummer: DE 812 845 193
>Geschäftsführer: Dr. Michael Halbherr, Karim Tähtivuori
>
>_______________________________________________
>Necessitas-devel mailing list
>Necessitas-devel at kde.org
>https://mail.kde.org/mailman/listinfo/necessitas-devel
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/necessitas-devel/attachments/20110722/af058ff2/attachment-0001.htm 