Richard Smith <[email protected]> writes: > On Fri, Dec 12, 2014 at 12:59 PM, Justin Bogner <[email protected]> wrote: >> Richard Smith <[email protected]> writes: >>> Thankyou! >>> >>> I think the change you're referring to is that it removed the >>> pruning of /x/.. /. That was not unintentional - there were no >>> tests for that behaviour, and it's wrong if x is a symlink - but I >>> had intended to investigate further prior to checkin and forgot. >> >> Yep, that's what I was looking at. I agree that the behaviour is a >> bit iffy, but if such a path were to survive to the VFS writer we'd >> assert, as it doesn't support path traversal. I'm looking into making >> a test that actually triggers that path so I can figure out what to >> do with it. > > Thanks, that sounds like a good start. This seems like a very tricky > problem, but it also seems important to get it right; directory > symlinks in include directories are not uncommon. > >> In any case, after looking a bit, the ".." change is not why these tests >> started failing. The output order changed here because removeDotPaths >> also improved on removePathTraversal by returning early if there are no >> changes. If removeDotPaths changes anything, the path separators are >> canonicalized and all end up being \\ on windows, but if it doesn't >> change anything some of the separators may be /, since that's how they >> were passed on the commandline. > > Aha, yes, that was an entirely unintentional change. (Is that actually a > problem, or was it just something the tests weren't prepared for?)
It could lead to some directories showing up redundantly in the vfs.yaml, each with a subset of the contents. I'm not sure if this would break anything or not, but it's certainly confusing. >> I'm not entirely sure how best to handle that. Maybe >> ModuleDependencyListener should canonicalize path separators as a >> separate step? > > Sounds like a good idea. Adding a call to llvm::sys::path::native should do > the trick. Thanks for figuring this out! Done in r224164. I've also added a TODO about the .. thing. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
