[lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py.

[rupprecht]

Summary

use_lldb_suite.py looks for use_lldb_suite_root.py by checking parent directories. If for some reason it doesn't exist, it keeps checking parent directories until it finds it.

However, this only breaks when the parent directory is None, but at least on Linux, dirname('/') == '/', so this will never be None.

This changes the lookup to stop if the dirname(lldb_root) is unchanged. It still stops if dirname returns None on some systems.

Additionally, this makes the failure mode more visible -- if the root is not found, it complains loudly instead of silently failing, having later modules that need lldb_root fail.

[labath]

Heh I fixed the same thing back in 67f6d842fab6 for scripts/use_lldb_suite.py, but of course I forgot to update this copy... The idea is good, just please make sure both files end up using the same solution.

(Also, if you have any ideas on how to avoid having multiple copies of this file around, then don't keep them to yourself. :) )

[rupprecht]

In D83840#2152706, @labath wrote:

Heh I fixed the same thing back in 67f6d842fab6 for scripts/use_lldb_suite.py, but of course I forgot to update this copy... The idea is good, just please make sure both files end up using the same solution.

Oh, and there's another one -- lldb/packages/Python/lldbsuite/__init__.py. I'll update it too.

(Also, if you have any ideas on how to avoid having multiple copies of this file around, then don't keep them to yourself. :) )

Not at the moment, but I have a feeling I'll be staring at these files long enough to come up with something.

[shafik]

@labath why do we need two copies of use_lldb_suite.py?

[labath]

In D83840#2154263, @shafik wrote:

@labath why do we need two copies of use_lldb_suite.py?

This script is responsible for setting up an appropriate python import path. Before we can import any code in lldb/third_party/Python/module or lldb/packages/Python, we need to add those paths to sys.path. And we cannot completely put this code into a central place because then we wouldn't know how to import that.

So, the way get around that is by placing this file into the same folder as the script that needs it. Then, the script can load this file using a relative import, and afterwards, it can import anything it wants. And since we have scripts needing this functionality in multiple places, we have multiple copies of the script.

At least that's the current state of the art. It's possible that there are better solutions, but we just don't know about them.

I wouldn't say this is a major problem. These files don't get a lot of traffic -- this is pretty much the second change since they were added five years ago, and the only reason we ran into this is because it caused problems in our weird internal distributed build setup. But removing the duplication would certainly be nice...

[JDevlieghere]

In D83840#2155308, @labath wrote:

In D83840#2154263, @shafik wrote:

@labath why do we need two copies of use_lldb_suite.py?

This script is responsible for setting up an appropriate python import path. Before we can import any code in lldb/third_party/Python/module or lldb/packages/Python, we need to add those paths to sys.path. And we cannot completely put this code into a central place because then we wouldn't know how to import that.

So, the way get around that is by placing this file into the same folder as the script that needs it. Then, the script can load this file using a relative import, and afterwards, it can import anything it wants. And since we have scripts needing this functionality in multiple places, we have multiple copies of the script.

At least that's the current state of the art. It's possible that there are better solutions, but we just don't know about them.

We could have CMake configure this at the cost of always having to use the corresponding scripts from the build directory? It seems like only analyze-project-deps.py and host_art_bt.py are importing lldb right now so it might be worth considering.

I wouldn't say this is a major problem. These files don't get a lot of traffic -- this is pretty much the second change since they were added five years ago, and the only reason we ran into this is because it caused problems in our weird internal distributed build setup. But removing the duplication would certainly be nice...

[labath]

In D83840#2156040, @JDevlieghere wrote:

In D83840#2155308, @labath wrote:

In D83840#2154263, @shafik wrote:

@labath why do we need two copies of use_lldb_suite.py?

This script is responsible for setting up an appropriate python import path. Before we can import any code in lldb/third_party/Python/module or lldb/packages/Python, we need to add those paths to sys.path. And we cannot completely put this code into a central place because then we wouldn't know how to import that.

So, the way get around that is by placing this file into the same folder as the script that needs it. Then, the script can load this file using a relative import, and afterwards, it can import anything it wants. And since we have scripts needing this functionality in multiple places, we have multiple copies of the script.

At least that's the current state of the art. It's possible that there are better solutions, but we just don't know about them.

We could have CMake configure this at the cost of always having to use the corresponding scripts from the build directory? It seems like only analyze-project-deps.py and host_art_bt.py are importing lldb right now so it might be worth considering.

The important part is who is importing use_lldb_suite, not lldb. By the looks of things host_art_bt.py is meant to be included from within lldb, so the path will be correct there.

Not being able to run analyze-project-deps from the source tree would be a minor inconvenience, so I'm not sure if it's worth it. But I am not opposed to the idea either. All that script really needs is the location of the repo root. Since it already has a lot of knowledge about the repo layout, we could just hardcode that too...

I don't know if it's the only script that needs that...

lldb/packages/Python/lldbsuite/__init__.py

 # Module level initialization for the `lldbsuite` module.
 
 import inspect
 import os
 import sys
 
 
 def find_lldb_root():
     lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe()))
     while True:
-        lldb_root = os.path.dirname(lldb_root)
-        if lldb_root is None:
-            return None
+        parent = os.path.dirname(lldb_root)
+        if parent == lldb_root: # dirname('/') == '/'
+            raise Exception("use_lldb_suite_root.py not found")
+        lldb_root = parent
 
         test_path = os.path.join(lldb_root, "use_lldb_suite_root.py")
         if os.path.isfile(test_path):
             return lldb_root
-    return None
 
 # lldbsuite.lldb_root refers to the root of the git/svn source checkout
 lldb_root = find_lldb_root()
 
 # lldbsuite.lldb_test_src_root refers to the root of the python test case tree
 # (i.e. the actual unit tests).
 lldb_test_root = os.path.join(lldb_root, "test", "API")

lldb/scripts/use_lldb_suite.py

 import inspect
 import os
 import sys
 
 
 def find_lldb_root():
     lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe()))
     while True:
         parent = os.path.dirname(lldb_root)
         if parent == lldb_root: # dirname('/') == '/'
-            break
+            raise Exception("use_lldb_suite_root.py not found")
         lldb_root = parent
 
         test_path = os.path.join(lldb_root, "use_lldb_suite_root.py")
         if os.path.isfile(test_path):
             return lldb_root
-    return None
 
 lldb_root = find_lldb_root()
-if lldb_root is not None:
-    import imp
-    fp, pathname, desc = imp.find_module("use_lldb_suite_root", [lldb_root])
-    try:
-        imp.load_module("use_lldb_suite_root", fp, pathname, desc)
-    finally:
-        if fp:
-            fp.close()

lldb/test/API/use_lldb_suite.py

 import inspect
 import os
 import sys
 
 
 def find_lldb_root():
     lldb_root = os.path.dirname(
         os.path.abspath(inspect.getfile(inspect.currentframe()))
     )
     while True:
-        lldb_root = os.path.dirname(lldb_root)
-        if lldb_root is None:
-            return None
+        parent = os.path.dirname(lldb_root)
+        if parent == lldb_root: # dirname('/') == '/'
+            raise Exception("use_lldb_suite_root.py not found")
+        lldb_root = parent
 
         test_path = os.path.join(lldb_root, "use_lldb_suite_root.py")
         if os.path.isfile(test_path):
             return lldb_root
-    return None
 
 lldb_root = find_lldb_root()
-if lldb_root is not None:
-    import imp
-    fp, pathname, desc = imp.find_module("use_lldb_suite_root", [lldb_root])
-    try:
-        imp.load_module("use_lldb_suite_root", fp, pathname, desc)
-    finally:
-        if fp:
-            fp.close()
+