Try to make exename portable. Refactor searchForIncludePath. #5185
Conversation
fruffy
added
the
core
fruffy
force-pushed
the
fruffy/exename
branch
2 times, most recently
from
e4df393 to
5d6bb23
Compare
Signed-off-by: fruffy <fruffy@nyu.edu>
Signed-off-by: fruffy <fruffy@nyu.edu>
fruffy
added
the
breaking-change
fruffy
added
run-ubuntu18
Signed-off-by: fruffy <fruffy@nyu.edu>
Signed-off-by: fruffy <fruffy@nyu.edu>
| bool ParserOptions::searchForIncludePath(const char *&includePathOut, | ||
| std::vector<cstring> userSpecifiedPaths, | ||
| const std::vector<cstring> &userSpecifiedPaths, | ||
| const char *exename) { |
There was a problem hiding this comment.
Can exename be converted to std::filesystem::path while here?
There was a problem hiding this comment.
This would break downstream use of that function, figured I should do that in a separate PR. Ditto for all the cstring typing of variables that should be a path.
| bool ParserOptions::searchForIncludePath(const char *&includePathOut, | ||
| std::vector<cstring> userSpecifiedPaths, | ||
| const std::vector<cstring> &userSpecifiedPaths, |
There was a problem hiding this comment.
Change to vector of std::filesystem::path?
| const char *separator = cwd[cwdLen - 1] == '/' ? "" : "/"; | ||
| std::filesystem::path getExecutablePath(const std::filesystem::path &suggestedPath) { | ||
| #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__) | ||
| // Find the path of the executable. We use a number of techniques that may fail or work on |
There was a problem hiding this comment.
There was a problem hiding this comment.
Figured we shouldn't depend on external dependencies for a utility like this. Particularly boost which we want to get rid of. Let me take a look at whereami
There was a problem hiding this comment.
Yes. Though there is bunch of domain knowledge here :(
fruffy
force-pushed
the
fruffy/exename
branch
2 times, most recently
from
87ef696 to
3d7dbe3
Compare
Signed-off-by: fruffy <fruffy@nyu.edu>
|
The Ubuntu failure is unrelated, should be fixed with #5177. |
Fixes #3812.
Ran into this while working on a PR. Turns out exename doesn't seem to properly work. Maybe this approach works better? Still trying to figure out the right approach for
getExecutablePath.Also use
std::filesystem::pathto set the include path instead of char manipulation.This change preserves all APIs, still marking as breaking in case there are some differences in behavior.