JDevlieghere added a comment. In D89334#2334881 <https://reviews.llvm.org/D89334#2334881>, @labath wrote:
> In D89334#2332452 <https://reviews.llvm.org/D89334#2332452>, @JDevlieghere > wrote: > >> In D89334#2331903 <https://reviews.llvm.org/D89334#2331903>, @labath wrote: >> >>> In D89334#2330287 <https://reviews.llvm.org/D89334#2330287>, @JDevlieghere >>> wrote: >>> >>>> I guess then the user should have called `command script import >>>> awesome_backtrace_analyzer` to import the package rather than the >>>> `main.py` inside it. But I get your point. FWIW just adding the `$ROOT` is >>>> how I did the original implementation before adding the tests for the >>>> nested directories and `.py` files that Dave suggested. It would solve >>>> this issues but then doesn't support those scenarios. I don't know if it >>>> would be less confusing that you can't pass a relative path to a `.py` >>>> file or that you can't import another module as you described. >>> >>> I don't think the two options are mutually exclusive. I'm pretty sure this >>> is just a limitation of our current importing code, which could be fixed to >>> import `awesome_backtrace_analyzer/main.py` as >>> `awesome_backtrace_analyzer.main` like it would be from python. >> >> I don't think we can do that in the general case without breaking users (see >> below). I guess we could do it for imports not relative to the current >> working directory, as this has never worked before. It would then replace >> the logic described by the comment on line 2793. > > In general, we cannot impose any kind of natural structure on the path the > user gives us -- in an absolute path, the python root could be anywhere. For > cwd-relative imports, we could pretend that the cwd is the root, but that > seems somewhat unintuitive (and breaks users) (*). Even the usage of the > directory of the sourced file as root seems moderately unintuitive to me, > though I think it might be a good fit for the motivational use case for this > feature. The issue is that we "resolve" cwd-relative paths to absolute paths and treat the result as an absolute path and the "." is only in the system path to make relative imports from inside the module work. We could do the same for the "source root" but that would mean adding yet another path to the system path. The more I think about it the more I feel like the current approach in this patch is fits best with what we're already doing. If it doesn't work for the user they can always adjust the system path in the top level (imported) module. 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
