[lldb-dev] FileSpec and normalization questions

[Greg Clayton via lldb-dev]

> On Apr 20, 2018, at 12:17 PM, Pavel Labath <labath at google.com> wrote:
> 
> On Fri, 20 Apr 2018 at 17:14, Greg Clayton <clayborg at gmail.com> wrote:
>>> On Apr 20, 2018, at 1:08 AM, Pavel Labath <labath at google.com> wrote:
>>> 
>>> 
>>> So, I can see the case for both, and I don't really have a clear
>>> preference. All I would say is, whichever way we choose, we should make
> it
>>> very explicit so that the users of FileSpec know what to expect.
> 
>> I would say that without a directory it is a wildcard match on base name
> alone, and with one, the partial directories must match if the path is
> relative, and the full directory must match if absolute. I will submit a
> patch that keeps leading "./" and "../" during normalization and we will
> see what people think.

I have that as part of my current patch, so don't worry about that.
> 
> Ok, what about multiple leading "./" components? Would it make sense to
> collapse those to a single one ("././././foo.cpp" -> "./foo.cpp")?

yes!

> 
> 
>>> 
>>> On Thu, 19 Apr 2018 at 19:37, Zachary Turner via lldb-dev <
>>> lldb-dev at lists.llvm.org> wrote:
>>>> I think I might have tried to replace some of the low level functions
> in
>>> FileSpec with the LLVM equivalents and gotten a few test failures, but I
>>> didn't have time to investigate.  It would be a worthwhile experiment
> for
>>> someone to try again if they have some cycles.
> 
>> I took a look at the llvm file stuff and it has llvm::sys::fs::real_path
> which always resolves symlinks _and_ normalizes the path. Would be nice to
> break it out into two parts by adding llvm::sys::fs::normalize_path and
> have llvm::sys::fs::real_path call it.
> 
>>> I can try to take a look at it. The way I remember it, I just copied
> these
>>> functions from llvm and replaced all #ifdefs with runtime checks, which
> is
>>> pretty much what you later did in llvm proper. Unless there has been
> some
>>> significant divergence since then, it shouldn't be hard to reconcile
> these.
> 
> So, I tried playing around with unifying the two implementations today. I
> didn't touch the normalization code, I just wanted to try to replace path
> parsing functions with the llvm ones.
> 
> In theory, it should be as simple as replacing our parsing code in
> FileSpec::SetFile with calls to llvm::sys::path::filename and
> ...::parent_path (to set m_filename and m_directory).
> 
> It turned out this was not as simple, and the reason is that the llvm path
> api's aren't completely self-consistent either. For example, for "//", the
> filename+parent_path functions will decompose it into "." and "", while the
> path iterator api will just report a single component ("//").
> 
> After a couple of hours of fiddling with +/- ones, I think I have came up
> with a working and consistent implementation, but I haven't managed to
> finish polishing it today. I'll try to upload the llvm part of the patch on
> Monday. (I'll refrain from touching the lldb code for now, to avoid
> interfering with your patch).

Sounds good. Feel free to replace my changes with LLVM stuff as long as our test suite passes as I am adding a bunch of tests to cover all the cases.

Greg