[lldb-dev] FileSpec and normalization questions

[Pavel Labath via lldb-dev]

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.

Ok, what about multiple leading "./" components? Would it make sense to
collapse those to a single one ("././././foo.cpp" -> "./foo.cpp")?


> >
> > 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).