BugFix: fixup failing MacOs CI builds#3636
Conversation
Inserts some symbolic links to account for the change of the library name for linking from `zziplib` to `zziplib-0` on MacOs! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
It shouldn't be needed but it did work in the other PR where I was hacking around trying to solve the MacOs CI build failures and it is the only functional difference I could see. It is not expected that the second (the CMake one) build job will pass. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Also reremove the manual insertion of the libraries into the QMake LIBS variable - as it should be done by Qt and pkg-config automagically. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Hopefully this will fix the second (CMake) Travis CI build job. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…ackage The added file is Public Domain licensed from the "Ogre" (Object-oriented Graphics Rendering Engine) project at http://www.ogre3d.org/. The file was sourced from: https://github.com/OGRECave/ogre/blob/master/CMake/Packages/FindZZip.cmake Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…acOs Reviewing the AppVeyor builds for Windows it appears that we are repeating this use of luazip instead of lua-zip there as well. As there is some tweaking and the compilation is done manually I have left comments about this in the relevant CI files... Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…ommit Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
CodeFactor likes you a lot now that you've deleted luazip.h! |
|
Hah! Were you aware that we have been using the 🤹 |
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
The link flag `-lz` is for the zlib library {https://zlib.net/}...
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Not off the top of my head. The less complexity, the better - less can break. I totally welcome the removal of things we don't need 🎉 |
|
Linux works well, but not macOS:
|
|
That was what I sort of expected - can you install the |
|
Also, that should be addressed by: Mudlet/installers#72 ... |
|
Cool! Merged that one and kicked this one to rebuild. |
|
So plonking that |
|
I haven't tried that, I'll try this build when it's complete. |
|
Ah, no, the MacOS Qmake build ended up with: INFO | macdeployqtfix terminated with success
cp: /Users/travis/.luarocks/lib/lua/5.1/brimworks/yajl.so: No such file or directory
Done. Your build exited with 0.Can you check to see where it goes if you do a EDit: oh, |
|
Hm... nowhere. It looks like the development environment on the machine broke:
Could you do it in Travis? |
|
🔎 You are building your luarock against Lua 5.3 not 5.1! Well the build log on that last run for the MacOS QMake (Job 1) looks okay: Installing https://luarocks.org/lua-yajl-2.0-1.src.rock
lua-yajl 2.0-1 depends on lua >= 5.1 (5.1-1 provided by VM)
lua_yajl.c:146:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
1 warning generated.
env MACOSX_DEPLOYMENT_TARGET=10.8 gcc -O2 -fPIC -I/usr/local/opt/lua@5.1/include/lua5.1 -c lua_yajl.c -o lua_yajl.o
env MACOSX_DEPLOYMENT_TARGET=10.8 gcc -bundle -undefined dynamic_lookup -all_load -o yajl.so lua_yajl.o -lyajl
lua-yajl 2.0-1 is now installed in /Users/travis/.luarocks (license: MIT/X11)
Installing https://luarocks.org/lua-zip-0.2-0.src.rock
lua-zip 0.2-0 depends on lua (5.1-1 provided by VM)
env MACOSX_DEPLOYMENT_TARGET=10.8 gcc -O2 -fPIC -I/usr/local/opt/lua@5.1/include/lua5.1 -c lua_zip.c -o lua_zip.o -I/usr/local/include
env MACOSX_DEPLOYMENT_TARGET=10.8 gcc -bundle -undefined dynamic_lookup -all_load -o brimworks/zip.so lua_zip.o -L/usr/local/lib -Wl,-rpath,/usr/local/lib -lzip
lua-zip 0.2-0 is now installed in /Users/travis/.luarocks (license: MIT)
On the other hand, examining the build
But there is no sign of a |
|
Ah, I think we might be installing lua-yajl twice (one in the main build, as it is needed for compilation - processing the translation statistics; and one in the installer as it is needed for the bundle). Yes! I see it - I was copying the lua |
|
Just kicked the number 1 Travis CI build job to rebuild a new .dmg... |
…veyor] Temporarily use a Testing PR in the mudlet/installers repo Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…veyor] Will need a further commit to revert to standard installer repo when this is shown to put the `zip.so` module in the right place for MacOs CI / installer builds and the installer repo is updated to include the change. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
The reason for this code is not clear - why is it needed on MacOs and not other ones...? Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Because the module is stored as brimworks/zip.so (to differentiate it from the other zip.so that we are replacing for MacOs builds) we have to bundle it in such a sub-directory. This also needs a tweak to the installer but that is already in place for a testing run. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
I shouldn't have put it in as it wasn't relevant and now break things. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
vadi2
left a comment
There was a problem hiding this comment.
Thanks a lot for patching this up! This was really tough to do. Cheers 🍺
Also another one that deletes more than it adds. Awesome 😉
|
Merging it in for you in order to update all PRs affected by this. |
|
I really hope you did a squash rather than a rebase merge - there is an awful lot of c**p in that chain of commits and they really ought not to be shown in the end result - oh you did - but all that mess is present in the commit message whereas I would have summarised it to to prune out the unnecessary stuff about Okay - FTR this means that MacOS developers compiling from sources will need to have the I need to re-tweak that ... I'll try and do that in the next couple of days. |
|
Why both? Only dealing with one will keep our code simpler (and thus less bugs) |
|
Yeah, but it means that instead of all OSes using the same module they use different ones and that will have to be documented somewhere - it is easier for the end user/hacker if they can source the one that works easier for them and makes things hard for us to debug a problem if the two modules do not behave exactly the same under ALL circumstances and we cannot swap between them just by changing the luarocks we have installed... |
|
Hmm so you're saying yes we should have one because one will have the same behaviour? It's a bit unclear |
|
It took me a while to work out that the name you give for a luarock need not be the name of the library/module that it installs and there can be compatible rocks that do the same thing. E.g.:
Anyhow that is now ready and works for me (on Linux) see: #3648 . |
* BugFix: fixup failing MacOs CI builds Inserts some symbolic links to account for the change of the library name for linking from `zziplib` to `zziplib-0` on MacOs! Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: add a missing double-quote from previous commit Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: put some debug output lines back in Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: fix a typo in previous commit Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: restore manual qmake LIBS addition [skip appveyor] It shouldn't be needed but it did work in the other PR where I was hacking around trying to solve the MacOs CI build failures and it is the only functional difference I could see. It is not expected that the second (the CMake one) build job will pass. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: correct error in numeric value use in file names Also reremove the manual insertion of the libraries into the QMake LIBS variable - as it should be done by Qt and pkg-config automagically. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: add zziblib to main project MacOs CMake libraries list Hopefully this will fix the second (CMake) Travis CI build job. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: try a different name for the zziblib libary Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: try adding a FindZZip.cmake file to aid finding the zziplib package The added file is Public Domain licensed from the "Ogre" (Object-oriented Graphics Rendering Engine) project at http://www.ogre3d.org/. The file was sourced from: https://github.com/OGRECave/ogre/blob/master/CMake/Packages/FindZZip.cmake Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Append: add a missing file of macros for use with previously added file Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert last four commits * Refactor: remove embedded luazip code and switch to lua-zip rock on MacOs Reviewing the AppVeyor builds for Windows it appears that we are repeating this use of luazip instead of lua-zip there as well. As there is some tweaking and the compilation is done manually I have left comments about this in the relevant CI files... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: remove a couple of remnants of luazip left behind from last commit Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert: don't change the Windows building of lua-zip right now... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert: another bit of don't change the Windows building of lua-zip now Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert: put back in a needed library for MacOs QMake builds The link flag `-lz` is for the zlib library {https://zlib.net/}... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Testing: find macOs qmake CI build placement of zip luarock [skip appveyor] Temporarily use a Testing PR in the mudlet/installers repo Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: correct location of temporary installer branch [skip appveyor] Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: temp installer to get lua-zip module right on MacOs [skip appveyor] Will need a further commit to revert to standard installer repo when this is shown to put the `zip.so` module in the right place for MacOs CI / installer builds and the installer repo is updated to include the change. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert: restore C loading of `zip.so` for MacOs case. [skip appveyor] The reason for this code is not clear - why is it needed on MacOs and not other ones...? Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: load zip.so from a sub-directory on MacOS [skip appveyor] Because the module is stored as brimworks/zip.so (to differentiate it from the other zip.so that we are replacing for MacOs builds) we have to bundle it in such a sub-directory. This also needs a tweak to the installer but that is already in place for a testing run. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revise: take out a code fragment again [skip appveyor] I shouldn't have put it in as it wasn't relevant and now break things. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Revert back to main repo of installers Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>
Inserts some symbolic links to account for the change of the library name for linking fromzziplibtozziplib-0on MacOs!Edit: rendered unnecessary as I have now excised the use of the Lua
luazipmodule (in favour oflua-zip) on MacOs to provide thezipmodule in the Lua sub-system and THAT does not have a dependency onzziplib(a.k.a.zzipa.k.a.libzzip)Signed-off-by: Stephen Lyons slysven@virginmedia.com