[lldb-dev] FileSpec and normalization questions

[Pavel Labath via lldb-dev]

The llvm patches have been posted as: <https://reviews.llvm.org/D45941>, <
https://reviews.llvm.org/D45942>.
On Fri, 20 Apr 2018 at 22:12, Greg Clayton <clayborg at gmail.com> wrote:



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