Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

third-party: update libuv to v1.32.0 #10978

Merged
merged 3 commits into from
Sep 25, 2019
Merged

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Sep 9, 2019

Waiting for new release for OpenBSD build failure: libuv/libuv#2458

TODO:

@blueyed
Copy link
Contributor Author

blueyed commented Sep 9, 2019

Build fails on OpenBSD (should be reported).
Tests fail on Windows (merged tag, not tried latest v1.x there).

@blueyed blueyed changed the title third-party: update libuv to v1.31.0 [WIP] third-party: update libuv to v1.31.0 Sep 9, 2019
@marvim marvim added the WIP label Sep 9, 2019
@justinmk
Copy link
Member

justinmk commented Sep 9, 2019

Build fails on OpenBSD (should be reported).

Failed installing luajit?

ldconfig: unknown option -- n

@erw7
Copy link
Contributor

erw7 commented Sep 10, 2019

Tests fail on Windows (merged tag, not tried latest v1.x there).

I think this is the effect of libuv/libuv#2324. I think we need to add b to mode of lua's io.open etc.

@blueyed
Copy link
Contributor Author

blueyed commented Sep 10, 2019

@justinmk

ldconfig: unknown option -- n

That's likely another issue (but apparently not fatal - filed in LuaJIT/LuaJIT#515).
The error with libuv is libuv/libuv#2466.

blueyed added a commit to blueyed/neovim that referenced this pull request Sep 10, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Sep 10, 2019

Re Windows test failures: fixed a test, most others are related to shada.
Also ":set bin" appears be affected? (test/functional/legacy/011_autocommands_spec.lua)

Would be good if somebody having Windows locally could pick it up from here.

@justinmk
Copy link
Member

justinmk commented Sep 10, 2019

Also ":set bin" appears be affected? (test/functional/legacy/011_autocommands_spec.lua)

Sounds like #10978 (comment) right?

Could we just do _set_fmode(_O_BINARY) somewhere on startup (for Windows only)? I.e. revert libuv/libuv#2324

@erw7
Copy link
Contributor

erw7 commented Sep 11, 2019

I tried the following changes and passed the test in my environment.

diff --git a/test/functional/preload.lua b/test/functional/preload.lua
index 1107b45d5..ff61c8ada 100644
--- a/test/functional/preload.lua
+++ b/test/functional/preload.lua
@@ -2,3 +2,12 @@
 -- Busted started doing this to help provide more isolation.  See issue #62
 -- for more information about this.
 local helpers = require('test.functional.helpers')(nil)
+local iswin = helpers.iswin
+if iswin() then
+  local ffi = require('ffi')
+  ffi.cdef[[
+  typedef int errno_t;
+  errno_t _set_fmode(int mode);
+  ]]
+  ffi.C._set_fmode(0x8000)
+end

@erw7
Copy link
Contributor

erw7 commented Sep 11, 2019

By the way, libuv v1.32.0 has been released. libuv/libuv#2419 is included, so if there is no problem, it is better to use v1.32.0. As a result, #10734 can be reverted.

@justinmk justinmk added the core label Sep 11, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Sep 11, 2019

By the way, libuv v1.32.0 has been released.

There are conflicts with our nvim branch when trying to merge the tag:

```diff diff --cc src/win/tty.c index 63816759,8f84bcd0..00000000 --- i/src/win/tty.c +++ w/src/win/tty.c @@@ -135,10 -117,11 +135,12 @@@ static int uv_tty_virtual_width = -1 * handle signalling SIGWINCH */

-static HANDLE uv__tty_console_handle = INVALID_HANDLE_VALUE;
+static HANDLE uv__tty_console_output_handle = INVALID_HANDLE_VALUE;
+static HANDLE uv__tty_console_input_handle = INVALID_HANDLE_VALUE;
static int uv__tty_console_height = -1;
static int uv__tty_console_width = -1;

  • static HANDLE uv__tty_console_resized = INVALID_HANDLE_VALUE;

  • static uv_mutex_t uv__tty_console_resize_mutex;

    static DWORD WINAPI uv__tty_console_resize_message_loop_thread(void* param);
    static void CALLBACK uv__tty_console_resize_event(HWINEVENTHOOK hWinEventHook,
    @@@ -197,14 -172,8 +201,15 @@@ void uv_console_init(void)
    QueueUserWorkItem(uv__tty_console_resize_message_loop_thread,
    NULL,
    WT_EXECUTELONGFUNCTION);

  • uv_mutex_init(&uv__tty_console_resize_mutex);
    

    }

  • uv__tty_console_input_handle = CreateFileW(L"CONIN$",

  •                                         GENERIC_READ | GENERIC_WRITE,
    
  •                                         FILE_SHARE_READ,
    
  •                                         0,
    
  •                                         OPEN_EXISTING,
    
  •                                         0,
    
  •                                         0);
    

}

@@@ -817,186 -733,14 +822,200 @@@ void uv_process_tty_read_raw_req(uv_loo
}
records_left--;

++<<<<<<< HEAD

  •  /* Ignore other events that are not key or mouse events. */
    
  •  if (handle->tty.rd.last_input_record.EventType != KEY_EVENT &&
    
  •      handle->tty.rd.last_input_record.EventType != MOUSE_EVENT) {
    
  •    continue;
    
  •  }
    
  •  /* See https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking */
    
  •  if (handle->tty.rd.last_input_record.EventType == MOUSE_EVENT &&
    
  •      uv__tty_mouse_mode) {
    
  •    COORD pos;
    
  •    char button, c, fbyte = 'M';
    
  •    static COORD old_pos = {-1, -1};
    
  •    static char old_button = -1;
    
  •    static DWORD number_of_buttons = 0;
    
  •    int i;
    
  •    uv_sem_wait(&uv_tty_output_lock);
    
  •    if (number_of_buttons == 0 &&
    
  •        !GetNumberOfConsoleMouseButtons(&number_of_buttons)) {
    
  •      number_of_buttons = 2;
    
  •    }
    
  •    button = old_button;
    
  •    switch (MEV.dwEventFlags) {
    
  •      case 0:
    
  •        /* release mouse button event */
    
  •        if ((MEV.dwButtonState & ((1 << 5) - 1)) == 0) {
    
  •          /* Ignore if X10 cmpatible mode or button is not pressed */
    
  •          if (old_button == -1 || uv__tty_mouse_mode == UV_TTY_MOUSE_MODE_X10) {
    
  •            old_button = -1;
    
  •            uv_sem_post(&uv_tty_output_lock);
    
  •            continue;
    
  •          }
    
  •          if (uv__tty_mouse_enc == UV_TTY_MOUSE_ENC_SGR) {
    
  •            fbyte = 'm';
    
  •          } else {
    
  •            button = 4;
    
  •          }
    
  •          old_button = -1;
    
  •        } else {
    
  •          /* press mouse button event */
    
  •          if (button != -1) {
    
  •            uv_sem_post(&uv_tty_output_lock);
    
  •            continue;
    
  •          }
    
  •          switch (MEV.dwButtonState) {
    
  •            case FROM_LEFT_1ST_BUTTON_PRESSED:
    
  •              button = 0;
    
  •              break;
    
  •            case FROM_LEFT_2ND_BUTTON_PRESSED:
    
  •              button = 1;
    
  •              break;
    
  •            case RIGHTMOST_BUTTON_PRESSED:
    
  •              button = number_of_buttons > 2 ? 2 : 1;
    
  •              break;
    
  •            default:
    
  •              /* Ignore because does not support more than 4 buttons */
    
  •              old_button = -1;
    
  •              uv_sem_post(&uv_tty_output_lock);
    
  •              continue;
    
  •          }
    
  •          old_button = button;
    
  •        }
    
  •        pos = uv_tty_make_mouse_coord(MEV);
    
  •        break;
    
  •      case MOUSE_MOVED:
    
  •        /* Ignore moved events in X10 compatibility mode and Normal
    
  •        * tracking mode.
    
  •        * Ignore moved events if button is not pressed in Button-event
    
  •        * tracking mode. */
    
  •        if (uv__tty_mouse_mode == UV_TTY_MOUSE_MODE_X10 ||
    
  •            uv__tty_mouse_mode == UV_TTY_MOUSE_MODE_NORMAL ||
    
  •            (uv__tty_mouse_mode == UV_TTY_MOUSE_MODE_BT && old_button == -1)) {
    
  •          uv_sem_post(&uv_tty_output_lock);
    
  •          continue;
    
  •        }
    
  •        /* Ignore moved events that occurred in same cell as before */
    
  •        pos = uv_tty_make_mouse_coord(MEV);
    
  •        if (pos.X == old_pos.Y && pos.Y == old_pos.Y) {
    
  •          uv_sem_post(&uv_tty_output_lock);
    
  •          continue;
    
  •        }
    
  •        button = old_button + 32;
    
  •        break;
    
  •      case MOUSE_WHEELED:
    
  •        /* If high word of dwButtonState is positive then forward rotation,
    
  •         * otherwise backword rotation. Therefore, backward rotation if the
    
  •         * most significant bit is set, forward rotation otherwise.*/
    
  •        if (((DWORD)1 << 31) & MEV.dwButtonState) {
    
  •          button = 65;
    
  •        } else {
    
  •          button = 64;
    
  •        }
    
  •        pos = uv_tty_make_mouse_coord(MEV);
    
  •        /* Wheel evnet doesn't cause a button relase event. */
    
  •        old_button = -1;
    
  •        break;
    
  •      default:
    
  •        uv_sem_post(&uv_tty_output_lock);
    
  •        continue;
    
  •    }
    
  •    old_pos = pos;
    
  •    if (uv__tty_mouse_mode != UV_TTY_MOUSE_MODE_X10) {
    
  •      if (MEV.dwControlKeyState == SHIFT_PRESSED) {
    
  •        button += 4;
    
  •      }
    
  •      if (MEV.dwControlKeyState == LEFT_ALT_PRESSED ||
    
  •          MEV.dwControlKeyState == RIGHT_ALT_PRESSED) {
    
  •        button += 8;
    
  •      }
    
  •      if (MEV.dwControlKeyState == LEFT_CTRL_PRESSED ||
    
  •          MEV.dwControlKeyState == RIGHT_CTRL_PRESSED) {
    
  •        button += 16;
    
  •      }
    
  •    }
    
  •    if (uv__tty_mouse_enc != UV_TTY_MOUSE_ENC_SGR) {
    
  •      /* Add ' '(32) to convert it to printable characters */
    
  •      button += ' ';
    
  •    }
    
  •    switch (uv__tty_mouse_enc) {
    
  •      case UV_TTY_MOUSE_ENC_X10:
    
  •        /* CSI M CbCxCy */
    
  •        snprintf(handle->tty.rd.last_key, 32, "\033[%c%c%c%c",
    
  •            fbyte, button, (unsigned char)pos.X, (unsigned char)pos.Y);
    
  •        break;
    
  •      case UV_TTY_MOUSE_ENC_EXT:
    
  •        /* CSI M CbCxCy */
    
  •        snprintf(handle->tty.rd.last_key, 32, "\033[%c%c", fbyte, button);
    
  •        i = strnlen(handle->tty.rd.last_key, 32);
    
  •        if (pos.X < 256) {
    
  •          /* ASCII */
    
  •          handle->tty.rd.last_key[i] = (unsigned char)pos.X;
    
  •          i++;
    
  •        } else {
    
  •          /* Encode upper 5bits to UTF-8 */
    
  •          c = (unsigned char)(0xC0 + ((pos.X >> 6) & 0x1F));
    
  •          handle->tty.rd.last_key[i] = c;
    
  •          i++;
    
  •          /* Encode lower 6bits to UTF-8 */
    
  •          c = (unsigned char)(0x80 + (pos.X & 0x3F));
    
  •          handle->tty.rd.last_key[i] = c;
    
  •          i++;
    
  •        }
    
  •        if (pos.Y < 256) {
    
  •          /* ASCII */
    
  •          handle->tty.rd.last_key[i] = (unsigned char)pos.Y;
    
  •          i++;
    
  •        } else {
    
  •          /* Encode upper 5bits to UTF-8 */
    
  •          c = (unsigned char)(0xC0 + ((pos.Y >> 6) & 0x1F));
    
  •          handle->tty.rd.last_key[i] = c;
    
  •          i++;
    
  •          /* Encode lower 6bits to UTF-8 */
    
  •          c = (unsigned char)(0x80 + (pos.Y & 0x3F));
    
  •          handle->tty.rd.last_key[i] = c;
    
  •          i++;
    
  •        }
    
  •        handle->tty.rd.last_key[i] = '\0';
    
  •        break;
    
  •      case UV_TTY_MOUSE_ENC_SGR:
    
  •        /* CSI  < Cb ; Cx ; Cy M or m */
    
  •        snprintf(handle->tty.rd.last_key, 32, "\033[<%d;%d;%d%c",
    
  •            button, pos.X, pos.Y, fbyte);
    
  •        break;
    
  •      case UV_TTY_MOUSE_ENC_URXVT:
    
  •        /* CSI Cb ; Cx ; Cy M */
    
  •        snprintf(handle->tty.rd.last_key, 32, "\033[%d;%d;%d%c",
    
  •            button, pos.X, pos.Y, fbyte);
    
  •        break;
    
  •    }
    
  •    handle->tty.rd.last_key_len = strnlen(handle->tty.rd.last_key, 32);
    
  •    handle->tty.rd.last_key_offset = 0;
    
  •    uv_sem_post(&uv_tty_output_lock);
    

++||||||| merged common ancestors
++ /* Ignore other events that are not key events. */
++ if (handle->tty.rd.last_input_record.EventType != KEY_EVENT) {
++=======

  •   /* We might be not subscribed to EVENT_CONSOLE_LAYOUT or we might be
    
  •    * running under some TTY emulator that does not send those events. */
    
  •   if (handle->tty.rd.last_input_record.EventType == WINDOW_BUFFER_SIZE_EVENT) {
    
  •     uv__tty_console_signal_resize();
    
  •   }
    
  •   /* Ignore other events that are not key events. */
    
  •   if (handle->tty.rd.last_input_record.EventType != KEY_EVENT) {
    

++>>>>>>> v1.32.0
continue;
}

@@@ -3054,31 -2368,19 +3096,43 @@@ static void uv__tty_console_signal_resi
CONSOLE_SCREEN_BUFFER_INFO sb_info;
int width, height;

  • if (!GetConsoleScreenBufferInfo(uv__tty_console_handle, &sb_info))
  • uv_sem_wait(&uv_tty_output_lock);
  • if (!GetConsoleScreenBufferInfo(uv__tty_console_output_handle, &sb_info)) {
  • uv_sem_post(&uv_tty_output_lock);
    return;
  • }
  • width = sb_info.dwSize.X;
  • if (uv__tty_console_buffer == NULL) {

  • width = sb_info.dwSize.X;

  • } else {

  • width = sb_info.srWindow.Right - sb_info.srWindow.Left + 1;

  • }
    height = sb_info.srWindow.Bottom - sb_info.srWindow.Top + 1;

  • uv_mutex_lock(&uv__tty_console_resize_mutex);
    if (width != uv__tty_console_width || height != uv__tty_console_height) {
    uv__tty_console_width = width;
    uv__tty_console_height = height;
    ++<<<<<<< HEAD

  • if (uv__tty_console_buffer != NULL) {

  •  COORD buffer_size;
    
  •  buffer_size.X = width;
    
  •  buffer_size.Y = height;
    
  •  SetConsoleScreenBufferSize(uv__tty_console_output_handle, buffer_size);
    
  • }

  • uv_sem_post(&uv_tty_output_lock);
    ++||||||| merged common ancestors
    ++=======

  • uv_mutex_unlock(&uv__tty_console_resize_mutex);
    

++>>>>>>> v1.32.0
uv__signal_dispatch(SIGWINCH);
++<<<<<<< HEAD

  • return;
    ++||||||| merged common ancestors
    ++=======
  • } else {
  • uv_mutex_unlock(&uv__tty_console_resize_mutex);
    

++>>>>>>> v1.32.0
}

  • uv_sem_post(&uv_tty_output_lock);
    }
</details>

@erw7
Copy link
Contributor

erw7 commented Sep 12, 2019

There are conflicts with our nvim branch when trying to merge the tag:

I created neovim/libuv#10.

@blueyed blueyed changed the title [WIP] third-party: update libuv to v1.31.0 [WIP] third-party: update libuv to v1.32.0 Sep 12, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Sep 12, 2019

Failures on Windows:

[ RUN      ] :terminal buffer sends data to the terminal when the ":put" command is used: 79.00 ms OK
test\functional\ui\screen.lua:587: Row 2 did not match.
Expected:
  |tty ready                                         |
  |*{1: }                                                 |
  |*                                                  |
  |                                                  |
  |                                                  |
  |                                                  |
  |{3:-- TERMINAL --}                                    |
Actual:
  |tty ready                                         |
  |*rows: 6, cols: 50                                 |
  |*{1: }                                                 |
  |                                                  |
  |                                                  |
  |                                                  |
  |{3:-- TERMINAL --}                                    |
To print the expect() call that would assert the current screen state, use
screen:snapshot_util(). In case of non-deterministic failures, use
screen:redraw_debug() to show all intermediate screen states.  
stack traceback:
	test\functional\ui\screen.lua:587: in function '_wait'
	test\functional\ui\screen.lua:370: in function 'expect'
	test\functional\terminal\helpers.lua:94: in function 'screen_setup'
	test/functional\terminal\buffer_spec.lua:16: in function <test/functional\terminal\buffer_spec.lua:12>
[ RUN      ] :terminal buffer handles loss of focus gracefully: 125.00 ms OK
[ RUN      ] :terminal buffer term_close() use-after-free #4393: 0.00 ms OK
[ RUN      ] :terminal buffer handles confirmations with :confirm: 15.00 ms OK
[ RUN      ] :terminal buffer handles confirmations with &confirm: 47.00 ms OK
[ RUN      ] No heap-buffer-overflow when using termopen(echo) #3161: 78.00 ms OK
[----------] 11 tests from test/functional\terminal\buffer_spec.lua (13312.00 ms total)
[----------] Running tests from test/functional\terminal\cursor_spec.lua
test\functional\ui\screen.lua:587: Row 2 did not match.
Expected:
  |tty ready                                         |
  |*{1: }                                                 |
  |*                                                  |
  |                                                  |
  |                                                  |
  |                                                  |
  |{3:-- TERMINAL --}                                    |
Actual:
  |tty ready                                         |
  |*rows: 6, cols: 50                                 |
  |*{1: }                                                 |
  |                                                  |
  |                                                  |
  |                                                  |
  |{3:-- TERMINAL --}                                    |
To print the expect() call that would assert the current screen state, use
screen:snapshot_util(). In case of non-deterministic failures, use
screen:redraw_debug() to show all intermediate screen states.  
stack traceback:
	test\functional\ui\screen.lua:587: in function '_wait'
	test\functional\ui\screen.lua:370: in function 'expect'
	test\functional\terminal\helpers.lua:94: in function 'screen_setup'
	test/functional\terminal\cursor_spec.lua:15: in function <test/functional\terminal\cursor_spec.lua:13>

@blueyed
Copy link
Contributor Author

blueyed commented Sep 12, 2019

rows: 6, cols: 50

This is coming/e via SIGWINCH.
btw: might be good to prefix it ("WINCH: ") to make this more obvious (it just fits with the existing tests in test/functional/terminal/mouse_spec.lua then. What do you think?

@blueyed
Copy link
Contributor Author

blueyed commented Sep 12, 2019

SIGWINCH related failures on Windows are not flaky. Likely related to libuv/libuv@7d950c0d not being merged properly (https://github.com/neovim/libuv/pull/10/files#r323690684).

Also 'exists() handles empty env variable' fails (behaves like Unix now): https://github.com/blueyed/neovim/blob/a2899d88516a2fea50d2a9f33d08179749749307/test/functional/eval/environ_spec.lua#L14-L18
Likely via libuv/libuv@9e80057.
Should we adjust the test? This results in different behavior to Vim on Windows, but I think it's OK (and Vim/Bram refused to take a patch for this ("undefined behavior")).

@justinmk
Copy link
Member

Also 'exists() handles empty env variable' fails (behaves like Unix now): https://github.com/blueyed/neovim/blob/a2899d88516a2fea50d2a9f33d08179749749307/test/functional/eval/environ_spec.lua#L14-L18
Likely via libuv/libuv@9e80057.
Should we adjust the test? This results in different behavior to Vim on Windows, but I think it's OK

Sure. Can always change it later.

btw: might be good to prefix it ("WINCH: ") to make this more obvious (it just fits with the existing tests in test/functional/terminal/mouse_spec.lua then. What do you think

Sounds good.

@erw7
Copy link
Contributor

erw7 commented Sep 13, 2019

Also 'exists() handles empty env variable' fails (behaves like Unix now): https://github.com/blueyed/neovim/blob/a2899d88516a2fea50d2a9f33d08179749749307/test/functional/eval/environ_spec.lua#L14-L18
Likely via libuv/libuv@9e80057.
Should we adjust the test? This results in different behavior to Vim on Windows, but I think it's OK

There seems to be a misunderstanding as far as the comment is seen. When clear({env={EMPTY_VAR = ""}}) is used, the environment variable block that libuv passes to the child process includes the empty environment variable EMPTY_VAR. Therefore, exists('$ EMPTY_VAR') returns 1 even on Windows. If you use command ("let $ EMPTY_VAR=''"), exists('$EMPTY_VAR') returns 0 on Windows.

I haven't tried it, but it should work in vim as well. However, since there is no way to create an empty environment variable in vim tests, the same tests that use neovim clear({env={EMPTY_VAR = ""}}) cannot be performed.

@blueyed
Copy link
Contributor Author

blueyed commented Sep 24, 2019

@erw7
Thanks for the explanation.

I am not sure though if you agree that the change to os_env_exists (https://github.com/neovim/neovim/pull/10978/files#diff-145b92bbfb361cae1385b1efb2dea428) is expected / good.
And/or if this PR is good to be merged then altogether.

@erw7
Copy link
Contributor

erw7 commented Sep 25, 2019

LGTM.

@blueyed blueyed changed the title [WIP] third-party: update libuv to v1.32.0 third-party: update libuv to v1.32.0 Sep 25, 2019
@blueyed blueyed merged commit 2621f44 into neovim:master Sep 25, 2019
@blueyed blueyed deleted the update-libuv branch September 25, 2019 09:52
saghul pushed a commit to libuv/libuv that referenced this pull request Sep 25, 2019
Fix an issue where WINDOWS_BUFFER_SIZE_EVENT occurs and unexpected
SIGWINCH is received before calling
uv__tty_console_resize_message_loop_thread.

Refs: neovim/neovim#10978 (comment)
PR-URL: #2478
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
musm pushed a commit to musm/libuv that referenced this pull request Jul 9, 2020
Fix an issue where WINDOWS_BUFFER_SIZE_EVENT occurs and unexpected
SIGWINCH is received before calling
uv__tty_console_resize_message_loop_thread.

Refs: neovim/neovim#10978 (comment)
PR-URL: libuv#2478
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
(cherry picked from commit 5013042)
musm pushed a commit to musm/libuv that referenced this pull request Jul 9, 2020
Fix an issue where WINDOWS_BUFFER_SIZE_EVENT occurs and unexpected
SIGWINCH is received before calling
uv__tty_console_resize_message_loop_thread.

Refs: neovim/neovim#10978 (comment)
PR-URL: libuv#2478
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
(cherry picked from commit 5013042)
@clason clason added core Nvim core functionality or code and removed core labels Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Nvim core functionality or code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants