Details
- Reviewers
kossebau - Group Reviewers
KDevelop - Commits
- R32:bbcf0cfa29c6: Use "gmake" command in custommake plugin on FreeBSD. Fixes unit test.
Diff Detail
- Repository
- R32 KDevelop
- Branch
- custommake
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 1678 Build 1696: arc lint + arc unit
Yay for more unit test fixes :)
Though... why does this change fix the unit test exactly? Without knowing more details, I am surprised that one cannot use the normal "make". "gmake" does not occur elsewhere as string, so why is this needed here, and why not elsewhere as well (not sure if there are other direct calls of the "make" tool).
Does the call rely on some GNU make features? If so, that should be added at least as comment in the code, so it is clear why plain "make" does not work.
Also, is "gmake" part of the normal FreeBSD system? Or would we need to check first this exists in the runtime?
plugins/custommake/makefileresolver/makefileresolver.cpp | ||
---|---|---|
98 | Type of make could be QLatin1String as well, given below in the concatenation QStringBuilder happily deals with QLatin1Strings, saves us one conversion/malloc at runtime. | |
100 | The reassignment looks strange, IMHO would be better to do #ifdef Q_OS_FREEBSD // on FreeBSD gmake is needed because foobar const QLatin1String makeExe("gmake"); #else const QLatin1String makeExec("make"); #endif |
The make coming with FreeBSD is BSD one, it doesn't support some GNU extensions.
"gmake" does not occur elsewhere as string, so why is this needed here, and why not elsewhere as well (not sure if there are other direct calls of the "make" tool).
Haven't found other "make" uses. And it fixed the test, after all.
Does the call rely on some GNU make features? If so, that should be added at least as comment in the code, so it is clear why plain "make" does not work.
MakeFileResolver::resolveIncludePathInternal calls make with --no-print-directory flag, which isn't supported by BSD make.
Also, is "gmake" part of the normal FreeBSD system? Or would we need to check first this exists in the runtime?
It is not, but what we can do if there is no gmake? Lets just require packagers to install gmake along with KDevelop.
I see, so that should be documented then in the code, please.
Also, is "gmake" part of the normal FreeBSD system? Or would we need to check first this exists in the runtime?
It is not, but what we can do if there is no gmake? Lets just require packagers to install gmake along with KDevelop.
For one, the plugin could disable itself, like others do if their runtime dep is not found :) Which though should be improved in general, as this happens silently and the user is left clueless.
In any case, MakeFileResolver::executeCommand() could get some error handling incl. a message box if the execution fails
To make packagers & people building kdevelop aware of that dep, best copy FindCppcheck.cmake to some Findgmake.cmake or similar and adapt to what is needed with gmake in case of FreeBSD, then add to plugins/custommake/CMakeLists.txt this:
if(CMAKE_SYSTEM_NAME STREQUAL "FreeBSD") find_package(gmake QUIET) set_package_properties(gmake PROPERTIES DESCRIPTION "The GNU make" PURPOSE "Required by the custommake plugin" TYPE RUNTIME ) endif()
It turned out that BSD make just don't need that flag, so add it conditionally.
Would it still make sense on FreeBSD to try findExecutable("gmake") first and use that if found?
Okay.
Would it still make sense on FreeBSD to try findExecutable("gmake") first and use that if found?
Not sure. If the normal make of FrreeBSD does anything that is needed here to estimate the include dirs, things should be fine as it is.
plugins/custommake/makefileresolver/makefileresolver.cpp | ||
---|---|---|
99 | Please add a comment saying why this is needed, e.g. // GNU make implicitly enables "-w" for sub-makes, we don't want that |