On Mon, Jun 29, 2026 at 6:48 PM Agatha Isabelle Moreira <[email protected]> wrote:
>
> Hi Xi,
>
> On Fri, Jun 26, 2026 at 06:22:42PM +0800, Xi Ruoyao wrote:
> > On Fri, 2026-06-26 at 01:30 -0300, Agatha Isabelle Moreira wrote:
> > > Stop `configure` and throw an error when `--with-isl` is passed without
> > > an argument.
> > >
> > > If someone by mistake passes `--with-isl` using the option only or
> > > sepparated by a space instead of `=` it currently tries to build it.
> > >
> > > Since this syntax is valid in several programs (i.e. those built using
> > > getopt_long), throwing an error might prevent build-time issues.
> > >
> > > ChangeLog:
> > >
> > >     * configure.ac: Validate the value passed for `--with-isl` to
> > >     detect 'yes' (no argument).
> > >
> > >     * configure: Regenerate `configure` with `autoreconf-2.69`.
> > >
> > > Signed-off-by: Agatha Isabelle Moreira <[email protected]>
> > > ---
> > >  configure    | 3 +++
> > >  configure.ac | 3 +++
> > >  2 files changed, 6 insertions(+)
> > >
> > > diff --git a/configure b/configure
> > > index 02cd3bb7997..69f1b54f954 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -9588,6 +9588,9 @@ if test "${with_isl+set}" = set; then :
> > >  fi
> > >
> > >
> > > +if test "x$with_isl" = "xyes"; then
> > > +  as_fn_error $? "--with-isl requires a PATH argument" "$LINENO" 5
> > > +fi
> > >  # Treat --without-isl as a request to disable
> > >  # GRAPHITE support and skip all following checks.
> > >  if test "x$with_isl" != "xno"; then
> > > diff --git a/configure.ac b/configure.ac
> > > index ed66e1ea2e1..7961b281400 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -2018,6 +2018,9 @@ AC_ARG_WITH(isl,
> > >      Equivalent to --with-isl-include=PATH/include
> > >      plus --with-isl-lib=PATH/lib])])
> > >
> > > +if test "x$with_isl" = "xyes"; then
> > > +  AC_MSG_ERROR([--with-isl requires a PATH argument])
> > > +fi
> >
> > I think it will be more consistent with --without-isl to treat --with-
> > isl as a request to bail out if isl is not available.
>
> I can certainly do that, but wouldn't possibly break the user
> expectation?
>
> I mean, if the user by mistake passes `--with-isl` or `--with-isl
> /something` they are likely expecting isl. If we disable it because of
> insufficient parameters were given they might go through all the build
> process and end up with isl disable thinking otherwise.
>
> That's why I thought this patch would be a good idea, if we error the
> user will correct their command before they can move on, eliminating
> user's broken assumptions about what they are running.

But isn't the current behavior of --with-isl to error out if ISL
cannot be found?
I see

# Treat --without-isl as a request to disable
# GRAPHITE support and skip all following checks.
if test "x$with_isl" != "xno"; then
  # Check for isl
  dnl Provide configure switches and initialize islinc & isllibs
  dnl with user input.
  ISL_INIT_FLAGS
  dnl The versions of isl that work for Graphite
  ISL_CHECK_VERSION()
  dnl Only execute fail-action, if isl has been requested.
  ISL_IF_FAILED([
    AC_MSG_ERROR([Unable to find a usable isl.  See config.log for details.])])
fi

also ISL_INIT_FLAGS explicitly handles --with-isl so you'll be breaking peoples
existing scripts?

>
> It makes sense?
>
> >
> > >  # Treat --without-isl as a request to disable
> > >  # GRAPHITE support and skip all following checks.
> > >  if test "x$with_isl" != "xno"; then
> >
> > --
> > Xi Ruoyao <[email protected]>
>
> --
> Agatha Isabelle Moreira
> Systems Engineer | C

Reply via email to