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

V1.45 #649

Merged
merged 17 commits into from
May 20, 2023
Merged

V1.45 #649

merged 17 commits into from
May 20, 2023

Conversation

zhaozg
Copy link
Member

@zhaozg zhaozg commented May 20, 2023

close #646

@squeek502
Copy link
Member

squeek502 commented May 20, 2023

My suggestion would be to remove the update lua to v5.4.6 and update lua-compat-5.3 to HEAD commits from this branch to see if updating Lua is the cause of the Valgrind errors. If it is, then we should just do that separately from updating Libuv.

See #646 (comment)

@zhaozg zhaozg force-pushed the v1.45 branch 2 times, most recently from 28292f9 to 763f07e Compare May 20, 2023 07:38
@zhaozg
Copy link
Member Author

zhaozg commented May 20, 2023

Ok, change finished.

@squeek502
Copy link
Member

Created a pull request with the remaining stuff: zhaozg#5

These functions are public but currently undocumented. Will wait to bind them until they have documentation.
@zhaozg
Copy link
Member Author

zhaozg commented May 20, 2023

Good jobs, Let wait appveyor report status.

@zhaozg
Copy link
Member Author

zhaozg commented May 20, 2023

failed https://ci.appveyor.com/project/racker-buildbot/luv/builds/47095901

ok 58 misc - uv.version and uv.version_string
  .\tests/test-misc.lua:31: assertion failed!
  stack traceback:
  	[C]: in function 'assert'
  	.\tests/test-misc.lua:31: in function 'fn'
  	.\lib/tap.lua:59: in function <.\lib/tap.lua:48>
  	[C]: in function 'xpcall'
  	.\lib/tap.lua:48: in function 'run'
  	.\lib/tap.lua:146: in function 'tap'
  	tests\run.lua:23: in main chunk
  	[C]: at 0x01205430
not ok 59 misc - memory size

@squeek502
Copy link
Member

Strange, will figure that out in a bit. I'm currently on Linux reporting the valgrind libuv stuff 😄

@zhaozg
Copy link
Member Author

zhaozg commented May 20, 2023

Strange, will figure that out in a bit. I'm currently on Linux reporting the valgrind libuv stuff 😄

Maybe lua cast it as signed number on 32 bits arch. I do some check.

@squeek502
Copy link
Member

Looks like you're right. Changing luv_get_available_memory to use lua_pushnumber should fix it I think.

@squeek502
Copy link
Member

Looks like pushunsigned won't work, since the lua-compat-5.3 implementation just casts to lua_Integer anyway:

https://github.com/lunarmodules/lua-compat-5.3/blob/8f8e4c6adb43e107f5902e784ef207dc3c8ca06b/c-api/compat-5.3.h#L258-L259

I think lua_pushnumber is okay, it's what luv_get_free_memory, luv_get_total_memory, etc use.

@zhaozg
Copy link
Member Author

zhaozg commented May 20, 2023

ok, I will back in 20 minutes, outsides now.

Copy link
Member

@squeek502 squeek502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a bad bug in the code I wrote, will submit a fix in a few minutes.

@squeek502
Copy link
Member

zhaozg#6

squeek502 and others added 2 commits May 20, 2023 18:52
… of `uv_thread_t*`

This happened to work since the uv_thread_t is at the start of the luv_thread_t structs so their pointers happen to be equal, but it's definitely worth fixing.
Comment on lines +82 to +84
'Dbghelp';
"Ole32";
"Shell32";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhaozg zhaozg merged commit 6979eb9 into luvit:master May 20, 2023
@zhaozg zhaozg deleted the v1.45 branch May 20, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request bind v1.45.0 new features
2 participants