[KDE/Mac] Review Request 128272: support -iframework and -F header search path options

[Milian Wolff]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128272/#review97255
-----------------------------------------------------------


Fix it, then Ship it!




two minor things which you could fix and then commit this with the kdevplatform change (remember, first the platform change, then wait for the CI to settle (yes - you need to poll), then push this kdevelop change).


projectmanagers/qmake/qmakeprojectfile.cpp (line 261)
<https://git.reviewboard.kde.org/r/128272/#comment65661>

    use auto here to use an initializer_list instead of a heap-allocated QStringList



projectmanagers/qmake/qmakeprojectfile.cpp (line 262)
<https://git.reviewboard.kde.org/r/128272/#comment65662>

    static const, but again - heap allocated container of two elements just to check whether arg equals a string? I'd rather you rewrite this, first introduce
    
        const QLatin1String f("-F");
        const QLatin1String iframework("-iframework");
    
    then below use it like this:
    
        if (arg == f || arg == iframework) {
           storeArg = true;
           continue;
        }
        if (arg.startsWith(f)) {
            fwDirs << arg.mid(f.length());
        } else if (arg.startsWith(iframework)) {
            fwDirs << arg.mid(iframework.length());
        } else if (storeArg) {
            fwDirs << arg;
        }
        storeArg = false;


- Milian Wolff


On July 5, 2016, 6:41 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128272/
> -----------------------------------------------------------
> 
> (Updated July 5, 2016, 6:41 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> This is a draft implementation for parser support of the `-iframework dir` and `-F dir` compiler arguments. On OS X these are the framework equivalents of `-isystem` and `-I` respectively, telling the compiler and/or linker where to find framework bundles.
> 
> I started out making the new code available on OS X only but that introduces a lot of #ifdefs for probably little benefit. On the contrary, clang supports the arguments on Linux too, presumably because clang is a functional cross-compiler that can generate Darwin Mach-O object files on Linux too.
> 
> For the 1st approach I propose to parse the framework directories, adding the effective header directories of the individual frameworks as if they were added explicitly. The framework directories are also added to a new list in the result structure. I presume that this is a prerequisite for adding them to the (lib)clang arguments of the clang parser.
> 
> 
> Diffs
> -----
> 
>   languages/clang/clangparsejob.cpp 8375eb5 
>   languages/clang/duchain/clangparsingenvironment.h c689132 
>   languages/clang/duchain/clangparsingenvironment.cpp b515037 
>   languages/clang/duchain/parsesession.cpp aae0661 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h 7a5184f 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp 24e532a 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.h 6d0d210 
>   languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp ebceb4d 
>   languages/plugins/custom-definesandincludes/idefinesandincludesmanager.h 5da71f2 
>   projectmanagers/cmake/cmakeimportjsonjob.cpp f064647 
>   projectmanagers/cmake/cmakemanager.h 3096b7d 
>   projectmanagers/cmake/cmakemanager.cpp 5c15e2f 
>   projectmanagers/cmake/cmakeprojectdata.h 60e8773 
>   projectmanagers/custom-buildsystem/custombuildsystemplugin.h 372b283 
>   projectmanagers/custom-buildsystem/custombuildsystemplugin.cpp b04647e 
>   projectmanagers/custommake/custommakemanager.h 33c2997 
>   projectmanagers/custommake/custommakemanager.cpp e2ce943 
>   projectmanagers/custommake/makefileresolver/makefileresolver.h debe977 
>   projectmanagers/custommake/makefileresolver/makefileresolver.cpp 97973d4 
>   projectmanagers/custommake/makefileresolver/tests/test_custommake.h 3ad0f36 
>   projectmanagers/custommake/makefileresolver/tests/test_custommake.cpp 368e83e 
>   projectmanagers/qmake/qmakemanager.h e5e3266 
>   projectmanagers/qmake/qmakemanager.cpp 123b474 
>   projectmanagers/qmake/qmakeprojectfile.h 027800c 
>   projectmanagers/qmake/qmakeprojectfile.cpp 221eee9 
> 
> Diff: https://git.reviewboard.kde.org/r/128272/diff/
> 
> 
> Testing
> -------
> 
> the unittest works as expected on OS X.
> 
> 20160625: the patch works as expected on OS X. The `-iframework /opt/local/libexec/qt5/Library/Frameworks` argument added by cmake to each compiler invocation is detected and put to use; Qt header files are found in the frameworks without a wrapper Qt5 header directory (`/opt/local/include/qt5`) added to the header search path. Header files from the system SDKs are found too
> 
> 
> File Attachments
> ----------------
> 
> the real companion patch for kdevplatform
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/06/22/67189f62-ec2c-4797-a315-cafc44fbbb6d__patch-support-kdevp-frameworks.diff
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20160710/111ec16b/attachment-0001.html>