arnodel / golua

A Lua compiler / runtime in Go
Apache License 2.0
93 stars 10 forks source link

Make Golua build and pass tests on Windows #81

Closed arnodel closed 2 years ago

arnodel commented 2 years ago

Build issues

  1. syscall.Getrusage is used to implement os.clock, which is unix platform-specific

Solution: implement os.clock using the wall clock on windows platform - less accurate but works

Test issues

  1. Go plugins are not supported on Windows platform and are used in golib.import

Solution: as not supported, set golib.import to nil on windows and disabled related tests

  1. Golua is using the current time microseconds as a source of entropy for math.randomseed but the clock resolution on Windows is 1s which makes is completely unsuitable

Solution: use crypto/rand to obtain entropy, which is better anyway

  1. Standard files (stdin, stdout, stderr) are identified via their file descriptor (0, 1, 2) which is unix platform-specific

Solution: file handlers for the standard files are only created once at the start, so add a closable field to File handles which is set to false for those and true for all other files.

  1. Some os error messages related to file manipulation are substantially different on Windows

Solution: test for those errors in a much ore generic way (not ideal but it would be a lot of work to abstract that in a cross-platform way)

  1. Golua relies in places on LF being the line separator, when it is CR+LF on Windows. In particular source code is normalized w.r.t. line endings before being passed to the lexer, which causes some issues on Windows

Solution: do not normalize source code line endings before lexing, but instead the lexer itself detects CR+LF / LF+CR sequences and normalizes on the fly.

  1. In the Lua Test Suite file.lua tests, there are some failures related to garbage collection not triggering closing of files as expected on Windows.

Solution (temporary): very hard for me to investigate this without a usable windows env, so for now this particular test is disabled in CI only for windows targets.

QA

As part of this PR, a Windows target has been added to the build matrix in GHA, so the Windows build, go tests and Lua Test Suite are now part of CI.

This means there is no manual QA for this PR.

codecov[bot] commented 2 years ago

Codecov Report

Merging #81 (0eaf14e) into lua5.4 (866962c) will increase coverage by 0.28%. The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           lua5.4      #81      +/-   ##
==========================================
+ Coverage   88.50%   88.78%   +0.28%     
==========================================
  Files          95       96       +1     
  Lines       10435    10456      +21     
==========================================
+ Hits         9235     9283      +48     
+ Misses        921      897      -24     
+ Partials      279      276       -3     
Impacted Files Coverage Δ
lib/golib/goimports/goimports.go 68.64% <ø> (ø)
lib/iolib/read.go 84.81% <0.00%> (ø)
lib/mathlib/mathlib.go 92.91% <40.00%> (+3.75%) :arrow_up:
lib/iolib/iolib.go 92.45% <77.77%> (+0.01%) :arrow_up:
lib/iolib/file.go 77.20% <81.81%> (+0.36%) :arrow_up:
luatesting/linechecker.go 80.32% <85.71%> (+0.69%) :arrow_up:
scanner/scanner.go 79.04% <86.36%> (+3.00%) :arrow_up:
lib/golib/golib.go 51.54% <100.00%> (+1.02%) :arrow_up:
luastrings/normalize.go 100.00% <100.00%> (ø)
scanner/states.go 95.37% <100.00%> (+6.05%) :arrow_up:
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 866962c...0eaf14e. Read the comment docs.