[RFC] Fix hang on job start caused by non-executable cwd option #9204

tweekmonster · 2018-11-05T22:53:31Z

Where should I add a test for this?

justinmk · 2018-11-05T23:45:59Z

Where should I add a test for this?

neovim/test/functional/core/job_spec.lua

Line 105 in 1347f36

it('fails to change to invalid `cwd`', function()

todo

add vim/vim@839e81e to NA list. version.c: update [ci skip] #9171

tweekmonster · 2018-11-06T04:13:37Z

@justinmk I'm having trouble understanding the QB failure. You have any insight?

bfredl · 2018-11-06T09:38:06Z

If you are speaking of the digraph failure, it is a know fragile test. Don't worry about it.

tweekmonster · 2018-11-06T16:37:47Z

@bfredl Thanks. I was actually confused because I couldn't see what failed. Now I know to look for not ok in the logs.

justinmk · 2018-11-06T20:26:15Z

src/nvim/os/fs.c

+    return false;
+  }
+
+#ifndef WIN32


we usually use "positive logic" (see is_executable), especially if both cases are covered. so #ifdef ... instead of #ifndef/

justinmk · 2018-11-06T20:27:34Z

test/functional/core/job_spec.lua

+    local _, err = pcall(function()
+      nvim('command', "call jobstart('pwd', {'cwd': '" .. dir .. "'})")
+    end)
+    ok(string.find(err, "E475: Invalid argument: expected valid directory$") ~= nil)


expect_err (in global_helpers.lua) makes this much easier.

expect_err('E475: Invalid argument: expected valid directory$', nvim, 'command', "call jobstart('pwd', {'cwd': '" .. dir .. "'})")

justinmk · 2018-11-06T20:32:01Z

test/functional/core/job_spec.lua

@@ -115,6 +115,19 @@ describe('jobs', function()
    ok(string.find(err, "E475: Invalid argument: expected valid directory$") ~= nil)
  end)

+  it('produces error when using non-executable `cwd`', function()
+    if iswin() then return end  -- N/A for Windows


Did the test fail on appveyor?

The Vim patch doesn't seem to skip Windows, although the problem in Nvim could be related to #8244 . (In that case, yes, let's skip for now)

I assumed that calling chmod would cause the test to fail and that making a directory executable on Windows wasn't really possible. Even if the test would pass, I figured it should be skipped on Windows because the description wouldn't make sense anyways.

jamessan · 2018-11-06T21:00:11Z

test/functional/core/job_spec.lua

+
+    local dir = 'Xtest_not_executable_dir'
+    mkdir(dir)
+    helpers.call('system', {'chmod', '0600', dir})


Why not use nvim's setfperm(..., 'rw-------')?

Because I copied that from another test 🤭

I'll use setfperm() because it looks cleaner.

vim-patch:8.0.0350: not enough test coverage for Perl vim-patch:8.0.1135: W_WINCOL() is always the same vim-patch:8.0.1280: Python None cannot be converted to a Vim type vim-patch:8.0.1308: the "Reading from stdin" message may be undesired vim-patch:8.0.1338: USE_IM_CONTROL is confusing and incomplete vim-patch:8.0.1343: MS-Windows: does not show colored emojis vim-patch:8.0.1350: cannot build with +eval and -multi_byte vim-patch:8.0.1829: MS-Windows: script for vimdiff can't handle ! chars vim-patch:8.0.1849: compiler warning for unused arguments, missing prototype vim-patch:8.1.0001: the netrw plugin does not work vim-patch:8.1.0006: syn_id2cterm_bg() may be undefined vim-patch:8.1.0012: misplaced #endif vim-patch:8.1.0021: clang warns for undefined behavior vim-patch:8.1.0041: attribute "width" missing from python window attribute list vim-patch:8.1.0051: MS-Windows: missing #endif vim-patch:8.1.0063: Mac: NSStringPboardType is deprecated vim-patch:8.1.0075: no Vim logo in README file vim-patch:8.1.0077: header of README file is not nice vim-patch:8.1.0079: superfluous space in messages vim-patch:8.1.0102: cannot build without syntax highlighting vim-patch:8.1.0104: can't build without the +eval feature vim-patch:8.1.0109: new po makefile missing from distribution vim-patch:8.1.0117: URL in install program still points to SourceForge vim-patch:8.1.0122: translators don't always understand the maintainer message vim-patch:8.1.0123: MS-Windows: colors are wrong after setting 'notgc' vim-patch:8.1.0124: has('vcon') returns true even for non-win32 terminal vim-patch:8.1.0127: build failure when disabling the session feature vim-patch:8.1.0128: building with MinGW does not work out-of-the-box vim-patch:8.1.0129: still some xterm-like terminals get a stray "p" vim-patch:8.1.0137: CI does not run with TCL vim-patch:8.1.0142: xterm and vt320 builtin termcap missing keypad keys vim-patch:8.1.0147: compiler warning when building with Python 3.7 vim-patch:8.1.0148: memory leak when using :tcl expr command vim-patch:8.1.0150: insufficient test coverage for Tcl vim-patch:8.1.0152: cannot easily run individual tests on MS-Windows vim-patch:8.1.0153: build with SHADOWDIR fails vim-patch:8.1.0155: evim.man missing from the distribution vim-patch:8.1.0157: old iTerm2 is not recognized, resulting in stray output vim-patch:8.1.0160: no Danish manual translations vim-patch:8.1.0162: Danish and German man pages are not installed vim-patch:8.1.0163: insufficient testing for Tcl vim-patch:8.1.0173: compiler warning on MS-Windows vim-patch:8.1.0176: overlapping string argument for strcpy() vim-patch:8.1.0178: warning for passing pointer to non-pointer argument vim-patch:8.1.0179: redundant condition for boundary check vim-patch:8.1.0180: static analysis errors in Lua interface vim-patch:8.1.0183: Lua API changed, breaking the build vim-patch:8.1.0185: running tests writes lua.vim even though it is not used vim-patch:8.1.0190: Perl refcounts are wrong vim-patch:8.1.0191: Perl test fails in 24 line terminal vim-patch:8.1.0197: Windows GUI: title for search/replace is wrong vim-patch:8.1.0201: newer Python uses "importlib" instead of "imp" vim-patch:8.1.0202: :version always shows +packages vim-patch:8.1.0203: building with Perl 5.28 fails on Windows vim-patch:8.1.0207: need many menu translation files to cover regions vim-patch:8.1.0209: stderr output from Ruby messes up display vim-patch:8.1.0215: no error if configure --with-x cannot configure X vim-patch:8.1.0217: compiler warning for variable set but not used vim-patch:8.1.0222: errors are reported for "make install" vim-patch:8.1.0232: Ruby error does not include backtrace vim-patch:8.1.0234: incorrect reference counting in Perl interface vim-patch:8.1.0236: Ruby build fails when ruby_intern is missing vim-patch:8.1.0237: Ruby on Cygwin doesn't always work vim-patch:8.1.0239: now Ruby build fails on other systems vim-patch:8.1.0246: build failure without the +eval feature vim-patch:8.1.0247: Python: error message for failing import is incorrect vim-patch:8.1.0249: GTK: when screen DPI changes Vim does not handle it vim-patch:8.1.0250: MS-Windows using VTP: windows size change incorrect vim-patch:8.1.0254: cannot build on MS-Windows; unused macro HAVE_HANDLE_DROP vim-patch:8.1.0260: no LGTM logo in README file vim-patch:8.1.0287: MAX is not defined everywhere vim-patch:8.1.0292: MS-Windows: the text "self-installing" confuses some users vim-patch:8.1.0301: GTK: input method popup displayed on wrong screen. vim-patch:8.1.0305: missing support for Lua 5.4 32 bits on Unix vim-patch:8.1.0319: bzero() function prototype doesn't work for Android vim-patch:8.1.0332: get Gdk-Critical error on first balloon show vim-patch:8.1.0346: building with Aap is outdated and unused vim-patch:8.1.0348: on Travis the slowest build is run last vim-patch:8.1.0357: instructions for tests are outdated vim-patch:8.1.0368: GTK code has too many #ifdefs and GTK 2.10 building fails vim-patch:8.1.0379: build dependencies are incomplete vim-patch:8.1.0380: "make proto" doesn't work well vim-patch:8.1.0383: missing source file rename vim-patch:8.1.0385: Coveralls badge doesn't update vim-patch:8.1.0386: cannot test with non-default option value vim-patch:8.1.0388: Coverity complains about possible NULL pointer use vim-patch:8.1.0390: scrollbars are not tested vim-patch:8.1.0391: building in a shadow directory fails vim-patch:8.1.0403: header file missing from distribution vim-patch:8.1.0405: too many #ifdefs for GTK vim-patch:8.1.0408: MSVC: cannot use the "x64" native compiler option vim-patch:8.1.0411: renamed file missing from distribution vim-patch:8.1.0412: cannot build with GTK 2.4 vim-patch:8.1.0413: test output is duplicated or missing vim-patch:8.1.0415: not actually using 16 colors with vtp vim-patch:8.1.0418: MS-Windows: cannot separate Lua include and library dirs vim-patch:8.1.0419: Cygwin: running cproto fails with -O2 vim-patch:8.1.0420: generating vim.lib when using ActivePerl 5.20.3 or later vim-patch:8.1.0421: MS-Windows: Ruby path is wrong for Ruby 1.9 and later vim-patch:8.1.0422: cannot create map file with MinGW vim-patch:8.1.0427: MS-Windows GUI: using invalid encoded file name vim-patch:8.1.0441: build failure without command line history vim-patch:8.1.0467: cannot build with Mac OS X 10.5 vim-patch:8.1.0472: dosinst command has a few flaws vim-patch:8.1.0474: directory where if_perl.c is written is inconsistent vim-patch:8.1.0477: tiny build fails vim-patch:8.1.0478: cannot build with perl using MinGW vim-patch:8.1.0480: MinGW build file uses different -I flags than MVC vim-patch:8.1.0482: MinGW "make clean" deletes all .exe files vim-patch:8.1.0483: MinGW does not build tee.exe vim-patch:8.1.0490: MS-Windows: doesn't handle missing glibwinpthread-1.dll vim-patch:8.1.0492: "Edit with existing Vim" list can get long vim-patch:8.1.0500: cleaning up in src/tee may not always work vim-patch:8.1.0521: cannot build with +eval but without +quickfix vim-patch:8.1.0534: MS-Windows installer uses different $HOME than Vim vim-patch:8.1.0541: help message in dosinst.c is outdated vim-patch:8.1.0485: term_start() does not check if directory is accessible #9204

Besides the "visible" improvements, this release features numerous internal improvements to the UI/screen code and test infrastructure. Numerous patches were merged from Vim, which are not mentioned below. FEATURES: 07ad5d7 clipboard: Support custom VimL functions neovim#9304 725da1f neovim#9401 win/TUI: Improve terminal/console support 7a8dadb neovim#9077 startup: Use $XDG_CONFIG_DIRS/nvim/sysinit.vim if it exists feec926 neovim#9299 support <cmd> mapping in more places 0653ed6 neovim#9028 diff/highlight: Show underline for low-priority CursorLine bddcbbb signs: Add "numhl" argument neovim#9113 05f9c7c clipboard: support Wayland (neovim#9230) 14ae394 neovim#9052 TUI: add support for undercurl and underline color 4fa3492 neovim#9023 man.vim: soft (dynamic) wrap neovim#9023 API: 8b39e4e neovim#6920 API: implement object namespaces b1aaa0a API: Implement nvim_win_set_buf() neovim#9100 8de87c7 neovim#8180 API: virtual text annotations (nvim_buf_set_virtual_text) 2b9fc9a neovim#8660 API: add nvim_buf_is_loaded() API: buf_get_lines, buf_line_count handle unloaded buffers 88f77c2 API: nvim_buf_get_offset_for_line 94841e5 API/UI: neovim#8221 ext_newgrid, ext_hlstate (use line-based rather than char-based updates) UI b5cfac0 neovim#8806 TUI: use BCE again more often, (smoother resizes/scrolling) 77b5e9a neovim#9315 screen: add missing status redraw when redraw_later(CLEAR) was used 5f15788 TUI: clip invalid regions on resize (neovim#8779), fixes neovim#8774 c936ae0 neovim#9193 TUI: improvements for scrolling and clearing f204274 neovim#9143 UI: disable clearing almost everywhere f4b2b66 neovim#9079 TUI: always use safe cursor movement after resize d36afaf neovim#9211 ui_options: also send when starting or from OptionSet 67f80d4 TUI: Avoid reset_cursor_color in old VTE neovim#9191 e55ebae neovim#9021 don't erase screen on `:hi Normal` during startup c5790d9 neovim#8915 TUI: Hint wrapped lines to terminals. FIXES: 231de72 RPC: turn errors from async calls into notifications 907ad92 TUI: Restore terminal title via "title stacking" (neovim#9407) cb76a8a genappimage: Unset $ARGV0 at invocation neovim#9376 b48efd9 neovim#9347 TUI: FreeBSD: Improve support for BSD vt console c16529a TUI: Konsole 18.07.70 supports DECSCUSR (neovim#9364) aec096f os/lang: use the correct LC_NUMERIC also for OS X 5fee0be provider: improve error message (neovim#9344) 3c42d7a TUI: alacritty supports set_cursor_color neovim#9353 7bff9a5 TUI: Alacritty supports DECSCUSR (neovim#9048) 57acfce macOS: infer primary language if $LANG is empty neovim#9345 bc132ae runtime/syntax: Fix highlighting of augroup contents (neovim#9328) 715fdfe neovim#9297 VimL/confirm(): Show dialog even if :silent 799d9c3 clipboard: Prefer xclip (neovim#9302) 6dae777 provider/nodejs: fix npm,yarn detection 16bc1e9 neovim#9218 channel: avoid buffering output when only terminal and no callbacks are active 72fecad neovim#8804 Fix crash in lang_init() on macOS if lang_region = NULL d581398 ruby: detect rbenv shims for other versions (neovim#8733) e568ac7 neovim#9123 third-party/unibilium: Fix parsing of extended capability entries c4c74c3 jobstart(): Fix hang on non-executable cwd neovim#9204 1cf50cb provider/nodejs: Simultaneously query npm and yarn neovim#9054 6c496db undo: Fix infinite loop if undo_read_byte returns EOF neovim#2880 f8f8357 neovim#9034 'swapfile: always show dialog' CHANGES: c236e80 neovim#9024 --embed: wait for UI unless --headless 180b50d neovim#9248 python: 'neovim' module was renamed to 'pynvim' 2000b6a neovim#8589 VimL: Remove legacy aliases "v:errmsg", "v:shell_error", "v:this_session" deb18a0 defaults: background=dark neovim#2894 (neovim#9205) c1187d4 defaults: win: 'shellpipe' for cmd.exe (neovim#8827)

This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed.

This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed. (cherry picked from commit b94e2c8)

[Backport release-0.7] revert: "jobstart(): Fix hang on non-executable cwd #9204"

…vim#19826) This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed.

marvim added the RFC label Nov 5, 2018

tweekmonster requested review from justinmk and jamessan November 6, 2018 16:42

justinmk reviewed Nov 6, 2018

View reviewed changes

jamessan reviewed Nov 6, 2018

View reviewed changes

tweekmonster force-pushed the os_isdir_executable branch from 605e2c5 to 918ad95 Compare November 7, 2018 05:49

tweekmonster added 3 commits November 7, 2018 00:52

os/fs.c: add os_isdir_executable()

4ab2551

eval.c: fix hang on job start caused by non-executable cwd option

458dc19

channel.c: assert cwd is an executable directory

2377580

tweekmonster force-pushed the os_isdir_executable branch from 918ad95 to 23acdc1 Compare November 7, 2018 05:53

test: jobstart() produces error when using non-executable cwd

f330f5b

tweekmonster force-pushed the os_isdir_executable branch from 23acdc1 to f330f5b Compare November 7, 2018 05:57

justinmk merged commit c4c74c3 into neovim:master Nov 7, 2018

justinmk removed the RFC label Nov 7, 2018

justinmk mentioned this pull request Nov 7, 2018

version.c: update [ci skip] #9171

Merged

zeertzjq mentioned this pull request Aug 17, 2022

Assertion failure on accessing disconnected remote filesystem #19825

Closed

zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Aug 17, 2022

revert: "jobstart(): Fix hang on non-executable cwd neovim#9204"

b94e2c8

This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed.

zeertzjq added a commit that referenced this pull request Aug 18, 2022

revert: "jobstart(): Fix hang on non-executable cwd #9204" (#19826)

36f0b50

This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed.

github-actions bot pushed a commit that referenced this pull request Aug 18, 2022

revert: "jobstart(): Fix hang on non-executable cwd #9204"

a7f4d0a

This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed. (cherry picked from commit b94e2c8)

jamessan added a commit that referenced this pull request Aug 18, 2022

Merge pull request #19833 from neovim/backport-19826-to-release-0.7

6ec04e5

[Backport release-0.7] revert: "jobstart(): Fix hang on non-executable cwd #9204"

smjonas pushed a commit to smjonas/neovim that referenced this pull request Dec 31, 2022

revert: "jobstart(): Fix hang on non-executable cwd neovim#9204" (neo…

9c5d07c

…vim#19826) This reverts commit c4c74c3. LibUV already gives an error for this, so this isn't needed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[RFC] Fix hang on job start caused by non-executable cwd option #9204

[RFC] Fix hang on job start caused by non-executable cwd option #9204

Uh oh!

tweekmonster commented Nov 5, 2018

Uh oh!

justinmk commented Nov 5, 2018 •

edited

Loading

Uh oh!

tweekmonster commented Nov 6, 2018

Uh oh!

bfredl commented Nov 6, 2018

Uh oh!

tweekmonster commented Nov 6, 2018

Uh oh!

justinmk Nov 6, 2018

Uh oh!

justinmk Nov 6, 2018 •

edited

Loading

Uh oh!

justinmk Nov 6, 2018 •

edited

Loading

Uh oh!

tweekmonster Nov 7, 2018

Uh oh!

jamessan Nov 6, 2018

Uh oh!

tweekmonster Nov 7, 2018

Uh oh!

Uh oh!

Uh oh!

[RFC] Fix hang on job start caused by non-executable cwd option #9204

[RFC] Fix hang on job start caused by non-executable cwd option #9204

Uh oh!

Conversation

tweekmonster commented Nov 5, 2018

Uh oh!

justinmk commented Nov 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

todo

Uh oh!

tweekmonster commented Nov 6, 2018

Uh oh!

bfredl commented Nov 6, 2018

Uh oh!

tweekmonster commented Nov 6, 2018

Uh oh!

justinmk Nov 6, 2018

Choose a reason for hiding this comment

Uh oh!

justinmk Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tweekmonster Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

jamessan Nov 6, 2018

Choose a reason for hiding this comment

Uh oh!

tweekmonster Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

justinmk commented Nov 5, 2018 •

edited

Loading

justinmk Nov 6, 2018 •

edited

Loading

justinmk Nov 6, 2018 •

edited

Loading