configuration: refactorize code scanning for executables #416
Conversation
|
|
||
| // Searches for an executable named file in the directories named by the PATH | ||
| // environment variable. | ||
| int look_path(const std::string &file, std::string *out); |
There was a problem hiding this comment.
You don't have to do that here and now, but I think it's high time to ditch c++03 in kcov (and the MacOS port already depends on newer), and then this would be better to write as
std::optional<std::string> look_path(...)
There was a problem hiding this comment.
Looking at CMake, for building kcov you use --std=c++17, for tests --std=c++0x and, only once, --std=c++1y (that is deprecated after C++14).
There was a problem hiding this comment.
Yes, the C++1y was used for the unit tests, which are sort of stale now anyway. I moved to c++17 after som MacOS rewrites, but forgot that it was done everywhere and not just for the Mac build (which is probably all code that needs it).
Anyway, you don't have to make that change here, I think it's fine as it is.
Add the look_path function for searching executables in directories named by the PATH environment variable in the utils source file. Refactorize the parse function in configuration.cc, using the new look_path function. This greatly simplify the code.
perillo
force-pushed
the
handle-out-dir-correctly-2
branch
from
e0f42ee
to
720eb98
Compare
|
Is seems that this change is also incorrect: https://github.com/SimonKagstrom/kcov/actions/runs/8208545224. I was sure that the behavior was the same as the original code. |
|
Let's merge this anyway, I'll resolve the test errors later. Thanks a lot! |
|
Thanks to you. |
Add the
look_pathfunction for searching executable in directories named by thePATHenvironment variable in theutilssource file.Refactorize the
parsefunction in configuration.cc, using the newlook_pathfunction. This greatly simplify the code.This PR implements an idea discussed #415.
The new change should behave the same as the current implementation, but unfortunately I can not verify this, because many tests fails on my system, probably due to incorrectly building the test suite.