labath added a comment. In D89334#2341889 <https://reviews.llvm.org/D89334#2341889>, @JDevlieghere wrote:
> In D89334#2341561 <https://reviews.llvm.org/D89334#2341561>, @labath wrote: > >> I'm afraid you've lost me there. Yes, we turn relative paths into absolute >> ones, but I don't see how that translates into adding less entries into >> sys.path. For each module that we import, we add `dirname(module_path)` to >> sys.path. If we import two modules from different directories, we will get >> two sys.path entries, even if the modules were specified as paths relative >> to the same directory. Canonicalizing the path to that directory will mean >> less entries. > > It doesn't, it translates to more: one for the `.` and one for > `dirname(module_path)`. Which basically translates to `n+1` entries because > the working directory doesn't change. I was saying we could do the exact same > thing for modules relative to the source-dir, but that means `2n` entries > because for every module we add `source_dir` and `dirname(module_path)` and > generally both will be different. My point was that it would solve the issue > of a relative import that you described last week, but at the cost at adding > twice as much entries, which means double the chance of an ambiguous (not > ambitious ;-) import. I think that was your main objection to the patch as it > is right now and I'm arguing that I think it's the best of the different > trade-offs. Why would we be adding both? We've already checked the path and know if the file exists in the cwd or not. What's the point in adding that? My impression was that this patch already avoids adding two paths in this case... Actually, this guessing of what they user meant to say is one of the things I don't like about this approach. I think it would be better if there was some way (command option or something) to specify where the module should be imported from. The docstring for that option could also explain the rule for how the module is being imported. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89334/new/ https://reviews.llvm.org/D89334 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
