labath added a comment. In D89334#2344765 <https://reviews.llvm.org/D89334#2344765>, @JDevlieghere wrote:
> For paths relative to the CWD we add both already: > > 1. We add "." to the sys path once: > https://github.com/llvm/llvm-project/blob/5d796645d6c8cadeb003715c33e231a8ba05b6de/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp#L3234 > 2. We "resolve" (make absolute) the relative path > (https://github.com/llvm/llvm-project/blob/5d796645d6c8cadeb003715c33e231a8ba05b6de/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp#L2747) > and if it exists add its dir to the sys path > (https://github.com/llvm/llvm-project/blob/5d796645d6c8cadeb003715c33e231a8ba05b6de/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp#L2785). > > For source-relative imports all this was a hypothetical solution to the > relative import from Python issue you described. I'm afraid we've completely desynchronized by this point. Let's try to reset. The algorithm I'm proposing is: if (is_relative_to_command_file(path)) {// how to implement that? ExtendPathIfNotExists(dirname(command_file)); import(path); } else { // Same as before path = make_absolute(path, cwd); ExtendPathIfNotExists(dirname(path)); import(basename(path)); } The algorithm that I think this patch implements is: if (is_relative_to_command_file(path)) // implemented by checking cwd for this file ? path = make_absolute(path, dirname(command_file); else path = make_absolute(path, cwd); ExtendPathIfNotExists(dirname(path)); import(basename(path)); Is that an accurate depiction? Both algorithms add at most one path entry for each import command. (I'm ignoring the '.' entry which gets added unconditionally.) For cwd-relative imports they behave the same way. The difference is in the command-relative imports. The first algorithm adds at most one path for each command file which executes import commands. The second one can add more -- if the imported scripts are in different directories, then all of those directories will be added to the path. >> 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. > > How are we guessing more than before? We're doing the exact same thing as for > relative paths, i.e. we resolve them and add the dir's path to the system > path (= 2 from above). But how do we know if the user meant to do a CWD-relative import or a command-relative one? That's an extra level of guessing (uncertainty). What if the file is present at both locations. I think it would be better if there was some way to explicitly say that you're importing a module using this command-relative scheme. And that this might give us an excuse to implement a more pythonic module import scheme (?) 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
