Skip to content

Commit b121f84

Browse files
KlaimDerThorsten
andauthored
Fixes various problems revealed by Windows CI runs (#702)
* applied #685 to async pr * added missing boost dependency * ignore root build dirs * added missing boost packages dependencies * debug duct tape and more info * removed msvc-specific instruction * tests wait for xpython's ready message instead of sleep * debug duct tape and more info * fi * fix * x6 * one minute * cleaner fix: lock python's GIL when destroying the debugger * cleanup python path deduction unicode-friendly (on windows) and absolute paths * removed debugging logs & cleaned error logs * imrpoved sub-process tracking output in tests * removed commented code * run 20 times on windows * run tests 20 times on unix * show test run number in CI logs * removed changes to python path deduction, improved wrong path detection and report instead * debugger reference decreased instead of object destroyed * removed some logs --------- Co-authored-by: DerThorsten <derthorstenbeier@gmail.com>
1 parent 07d0574 commit b121f84

10 files changed

Lines changed: 239 additions & 79 deletions

File tree

.github/workflows/main.yml

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,13 @@ jobs:
6969
run: xpython --version
7070

7171
- name: Test xeus-python C++
72-
run: ./test_xeus_python
73-
timeout-minutes: 4
72+
run: |
73+
for i in $(seq 1 10);
74+
do
75+
echo "tests run $i"
76+
./test_xeus_python
77+
done
78+
timeout-minutes: 40
7479
working-directory: build/test
7580

7681
- name: Test xeus-python Python
@@ -139,8 +144,10 @@ jobs:
139144
- name: Test xeus-python C++
140145
shell: cmd /C call {0}
141146
run: |
142-
micromamba activate xeus-python
143-
test_xeus_python
144-
timeout-minutes: 4
147+
for /L %%i in (1,1,20) do (
148+
echo tests run %%i
149+
test_xeus_python
150+
)
151+
timeout-minutes: 40
145152
working-directory: build\test
146153

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ docs/build/
4444

4545
# Build directory
4646
build/
47+
/build-*/
4748

4849
# generated kernel specs
4950
/share/jupyter/kernels/xpython/kernel.json

environment-dev.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,7 @@ dependencies:
2525
- jupyter_kernel_test<0.8
2626
- doctest
2727
- pluggy=1.3
28+
- libboost
29+
- libboost-devel
30+
- libboost-headers
31+

include/xeus-python/xdebugger.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <memory>
2121
#include <mutex>
2222
#include <set>
23+
#include <thread>
2324

2425
#include "nlohmann/json.hpp"
2526
#include "pybind11/pybind11.h"

src/main.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,32 @@ int main(int argc, char* argv[])
7777
PyConfig_InitPythonConfig(&config);
7878
// config.isolated = 1;
7979

80+
constexpr std::string_view python_path_help = "PYTHONHOME or PYTHON_EXECUTABLE environment variables can be used to specify the correct path";
81+
const auto fail_with_error_message = [](const auto&... message_parts) {
82+
std::stringstream message_stream;
83+
((message_stream << message_parts), ...);
84+
auto message = message_stream.str();
85+
std::cerr << message << std::endl;
86+
throw std::runtime_error(std::move(message));
87+
};
88+
8089
// Setting Program Name
8190
static const std::string executable(xpyt::get_python_path());
8291
static const std::wstring wexecutable(executable.cbegin(), executable.cend());
92+
if (!std::filesystem::exists(wexecutable))
93+
{
94+
fail_with_error_message("cannot find python executable, tried '", executable, "' - ", python_path_help);
95+
}
8396
config.program_name = const_cast<wchar_t*>(wexecutable.c_str());
8497

8598
// Setting Python Home
8699
static const std::string pythonhome{ xpyt::get_python_prefix() };
87-
static const std::wstring wstr(pythonhome.cbegin(), pythonhome.cend());;
88-
config.home = const_cast<wchar_t*>(wstr.c_str());
100+
static const std::wstring wpythonhome(pythonhome.cbegin(), pythonhome.cend());
101+
if (!std::filesystem::exists(wpythonhome))
102+
{
103+
fail_with_error_message("cannot find python home directory, tried '", pythonhome, "' - ", python_path_help);
104+
}
105+
config.home = const_cast<wchar_t*>(wpythonhome.c_str());
89106
xpyt::print_pythonhome();
90107

91108
// Implicitly pre-initialize Python

src/xdebugger.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ namespace xpyt
7272

7373
debugger::~debugger()
7474
{
75+
// release/destroy the debugger python object while GIL is acquired
76+
pybind11::gil_scoped_acquire gil_lock;
77+
m_pydebugger.dec_ref();
7578
}
7679

7780
nl::json debugger::inspect_variables_request(const nl::json& message)
@@ -286,7 +289,7 @@ namespace xpyt
286289
py::gil_scoped_acquire acquire;
287290
py::module xeus_python_shell = py::module::import("xeus_python_shell.debugger");
288291
m_pydebugger = xeus_python_shell.attr("XDebugger")();
289-
292+
290293
// Get debugpy version
291294
std::string expression = "debugpy.__version__";
292295
std::string version = (eval(py::str(expression))).cast<std::string>();

src/xinterpreter.cpp

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -342,29 +342,52 @@ namespace xpyt
342342

343343
// Reset traceback
344344
m_ipython_shell.attr("last_error") = py::none();
345-
346345
try
347346
{
348347
exec(py::str(code));
349348
return xeus::create_successful_reply();
350349
}
351350
catch (py::error_already_set& e)
352351
{
353-
// This will grab the latest traceback and set shell.last_error
354-
m_ipython_shell.attr("showtraceback")();
355-
356-
py::list pyerror = m_ipython_shell.attr("last_error");
357-
358-
xerror error = extract_error(pyerror);
352+
try{
353+
// This will grab the latest traceback and set shell.last_error
354+
m_ipython_shell.attr("showtraceback")();
355+
py::list pyerror = m_ipython_shell.attr("last_error");
356+
xerror error = extract_error(pyerror);
359357

360-
publish_execution_error(error.m_ename, error.m_evalue, error.m_traceback);
361-
error.m_traceback.resize(1);
362-
error.m_traceback[0] = code;
358+
publish_execution_error(error.m_ename, error.m_evalue, error.m_traceback);
363359

364-
return xeus::create_error_reply(error.m_ename, error.m_evalue, error.m_traceback);
360+
error.m_traceback.resize(1);
361+
error.m_traceback[0] = code;
362+
return xeus::create_error_reply(error.m_ename, error.m_evalue, error.m_traceback);
363+
}
364+
catch (py::error_already_set& e)
365+
{
366+
std::cerr << "an error occurred during error handling: "<<e.what()<<std::endl;
367+
return xeus::create_error_reply("ErrorDuringErrorHandling", e.what(), std::vector<std::string>());
368+
}
369+
catch(std::exception& e)
370+
{
371+
std::cerr << "a standard exception occurred during error handling: "<<e.what()<<std::endl;
372+
return xeus::create_error_reply("ExceptionDuringErrorHandling", e.what(), std::vector<std::string>());
373+
}
374+
catch (...)
375+
{
376+
std::cerr << "an unknown error occurred during error handling"<<std::endl;
377+
return xeus::create_error_reply("UnknownErrorDuringErrorHandling", "", std::vector<std::string>());
378+
}
379+
}
380+
catch(std::exception& e)
381+
{
382+
return xeus::create_error_reply("Exception", e.what(), std::vector<std::string>());
383+
}
384+
catch (...)
385+
{
386+
return xeus::create_error_reply("UnknownError", "", std::vector<std::string>());
365387
}
366388
}
367389

390+
368391
void interpreter::set_request_context(xeus::xrequest_context context)
369392
{
370393
py::gil_scoped_acquire acquire;

src/xpaths.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ namespace xpyt
5252

5353
std::string get_python_path()
5454
{
55+
const char* python_exe_environment = std::getenv("PYTHON_EXECUTABLE");
56+
if (python_exe_environment != nullptr && std::strlen(python_exe_environment) != 0)
57+
{
58+
static const std::string python_exe_path = python_exe_environment;
59+
return python_exe_path;
60+
}
61+
5562
std::string python_prefix = get_python_prefix();
5663
#ifdef _WIN32
5764
if (python_prefix.back() != '\\')

test/CMakeLists.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ endif()
4545
find_package(Threads)
4646
find_package(doctest)
4747

48+
cmake_policy(SET CMP0167 OLD) # use find boost from old cmake
49+
find_package(Boost REQUIRED COMPONENTS process filesystem)
50+
4851
include_directories(${GTEST_INCLUDE_DIRS} SYSTEM)
4952

5053
set(XEUS_PYTHON_TESTS
@@ -70,7 +73,7 @@ set_target_properties(test_xeus_python PROPERTIES
7073
)
7174

7275
include_directories(${PYTHON_INCLUDE_DIRS})
73-
target_link_libraries(test_xeus_python ${PYTHON_LIBRARIES} xeus-zmq doctest::doctest ${CMAKE_THREAD_LIBS_INIT})
76+
target_link_libraries(test_xeus_python ${PYTHON_LIBRARIES} xeus-zmq doctest::doctest Boost::headers Boost::filesystem Boost::process ${CMAKE_THREAD_LIBS_INIT})
7477
target_include_directories(test_xeus_python PRIVATE ${XEUS_PYTHON_INCLUDE_DIR})
7578

7679
add_custom_target(xtest COMMAND test_xeus_python DEPENDS test_xeus_python)

0 commit comments

Comments
 (0)