From 1c582ab51d49ed7e7d792e7612f95ac60e17fcba Mon Sep 17 00:00:00 2001
From: Andy C <andy@oilshell.org>
Date: Sun, 5 Jan 2025 19:04:52 -0500
Subject: [PATCH] [osh] Test for FNM_EXTMATCH at parse time!

So if you run on musl libc with extended globs, you will get a loud
error, rather than a silent one.
---
 NINJA-config.sh           |  2 --
 build/py.sh               |  9 +++++++++
 configure                 | 12 ++++++++++--
 core/state.py             |  3 ++-
 cpp/libc.cc               |  3 +--
 osh/word_parse.py         |  6 ++++++
 pyext/libc.c              |  9 +--------
 test/configure-effects.sh | 11 ++++++++---
 8 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/NINJA-config.sh b/NINJA-config.sh
index 69d03127c7..9d0dea3909 100755
--- a/NINJA-config.sh
+++ b/NINJA-config.sh
@@ -127,8 +127,6 @@ main() {
   echo DEPS prebuilt/ninja/*/deps.txt
 
   echo
-  # Special _OIL_DEV for -D GC_TIMING
-  _OIL_DEV=1 ./configure
 
   # Reads the deps.txt files above
   PYTHONPATH=. build/ninja_main.py
diff --git a/build/py.sh b/build/py.sh
index 31e0210a5c..3fe7012c66 100755
--- a/build/py.sh
+++ b/build/py.sh
@@ -364,7 +364,14 @@ py-extensions() {
   fastfunc
 }
 
+do-configure() {
+  # Special _OIL_DEV for -D GC_TIMING
+  _OIL_DEV=1 ./configure
+}
+
 minimal() {
+  do-configure
+
   build/stamp.sh write-git-commit
 
   py-source
@@ -410,6 +417,8 @@ time-helper() {
 }
 
 all() {
+  do-configure
+
   rm -f *.so  # 12/2019: to clear old symlinks, maybe get rid of
 
   build/stamp.sh write-git-commit
diff --git a/configure b/configure
index 0d29cd3cea..5a1f8821c8 100755
--- a/configure
+++ b/configure
@@ -369,7 +369,7 @@ check_sizeof() {
   echo "#define $pp_var $actual_bytes"
 }
 
-detect_c_language() {
+echo_libc() {
   # Exported by pyext/libc.c
   if test "$have_fnm_extmatch" = 1; then
     echo '#define HAVE_FNM_EXTMATCH 1'
@@ -382,6 +382,10 @@ detect_c_language() {
   else
     echo '#define HAVE_GLOB_PERIOD 0'
   fi
+}
+
+detect_c_language() {
+  echo_libc
   echo
 
   # This is the equivalent of AC_CHECK_SIZEOF(int, 4)
@@ -535,12 +539,16 @@ main() {
   echo_shell_vars > $sh_out
   log "Wrote $sh_out"
 
+  local c_out=_build/detected-config.h
+
   # Fast mode
   if test -n "$_OIL_DEV"; then
+    # Do only this subset
+    echo_libc > $c_out
+    log "Wrote $c_out"
     return
   fi
 
-  local c_out=_build/detected-config.h
   detect_c_language > $c_out
   log "Wrote $c_out"
 }
diff --git a/core/state.py b/core/state.py
index 5ab17620e0..76c63e1017 100644
--- a/core/state.py
+++ b/core/state.py
@@ -350,7 +350,8 @@ def _MaybeWarnDotglob():
     if HAVE_GLOB_PERIOD == 0:
         # GNU libc and musl libc have GLOB_PERIOD, but Android doesn't
         print_stderr(
-            "osh warning: GLOB_PERIOD wasn't found in libc, so 'shopt -s dotglob' won't work")
+            "osh warning: GLOB_PERIOD wasn't found in libc, so 'shopt -s dotglob' won't work"
+        )
 
 
 class MutableOpts(object):
diff --git a/cpp/libc.cc b/cpp/libc.cc
index 5dad8d4715..99f47e4946 100644
--- a/cpp/libc.cc
+++ b/cpp/libc.cc
@@ -40,8 +40,7 @@ int fnmatch(BigStr* pat, BigStr* str, int flags) {
 #ifdef FNM_EXTMATCH
   flags |= FNM_EXTMATCH;
 #else
-  // TODO: We should detect this at ./configure time, and then maybe flag these
-  // at parse time, not runtime
+  // Detected by ./configure
 #endif
 
   int result = ::fnmatch(pat->data_, str->data_, flags);
diff --git a/osh/word_parse.py b/osh/word_parse.py
index 84159f9c1e..f0f77075e1 100644
--- a/osh/word_parse.py
+++ b/osh/word_parse.py
@@ -104,6 +104,8 @@
 from osh import word_compile
 from mycpp.mylib import tagswitch
 
+from libc import HAVE_FNM_EXTMATCH
+
 from typing import List, Optional, Tuple, cast
 from typing import TYPE_CHECKING
 if TYPE_CHECKING:
@@ -1855,6 +1857,10 @@ def _ReadCompoundWord3(self, lex_mode, eof_type, empty_ok):
                     done = True
 
                 else:
+                    if HAVE_FNM_EXTMATCH == 0:
+                        p_die(
+                            "Extended glob won't work without FNM_EXTMATCH support in libc",
+                            self.cur_token)
                     part = self._ReadExtGlob()
                 w.parts.append(part)
 
diff --git a/pyext/libc.c b/pyext/libc.c
index f0a15286b7..290cec0d0c 100644
--- a/pyext/libc.c
+++ b/pyext/libc.c
@@ -63,15 +63,8 @@ func_fnmatch(PyObject *self, PyObject *args) {
     return NULL;
   }
 
-  // NOTE: Testing for __GLIBC__ is the version detection anti-pattern.  We
-  // should really use feature detection in our configure script.  But I plan
-  // to get rid of the dependency on FNM_EXTMATCH because it doesn't work on
-  // musl libc (or OS X).  Instead we should compile extended globs to extended
-  // regex syntax.
-#ifdef __GLIBC__
+#ifdef FNM_EXTMATCH
   flags |= FNM_EXTMATCH;
-#else
-  debug("Warning: FNM_EXTMATCH is not defined");
 #endif
 
   int ret = fnmatch(pattern, str, flags);
diff --git a/test/configure-effects.sh b/test/configure-effects.sh
index c9e1e12af1..99a836e7c9 100755
--- a/test/configure-effects.sh
+++ b/test/configure-effects.sh
@@ -37,15 +37,18 @@ test-osh() {
   # GLOB_PERIOD
   $osh -x -c 'shopt -s dotglob'
 
+  set +o errexit
+  # HAVE_PWENT
+  $osh -x -c 'compgen -A user'
+
   # FNM_EXTMATCH in glob()
   # Hm this will works
   $osh -x -c 'echo */*/t@(*.asdl|*.sh)'
+  echo status=$?
 
   # FNM_EXTMATCH in fnmatch()
   $osh -x -c 'case foo.py in @(*.asdl|*.py)) echo py ;; esac'
-
-  # HAVE_PWENT
-  $osh -x -c 'compgen -A user'
+  echo status=$?
 }
 
 cpp() {
@@ -59,6 +62,8 @@ py() {
 
   ln -s -f -v oil.ovm _bin/osh
 
+  #test-osh bin/osh
+
   test-osh _bin/osh
 }