Re: [PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 27, 2018, 17:02 UTC
Message-ID: <CAPig+cQ4p8kgAWji3r6WnudZdT4TOG15s1ip6p5SXmTec25mPw@mail.gmail.com>
In-Reply-To: <20181226230523.16572-3-randall.s.becker@rogers.com>

On Wed, Dec 26, 2018 at 6:05 PM <randall.s.becker@rogers.com> wrote:

Show 12 quoted lines

> A number of configuration options are not automatically detected by
> configure mechanisms, including the location of Perl and Python.
> [...]
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
> diff --git a/config.mak.uname b/config.mak.uname
> @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>         # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
> -       PERL_PATH = ${prefix}/bin/perl
> -       PYTHON_PATH = ${prefix}/bin/python
> +       PERL_PATH = /usr/bin/perl
> +       PYTHON_PATH = /usr/bin/python

Is the in-code comment about ${prefix} still applicable after this change?

> +       # The current /usr/coreutils/rm at lowest support level does not work
> +       # with the git test structure. Default to the older rm.
> +       RM = /bin/rm -f

This comment would be far more helpful if it explained in what way 'rm' actually fails with the test suite. Without that information, the comment is effectively useless.

Show 7 quoted lines

>         # As detected by './configure'.
>         # Missdetected, hence commented out, see below.
>         #NO_CURL = YesPlease
>         # Added manually, see above.
> +       # Missdetected, hence commented out, see below.
> +       #NO_CURL = YesPlease
> +       # Added manually, see above.

These added lines are just duplicating the existing line immediately above.

> +       # Not detected by ./configure. Add manually.
> +       NEEDS_SSL_WITH_CURL = YesPlease
> +       NEEDS_CRYPTO_WITH_SSL = YesPlease
> +       HAVE_DEV_TTY = YesPlease

I find these comments about 'configure' "misdetecting" or "not detecting" features confusing. The point of config.mak.uname is to provide sane defaults for people building without using the 'configure' script, so it feels weird to be talking about 'configure' here. Also, what does it mean to say that 'configure' "misdetects"? Does that mean that it fails to write assignments such as "NO_CURL = YesPlease" to config.autogen or does it mean that it writes incorrect assignments to that file?

Show 6 quoted lines

> @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> +       ifdef NO_PTHREADS
> +       else # WIP, use of Posix User Threads is planned but not working yet
> +               PTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include
> +               PTHREAD_LIBS = -lput
> +       endif

Why not a simpler 'ifndef'?