[llvm] r314915 - [test] Fix append_path in the empty case

[Francis Ricci via llvm-commits]

Ahh sorry, I misread your diff. Either way, adding the extra layer of
quoting looks fine to me - I can take care of it if you'd like, but
you can feel free (especially since you have the ability to test
against the broken use case)

On Sat, Oct 7, 2017 at 1:49 AM, Francis Ricci <francisjricci at gmail.com> wrote:
> You can feel free to change it from single to double quotes if that fixes
> your issue
>
>
> On Fri, Oct 6, 2017, 8:02 PM Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
> wrote:
>>
>> Hi Francis,
>>
>> On Wed, Oct 4, 2017 at 10:30 AM, Francis Ricci via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: fjricci
>> > Date: Wed Oct  4 10:30:28 2017
>> > New Revision: 314915
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=314915&view=rev
>> > Log:
>> > [test] Fix append_path in the empty case
>> >
>> > Summary:
>> > normpath() was being called on an empty string and appended to
>> > the environment variable in the case where the environment variable
>> > was unset. This led to ":." being appended to the path, since
>> > normpath() of an empty string is '.', presumably to represent cwd.
>>
>> Thanks for fixing this. However, I'm not quite sure I get part of
>> this. Looking at parse_flags() in
>> lib/sanitizer_common/sanitizer_flag_parser.cc, it looks to me like the
>> separator is a whitespace. os.path.pathsep defaults to ":" on darwin,
>> as you notice in your comment. I guess was never a issue for
>> ASAN_OPTIONS because it had never more than one?
>>
>> Now I get issues with these changes when I run this in macOS 10.13.
>> The workaround I found was to actually quote the arg:
>>
>> diff --git a/utils/lit/lit/llvm/config.py b/utils/lit/lit/llvm/config.py
>> index 87851b3cb1a..c381e2fd040 100644
>> --- a/utils/lit/lit/llvm/config.py
>> +++ b/utils/lit/lit/llvm/config.py
>> @@ -82,7 +82,7 @@ class LLVMConfig(object):
>>              if re.match(r'^x86_64.*-apple', target_triple):
>>                  if 'address' in sanitizers:
>>                      self.with_environment(
>> -                        'ASAN_OPTIONS', 'detect_leaks=1',
>> append_path=True)
>> +                        'ASAN_OPTIONS', "'detect_leaks=1'",
>> append_path=True)
>>              if re.match(r'^x86_64.*-linux', target_triple):
>>                  features.add('x86_64-linux')
>>              if re.match(r'.*-win32$', target_triple):
>>
>> Ideas?
>>
>> --
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc