link-grammar icon indicating copy to clipboard operation
link-grammar copied to clipboard

Emscripten build fails

Open ampli opened this issue 2 years ago • 3 comments

In recent CI checks, in the Emscripten configuration LT_INIT checks for threads.h, as can be seen from the following output (still exists in PR #1365 from Jan 13 2023):

checking for sysroot... no
checking for a working dd... /usr/bin/dd
checking how to truncate binary pipes... /usr/bin/dd bs=4096 count=1
checking for mt... mt
checking if mt is a manifest tool... no
checking for stdio.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for strings.h... yes
checking for sys/stat.h... yes
checking for sys/types.h... yes
checking for unistd.h... yes
checking for vfork.h... no
checking for threads.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs

Note the check for threads.h. It defines HAVE_THREADS_H, which protects the threads stuff that shouldn't be used if Emscripten is configured. This happens in my development system too.

It seems this is something new, as in the CI output for the Emscription configuration in PR #1352 (5 Nov 2022) threads.h doesn't appear:

checking for sysroot... no
checking for a working dd... /usr/bin/dd
checking how to truncate binary pipes... /usr/bin/dd bs=4096 count=1
checking for mt... mt
checking if mt is a manifest tool... no
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs

Also note that in the older output there is the message "checking for ANSI C header files." instead of the check for stdio.h. This Emscription configuration doesn't.

I thought this could be due to a recent change in configure.ac, that removed or reordered something that changed the behaviour of LT_INIT. However, I couldn't find such a change, and when I tried to configure Emscripten in my system using old branches, LT_INIT there also detected threads.h.

I also couldn't find any reference that checking for threads.h is something new in LT_INIT. So this is a mystery for me.

I can think of two ways to fix this problem:

  1. Add AC_DEFINE(HAVE_THREADS_H, 0) if Emscripten is defined. This should work but is not so nice since it the definitions will then be -DHAVE_THREADS_H=1 ... -DHAVE_THREADS_H=0.
  2. If Emscripten configuration is detected, define _EMSCRIPTEN__. Then add its check in the source code:
- #if HAVE_THREADS_H
+ #if HAVE_THREADS_H && !__EMSCRIPTYEN__

Option 1 is easier to implement, as only a change in configure is needed. But I think 2 is better, as it is clearer. I can send a PR for 1 or 2, as desired.

ampli avatar Jan 15 '23 23:01 ampli

Option 2 seems clearer. Can we close #1352 after this?

linas avatar Jan 16 '23 00:01 linas

Option 2 seems clearer.

I will send the PR tomorrow.

Can we close #1352 after this?

Yes.

ampli avatar Jan 16 '23 01:01 ampli

I will issue a version 5.12.1 shortly after that, since the regex fix seems important enough to require that.

linas avatar Jan 16 '23 01:01 linas