-
Notifications
You must be signed in to change notification settings - Fork 185
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
fix: Use os_uname() to check for Linux #686
Conversation
I think should use https://github.com/luvit/luv/blob/master/docs.md#uvos_uname |
I've changed it. |
The minimum supported libuv doesn't provide |
I don't think simply checking if local popen_handle = io.popen('uname -s')
local uname_os = assert(popen_handle:read('*a'))
popen_handle:close()
isLinux = uname_os:lower() == 'linux' I would also like to point out that in the current code it seems to me like |
I've updated it to use
|
Looks like |
Looks like IF(NOT WIN32)
ADD_DEFINITIONS(-DLUA_USE_POSIX)
ENDIF(NOT WIN32) to |
In build environments like Fedora's mockbuild, there is no OSTYPE environment variable set. Use a more secure method to detect Linux.
@squeek502 I think the CI tests need to be manually re-triggered? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a nice improvement to make the tests less dependent on the CI environment!
I've tested it with fedora mockbuild and the patch works fine. |
In build environments like Fedora's mockbuild, there is no OSTYPE environment variable set. Use a more secure method to detect Linux. Details:
man os-release
https://www.man7.org/linux/man-pages/man5/os-release.5.html