labath added a comment.

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.

Speaking of users, if you know any, it might be interesting to ask them about 
this and see whether they'd be interested in such a thing. I don't write python 
scripts, so I'm just hypothesizing. (and trying to ensure we don't dig a bigger 
hole for ourselves than we already have.)

(*) On the flip side, we are already adding `.` to the path, so this would kind 
of make sense.


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

Reply via email to