From 489cb0dc8ed3883d605ab4d0dc46c23cda157d84 Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Tue, 10 Oct 2023 22:25:41 +0300 Subject: [PATCH 1/8] ci(actions): Remove custom Lua compilation options, upstream defaults work now --- .github/workflows/test.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b6678af8a..fea98557a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,12 +13,12 @@ jobs: fail-fast: false matrix: luaVersion: - - [ '5.4', 'MYCFLAGS=-fPIC' ] - - [ '5.3', 'MYCFLAGS=-fPIC' ] - - [ '5.2', 'MYCFLAGS=-fPIC' ] - - [ '5.1', 'CFLAGS="-O2 -Wall -DLUA_USE_LINUX -fPIC"' ] - - [ 'luajit', 'XCFLAGS=-fPIC' ] - # - [ 'luajit-openresty', 'XCFLAGS=-fPIC' ] + - [ '5.4' ] + - [ '5.3' ] + - [ '5.2' ] + - [ '5.1' ] + - [ 'luajit' ] + # - [ 'luajit-openresty' ] runs-on: ubuntu-22.04 name: Test on Lua ${{ matrix.luaVersion[0] }} steps: From efc64d3e4fef727d302de467aa56e55e28a3df0a Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Tue, 10 Oct 2023 22:30:38 +0300 Subject: [PATCH 2/8] ci(actions): Enable testing on OpenResty fork of LuaJIT --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fea98557a..0877be3b9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: - [ '5.2' ] - [ '5.1' ] - [ 'luajit' ] - # - [ 'luajit-openresty' ] + - [ 'luajit-openresty' ] runs-on: ubuntu-22.04 name: Test on Lua ${{ matrix.luaVersion[0] }} steps: From 138cebcd6f79c5c0b6004da1ffe76dccd0e4ffb3 Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Tue, 10 Oct 2023 22:38:01 +0300 Subject: [PATCH 3/8] ci(actions): Overhaul run order and timeouts --- .github/workflows/test.yml | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0877be3b9..2726f6798 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,14 +13,14 @@ jobs: fail-fast: false matrix: luaVersion: - - [ '5.4' ] - - [ '5.3' ] - - [ '5.2' ] - - [ '5.1' ] - - [ 'luajit' ] - - [ 'luajit-openresty' ] + - '5.4' + - '5.3' + - '5.2' + - '5.1' + - 'luajit' + - 'luajit-openresty' runs-on: ubuntu-22.04 - name: Test on Lua ${{ matrix.luaVersion[0] }} + name: Test on Lua ${{ matrix.luaVersion }} steps: - name: Checkout uses: actions/checkout@v4 @@ -38,12 +38,15 @@ jobs: with: path: | lua_modules - key: luarocks-${{ matrix.luaVersion[0] }}-${{ hashFiles('Makefile-luarocks', 'sile.rockspec.in') }} + key: luarocks-${{ matrix.luaVersion }}-${{ hashFiles('Makefile-luarocks', 'sile.rockspec.in') }} + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install fonts-sil-gentiumplus libarchive-tools libfontconfig1-dev libharfbuzz-dev libicu-dev libpng-dev poppler-utils - name: Setup ‘lua’ uses: leafo/gh-actions-lua@v10 with: - luaVersion: ${{ matrix.luaVersion[0] }} - luaCompileFlags: ${{ matrix.luaVersion[1] }} + luaVersion: ${{ matrix.luaVersion }} - name: Setup ‘luarocks’ uses: leafo/gh-actions-luarocks@v4 - name: Prep system Lua for use @@ -58,10 +61,6 @@ jobs: LUA_INCLUDE=-I$(deepest $PWD/.lua/include) MAKEFLAGS=-j$(nproc) -Otarget EOF - - name: Install system dependencies - run: | - sudo apt-get update - sudo apt-get install fonts-sil-gentiumplus libarchive-tools libfontconfig1-dev libharfbuzz-dev libicu-dev libpng-dev poppler-utils - name: Configure run: | ./bootstrap.sh @@ -69,21 +68,24 @@ jobs: --enable-developer LUACHECK=false NIX=false \ --disable-font-variations \ --without-system-luarocks \ - --with${{ startsWith(matrix.luaVersion[0], '5') && 'out' || '' }}-luajit \ + --with${{ !startsWith(matrix.luaVersion, 'luajit') && 'out' || '' }}-luajit \ --without-manual - name: Make run: | make + - name: Prove SILE runs at all + run: | + make selfcheck - name: Test Busted - timeout-minutes: ${{ runner.debug && 60 || 6 }} + timeout-minutes: ${{ runner.debug && 20 || 2 }} run: | make busted - name: Test Regressions - timeout-minutes: ${{ runner.debug && 60 || 6 }} + timeout-minutes: ${{ runner.debug && 20 || 2 }} run: | make regressions - name: Upload artifacts uses: actions/upload-artifact@v3 with: - name: test-${{ matrix.luaVersion[0] }}-actuals + name: test-${{ matrix.luaVersion }}-actuals path: tests/*.actual From 713434dadbc271299c8548dd2f2d4af57c1eec62 Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Wed, 11 Oct 2023 00:17:58 +0300 Subject: [PATCH 4/8] fix(build): Make sure vendored luarocks isn't a phony target that runs repeatedly --- Makefile-luarocks | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Makefile-luarocks b/Makefile-luarocks index 46e5c8ef3..54387731d 100644 --- a/Makefile-luarocks +++ b/Makefile-luarocks @@ -4,16 +4,18 @@ if !SYSTEM_LUAROCKS LUAMODLOCK := sile-dev-1.rockslock LOCALLUAROCKS := $(LUAROCKS) --tree lua_modules --lua-version $(LUA_VERSION) -TMPFILE != mktemp genrockslock := $(LOCALLUAROCKS) $(LUAROCKSARGS) list --porcelain | $(AWK) '{print $$1 " " $$2}' rocksmatch := ( T=$$(mktemp); trap 'rm -f "$$T"' EXIT HUP TERM; $(genrockslock) > "$$T"; $(CMP) -s $(LUAMODLOCK) "$$T" ) -installrocks: $(LUAMODLOCK) $(shell $(rocksmatch) || echo lua_modules) +LUAROCKSMANIFEST := lua_modules/lib/luarocks/rocks-$(LUA_VERSION)/manifest -lua_modules: $(LUAMODSPEC) $(shell $(rocksmatch) || echo force) +installrocks: $(LUAMODLOCK) $(shell $(rocksmatch) || echo $(LUAROCKSMANIFEST)) + +$(LUAROCKSMANIFEST): $(LUAMODSPEC) $(shell $(rocksmatch) || echo force) $(LOCALLUAROCKS) $(LUAROCKSARGS) install --only-deps $< + touch $@ -$(LUAMODLOCK): lua_modules $(LUAMODSPEC) +$(LUAMODLOCK): $(LUAROCKSMANIFEST) $(LUAMODSPEC) $(genrockslock) > $@ else LUAMODLOCK := From 640ded0a90e427124f555a2a48d263cde5300d7d Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Wed, 11 Oct 2023 11:36:59 +0300 Subject: [PATCH 5/8] fix(core): Allocate exactly what we use, not a guess with an extra just in case --- packages/footnotes/init.lua | 2 +- src/justenoughharfbuzz.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/footnotes/init.lua b/packages/footnotes/init.lua index 2d8a7cea3..4adb97888 100644 --- a/packages/footnotes/init.lua +++ b/packages/footnotes/init.lua @@ -73,7 +73,7 @@ function package:registerCommands () -- Apply the font before boxing, so relative baselineskip applies #1027 local material SILE.call("footnote:font", {}, function () - material = SILE.call("vbox", {}, function () + material = SILE.call("vbox", {}, function () SILE.call("footnote:atstart", options) SILE.call("footnote:counter", options) SILE.process(content) diff --git a/src/justenoughharfbuzz.c b/src/justenoughharfbuzz.c index f8d9ba340..cbc150a18 100644 --- a/src/justenoughharfbuzz.c +++ b/src/justenoughharfbuzz.c @@ -185,12 +185,12 @@ int shape (lua_State *L) { } glyph_info = hb_buffer_get_glyph_infos(buf, &glyph_count); glyph_pos = hb_buffer_get_glyph_positions(buf, &glyph_count); - lua_checkstack(L, glyph_count*10); for (j = 0; j < glyph_count; ++j) { char namebuf[255]; hb_glyph_extents_t extents = {0,0,0,0}; hb_font_get_glyph_extents(hbFont, glyph_info[j].codepoint, &extents); + lua_checkstack(L, 3); lua_newtable(L); lua_pushstring(L, "name"); hb_font_get_glyph_name( hbFont, glyph_info[j].codepoint, namebuf, 255 ); @@ -206,21 +206,25 @@ int shape (lua_State *L) { baseline, and we should use that and take out this condition. */ if (direction != HB_DIRECTION_TTB) { if (glyph_pos[j].x_offset) { + lua_checkstack(L, 2); lua_pushstring(L, "x_offset"); lua_pushnumber(L, glyph_pos[j].x_offset * point_size / upem); lua_settable(L, -3); } if (glyph_pos[j].y_offset) { + lua_checkstack(L, 2); lua_pushstring(L, "y_offset"); lua_pushnumber(L, glyph_pos[j].y_offset * point_size / upem); lua_settable(L, -3); } } + lua_checkstack(L, 2); lua_pushstring(L, "gid"); lua_pushinteger(L, glyph_info[j].codepoint); lua_settable(L, -3); + lua_checkstack(L, 2); lua_pushstring(L, "index"); lua_pushinteger(L, glyph_info[j].cluster); lua_settable(L, -3); @@ -240,17 +244,21 @@ int shape (lua_State *L) { width = glyphAdvance; glyphAdvance = height; } + lua_checkstack(L, 2); lua_pushstring(L, "glyphAdvance"); lua_pushnumber(L, glyphAdvance); lua_settable(L, -3); + lua_checkstack(L, 2); lua_pushstring(L, "width"); lua_pushnumber(L, width); lua_settable(L, -3); + lua_checkstack(L, 2); lua_pushstring(L, "height"); lua_pushnumber(L, height); lua_settable(L, -3); + lua_checkstack(L, 2); lua_pushstring(L, "depth"); lua_pushnumber(L, -tHeight - height); lua_settable(L, -3); From 85f664920fb7ff50a1d37a2ea3cd47e416e49913 Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Wed, 11 Oct 2023 01:08:36 +0300 Subject: [PATCH 6/8] ci(actions): Allow skipping busted tests with Lua 5.1 in borked CI runner --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2726f6798..fcee6ccdf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -77,6 +77,7 @@ jobs: run: | make selfcheck - name: Test Busted + continue-on-error: ${{ matrix.luaVersion == '5.1' }} timeout-minutes: ${{ runner.debug && 20 || 2 }} run: | make busted From ace688ce5fc6b5c808400e8b111147cc8de30a30 Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Tue, 10 Oct 2023 12:45:05 +0300 Subject: [PATCH 7/8] chore(tooling): Explicitly set tested Lua version to ease local testing --- Makefile.am | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index 95b3f8988..894c0f64b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -228,14 +228,14 @@ luacheck: busted: $(SILE) $(addprefix .fonts/,$(TESTFONTFILES)) $(BUSTEDSPECS) set -f; IFS=';' -if SYSTEM_LUAROCKS - packagecpath=(./{,core/}?.$(SHARED_LIB_EXT)) packagepath=(./{,lua-libraries/}?{,/init}.lua) -else - packagecpath=(./{,core/,lua_modules/lib/lua/$(LUA_VERSION)/}?.$(SHARED_LIB_EXT)) - packagepath=(./{,lua_modules/share/lua/$(LUA_VERSION)/,lua-libraries/}?{,/init}.lua) + packagecpath=(./{,core/,{libtexpdf,justenough}/.libs/}?.$(SHARED_LIB_EXT)) +if !SYSTEM_LUAROCKS + packagepath+=(./lua_modules/share/lua/$(LUA_VERSION)/?{,/init}.lua) + packagecpath+=(./lua_modules/lib/lua/$(LUA_VERSION)/?.$(SHARED_LIB_EXT)) endif - $(LOCALTESTFONTS) $(BUSTED) --cpath="$${packagecpath[*]};;" --lpath="$${packagepath[*]};;" $(BUSTEDFLAGS) . +# Note: use of --lua causes this to be passed back through a shell loosing one layer of quoting. Drop single quotes if removing. + $(LOCALFONTS) $(BUSTED) --lua=$(LUA) --lpath="'$${packagepath[*]};;'" --cpath="'$${packagecpath[*]};;'" $(BUSTEDFLAGS) . coverage: export SILE_COVERAGE=1 coverage: BUSTEDFLAGS = -c From f4886437d0ebf229db1c2779a8a324bf441efc1a Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Wed, 11 Oct 2023 13:11:20 +0300 Subject: [PATCH 8/8] fix(core): Correct usage of HarfBuzz when passing a filtered list of shapers --- src/justenoughharfbuzz.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/justenoughharfbuzz.c b/src/justenoughharfbuzz.c index cbc150a18..a576da9c3 100644 --- a/src/justenoughharfbuzz.c +++ b/src/justenoughharfbuzz.c @@ -145,9 +145,9 @@ int shape (lua_State *L) { double point_size = luaL_checknumber(L, 6); const char * featurestring = luaL_checkstring(L, 7); char * shaper_list_string = luaL_checkstring(L, 8); - char ** shaper_list = NULL; + const char * const* shaper_list = NULL; if (strlen(shaper_list_string) > 0) { - shaper_list = scan_shaper_list(shaper_list_string); + shaper_list = (const char * const*)scan_shaper_list(shaper_list_string); } hb_direction_t direction;