CI: add tests for various MSYS2 environments #132
Conversation
|
Nice, thanks! I think there's a dependency on #131 though; it would be nice to fix that one first so that we don't need to comment out the 32-bit builds. |
|
@aklomp, if you're ok with the changes here, I think the next steps are either:
|
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because MinGW does not emulate the `writev(2)' system call. Fix this by detecting MinGW at compile time and optionally emulating `writev(2)' as a series of `write(2)' calls. Resolves aklomp#127.
|
@aklomp, I rebased on master and removed the comment on ming 32-bit. |
| - { msystem: mingw64, env: mingw-w64-x86_64- } | ||
| - { msystem: ucrt64, env: mingw-w64-ucrt-x86_64- } | ||
| # - { msystem: clang32, env: mingw-w64-clang-i686- } disabled, lld does not support the "-r" option | ||
| # - { msystem: clang64, env: mingw-w64-clang-x86_64- } disabled, lld does not support the "-r" option |
There was a problem hiding this comment.
Does it support -i or --relocatable? Both are synonyms of -r.
There was a problem hiding this comment.
No. The feature is supported for ELF & Mach-O targets but not for Windows.
| @@ -15,7 +17,9 @@ endif | |||
|
|
|||
| test: clean test_base64 benchmark | |||
| ./test_base64 | |||
| ifeq (, $(findstring mingw, $(TARGET))) # no "/dev/urandom" on Windows | |||
There was a problem hiding this comment.
There's a really old pull request, #6, which fixes this on Windows. Now that we have a provisional CI environment for Windows, I can try to revive that branch and see if I can get the benchmark to work.
Having a working Windows benchmark would also be great for checking in CI that certain changes did not introduce speed regressions on that platform.
| sed -e 's/^/_/' $< > $@ | ||
| else | ||
| cp -f $< $@ | ||
| endif |
There was a problem hiding this comment.
Honestly I'm not a big fan of this compatibility block, though I understand why it's needed. In the longer term I think we should try to get rid of the exports file entirely and rely on compiler/linker features to mark functions as hidden or visible.
There was a problem hiding this comment.
You mean like annotating these symbols as BASE64_SYMBOL_PRIVATE?
There was a problem hiding this comment.
I don't like this either but if we're to keep the current semantic, it's needed.
It's not the same as BASE64_SYMBOL_PRIVATE:
BASE64_SYMBOL_PRIVATE only acts on the visibility attribute of GLOBAL symbols e.g. even if base64_stream_encode_plain is marked as hidden it's still linkable (if building a static library or bare object).
--keep-global-symbols acts on LOCAL/GLOBAL attribute. It effectively marks symbols that are not in the list as LOCAL & those are not linkable anymore (as if we only had 1 translation unit - simulated by ld -r - and the function was declared static).
I wouldn't mind if the makefile produced the same things as the CMake build does for a static library (just something like ar -o libbase64.a *.o, getting rid of the ld -r and objcopy).
Another option is not to support makefile builds for Windows at all.
There was a problem hiding this comment.
My idea for the future roadmap is basically to engineer the library such that all functions start with akbase64_ (which is probably what I will rename this library to, see #103), and that all functions are declared static, except for those that are part of the public API. Those two measures should remove all potential naming conflicts in practice. Then we can also get rid of the exports file.
Until then, I suggest we keep going as usual, so using this Makefile hack/complication out of necessity.
aklomp
changed the title
aklomp
changed the title
aklomp
changed the title
This PR adds CI tests for various MSYS2 environments (msys, mingw64, clang32 & clang64).
It's based on top of 17a9ab9 and also adds a fix for mingw makefile builds.
Resolves #127.