r371027 - Revert r361885 "[Driver] Fix -working-directory issues"

Mikael Holmén via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 1 00:27:11 PDT 2019


Hi,

On 2019-09-27 10:13, Hans Wennborg wrote:
> Committed a fix in r373049, and I'll ask to have it merged for 9.0.1
> as well. Sorry again for the breakage.

No worries. Thanks for the fix!

/Mikael

> 
> On Thu, Sep 26, 2019 at 7:09 PM Mikael Holmén
> <mikael.holmen at ericsson.com> wrote:
>>
>> Hi,
>>
>> As Karl-Johan said I'll be out of office for a few days so please submit a fix if you can find the time.
>>
>> Thanks,
>> Mikael
>>
>> Den 26 sep. 2019 15:39 skrev Hans Wennborg <hans at chromium.org>:
>>
>> On Thu, Sep 26, 2019 at 12:55 PM Mikael Holmén via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>>
>>> Hi Hans,
>>>
>>> I'm a bit suspicious against the part
>>>
>>>   > This also revertes the part of r369938 which checked that
>>> -working-directory works.
>>>
>>> in this revert.
>>>
>>> You do:
>>>
>>>   > +  SmallString<128> Buf;
>>>   > +  if (!llvm::sys::fs::current_path(Buf))
>>>   > +    Buf = ".";
>>>   > +  CDB << "{ \"directory\": \"" << escape(Buf) << "\"";
>>>
>>> But if I look at r369938 it seems like before r369938 it looked like
>>>
>>> -  SmallString<128> Buf;
>>> -  if (llvm::sys::fs::current_path(Buf))
>>> -    Buf = ".";
>>> -  CDB << "{ \"directory\": \"" << escape(Buf) << "\"";
>>>
>>> Note the difference in the condition, where you do
>>>
>>>    if (!llvm::sys::fs::current_path(Buf))
>>>
>>> but before it was
>>>
>>>    if (llvm::sys::fs::current_path(Buf))
>>>
>>>
>>> We noticed this since
>>>
>>>    clang -MJ test.json -O2 -nostdinc empty.c -S -o - && cat test.json
>>>
>>> started to behave differently after this revert. The "directory: " part
>>> in the output suddenly became ".".
>>
>> You're right, it definitely looks like I messed up the revert :-(
>> Looking at the code again, I was probably thrown off by the
>> current_path returning false on success.
>>
>> Since you've been investigating, are you set up to land a fix with a
>> test maybe? Otherwise I can do it, please let me know.
>>
>> Thanks,
>> Hans


More information about the cfe-commits mailing list