[Lldb-commits] [PATCH] D54942: [PDB] Make PDB lit tests use the new builder

Thu Dec 6 21:23:44 PST 2018

Thank you! It seems that your commit have fixed the bug - it haven't been
reproduced on `lldb-x64-windows-ninja` since your commit.

On Thu, Dec 6, 2018 at 9:45 PM Zachary Turner <zturner at google.com> wrote:

> Hopefully fixed in r348511.  I went with the approach of concatenating the
> executable file name, for two reasons.  First, it's easier, since dealing
> with the tmpfile module and mkstemp and sorts has its own set of
> complexities and issues.  Secondly, because it means the build artifacts
> will still be in a normal place (instead of in temporary directory) and
> with an intuitive name.  So this means if a test failed it might be easier
> for a developer to inspect the build artifacts to try to determine what
> went wrong.
>
> On Thu, Dec 6, 2018 at 9:22 AM Stella Stamenova <stilis at microsoft.com>
> wrote:
>
>> I am in favor of unique filenames because it will guarantee we avoid any
>> problems like this in the future, but in most cases, as long as the names
>> are unique **in the test suite**, that should be sufficient. So I don’t
>> have any objections to Zachary’s approach of making the filenames unique
>> enough.
>>
>>
>>
>> Thanks,
>>
>> -Stella
>>
>>
>>
>> *From:* Aleksandr Urakov <aleksandr.urakov at jetbrains.com>
>> *Sent:* Thursday, December 6, 2018 12:25 AM
>> *To:* Zachary Turner <zturner at google.com>
>> *Cc:* reviews+D54942+public+2603ca548f36d222 at reviews.llvm.org; Stella
>> Stamenova <stilis at microsoft.com>; llvm-commits at lists.llvm.org; Hafiz
>> Abid Qadeer <abidh.haq at gmail.com>; Raphael Isemann <teemperor at gmail.com>;
>> lldb-commits at lists.llvm.org; sanimir at subpath.org; chirag at raincode.com;
>> Brian Gianforcaro <b.gianfo at gmail.com>
>>
>>
>> *Subject:* Re: [PATCH] D54942: [PDB] Make PDB lit tests use the new
>> builder
>>
>>
>>
>> Yes, this solution would solve the current problem. But theoretically the
>> uniqueness may be defined by the directory, where the file is located, not
>> by the name. It looks very unlikely, but who knows...
>>
>>
>>
>> I thought about what Stella have said, and it seems that I have found a
>> good solution for this. We can use something like `tempfile` (
>> https://docs.python.org/3.7/library/tempfile.html
>> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.python.org%2F3.7%2Flibrary%2Ftempfile.html&data=02%7C01%7Cstilis%40microsoft.com%7Cbdbb5017a9b24a0ad26708d65b5458e0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636796815166605217&sdata=JXoPXUaLdantqlee9S0E7Huv8WIDoLHltXyn67GnABY%3D&reserved=0>)
>> for every internally generated file of the script. It will guarantee us the
>> uniqueness of the file. But for the uniqueness of files named by script
>> parameters the script user will remain responsible.
>>
>>
>>
>> What do you think about the such approach?
>>
>>
>>
>> On Wed, Dec 5, 2018 at 11:55 PM Zachary Turner <zturner at google.com>
>> wrote:
>>
>> I was thinking that we could just automatically compute the output file
>> names as:
>>
>>
>>
>> os.path.join(out_dir, basename(output_file) + '.' +
>> basename(input_file) + '.obj')
>>
>>
>>
>> Currently it's just
>>
>>
>>
>> os.path.join(out_dir, basename(input_file) + '.obj')
>>
>>
>>
>> which is why I think the problem occurs.
>>
>>
>>
>> On Wed, Dec 5, 2018 at 12:47 PM Aleksandr Urakov <
>> aleksandr.urakov at jetbrains.com> wrote:
>>
>> With such solution there would be even no need to change the current
>> commit. But I'm not sure that it's trivial to do - the output file name may
>> contain path with directories. May be we can replace slashes with
>> underscores in the output file path and concatenate it with the object file
>> name? Or even replace slashes in the source file path and concatenate it
>> with the output file path - so object files will be located in the same
>> place as the output file.
>>
>> Am Mi., 5. Dez. 2018, 23:30 hat Zachary Turner <zturner at google.com>
>> geschrieben:
>>
>> It is not possible to specify object file name in compile and link mode.
>> But perhaps we can just change the default object file name to include
>> something from the output file as well
>>
>> On Wed, Dec 5, 2018 at 12:26 PM Aleksandr Urakov via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>> aleksandr.urakov added a subscriber: zturner.
>> aleksandr.urakov added a comment.
>>
>> The similar problem with `typedefs.test` is here:
>> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1940/steps/test/logs/stdio
>> <https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Flab.llvm.org%3A8014%2Fbuilders%2Flldb-x64-windows-ninja%2Fbuilds%2F1940%2Fsteps%2Ftest%2Flogs%2Fstdio&data=02%7C01%7Cstilis%40microsoft.com%7Cbdbb5017a9b24a0ad26708d65b5458e0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636796815166615226&sdata=CGSQScg8GAQnLurJBOCf1JaoT4hFhluoD6fVkvdECxw%3D&reserved=0>
>>
>> I have an assumption about the cause of the problem. Are the tests
>> running in parallel? In this case `typedefs.test` and `enums-layout.test`
>> are writing to the same object file together, because they both are
>> compiled from the same source.
>>
>> @zturner Is it possible to specify object file's name in
>> `compile-and-link` mode? Then we can specify different names in different
>> tests for both object files and executables. But I think that splitting the
>> source or combining the tests would be a better idea.
>>
>> I will fix it only tomorrow, because I'm already at home today. Feel free
>> to revert it if needed.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D54942/new/
>> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD54942%2Fnew%2F&data=02%7C01%7Cstilis%40microsoft.com%7Cbdbb5017a9b24a0ad26708d65b5458e0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636796815166625231&sdata=nagEt0RLn04uZdpSBsoisT4%2BxhlMLMK5vXYv3UkKdrg%3D&reserved=0>
>>
>> https://reviews.llvm.org/D54942
>> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD54942&data=02%7C01%7Cstilis%40microsoft.com%7Cbdbb5017a9b24a0ad26708d65b5458e0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636796815166625231&sdata=5O%2FLR7P8QEPgE7bhN0z9Szk0UgV1sChq8QNhAvzcHaU%3D&reserved=0>
>>
>>
>>
>>
>>
>> --
>>
>> Aleksandr Urakov
>>
>> Software Developer
>>
>> JetBrains
>>
>> http://www.jetbrains.com
>> <https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.jetbrains.com&data=02%7C01%7Cstilis%40microsoft.com%7Cbdbb5017a9b24a0ad26708d65b5458e0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636796815166635240&sdata=upmrCJ165lcxyFu8cMo250E88LUpK8MG4G2ZjMpsp2A%3D&reserved=0>
>>
>> The Drive to Develop
>>
>

-- 
Aleksandr Urakov
Software Developer
JetBrains
http://www.jetbrains.com
The Drive to Develop
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181207/53f54809/attachment-0001.html>