feat: add partial support for WASI #75

kkebo · 2024-10-12T06:57:09Z

Description

This change allows building the cmark-gfm target for both wasm32-unknown-wasi and wasm32-unknown-wasip1-threads. The other targets are not tested.

The current problem

The differences between two target triples are as follows:

wasm32-unknown-wasi
- C
  - can import unistd.h
  - has __wasi__
  - does not have __unix__
  - does not have _REENTRANT
  - does not have _POSIX_THREADS
  - does not support pthreads API at all
- Swift
  - can use #if os(WASI)
- SwiftPM
  - BuildSettingsCondition's Platform is .wasi
wasm32-unknown-wasip1-threads
- C
  - can import unistd.h
  - has __wasi__
  - does not have __unix__
  - has _REENTRANT (defined in wasi-libc)
  - has _POSIX_THREADS (defined in wasi-libc)
  - supports a subset of pthreads API
- Swift
  - can use #if os(WASI)
- SwiftPM
  - BuildSettingsCondition's Platform is .wasi

Therefore, CMARK_THREADING must not be defined when building for wasm32-unknown-wasi, but must be defined when building for wasm32-unknown-wasip1-threads.

However, when building with CMake, it is controllable, but when building with SwiftPM, CMARK_THREADING is always defined.

Proposed solution

Building with CMake and Make
- Current
  - When running with -DCMARK_THREADING=OFF (default), CMARK_THREADING will not be defined in the C compiler.
  - When running with -DCMARK_THREADING=ON, CMARK_THREADING will be defined in the C compiler.
- New
  - When running with -DCMARK_THREADING=OFF (default), CMARK_THREADING will be defined in config.h and be 0.
  - When running with -DCMARK_THREADING=ON, CMARK_THREADING will be defined in config.h and be 1.
Building with SwiftPM
- Current: .define("CMARK_THREADING") will be added in Package.swift.
- New: .define("CMARK_THREADING") will not be added in Package.swift. (reverting aef4761)
  - Instead, CMARK_THREADING will automatically be defined as one of the following values in src/include/cmark-gfm_config.h.
    - 0 if building for targets that do not support threading (e.g. wasm32-unknown-wasi)
    - 1 if building for the other targets including wasm32-unknown-wasip1-threads

I reverted 6f64876 because I needed src/config.h.in and src/include/cmark-gfm_config.h.

Checks

I've tested this PR (149b31f) by the following ways on Linux.

Linux Raspberry-beetle 6.6.51+rpt-rpi-2712 #1 SMP PREEMPT Debian 1:6.6.51-1+rpt3 (2024-10-08) aarch64 GNU/Linux

Building with SwiftPM

swift-DEVELOPMENT-SNAPSHOT-2024-09-25-a:

Install swiftly
swiftly install main-snapshot-2024-09-25
swiftly use main-snapshot-2024-09-25
swift sdk install https://github.com/swiftwasm/swift/releases/download/swift-wasm-DEVELOPMENT-SNAPSHOT-2024-09-26-a/swift-wasm-DEVELOPMENT-SNAPSHOT-2024-09-26-a-wasm32-unknown-wasi.artifactbundle.zip --checksum 34c53bac6f41502f26056fd14bbf4757eca2cb93fdab089dd25449bc17cbc3c8 (instructions)
swift build --swift-sdk wasm32-unknown-wasi --target cmark-gfm
swift sdk install https://github.com/swiftwasm/swift/releases/download/swift-wasm-DEVELOPMENT-SNAPSHOT-2024-09-26-a/swift-wasm-DEVELOPMENT-SNAPSHOT-2024-09-26-a-wasm32-unknown-wasip1-threads.artifactbundle.zip --checksum 588749ed32af29c8fdd64c37ee61531c62574197f058c11c43848d74e6683a41 (instructions)
swift build --swift-sdk wasm32-unknown-wasip1-threads --target cmark-gfm
swift build

$ swift --version
Swift version 6.1-dev (LLVM 89ccf4b8a46135a, Swift 6a5ae8d5df144dd)
Target: aarch64-unknown-linux-gnu
$ swift build --swift-sdk wasm32-unknown-wasi --target cmark-gfm
Building for debugging...
[27/27] Compiling cmark-gfm scanners.c
Build of target: 'cmark-gfm' complete! (3.61s)
$ swift build --swift-sdk wasm32-unknown-wasip1-threads --target cmark-gfm
Building for debugging...
[27/27] Compiling cmark-gfm scanners.c
Build of target: 'cmark-gfm' complete! (3.48s)
$ swift build
Building for debugging...
[42/42] Linking api_test
Build complete! (2.29s)

Building with CMake and Make

-DCMARK_THREADING=ON

$ cmake -S . -B build -DCMARK_THREADING=ON
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER - Failed
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS - Success
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Checking _FILE_OFFSET_BITS for large files
-- Checking _FILE_OFFSET_BITS for large files - not needed
-- Found Python3: /usr/bin/python3 (found version "3.11.2") found components: Interpreter
-- Configuring done
-- Generating done
-- Build files have been written to: /home/kebo/swift-cmark/build
$ cmake --build build
[  2%] Building C object src/CMakeFiles/libcmark-gfm.dir/arena.c.o
In file included from /home/kebo/swift-cmark/src/arena.c:6:
/home/kebo/swift-cmark/src/include/mutex.h:24:24: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   24 | CMARK_DEFINE_ONCE(NAME); \
      |                        ^
/home/kebo/swift-cmark/src/arena.c:8:1: note: in expansion of macro â��CMARK_DEFINE_LOCKâ��
    8 | CMARK_DEFINE_LOCK(arena)
      | ^~~~~~~~~~~~~~~~~
[  4%] Building C object src/CMakeFiles/libcmark-gfm.dir/blocks.c.o
[  7%] Building C object src/CMakeFiles/libcmark-gfm.dir/buffer.c.o
[  9%] Building C object src/CMakeFiles/libcmark-gfm.dir/cmark.c.o
[ 11%] Building C object src/CMakeFiles/libcmark-gfm.dir/cmark_ctype.c.o
[ 14%] Building C object src/CMakeFiles/libcmark-gfm.dir/commonmark.c.o
[ 16%] Building C object src/CMakeFiles/libcmark-gfm.dir/footnotes.c.o
[ 19%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_href_e.c.o
[ 21%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_html_e.c.o
[ 23%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_html_u.c.o
[ 26%] Building C object src/CMakeFiles/libcmark-gfm.dir/html.c.o
[ 28%] Building C object src/CMakeFiles/libcmark-gfm.dir/inlines.c.o
[ 30%] Building C object src/CMakeFiles/libcmark-gfm.dir/iterator.c.o
[ 33%] Building C object src/CMakeFiles/libcmark-gfm.dir/latex.c.o
[ 35%] Building C object src/CMakeFiles/libcmark-gfm.dir/linked_list.c.o
[ 38%] Building C object src/CMakeFiles/libcmark-gfm.dir/man.c.o
[ 40%] Building C object src/CMakeFiles/libcmark-gfm.dir/map.c.o
[ 42%] Building C object src/CMakeFiles/libcmark-gfm.dir/node.c.o
In file included from /home/kebo/swift-cmark/src/node.c:6:
/home/kebo/swift-cmark/src/include/mutex.h:24:24: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   24 | CMARK_DEFINE_ONCE(NAME); \
      |                        ^
/home/kebo/swift-cmark/src/node.c:10:1: note: in expansion of macro â��CMARK_DEFINE_LOCKâ��
   10 | CMARK_DEFINE_LOCK(nextflag)
      | ^~~~~~~~~~~~~~~~~
/home/kebo/swift-cmark/src/include/mutex.h:24:24: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   24 | CMARK_DEFINE_ONCE(NAME); \
      |                        ^
/home/kebo/swift-cmark/src/node.c:11:1: note: in expansion of macro â��CMARK_DEFINE_LOCKâ��
   11 | CMARK_DEFINE_LOCK(safety)
      | ^~~~~~~~~~~~~~~~~
[ 45%] Building C object src/CMakeFiles/libcmark-gfm.dir/plaintext.c.o
[ 47%] Building C object src/CMakeFiles/libcmark-gfm.dir/plugin.c.o
[ 50%] Building C object src/CMakeFiles/libcmark-gfm.dir/references.c.o
[ 52%] Building C object src/CMakeFiles/libcmark-gfm.dir/registry.c.o
In file included from /home/kebo/swift-cmark/src/registry.c:7:
/home/kebo/swift-cmark/src/include/mutex.h:24:24: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   24 | CMARK_DEFINE_ONCE(NAME); \
      |                        ^
/home/kebo/swift-cmark/src/registry.c:16:1: note: in expansion of macro â��CMARK_DEFINE_LOCKâ��
   16 | CMARK_DEFINE_LOCK(extensions);
      | ^~~~~~~~~~~~~~~~~
/home/kebo/swift-cmark/src/registry.c:16:30: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   16 | CMARK_DEFINE_LOCK(extensions);
      |                              ^
[ 54%] Building C object src/CMakeFiles/libcmark-gfm.dir/render.c.o
[ 57%] Building C object src/CMakeFiles/libcmark-gfm.dir/scanners.c.o
[ 59%] Building C object src/CMakeFiles/libcmark-gfm.dir/syntax_extension.c.o
[ 61%] Building C object src/CMakeFiles/libcmark-gfm.dir/utf8.c.o
[ 64%] Building C object src/CMakeFiles/libcmark-gfm.dir/xml.c.o
[ 66%] Linking C static library libcmark-gfm.a
[ 66%] Built target libcmark-gfm
[ 69%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/autolink.c.o
[ 71%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/core-extensions.c.o
/home/kebo/swift-cmark/extensions/core-extensions.c:22:30: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   22 | CMARK_DEFINE_ONCE(registered);
      |                              ^
[ 73%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/ext_scanners.c.o
[ 76%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/strikethrough.c.o
[ 78%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/table.c.o
[ 80%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/tagfilter.c.o
[ 83%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/tasklist.c.o
[ 85%] Linking C static library libcmark-gfm-extensions.a
[ 85%] Built target libcmark-gfm-extensions
[ 88%] Building C object src/CMakeFiles/cmark-gfm.dir/__/bin/main.c.o
[ 90%] Linking C executable cmark-gfm
[ 90%] Built target cmark-gfm
[ 92%] Building CXX object api_test/CMakeFiles/api_test.dir/cplusplus.cpp.o
[ 95%] Building C object api_test/CMakeFiles/api_test.dir/harness.c.o
[ 97%] Building C object api_test/CMakeFiles/api_test.dir/main.c.o
/home/kebo/swift-cmark/api_test/main.c: In function â��reentrant_parse_inline_extâ��:
/home/kebo/swift-cmark/api_test/main.c:1367:32: warning: ISO C forbids conversion of object pointer to function pointer type [-Wpedantic]
 1367 |     reentrant_call_func func = (reentrant_call_func)priv;
      |                                ^
/home/kebo/swift-cmark/api_test/main.c: In function â��parser_interruptâ��:
/home/kebo/swift-cmark/api_test/main.c:1391:46: warning: ISO C forbids conversion of function pointer to object pointer type [-Wpedantic]
 1391 |   cmark_syntax_extension_set_private(my_ext, (void *)&run_inner_parser, NULL);
      |                                              ^
[100%] Linking CXX executable api_test
[100%] Built target api_test
$ cat build/src/config.h
#ifndef CMARK_CONFIG_H
#define CMARK_CONFIG_H

#ifdef __cplusplus
extern "C" {
#endif

#define CMARK_THREADING 1

#ifdef __cplusplus
}
#endif

#endif

-DCMARK_THREADING=OFF

$ cmake -S . -B build -DCMARK_THREADING=OFF
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER - Failed
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS - Success
-- Checking _FILE_OFFSET_BITS for large files
-- Checking _FILE_OFFSET_BITS for large files - not needed
-- Found Python3: /usr/bin/python3 (found version "3.11.2") found components: Interpreter
-- Configuring done
-- Generating done
-- Build files have been written to: /home/kebo/swift-cmark/build
$ cmake --build build
[  2%] Building C object src/CMakeFiles/libcmark-gfm.dir/arena.c.o
[  4%] Building C object src/CMakeFiles/libcmark-gfm.dir/blocks.c.o
[  7%] Building C object src/CMakeFiles/libcmark-gfm.dir/buffer.c.o
[  9%] Building C object src/CMakeFiles/libcmark-gfm.dir/cmark.c.o
[ 11%] Building C object src/CMakeFiles/libcmark-gfm.dir/cmark_ctype.c.o
[ 14%] Building C object src/CMakeFiles/libcmark-gfm.dir/commonmark.c.o
[ 16%] Building C object src/CMakeFiles/libcmark-gfm.dir/footnotes.c.o
[ 19%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_href_e.c.o
[ 21%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_html_e.c.o
[ 23%] Building C object src/CMakeFiles/libcmark-gfm.dir/houdini_html_u.c.o
[ 26%] Building C object src/CMakeFiles/libcmark-gfm.dir/html.c.o
[ 28%] Building C object src/CMakeFiles/libcmark-gfm.dir/inlines.c.o
[ 30%] Building C object src/CMakeFiles/libcmark-gfm.dir/iterator.c.o
[ 33%] Building C object src/CMakeFiles/libcmark-gfm.dir/latex.c.o
[ 35%] Building C object src/CMakeFiles/libcmark-gfm.dir/linked_list.c.o
[ 38%] Building C object src/CMakeFiles/libcmark-gfm.dir/man.c.o
[ 40%] Building C object src/CMakeFiles/libcmark-gfm.dir/map.c.o
[ 42%] Building C object src/CMakeFiles/libcmark-gfm.dir/node.c.o
[ 45%] Building C object src/CMakeFiles/libcmark-gfm.dir/plaintext.c.o
[ 47%] Building C object src/CMakeFiles/libcmark-gfm.dir/plugin.c.o
[ 50%] Building C object src/CMakeFiles/libcmark-gfm.dir/references.c.o
[ 52%] Building C object src/CMakeFiles/libcmark-gfm.dir/registry.c.o
/home/kebo/swift-cmark/src/registry.c:16:30: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   16 | CMARK_DEFINE_LOCK(extensions);
      |                              ^
[ 54%] Building C object src/CMakeFiles/libcmark-gfm.dir/render.c.o
[ 57%] Building C object src/CMakeFiles/libcmark-gfm.dir/scanners.c.o
[ 59%] Building C object src/CMakeFiles/libcmark-gfm.dir/syntax_extension.c.o
[ 61%] Building C object src/CMakeFiles/libcmark-gfm.dir/utf8.c.o
[ 64%] Building C object src/CMakeFiles/libcmark-gfm.dir/xml.c.o
[ 66%] Linking C static library libcmark-gfm.a
[ 66%] Built target libcmark-gfm
[ 69%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/autolink.c.o
[ 71%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/core-extensions.c.o
/home/kebo/swift-cmark/extensions/core-extensions.c:22:30: warning: ISO C does not allow extra â��;â�� outside of a function [-Wpedantic]
   22 | CMARK_DEFINE_ONCE(registered);
      |                              ^
[ 73%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/ext_scanners.c.o
[ 76%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/strikethrough.c.o
[ 78%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/table.c.o
[ 80%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/tagfilter.c.o
[ 83%] Building C object extensions/CMakeFiles/libcmark-gfm-extensions.dir/tasklist.c.o
[ 85%] Linking C static library libcmark-gfm-extensions.a
[ 85%] Built target libcmark-gfm-extensions
[ 88%] Building C object src/CMakeFiles/cmark-gfm.dir/__/bin/main.c.o
[ 90%] Linking C executable cmark-gfm
[ 90%] Built target cmark-gfm
[ 92%] Building CXX object api_test/CMakeFiles/api_test.dir/cplusplus.cpp.o
[ 95%] Building C object api_test/CMakeFiles/api_test.dir/harness.c.o
[ 97%] Building C object api_test/CMakeFiles/api_test.dir/main.c.o
/home/kebo/swift-cmark/api_test/main.c: In function â��reentrant_parse_inline_extâ��:
/home/kebo/swift-cmark/api_test/main.c:1367:32: warning: ISO C forbids conversion of object pointer to function pointer type [-Wpedantic]
 1367 |     reentrant_call_func func = (reentrant_call_func)priv;
      |                                ^
/home/kebo/swift-cmark/api_test/main.c: In function â��parser_interruptâ��:
/home/kebo/swift-cmark/api_test/main.c:1391:46: warning: ISO C forbids conversion of function pointer to object pointer type [-Wpedantic]
 1391 |   cmark_syntax_extension_set_private(my_ext, (void *)&run_inner_parser, NULL);
      |                                              ^
[100%] Linking CXX executable api_test
[100%] Built target api_test
$ cat build/src/config.h
#ifndef CMARK_CONFIG_H
#define CMARK_CONFIG_H

#ifdef __cplusplus
extern "C" {
#endif

#define CMARK_THREADING 0

#ifdef __cplusplus
}
#endif

#endif

Building as an external dependency with SwiftPM

This CI workflow builds swift-format's executable and test executable for wasm32-unknown-wasi. swift-format depends on swift-cmark, and the following one is using this PR's branch. The test executable runs on a WebAssembly runtime (wasmtime). As you can see, all tests cases have passed.

https://github.com/kkebo/swift-format/actions/runs/11669491583

This change allows building the `cmark-gfm` target for both `wasm32-unknown-wasi` and `wasm32-unknown-wasip1-threads`. The other targets are not tested. The differences between two target triples: - `wasm32-unknown-wasi` - C - can import `unistd.h` - has `__wasi__` - does not have `__unix__` - **does not have `_REENTRANT`** - **does not have `_POSIX_THREADS`** - **does not support pthreads API at all** - Swift - can use `#if os(WASI)` - SwiftPM - `BuildSettingsCondition`'s `Platform` is `.wasi` - `wasm32-unknown-wasip1-threads` - C - can import `unistd.h` - has `__wasi__` - does not have `__unix__` - **has `_REENTRANT` (defined in wasi-libc)** - **has `_POSIX_THREADS` (defined in wasi-libc)** - **supports a subset of pthreads API** - Swift - can use `#if os(WASI)` - SwiftPM - `BuildSettingsCondition`'s `Platform` is `.wasi`

QuietMisdreavus · 2024-10-31T15:07:52Z

I feel like it would be nice to still have the ability to definitively disable the threading support, since upstream cmark-gfm doesn't have it. What do you think of adding a kind of CMARK_NO_THREADING option that can be set from CMake? The top-level CMakeLists.txt would be updated like this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 23eca3c5..4f741228 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -24,7 +24,8 @@ set(CMAKE_INCLUDE_CURRENT_DIR YES)
 
 option(CMARK_FUZZ_QUADRATIC "Build quadratic fuzzing harness" OFF)
 option(CMARK_LIB_FUZZER "Build libFuzzer fuzzing harness" OFF)
-option(CMARK_THREADING "Add locks around static accesses" OFF)
+option(CMARK_THREADING "Add locks around static accesses; will autodetect based on platform if not set" OFF)
+option(CMARK_NO_THREADING "Disable thread-support auto-detection" OFF)
 
 if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
     message(FATAL_ERROR "Do not build in-source.\nPlease remove CMakeCache.txt and the CMakeFiles/ directory.\nThen: mkdir build ; cd build ; cmake .. ; make")
@@ -44,6 +45,9 @@ if(CMARK_THREADING)
   include(FindThreads)
   add_compile_definitions(CMARK_THREADING)
 endif()
+if(CMARK_NO_THREADING)
+  add_compile_definitions(CMARK_NO_THREADING)
+endif()
 include(GNUInstallDirs)
 
 if(NOT MSVC OR CMAKE_HOST_SYSTEM_NAME STREQUAL Windows)

And then you'd guard the autodetect you added with a #if !defined(CMARK_THREADING) && !defined(CMARK_NO_THREADING) so that it's still enabled by default (including when built as a Swift package), but can be explicitly disabled as well if desired.

kkebo · 2024-10-31T15:56:09Z

Thank you for your suggestion. That sounds great, but I feel the naming is a little confusing. Users could specify both options at the same time like cmake -S . -B build -DCMARK_THREADING=ON -DCMARK_NO_THREADING=ON.

Instead, what do you think about undoing the behavior of CMARK_THREADING and changing the direction of this PR to the following?

diff --git a/api_test/harness.c b/api_test/harness.c
index f561aeac..a00ae9ef 100644
--- a/api_test/harness.c
+++ b/api_test/harness.c
@@ -54,7 +54,7 @@ void INT_EQ(test_batch_runner *runner, int got, int expected, const char *msg,
   }
 }
 
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__wasi__)
 #include <unistd.h>
 
 static char *write_tmp(char const *header, char const *data) {
@@ -79,7 +79,7 @@ void STR_EQ(test_batch_runner *runner, const char *got, const char *expected,
   va_end(ap);
 
   if (!cond) {
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__wasi__)
     char *got_fn = write_tmp("actual\n", got);
     char *expected_fn = write_tmp("expected\n", expected);
     char buf[1024];
diff --git a/src/include/mutex.h b/src/include/mutex.h
index 59427f34..d9f12554 100644
--- a/src/include/mutex.h
+++ b/src/include/mutex.h
@@ -3,9 +3,13 @@
 
 #include <stdbool.h>
 
+#if defined(CMARK_THREADING) && defined(__wasi__) && !defined(_REENTRANT)
+#undef CMARK_THREADING
+#endif
+
 #ifdef CMARK_THREADING
 
-#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
+#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__wasi__)
 #include <unistd.h>
 #endif

kkebo · 2024-10-31T15:59:19Z

I would like to hear @kateinoigakukun's opinion too, if possible.

kateinoigakukun · 2024-11-01T03:37:47Z

@kkebo I'm personally not a big fan of undefining a definition as I feel "including a header results in disappearing a definition" is a bit confusing.

How about interpreting CMARK_THREADING definition as a value with #if instead of just testing its existence.
In CMake build, we can just propagate CMARK_THREADING CMake variable value, and in SwiftPM build, we will get a way to explicitly turn off the support by swift build -Xcc -DCMARK_THREADING=0.

kkebo · 2024-11-03T18:00:46Z

Thanks for the advice. It seems to be possible to express the three cases (on, off, auto) in that way.

CMake
- Current
  - When running with -DCMARK_THREADING=OFF (default), CMARK_THREADING will not be defined in C.
  - When running with -DCMARK_THREADING=ON, CMARK_THREADING will be defined in C.
- New
  - When running with -DCMARK_THREADING=OFF (default), CMARK_THREADING will be 0 in C.
  - When running with -DCMARK_THREADING=ON, CMARK_THREADING will be 1 in C.
SwiftPM
- Current: CMARK_THREADING will be defined in C.
- New: CMARK_THREADING will not be defined in C.
In C,
- Current
  - When CMARK_THREADING is not defined, it will fall back to the branch for no threading support.
  - When CMARK_THREADING is is defined, it will execute the branch for threading support.
- New
  - When CMARK_THREADING is 0, it will fall back to the branch for no threading support.
  - When CMARK_THREADING is 1, it will execute the branch for threading support.
  - When CMARK_THREADING is not defined,
    - If building for targets that do not support threading (e.g. wasm32-unknown-wasi), CMARK_THREADING will be defined and be 0 when including mutex.h.
    - If building for the other targets including wasm32-unknown-wasip1-threads, CMARK_THREADING will be defined and be 1 when including mutex.h.

I felt that this method seemed to be the best at the moment.

new diff from gfm branch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 23eca3c5..02596d5d 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -42,7 +42,9 @@ include(FindAsan)
 if(CMARK_THREADING)
   set(THREADS_PREFER_PTHREAD_FLAG YES)
   include(FindThreads)
-  add_compile_definitions(CMARK_THREADING)
+  add_compile_definitions(CMARK_THREADING=1)
+else()
+  add_compile_definitions(CMARK_THREADING=0)
 endif()
 include(GNUInstallDirs)
 
diff --git a/Package.swift b/Package.swift
index f1acc5f3..618c280a 100644
--- a/Package.swift
+++ b/Package.swift
@@ -11,12 +11,9 @@ import PackageDescription
     // link time.
     let cSettings: [CSetting] = [
         .define("CMARK_GFM_STATIC_DEFINE", .when(platforms: [.windows])),
-        .define("CMARK_THREADING"),
     ]
 #else
-    let cSettings: [CSetting] = [
-        .define("CMARK_THREADING"),
-    ]
+    let cSettings: [CSetting] = []
 #endif
 
 let package = Package(
diff --git a/api_test/harness.c b/api_test/harness.c
index f561aeac..a00ae9ef 100644
--- a/api_test/harness.c
+++ b/api_test/harness.c
@@ -54,7 +54,7 @@ void INT_EQ(test_batch_runner *runner, int got, int expected, const char *msg,
   }
 }
 
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__wasi__)
 #include <unistd.h>
 
 static char *write_tmp(char const *header, char const *data) {
@@ -79,7 +79,7 @@ void STR_EQ(test_batch_runner *runner, const char *got, const char *expected,
   va_end(ap);
 
   if (!cond) {
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__wasi__)
     char *got_fn = write_tmp("actual\n", got);
     char *expected_fn = write_tmp("expected\n", expected);
     char buf[1024];
diff --git a/src/include/mutex.h b/src/include/mutex.h
index 59427f34..25630622 100644
--- a/src/include/mutex.h
+++ b/src/include/mutex.h
@@ -3,9 +3,17 @@
 
 #include <stdbool.h>
 
-#ifdef CMARK_THREADING
+#ifndef CMARK_THREADING
+#if defined(__wasi__) && !defined(_REENTRANT)
+#define CMARK_THREADING 0
+#else
+#define CMARK_THREADING 1
+#endif
+#endif
+
+#if CMARK_THREADING
 
-#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
+#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(__wasi__)
 #include <unistd.h>
 #endif

In addition to successful builds for wasm32-unknown-wasi and wasm32-unknown-wasip1-threads, I've tested CMake too.

CMake logs

$ cmake -S . -B build -DCMARK_THREADING=OFF
-- Configuring done
-- Generating done
-- Build files have been written to: /home/kebo/swift-cmark/build
$ grep CMARK_THREADING build
build/extensions/CMakeFiles/libcmark-gfm-extensions.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=0

build/CMakeCache.txt
285:CMARK_THREADING:BOOL=OFF

build/api_test/CMakeFiles/api_test.dir/flags.make
6:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=0
12:CXX_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=0

build/src/CMakeFiles/libcmark-gfm.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=0

build/src/CMakeFiles/cmark-gfm.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=0
$ cmake -S . -B build -DCMARK_THREADING=ON
-- Configuring done
-- Generating done
-- Build files have been written to: /home/kebo/swift-cmark/build
$ grep CMARK_THREADING build
build/extensions/CMakeFiles/libcmark-gfm-extensions.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=1

build/CMakeCache.txt
285:CMARK_THREADING:BOOL=ON

build/api_test/CMakeFiles/api_test.dir/flags.make
6:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=1
12:CXX_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=1

build/src/CMakeFiles/libcmark-gfm.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=1

build/src/CMakeFiles/cmark-gfm.dir/flags.make
5:C_DEFINES = -DCMARK_GFM_STATIC_DEFINE -DCMARK_THREADING=1

kkebo · 2024-11-03T18:23:26Z

@QuietMisdreavus If this change addresses your concerns, I would change the PR to this.

compnerd · 2024-11-04T05:56:52Z

This would mean that the library built with the C compiler could have threading enabled, but then when imported into swift would not have threading enabled. This feels like it would cause issues. Perhaps we need to consider having a config.h generated instead. That could be checked in for SPM usage.

This reverts commit 6f64876.

This reverts commit aef4761.

kkebo · 2024-11-04T17:47:15Z

@compnerd Thank you for your comment. It would certainly be better to use config.h to make the CMARK_THREADING value persistent. I've pushed commits and rewritten the description.

I referred to swift-cmark's previous commits and https://github.com/github/cmark-gfm.

In addition, as it seemed more appropriate to place the code for when CMARK_THREADING is not defined in cmark-gfm_config.h rather than in mutex.h, I changed it.

compnerd

This seems reasonable to me. Please get @QuietMisdreavus to sign off on this.

kateinoigakukun requested a review from compnerd October 12, 2024 07:01

kkebo added 3 commits November 5, 2024 01:42

Revert "remove empty config.h and cmark-gfm_config.h headers"

c0cf35c

This reverts commit 6f64876.

Revert "build: hoist CMARK_THREADING to a build-only config"

08c5a51

This reverts commit aef4761.

build: change CMARK_THREADING to 0 or 1

149b31f

compnerd reviewed Nov 4, 2024

View reviewed changes

kkebo mentioned this pull request Nov 14, 2024

[DNM] PR test for wasm32-wasi branch of kkebo/swift-cmark kkebo/swift-format#106

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add partial support for WASI #75

feat: add partial support for WASI #75

kkebo commented Oct 12, 2024 •

edited

Loading

QuietMisdreavus commented Oct 31, 2024

kkebo commented Oct 31, 2024

kkebo commented Oct 31, 2024

kateinoigakukun commented Nov 1, 2024

kkebo commented Nov 3, 2024 •

edited

Loading

kkebo commented Nov 3, 2024

compnerd commented Nov 4, 2024

kkebo commented Nov 4, 2024 •

edited

Loading

compnerd left a comment

feat: add partial support for WASI #75

Are you sure you want to change the base?

feat: add partial support for WASI #75

Conversation

kkebo commented Oct 12, 2024 • edited Loading

Description

The current problem

Proposed solution

Checks

Building with SwiftPM

Building with CMake and Make

Building as an external dependency with SwiftPM

QuietMisdreavus commented Oct 31, 2024

kkebo commented Oct 31, 2024

kkebo commented Oct 31, 2024

kateinoigakukun commented Nov 1, 2024

kkebo commented Nov 3, 2024 • edited Loading

kkebo commented Nov 3, 2024

compnerd commented Nov 4, 2024

kkebo commented Nov 4, 2024 • edited Loading

compnerd left a comment

Choose a reason for hiding this comment

kkebo commented Oct 12, 2024 •

edited

Loading

kkebo commented Nov 3, 2024 •

edited

Loading

kkebo commented Nov 4, 2024 •

edited

Loading