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

tl .24.1 - bad argument to popen #839

Open
SlashScreen opened this issue Oct 25, 2024 · 9 comments
Open

tl .24.1 - bad argument to popen #839

SlashScreen opened this issue Oct 25, 2024 · 9 comments
Labels
not a Teal bug? This sounds like an issue somewhere else

Comments

@SlashScreen
Copy link

I just now updated tl to the latest version as of writing, and it gives me the following error:

[Error] C:\Users\talle\scoop\apps\lua\current\lua54.exe: ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:672: bad argument teal-language/vscode-teal#2 to 'popen' (invalid mode) stack traceback: [C]: in function 'io.popen' ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:672: in upvalue 'cd' ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:689: in upvalue 'normalize' ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:724: in upvalue 'already_loaded' ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:738: in field '?' ...\current\rocks\lib\luarocks\rocks-5.4\tl\0.24.1-1\bin\tl:977: in main chunk [C]: in ?

The line in neal seems to be:
local pd = io.popen("cd", "rb")

@pdesaulniers
Copy link
Member

What happens if you run tl check on the command line? Does it give you the same error?

@SlashScreen
Copy link
Author

Yes.

@pdesaulniers
Copy link
Member

pdesaulniers commented Oct 26, 2024

Glancing at the code, it seems like tl check fails when the PWD environment variable is not defined. The script tries to read the output of cd using rb mode, which doesn't appear to be supported in Lua 5.4.7.

Reproducing the issue on Linux:

$ unset PWD
$ tl check tl.tl
tl: [string "tl"]:671: bad argument #2 to 'popen' (invalid mode)
stack traceback:
        [C]: in function 'io.popen'
        [string "tl"]:671: in upvalue 'cd'
        [string "tl"]:688: in upvalue 'normalize'
        [string "tl"]:721: in upvalue 'already_loaded'
        [string "tl"]:737: in field '?'
        [string "tl"]:976: in main chunk
        [C]: in ?

(though I assume this code path was intended for Windows, since cd doesn't typically output the current directory on Linux)

@pdesaulniers pdesaulniers transferred this issue from teal-language/vscode-teal Oct 26, 2024
@hishamhm
Copy link
Member

@pdesaulniers Thanks for investigating!

(though I assume this code path was intended for Windows, since cd doesn't typically output the current directory on Linux)

...which seems to be the case for @SlashScreen. According to the Lua 5.4 source code, "rb" is supported by io.popen on Windows when the LUA_USE_WINDOWS define is set.

@SlashScreen How was this lua54.exe compiled?

Does the error happen when using the tl.exe build from the official release?

@hishamhm hishamhm added the needs more info Further information is requested label Oct 26, 2024
@SlashScreen
Copy link
Author

@hishamhm lua54.exe is from the scoop PM for windows.
using the tl.exe seems to work as expected, as well as somehow restoring functionality to the vscode plugin when put in the directory with the code.

@hishamhm
Copy link
Member

@SlashScreen in that case, my diagnostic is that somehow lua54.exe was compiled by scoop without the LUA_USE_WINDOWS define (which should have been autodetected in luaconf.h due to Windows compilers defining _WIN32). Looks like root cause for the problem lies in the scoop PM package, then.

@SlashScreen
Copy link
Author

Hmm, okay. I'll have to take a look at that, then. was this calling method used in previous versions of teal? Since earlier versions worked just fine.

@hishamhm
Copy link
Member

Hmm, okay. I'll have to take a look at that, then.

Thank you!

was this calling method used in previous versions of teal? Since earlier versions worked just fine.

No, they were missing the "b" option in open and popen calls on Windows, which we added in d210ff9.

@hishamhm hishamhm removed the needs more info Further information is requested label Oct 27, 2024
@SlashScreen
Copy link
Author

I'm unable to test another lua build, because I have screwed up my lua and luarocks installation. I'll try again once I fix it.

@hishamhm hishamhm added the not a Teal bug? This sounds like an issue somewhere else label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not a Teal bug? This sounds like an issue somewhere else
Projects
None yet
Development

No branches or pull requests

3 participants