[Lldb-commits] [lldb] r335236 - ScriptInterpreterPython cleanup
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Jun 21 10:42:17 PDT 2018
Thanks for the explanation.
The ifdef parts should be reverted by r335236.
On Thu, 21 Jun 2018 at 18:39, Greg Clayton <clayborg at gmail.com> wrote:
>
>
>
> > On Jun 21, 2018, at 10:30 AM, Pavel Labath via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> >
> > Sure, I can do that. The though of xcode compatibility did enter my
> > mind briefly while doing this, but I just assumed that XCode always
> > builds with python enabled. I guess I was wrong.
> >
> > Out of curiosity, what are you using the !Python build for?
>
> Apple builds LLDB for arm and arm64 for native execution on iOS devices. iOS devices don't have python.
>
>
> > On Thu, 21 Jun 2018 at 18:16, Jim Ingham <jingham at apple.com> wrote:
> >>
> >> This change will break building from Xcode without Python. It will take a bunch of monkeying with the project files to achieve the same effect as your cmake changes, and I don't have time to do that right now. I have no problem with changing cmake to just not build the relevant files, but can you leave the #ifdef's in place so you don't break the Xcode build? They don't seem to add enough value to warrant the breakage.
> >>
> >> Thanks,
> >>
> >> Jim
> >>
> >>> On Jun 21, 2018, at 7:09 AM, Pavel Labath via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> >>>
> >>> Author: labath
> >>> Date: Thu Jun 21 07:09:15 2018
> >>> New Revision: 335236
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=335236&view=rev
> >>> Log:
> >>> ScriptInterpreterPython cleanup
> >>>
> >>> Instead of #ifdef-ing the contents of all files in the plugin for all
> >>> non-python builds, just disable the plugin at the cmake level. Also,
> >>> remove spurious extra linking of the Python plugin in liblldb. This
> >>> plugin is already included as a part of LLDB_ALL_PLUGINS variable.
> >>>
> >>> Modified:
> >>> lldb/trunk/source/API/CMakeLists.txt
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/CMakeLists.txt
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h
> >>>
> >>> Modified: lldb/trunk/source/API/CMakeLists.txt
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/CMakeLists.txt?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/API/CMakeLists.txt (original)
> >>> +++ lldb/trunk/source/API/CMakeLists.txt Thu Jun 21 07:09:15 2018
> >>> @@ -108,11 +108,6 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND
> >>> PROPERTY COMPILE_FLAGS " -Wno-sequence-point -Wno-cast-qual")
> >>> endif ()
> >>>
> >>> -target_link_libraries(liblldb PRIVATE
> >>> - lldbPluginScriptInterpreterNone
> >>> - lldbPluginScriptInterpreterPython
> >>> - )
> >>> -
> >>> set_target_properties(liblldb
> >>> PROPERTIES
> >>> VERSION ${LLDB_VERSION}
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/CMakeLists.txt
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/CMakeLists.txt?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/CMakeLists.txt (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/CMakeLists.txt Thu Jun 21 07:09:15 2018
> >>> @@ -1,2 +1,4 @@
> >>> add_subdirectory(None)
> >>> -add_subdirectory(Python)
> >>> +if (NOT LLDB_DISABLE_PYTHON)
> >>> + add_subdirectory(Python)
> >>> +endif()
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt Thu Jun 21 07:09:15 2018
> >>> @@ -1,4 +1,4 @@
> >>> -if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows" AND NOT LLDB_DISABLE_PYTHON)
> >>> +if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
> >>> # Call a python script to gather the arch-specific libdir for
> >>> # modules like the lldb module.
> >>> execute_process(
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp Thu Jun 21 07:09:15 2018
> >>> @@ -1,5 +1,4 @@
> >>> -//===-- PythonDataObjects.cpp ------------------------------------*- C++
> >>> -//-*-===//
> >>> +//===-- PythonDataObjects.cpp -----------------------------------*- C++ -*-===//
> >>> //
> >>> // The LLVM Compiler Infrastructure
> >>> //
> >>> @@ -8,12 +7,6 @@
> >>> //
> >>> //===----------------------------------------------------------------------===//
> >>>
> >>> -#ifdef LLDB_DISABLE_PYTHON
> >>> -
> >>> -// Python is disabled in this build
> >>> -
> >>> -#else
> >>> -
> >>> #include "PythonDataObjects.h"
> >>> #include "ScriptInterpreterPython.h"
> >>>
> >>> @@ -1035,5 +1028,3 @@ bool PythonFile::GetUnderlyingFile(File
> >>> file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
> >>> return file.IsValid();
> >>> }
> >>> -
> >>> -#endif
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h Thu Jun 21 07:09:15 2018
> >>> @@ -10,8 +10,6 @@
> >>> #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
> >>> #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
> >>>
> >>> -#ifndef LLDB_DISABLE_PYTHON
> >>> -
> >>> // LLDB Python header must be included first
> >>> #include "lldb-python.h"
> >>>
> >>> @@ -470,5 +468,3 @@ public:
> >>> } // namespace lldb_private
> >>>
> >>> #endif
> >>> -
> >>> -#endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp Thu Jun 21 07:09:15 2018
> >>> @@ -7,8 +7,6 @@
> >>> //
> >>> //===----------------------------------------------------------------------===//
> >>>
> >>> -#ifndef LLDB_DISABLE_PYTHON
> >>> -
> >>> // LLDB Python header must be included first
> >>> #include "lldb-python.h"
> >>>
> >>> @@ -166,5 +164,3 @@ std::string PythonExceptionState::ReadBa
> >>>
> >>> return retval;
> >>> }
> >>> -
> >>> -#endif
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h Thu Jun 21 07:09:15 2018
> >>> @@ -10,8 +10,6 @@
> >>> #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONEXCEPTIONSTATE_H
> >>> #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONEXCEPTIONSTATE_H
> >>>
> >>> -#ifndef LLDB_DISABLE_PYTHON
> >>> -
> >>> #include "PythonDataObjects.h"
> >>>
> >>> namespace lldb_private {
> >>> @@ -53,5 +51,3 @@ private:
> >>> }
> >>>
> >>> #endif
> >>> -
> >>> -#endif
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Thu Jun 21 07:09:15 2018
> >>> @@ -7,12 +7,6 @@
> >>> //
> >>> //===----------------------------------------------------------------------===//
> >>>
> >>> -#ifdef LLDB_DISABLE_PYTHON
> >>> -
> >>> -// Python is disabled in this build
> >>> -
> >>> -#else
> >>> -
> >>> // LLDB Python header must be included first
> >>> #include "lldb-python.h"
> >>>
> >>> @@ -3191,30 +3185,18 @@ void ScriptInterpreterPython::AddToSysPa
> >>> PyRun_SimpleString(statement.c_str());
> >>> }
> >>>
> >>> -// void
> >>> -// ScriptInterpreterPython::Terminate ()
> >>> -//{
> >>> -// // We are intentionally NOT calling Py_Finalize here (this would be the
> >>> -// logical place to call it). Calling
> >>> -// // Py_Finalize here causes test suite runs to seg fault: The test suite
> >>> -// runs in Python. It registers
> >>> -// // SBDebugger::Terminate to be called 'at_exit'. When the test suite
> >>> -// Python harness finishes up, it calls
> >>> -// // Py_Finalize, which calls all the 'at_exit' registered functions.
> >>> -// SBDebugger::Terminate calls Debugger::Terminate,
> >>> -// // which calls lldb::Terminate, which calls ScriptInterpreter::Terminate,
> >>> -// which calls
> >>> -// // ScriptInterpreterPython::Terminate. So if we call Py_Finalize here, we
> >>> -// end up with Py_Finalize being called from
> >>> -// // within Py_Finalize, which results in a seg fault.
> >>> -// //
> >>> -// // Since this function only gets called when lldb is shutting down and
> >>> -// going away anyway, the fact that we don't
> >>> -// // actually call Py_Finalize should not cause any problems (everything
> >>> -// should shut down/go away anyway when the
> >>> -// // process exits).
> >>> -// //
> >>> -//// Py_Finalize ();
> >>> -//}
> >>> -
> >>> -#endif // #ifdef LLDB_DISABLE_PYTHON
> >>> +// We are intentionally NOT calling Py_Finalize here (this would be the logical
> >>> +// place to call it). Calling Py_Finalize here causes test suite runs to seg
> >>> +// fault: The test suite runs in Python. It registers SBDebugger::Terminate to
> >>> +// be called 'at_exit'. When the test suite Python harness finishes up, it
> >>> +// calls Py_Finalize, which calls all the 'at_exit' registered functions.
> >>> +// SBDebugger::Terminate calls Debugger::Terminate, which calls lldb::Terminate,
> >>> +// which calls ScriptInterpreter::Terminate, which calls
> >>> +// ScriptInterpreterPython::Terminate. So if we call Py_Finalize here, we end
> >>> +// up with Py_Finalize being called from within Py_Finalize, which results in a
> >>> +// seg fault. Since this function only gets called when lldb is shutting down
> >>> +// and going away anyway, the fact that we don't actually call Py_Finalize
> >>> +// should not cause any problems (everything should shut down/go away anyway
> >>> +// when the process exits).
> >>> +//
> >>> +// void ScriptInterpreterPython::Terminate() { Py_Finalize (); }
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Thu Jun 21 07:09:15 2018
> >>> @@ -10,12 +10,6 @@
> >>> #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTINTERPRETERPYTHON_H
> >>> #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTINTERPRETERPYTHON_H
> >>>
> >>> -#ifdef LLDB_DISABLE_PYTHON
> >>> -
> >>> -// Python is disabled in this build
> >>> -
> >>> -#else
> >>> -
> >>> // C Includes
> >>> // C++ Includes
> >>> #include <memory>
> >>> @@ -571,6 +565,4 @@ protected:
> >>>
> >>> } // namespace lldb_private
> >>>
> >>> -#endif // LLDB_DISABLE_PYTHON
> >>> -
> >>> #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTINTERPRETERPYTHON_H
> >>>
> >>> Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h
> >>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h?rev=335236&r1=335235&r2=335236&view=diff
> >>> ==============================================================================
> >>> --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h (original)
> >>> +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/lldb-python.h Thu Jun 21 07:09:15 2018
> >>> @@ -1,5 +1,4 @@
> >>> -//===-- lldb-python.h --------------------------------------------*- C++
> >>> -//-*-===//
> >>> +//===-- lldb-python.h -------------------------------------------*- C++ -*-===//
> >>> //
> >>> // The LLVM Compiler Infrastructure
> >>> //
> >>> @@ -14,9 +13,6 @@
> >>> // Python.h needs to be included before any system headers in order to avoid
> >>> // redefinition of macros
> >>>
> >>> -#ifdef LLDB_DISABLE_PYTHON
> >>> -// Python is disabled in this build
> >>> -#else
> >>> #include "llvm/Support/Compiler.h"
> >>> #if defined(_WIN32)
> >>> // If anyone #includes Host/PosixApi.h later, it will try to typedef pid_t. We
> >>> @@ -36,6 +32,5 @@
> >>>
> >>> // Include python for non windows machines
> >>> #include <Python.h>
> >>> -#endif // LLDB_DISABLE_PYTHON
> >>>
> >>> #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_H
> >>>
> >>>
> >>> _______________________________________________
> >>> lldb-commits mailing list
> >>> lldb-commits at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >>
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
More information about the lldb-commits
mailing list