svinota / pyroute2

Python Netlink and PF_ROUTE library — network configuration and monitoring
https://pyroute2.org/
Other
930 stars 244 forks source link

Unconditionally importing readline causes curses resize handler to break #1161

Closed Gunni closed 4 months ago

Gunni commented 5 months ago

Hey,

We had an issue in our curses application after introducing pyroute2.

What happens is as far as we can tell:

  1. we import pyroute2
  2. it imports readline unconditionally because the Cli module uses it
  3. readline adds a SIGWINCH handler
  4. we import curses
  5. we start a curses wrapper
  6. curses wrapper checks if a SIGWINCH handler exists
  7. if it exists, it does nothing (breaking resizing)
  8. if it does not exist, it installs a resize handler

Here is a fairly minimal example script I found somewhere, observe how importing pyroute2 breaks resizing the window:

import sys,os
import curses

# comment this in/out
#import pyroute2
# this one is also affected
#import pyroute2.IPRoute

def draw_menu(stdscr):
    k = 0
    cursor_x = 0
    cursor_y = 0

    # Clear and refresh the screen for a blank canvas
    stdscr.clear()
    stdscr.refresh()

    # Start colors in curses
    curses.start_color()
    curses.init_pair(1, curses.COLOR_CYAN, curses.COLOR_BLACK)
    curses.init_pair(2, curses.COLOR_RED, curses.COLOR_BLACK)
    curses.init_pair(3, curses.COLOR_BLACK, curses.COLOR_WHITE)

    # Loop where k is the last character pressed
    while (k != ord('q')):

        # Initialization
        stdscr.clear()
        height, width = stdscr.getmaxyx()

        if k == curses.KEY_DOWN:
            cursor_y = cursor_y + 1
        elif k == curses.KEY_UP:
            cursor_y = cursor_y - 1
        elif k == curses.KEY_RIGHT:
            cursor_x = cursor_x + 1
        elif k == curses.KEY_LEFT:
            cursor_x = cursor_x - 1

        cursor_x = max(0, cursor_x)
        cursor_x = min(width-1, cursor_x)

        cursor_y = max(0, cursor_y)
        cursor_y = min(height-1, cursor_y)

        # Declaration of strings
        title = "Curses example"[:width-1]
        subtitle = "Written by Clay McLeod"[:width-1]
        keystr = "Last key pressed: {}".format(k)[:width-1]
        statusbarstr = "Press 'q' to exit | STATUS BAR | Pos: {}, {}".format(cursor_x, cursor_y)
        if k == 0:
            keystr = "No key press detected..."[:width-1]

        # Centering calculations
        start_x_title = int((width // 2) - (len(title) // 2) - len(title) % 2)
        start_x_subtitle = int((width // 2) - (len(subtitle) // 2) - len(subtitle) % 2)
        start_x_keystr = int((width // 2) - (len(keystr) // 2) - len(keystr) % 2)
        start_y = int((height // 2) - 2)

        # Rendering some text
        whstr = "Width: {}, Height: {}".format(width, height)
        stdscr.addstr(0, 0, whstr, curses.color_pair(1))

        # Render status bar
        stdscr.attron(curses.color_pair(3))
        stdscr.addstr(height-1, 0, statusbarstr)
        stdscr.addstr(height-1, len(statusbarstr), " " * (width - len(statusbarstr) - 1))
        stdscr.attroff(curses.color_pair(3))

        # Turning on attributes for title
        stdscr.attron(curses.color_pair(2))
        stdscr.attron(curses.A_BOLD)

        # Rendering title
        stdscr.addstr(start_y, start_x_title, title)

        # Turning off attributes for title
        stdscr.attroff(curses.color_pair(2))
        stdscr.attroff(curses.A_BOLD)

        # Print rest of text
        stdscr.addstr(start_y + 1, start_x_subtitle, subtitle)
        stdscr.addstr(start_y + 3, (width // 2) - 2, '-' * 4)
        stdscr.addstr(start_y + 5, start_x_keystr, keystr)
        stdscr.move(cursor_y, cursor_x)

        # Refresh the screen
        stdscr.refresh()

        # Wait for next input
        k = stdscr.getch()

def main():
    curses.wrapper(draw_menu)

if __name__ == "__main__":
    main()
svinota commented 5 months ago

Trying to reorganize the modules so the readline will not be imported unconditionally.

svinota commented 5 months ago

Pls check if this solution will work for you. Now the readline module is imported in the cli script only, so it doesn't affect the rest of the library.

svinota commented 5 months ago

If it works, then I merge it in the main branch and tag the next release.

svinota commented 4 months ago

Merging, 'cause it's a good idea to move the import anyways.

Gunni commented 4 months ago

Hey, just tested it with the code in https://github.com/svinota/pyroute2/pull/1165.

I can confirm that it works for my use-case.

Note that I did not test the cli module.

Note that this bug may also be fixed https://github.com/svinota/pyroute2/issues/625.

Please publish a new version on PyPi when you can!

svinota commented 4 months ago

Will be rolled out this weekend. Thanks for the feedback!