Skip to content

Commit

Permalink
[osh] Test for FNM_EXTMATCH at parse time!
Browse files Browse the repository at this point in the history
So if you run on musl libc with extended globs, you will get a loud
error, rather than a silent one.
  • Loading branch information
Andy C committed Jan 6, 2025
1 parent 950120a commit 1c582ab
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 18 deletions.
2 changes: 0 additions & 2 deletions NINJA-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions build/py.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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"
}
Expand Down
3 changes: 2 additions & 1 deletion core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
3 changes: 1 addition & 2 deletions cpp/libc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions osh/word_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
9 changes: 1 addition & 8 deletions pyext/libc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions test/configure-effects.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -59,6 +62,8 @@ py() {

ln -s -f -v oil.ovm _bin/osh

#test-osh bin/osh

test-osh _bin/osh
}

Expand Down

0 comments on commit 1c582ab

Please sign in to comment.