Skip to content

fixes#4

Merged
JohanMabille merged 31 commits intojupyter-xeus:mainfrom
DerThorsten:main
Jan 14, 2026
Merged

fixes#4
JohanMabille merged 31 commits intojupyter-xeus:mainfrom
DerThorsten:main

Conversation

@DerThorsten
Copy link
Member

@DerThorsten DerThorsten commented Dec 17, 2025

  • drain all messages instead of just one
  • remove controller handling from xuv_runner since that only needs to handle the shell. the controll messages are handled via the xcontrol_default_runner (thats intended, isnt it @IsabelParedes ?)
  • fix typo (XEUX_UV_API -> XEUS_UV_API )
  • renamed header for consitency (xeus-uv.hpp -> xeus_uv.hpp)
  • a critical missing #include "xeus-uv/xserver_uv.hpp" in "xeus-uv/xserver_uv.cpp". With that beeing missing, theXEUS_UV_API` visibility was not considered for that symbol which made that symbol hidden in the dll/so
  • use macos14 instead of deprecated macos13:
    • this means we drop mac-intel because macos13 where the last intel based mac runners, now its all arm

Open questions / points:

  • I am tempted to rename xhook_base to something more descriptive
  • windows tests seem to pass even when there are headers missing and it cannot build => windows seems to always pass even when the build should fail...

@DerThorsten DerThorsten changed the title minor refactorings minor refactor Dec 17, 2025
@DerThorsten DerThorsten changed the title minor refactor fixes Jan 13, 2026
@JohanMabille
Copy link
Member

JohanMabille commented Jan 13, 2026

Remove controller handling from xuv_runner since that only needs to handle the shell. the controll messages are handled via the xcontrol_default_runner (thats intended, isnt it @IsabelParedes ?)

The "controller" here is not the control channel, it is the socket used for internal communication between the control and the shell channels (typically used for stopping the shell thread, or for sending code to execute to start the debugger). It must not be removed.

@DerThorsten
Copy link
Member Author

Remove controller handling from xuv_runner since that only needs to handle the shell. the controll messages are handled via the xcontrol_default_runner (thats intended, isnt it @IsabelParedes ?)

The "controller" here is not the control channel, it is the socket used for internal communication between the control and the shell channels (typically used for stopping the shell thread, or for sending code to execute to start the debugger). It must not be removed.

Since the tests passed even without it, maybe we should extend the s.t. they would not pass without these handlers

@JohanMabille
Copy link
Member

I would have expected the shutdown_request test fail.

{
if (!p_loop)
{
std::cerr << "No loop provided, using default loop." << std::endl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...overenthusiastic cleanup

@DerThorsten
Copy link
Member Author

I would have expected the shutdown_request test fail.

but it was not failing...and restarting the kernel (ie python kernel) seemed to work just fine.
I did never see any message on the controll part of the runner, thats why I was removing it. So something seems to be odd there.

@JohanMabille
Copy link
Member

Indeed that's weird. Anyway, we'll need it for the debugger.

@JohanMabille JohanMabille merged commit 9067073 into jupyter-xeus:main Jan 14, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants