<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 10, 2016 at 11:14 PM, Bruno Cardoso Lopes <span dir="ltr"><<a href="mailto:bruno.cardoso@gmail.com" target="_blank">bruno.cardoso@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Hi Sean,<br>
<span class=""><br>
> So sorry! It had gone quiet for a few hours and I wanted to avoid leaving it<br>
> red any longer.<br>
<br>
</span>No problem, the priority is to get bot greens!<br>
<span class=""><br>
> Does any platform define a guaranteed directory iteration order? I feel like<br>
> depending on it in the first place is unwise (or maybe I'm misunderstanding?<br>
> e.g. I'm thinking that we should sort the results of directory iteration to<br>
> guarantee a particular order if needed for a comparison in a test)<br>
<br>
</span>Yes, I was thinking about that too, however I'm not yet sure this is<br>
actually happening due to iteration order.<br>
<span class=""><br>
>><br>
>> Or can you help me debug it once I land it again?<br>
><br>
><br>
> Sure, but like I mentioned the nature of this error is very troubling to me<br>
> (high-level API use running into low-level system issues). It suggests that<br>
> there is an incorrect layering or something. I hate these annoying windows<br>
> path issues as much as everyone else, so I'd rather we focus energy on<br>
> building an appropriate layering to consolidate all the special handling in<br>
> one place than to continually band-aid issues as they creep into all parts<br>
> of the codebase. VirtualFileSystem.cpp already has quite a bit of technical<br>
> debt that violates our policies in this regard (platform-specific #ifdef's<br>
> outside libSupport; I count 3 in VirtualFileSystem.cpp which is a huge<br>
> number considering how recent that code is).<br>
<br>
</span>Totally agree. Part of that is on me - I'm actually relying on<br>
<a href="http://reviews.llvm.org/D19842" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19842</a> as a base to do a proper clean up by<br>
using a target independent version of realpath. FileManager is likely<br>
benefiting from that as well (and has itself some layering violations<br>
I took precedence from). However, I'm not even sure what are the root<br>
causes that make stuff fails on windows, given that, it's hard to know<br>
what proper layering should look like, because aside windows-avoid<br>
hacks, the rest looks fine to me; I count on you then to help me with<br>
that, feel free to file PRs about anything specific :)<br></blockquote><div><br></div><div>I'm glad to help, but it makes me uneasy to have somebody working on a filesystem abstraction that does not have ready access to test and debug their changes across the major host platforms that LLVM supports (linux, mac, windows). Is there any way you can get access? I don't think that windows 10 is critical here; win7 or win8 are fine.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span class=""><br>
>><br>
>> I might<br>
>> check-in the new tests separately from the rest to make it easier to<br>
>> revert.<br>
><br>
><br>
> I don't think there's precedent for doing this in LLVM (or at least it would<br>
> be considered as bad practice). We typically check in tests alongside the<br>
> code being tested specifically to avoid the temptation to leave<br>
> untested/broken code in tree.<br>
<br>
</span>True. However it's kinda subtle in this case: (1) this patch is not<br>
really a feature, it just re-arranges the tree to allow faster search,<br>
so the current tests passing should be enough. OTOH, the side-effect<br>
is that it fixes directory iteration because different roots pointing<br>
to the same path now can be seen as the same by directory iteration. I<br>
had the intention to test that, to make sure we don't regress in the<br>
future. (2) I have no other way to "debug" on windows10-ps4 platform<br>
besides reducing-by-commit. Anyway, I'm gonna try a more minimal<br>
testcase first.<br>
<div class=""><div class="h5"><br>
--<br>
Bruno Cardoso Lopes<br>
<a href="http://www.brunocardoso.cc" rel="noreferrer" target="_blank">http://www.brunocardoso.cc</a><br>
</div></div></blockquote></div><br></div></div>