Fix detection of include dirs with gnu compiler and non US locale #2564
Conversation
mesonbuild/compilers/compilers.py
Outdated
| @@ -923,11 +923,14 @@ def get_largefile_args(compiler): | |||
| def gnulike_default_include_dirs(compiler, lang): | |||
| if lang == 'cpp': | |||
| lang = 'c++' | |||
| env = os.environ.copy() | |||
| env["LC_ALL"]='C' | |||
There was a problem hiding this comment.
[Flake8]
missing whitespace around operator
Auto detection was based on parsing gcc's output so we have to ensure that it is always 'C'. Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
jeandet
force-pushed
the
fix_boost_detection_with_wrong_locale
branch
from
721dcec
to
f15a57f
Compare
| p = subprocess.Popen( | ||
| compiler + ['-x{}'.format(lang), '-E', '-v', '-'], | ||
| stdin=subprocess.DEVNULL, | ||
| stderr=subprocess.PIPE, | ||
| stdout=subprocess.PIPE | ||
| stdout=subprocess.PIPE, | ||
| env=env | ||
| ) | ||
| stderr = p.stderr.read().decode('utf-8') |
There was a problem hiding this comment.
Using 'C' and 'utf-8' is inconsistent, no? And C.utf8 is not necessarily widespread. Would setting LC_MESSAGES work?
There was a problem hiding this comment.
Would the compiler complain though?
There was a problem hiding this comment.
If there are paths that cannot be represented with ASCII, then maybe?
There was a problem hiding this comment.
@QuLogic
I'm not a locale/encoding expert.
I would say that setting LC_MESSAGES might be enough while setting LC_ALL isn't wrong.
Since UTF-8 can encode any char and is ASCII compatible I don't see anything better, do you have failing test cases?
There was a problem hiding this comment.
The problem is not on the Python side, but on the compiler side as 'C' implies ASCII. Now, I don't have any non-ASCII paths, so I don't really have a test case or any idea if the compiler would even output this correctly.
There was a problem hiding this comment.
I did a test on my laptop with this folder name 'test_£_♣_が' it works.
I'm kind of stuck. My PR mainly ensures that user local doesn't changes gcc output.
I really don't know what should I do about UTF-8.
For now, any user with a non english linux box may suffer this issue.
There was a problem hiding this comment.
Maybe it should set an english utf-8 locale?
There was a problem hiding this comment.
I think this should be considered when a real world problem with C arrives. As it is now, this PR improves the situation for anyone with a non english locale A LOT.
|
I think the function should print a warning if no directories are found. That will make bug reports easier in the future. |
mesonbuild/compilers/compilers.py
Outdated
| @@ -946,6 +950,8 @@ def gnulike_default_include_dirs(compiler, lang): | |||
| break | |||
| else: | |||
| paths.append(line[1:]) | |||
| if 0==len(paths): | |||
There was a problem hiding this comment.
[Flake8]
missing whitespace around operator
jeandet
force-pushed
the
fix_boost_detection_with_wrong_locale
branch
from
c2d24a3
to
10a6d50
Compare
mesonbuild/compilers/compilers.py
Outdated
| @@ -946,6 +950,8 @@ def gnulike_default_include_dirs(compiler, lang): | |||
| break | |||
| else: | |||
| paths.append(line[1:]) | |||
| if 0 == len(paths): | |||
There was a problem hiding this comment.
it isn't very pythonic to write your test in reverse order "yoda condition" because you are not allowed to assign in an if statement anyway. So there is no risk of writing if a = 0: instead of if a == 0:
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
jeandet
force-pushed
the
fix_boost_detection_with_wrong_locale
branch
from
10a6d50
to
dfab587
Compare
|
I tagged this for 0.43.1 but @nirbheek makes the final call on that. |
…ith_wrong_locale Fix detection of include dirs with gnu compiler and non US locale
Auto detection was based on parsing gcc's output so we have to
ensure that it is always 'C'.