leafo / pgmoon

A pure Lua Postgres driver for use in OpenResty & more
MIT License
390 stars 93 forks source link

Lua53 compat #97

Closed jprjr closed 3 years ago

jprjr commented 3 years ago

Hi there! This PR adds a library that attempts to use lua's native bitwise operators via pcall + load (to prevent parse errors on older lua versions), also adds Lua 5.4 to travis and updates 5.3 to the latest.

jprjr commented 3 years ago

This last push should pass travis's tests, let me know if you'd like me to squash it or anything.

There were a few issues in the tests themselves -- tests that asserted os.execute returned 0 were failing because on lua 5.2+, os.execute returns 3 values now, instead of 1, and the first returned value is a boolean instead of the exit code.

lua5.1
Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print(os.execute('true'))
0
> 

vs

lua5.2
Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
> print(os.execute('true'))
true    exit    0

So some of the setup steps weren't passing, I wrapped those os.execute calls into tables and just check the last element is zero.

Additionally, I updated the lua-cjson module to pull from git since your latest compatibility updates haven't been published to luarocks yet.

I wrote some luabitop-compatible wrapper functions around lua's native bitwise operators, and wrote spec tests to make sure everything behaves the same.

There was also a fix for checking the number of affected query rows - on lua5.1 and luajit, tonumber works fine with strings with embedded zeroes, but on lua5.2+ it returns nil. I just used a group so match only sends the digits to tonumber.

jprjr commented 3 years ago

One last group of commits here - Travis seems to take an extraordinarily long time to run, I created a Dockerfile + docker-compose to perform local testing.

I tried to work around how Docker caches things to minimize build time - the first build will take a bit as it installs postgres, lua, dependencies, etc -- but subsequent builds will only have to run the last few steps (copying the repo into the image).

Then for testing you can just run:

docker-compose build
docker-compose run lua5.1
docker-compose run lua5.2
...

There's some duplication here since I basically had to copy the travis matrix into Docker-compose (ie, each entry in the matrix is now an entry in the docker-compose file). And changing anything in .travis.yml or the .travis folder will trigger a more full rebuild.

I also added testing for 32bit builds of lua5.3 and lua5.4 (to be extra certain those bitwise operations wrappers behave exactly the same, everywhere).

jprjr commented 3 years ago

I'm going to squash this a bit and open up new PRs, one focused on just the Lua-related changes, another for Travis-related changes, and a last one for adding Docker.