Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emscripten build fails #1374

Open
ampli opened this issue Jan 15, 2023 · 3 comments
Open

Emscripten build fails #1374

ampli opened this issue Jan 15, 2023 · 3 comments

Comments

@ampli
Copy link
Member

ampli commented Jan 15, 2023

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.

@linas
Copy link
Member

linas commented Jan 16, 2023

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

@ampli
Copy link
Member Author

ampli commented Jan 16, 2023

Option 2 seems clearer.

I will send the PR tomorrow.

Can we close #1352 after this?

Yes.

@linas
Copy link
Member

linas commented Jan 16, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants