Add support for building with Zig #598
Conversation
|
@ivmai here's a first draft of adding in support for the zig build system. Please let me know what you think!? :) |
|
I mean, there are things that are missing, like building dynamic libraries so I don't consider this ready for merge. I just wanted to get the current version out there to get feedback on the overall look and feel of it! |
|
@ivmai so I've added in compilation and running of the basic gctest as well. It works when building a static lib and disabling threads: kll@Boxy:~/terastream/acton-deps/bdwgc$ ~/terastream/acton/dist/zig/zig build -Dbuild_tests -Dbuild_shared_libs=false -Denable_threads=false test
Switched to incremental mode
Reading dirty bits from /proc
Completed 6 tests
Allocated 4154109 collectable objects
Allocated 1218 uncollectable objects
Allocated 8219232 atomic objects
Reallocated 756 objects
Garbage collection after fork is tested too
Finalized 13216/13216 objects - finalization is probably OK
Total number of bytes allocated is 1143245488
Total memory use by allocated blocks is 638976 bytes
Final heap size is 172032 bytes
Obtained 12488704 bytes from OS
Final number of reachable objects is 2423
Full/world-stopped collections took 23/484 ms
Completed 2475 collections
Collector appears to work
kll@Boxy:~/terastream/acton-deps/bdwgc$ If I enable threads, I get a failure: kll@Boxy:~/terastream/acton-deps/bdwgc$ ~/terastream/acton/dist/zig/zig build -Dbuild_tests -Dbuild_shared_libs=false -Denable_threads=true test
Switched to incremental mode
Reading dirty bits from /proc
GC_is_visible produced wrong failure indication
Test failed
run gctest: error: the following command terminated unexpectedly:
/home/kll/terastream/acton-deps/bdwgc/zig-cache/o/71dd4f8899463605f4330d1f46afd9e6/gctest
Build Summary: 19/21 steps succeeded; 1 failed (disable with --summary none)
test transitive failure
└─ run gctest failure
error: the following build command failed with exit code 1:
/home/kll/terastream/acton-deps/bdwgc/zig-cache/o/6ca84adc5567722c0e59ba2f7e1f9200/build /home/kll/terastream/acton/dist/zig/zig /home/kll/terastream/acton-deps/bdwgc /home/kll/terastream/acton-deps/bdwgc/zig-cache /home/kll/.cache/zig --seed 0x2b9282f8 -Dbuild_tests -Dbuild_shared_libs=false -Denable_threads=true test
kll@Boxy:~/terastream/acton-deps/bdwgc$Similarly when building a shared lib also without threads, I get a different error: kll@Boxy:~/terastream/acton-deps/bdwgc$ ~/terastream/acton/dist/zig/zig build -Dbuild_tests -Dbuild_shared_libs=true -Denable_threads=false test
Switched to incremental mode
Reading dirty bits from /proc
List reversal produced incorrect list - collector is broken
Test failed
run gctest: error: the following command terminated unexpectedly:
/home/kll/terastream/acton-deps/bdwgc/zig-cache/o/9285cc5d5ea49b5fec8b8149ebdb78e2/gctest
Build Summary: 19/21 steps succeeded; 1 failed (disable with --summary none)
test transitive failure
└─ run gctest failure
error: the following build command failed with exit code 1:
/home/kll/terastream/acton-deps/bdwgc/zig-cache/o/6ca84adc5567722c0e59ba2f7e1f9200/build /home/kll/terastream/acton/dist/zig/zig /home/kll/terastream/acton-deps/bdwgc /home/kll/terastream/acton-deps/bdwgc/zig-cache /home/kll/.cache/zig --seed 0x72fd843e -Dbuild_tests -Dbuild_shared_libs=true -Denable_threads=false test
kll@Boxy:~/terastream/acton-deps/bdwgc$ When building with the equivalent options for CMake, the tests pass just fine. I've been trying to pin down where the compilation differs but haven't found anything so far. It could be that it's a difference due to zig default clang flags. I wonder if the error messages can guide to the error but I don't quite understand them, in particular the "GC_is_visible produced wrong failure indication". Could you possibly point me in the general direction where I should dig deeper? I just pushed the latest code that fails as described above. |
|
Nevermind, I found the cause. The flags were only applied to compilation of the lib, not of gctest itself and that discrepancy caused the failures. Having fixed that, I seem to be having some success with the tests. More to be reported! |
plajjan
force-pushed
the
zig-build
branch
9 times, most recently
from
177780f to
04f3a1d
Compare
|
I am reviewing the patch, I will commit some changes tomorrow. |
|
@ivmai I'm seeing some intermittent test failures, like in https://github.com/ivmai/bdwgc/actions/runs/7273056790 where 1 out of 12 jobs failed. I can reproduce it locally where it is intermittent. I think it only happens with assertions enabled. For example, here are two runs, one succeeding and one failing: Click meI'm pretty sure I've gotten the compilation flags etc correct by now so I'm really quite stumped by this and I'm not sure what to do. |
|
Please create a separate issue about the failure. |
|
I discovered one more |
|
Ok, I addressed the warning / error in mallo.c and enabled |
|
There are actually 2 tests out of the 12 that intermittently fail and we can see it failing in this last run after my latest commit, see https://github.com/ivmai/bdwgc/actions/runs/7281080470?pr=598 |
|
@ivmai Apart from this intermittently failing test I think I've addressed all your feedback and improved various other things along the way. I don't think I have any more changes planned, so I think another review of build.zig would be in place :) |
This adds a build.zig that allows building the garbage collector using the Zig build system. It implements a subset of the configuration options offered by CMake and following the naming convention in CMake for all those options. There are two new GitHub Actions workflows: - zig build is essentially a copy of the cmake build workflow - it builds the collector and runs tests - zig xbuild uses zig to cross-compile for different platforms - we don't actually run the tests as that would require emulation The README is slightly updated, not just adding an explanation of zig but restructuring the whole section on how to build the GC somewhat. The build.zig follows the format used in zig v0.12. The final 0.12 version is not out yet, so users will have to use a build from the zig master branch for now. This is not ideal but the whole zig build system is so new that there have been backwards incompatible changes from 0.11 which is why we opt to use 0.12 that is likely going to be closer to the final form. There is no support for installing documentation currently.
(refactoring) PR #598 (bdwgc). * CMakeLists.txt (SRC): Order items lexicographically. * Makefile.am [!SINGLE_GC_OBJ] (libgc_la_SOURCES): Likewise.
PR #598 (bdwgc). * malloc.c [REDIRECT_FREE && !REDIRECT_MALLOC_IN_HEADER && IGNORE_FREE] (free): Specify p is an unused argument.
PR #598 (bdwgc). * README.md (Installation and Portability): Rename to "Building and Installing"; move Portability to a separate section; describe every build system (cmake, autoconf, nmake, Makefile.direct, manual C compilation) in a separate sub-section; update time how long gctest may be running; move information about configurable macros and libatomic_ops to separate sub-sections. Co-authored-by: Ivan Maidanski <[email protected]>
|
I have push your change of readme with some modification. Plus 2 your changes. |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| ttriple: [ aarch64-linux-musl, wasm32-wasi, x86_64-linux-gnu.2.27, x86_64-linux-musl, x86_64-windows-gnu ] |
There was a problem hiding this comment.
How to cross-compile for MacOS? I've failed to
There was a problem hiding this comment.
Should be a simple matter of: zig build -Dtarget=x86_64-macos-none
Or aarch64-macos-none for apple silicon...
There was a problem hiding this comment.
This results in an error:
.../bdwgc/extra/../os_dep.c:4354:10: error: 'mach/exception.h' file not found
There was a problem hiding this comment.
There was a problem hiding this comment.
@ivmai ah yes, that's correct. I forgot I'm running with a patched zig. Zig doesn't ship with the correct headers just yet. I opened an issue for this ziglang/zig#18257. They're usually quite fast adding headers but I'm guessing it'll be after the holidays.
In Acton we add in a few extra headers, see https://github.com/actonlang/acton/tree/main/deps/zig-extras/lib/libc/include/any-macos-any
You can do the same, just copy into your zig/lib/...
There was a problem hiding this comment.
Okay, let's wait for right nightly build and then add macos targets.
There was a problem hiding this comment.
Right yes, agreed, let's wait for it.. I was just thinking if you wanted to test stuff locally first :)
PR #598 (bdwgc). This adds a build.zig that allows building the garbage collector using the Zig build system. It implements a subset of the configuration options offered by CMake and following the naming convention in CMake for all those options. The README is slightly updated adding an explanation of zig. The build.zig follows the format used in zig v0.12.0-dev.1814. The final 0.12 version is not out yet, so users will have to use a build from the zig master branch for now. This is not ideal but the whole zig build system is so new that there have been backwards incompatible changes from 0.11 which is why we opt to use 0.12 that is likely going to be closer to the final form. * README.md (Building and Installing): Add subsection about Zig. * Makefile.am (EXTRA_DIST): Add build.zig item. * build.zig: New file. Co-authored-by: Ivan Maidanski <[email protected]>
PR #598 (bdwgc). zig-xbuild uses zig to cross-compile for different platforms; we do not actually run the tests as that would require emulation.
|
I've pushed your PR with plenty of modifications. Please test it. |
I've created #601 |
PR #598 (bdwgc). zig build is essentially a copy of the cmake build workflow - it builds the collector and runs its tests in various configurations. Co-authored-by: Ivan Maidanski <[email protected]>
(fix of commit 2666e3e) PR #598 (bdwgc). Zig windows-gnu target is MinGW-based build. In this case pthreads library is not used by bdwgc. * build.zig [enable_threads] (build): Move GC_THREADS and PARALLEL_MARK macros definition upper; add TODO item about Cygwin; add gc_dlopen.c, specific.c to source_files only if t.os.tag!=.windows; define _REENTRANT, HANDLE_FORK, GC_USESIGRT_SIGNALS macros only if t.os.tag!=.windows; do not add threadkeytest to tests if t.os.tag==.windows. * build.zig (build): Set have_getcontext to false if t.os.tag==.windows. * build.zig (build): Do not define HAVE_DLADDR macro if t.os.tag==.windows. * build.zig [install_headers && enable_threads] (build): Do not install gc_pthread_redirects.h if t.os.tag!=.windows.
This adds a build.zig that allows building the garbage collector using the Zig build system. It implements a subset of the configuration options offered by CMake and following the naming convention in CMake for all those options.
There is a new GitHub Actions configuration file that runs a job that uses zig to compile the collector and targetting different targets using cross-compilation. It doesn't run any actual tests (and it should!).
The README is slightly updated, not just adding an explanation of zig but restructuring the whole section on how to build the GC somewhat. I moved mentions of libatomic_ops to a separate heading. All? modern compilers include atomic intrinsics so it feels like we don't have to include mentions of libatomic_ops in the default "how to build this" section but can mention it a little later. I tried to make it easier to read and more direct and to the point for new users. Let me know if you don't agree with this restructuring and I'll change back. I think there's more stuff to reword in the README but I don't want to focus too much on that in this PR since it's really about introducing zig.
The build.zig follows the format used in zig v0.12. The final 0.12 version is not out yet, so users will have to use a build from the zig master branch for now. This is not ideal but the whole zig build system is so new that there have been backwards incompatible changes from 0.11 which is why we opt to use 0.12 that is likely going to be closer to the final form.