Fix resource dir issue #1173
Fix resource dir issue #1173
Conversation
martong
force-pushed
the
fix_resource_dir2
branch
2 times, most recently
from
2591089 to
90e60bc
Compare
martong
force-pushed
the
fix_resource_dir2
branch
from
90e60bc to
fc0ab38
Compare
martong
added
bugfix 🔨
analyzer 📈
|
|
||
| resource_inc = compiler_resource_dir | ||
| # Resource include path must end with "/include". | ||
| basename = os.path.basename(os.path.normpath(resource_inc)) |
There was a problem hiding this comment.
Maybe better to move this logic to __get_compiler_resource_dir?
There was a problem hiding this comment.
On a long run we plan to use a function which takes both the system includes and the resource dir as a parameter, so we could implement just in one place the madness around this header include order.
| context.ld_lib_path_extra) | ||
| # Overwrite PATH to contain only the parent of the clang binary. | ||
| if os.path.isabs(config_handler.analyzer_binary): | ||
| check_env['PATH'] = os.path.dirname(config_handler.analyzer_binary) |
There was a problem hiding this comment.
Is it a good idea to overwrite the PATH? What is the motivation behind this?
There was a problem hiding this comment.
Or in case the point is to always resolve to the absolute clang binary, maybe we could just set the binary instead of invoking the function that looks up the binary in the path?
There was a problem hiding this comment.
The idea is to lookup clang from $PATH when it is a relative path. This is handled with the local object on the stack with the name check_env.
However, when clang-tidy is absolute we overwrite the PATH in check_env so we lookup only in the parent dir of clang-tidy.
It might be confusing to append to the path, because we may end up with /a/b/c/clang for /x/y/z/clang-tidy. This is true nevertheless in the relative case too, but that we cannot work around.
There was a problem hiding this comment.
I think if we want tidy and clang from the same directory we should modify the lookup function.
Overwriting the path might have negative consequences when we use this environment. (E.g.: we can not even issue a cd command if we pass this environment to a Popen or similar functions.)
There was a problem hiding this comment.
Simply setting the binary will miss the binary e.g. clang-4.0, this the reason to use resolve_missing_binary.
There was a problem hiding this comment.
We don't write over the effective PATH in the global env. We use a local variable and we overwrite the path in that local variable.
|
|
||
| resource_inc = compiler_resource_dir | ||
| # Resource include path must end with "/include". | ||
| basename = os.path.basename(os.path.normpath(resource_inc)) |
Closes #1171.