Adjust search mingw32 lib search to support llvm-mingw32 compiler. #1138
Conversation
| --~ 2020-01-23: putting ?.lib first allows use of the llvm-mingw32 compiler toolchain | ||
| --~ https://github.com/mstorsjo/llvm-mingw | ||
| --~ The LLVM linker doesn't support linking to a dll (GNU ld does). | ||
| lib = { "?.lib", "lib?.dll.a", "?.dll.a", "lib?.a", "cyg?.dll", "lib?.dll", "?.dll" }, |
There was a problem hiding this comment.
Is there a way to check that this change does not affect non-LLVM mingw32 in some way? Not sure if the testsuite (and the CI run for spec/build_spec.lua in Appveyor) covers it.
There was a problem hiding this comment.
Not sure if the testsuite (and the CI run for spec/build_spec.lua in Appveyor) covers it.
I think there may be a chance it does, in the very latest code of master. You might want to rebase this PR over the latest origin/master and force-push it so we get a new CI run.
There was a problem hiding this comment.
On deeper inspection, I don't think the test suite actually checks that the build of a module with an external dependency works correctly (i.e. that it produces a module that the Lua interpreter is really able to load and use). I'm reluctant about merging this before further testing, I can try my hand at extending the tests (probably next week).
There was a problem hiding this comment.
I got a build working last night so I can move forward and test; no rush on up-streaming it. What platform are you concerned about? Windows, Linux?
Codecov Report
@@ Coverage Diff @@
## master #1138 +/- ##
=========================================
+ Coverage 85.3% 85.41% +0.1%
=========================================
Files 94 94
Lines 11304 11325 +21
=========================================
+ Hits 9643 9673 +30
+ Misses 1661 1652 -9
Continue to review full report at Codecov.
|
|
The other platform that would be affected by this change would be mingw
with gcc.
I recently added a test that builds a dummy external library on the spot
(look for "fixturedep") and uses it as an external dependency, but the test
doesn't go as far as verifying that the linkage was correct (i.e. that the
Lua module is actually able to use the C library successfully).
|
- .gitignore - updated to include windows binary build outputs. - all_in_one - changed default path for external dependencies (e.g. mingw) - binary/lua-bz2-0.1.0-1.rockspec - changed protocol for anonymous download
Hello,
As per #1135, I was able to get the llvm-mingw32 toolchain to work with luarocks. I'm not sure why the mingw32 internal build script doesn't respect variables.LUALIB from the config file, but the fix is minor. I simply adjusted the search path as provided by cfg.lua. I moved the
?.libsearch string to the front of the table definition. I don't expect this to affect mingw32-gcc, but that's a guess (based on an old faded memory) on my part. I suspect it either doesn't generate lib files (so won't find it in a search string) or is agnostic of their use. I'm indifferent to leaving the comment in (e.g. take it out if you feel). I thought it appropriate given there was an opposing comment there before.