Re: readline in base 7.0.7

[Michael Davidsaver via Core-talk <core-talk at aps.anl.gov>]

On 10/1/22 05:48, Mark Rivers wrote:
> Hi Michael,
>
> distclean fixed the problem.  I didn't know I needed to clean since it was a .c file I changed.
I wish this were never necessary.  However, a syntax error
in a generated Makefile snippet is currently a bit of a
special case as it is preventing the rule to regenerated
from running.

While it isn't actually necessary for configure/Makefile
to include the cfg/TOOLCHAIN* files.  We currently don't have
a way to short-circuit the include from configure/CONFIG
which is included by every Makefile.


> readline is now used with no change to the configure/os files.
>
> Thanks,
> Mark
>
>
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> *From:* Michael Davidsaver <mdavidsaver at gmail.com>
> *Sent:* Friday, September 30, 2022 6:00 PM
> *To:* Mark Rivers <rivers at cars.uchicago.edu>
> *Cc:* core-talk at aps.anl.gov <core-talk at aps.anl.gov>
> *Subject:* Re: readline in base 7.0.7
> On 9/30/22 15:43, Mark Rivers wrote:
> > Michael,
> >
> > I had already tried eliminating the continuation lines, but that did not help.
> >
> > I just copied that new line from your PR and pasted it into toolchain.c.  It still fails.
> Can you check if the generated files were actually re-created?
>
> eg.
>
> > cfg/TOOLCHAIN.linux-x86_64.linux-x86_64
> > configure/O.linux-x86_64/TOOLCHAIN.linux-x86_64.linux-x86_64
> A "make distclean" should remove them..
>
>
> ...
> > This is make output on Centos 7.
> fyi. The centos-7 CI build for this PR passes:
>
> https://github.com/epics-base/epics-base/actions/runs/3055627011/jobs/4928888222 <https://github.com/epics-base/epics-base/actions/runs/3055627011/jobs/4928888222>
>
>
> > corvette:epics-devel/base-7.0.7/configure>make
> >
> > make -C O.linux-x86_64 -f ../Makefile TOP=../.. \
> >
> >       T_A=linux-x86_64 install
> >
> > make[1]: Entering directory `/usr/local/epics-devel/base-7.0.7/configure/O.linux-x86_64'
> >
> > ../../cfg/TOOLCHAIN.linux-x86_64.linux-x86_64:13: *** missing separator.  Stop.
> >
> > make[1]: Leaving directory `/usr/local/epics-devel/base-7.0.7/configure/O.linux-x86_64'
> >
> > make: *** [install.linux-x86_64] Error 2
> >
> > I then tested on Ubuntu 22, and it worked in the same source directory.
> >
> > Thanks,
> >
> > Mark
> >
> > -----Original Message-----
> > From: Michael Davidsaver <mdavidsaver at gmail.com>
> > Sent: Friday, September 30, 2022 5:07 PM
> > To: Mark Rivers <rivers at cars.uchicago.edu>
> > Cc: core-talk at aps.anl.gov
> > Subject: Re: readline in base 7.0.7
> >
> > On 9/30/22 14:50, Mark Rivers wrote:
> >
> >   > Hi Michael,
> >
> >   >
> >
> >   > I just tried your patch, and it does not build.
> >
> > Andrew had me turn this into a PR.  What I have there now does build, and is different to what I posted previously.  So I must have hit the same error as you.  I guess I was getting the line continuations wrong somehow?  (then got frustrated and removed them)
> >
> > https://github.com/epics-base/epics-base/pull/296 <https://github.com/epics-base/epics-base/pull/296> <https://github.com/epics-base/epics-base/pull/296 <https://github.com/epics-base/epics-base/pull/296>>
> >
> >   > corvette:local/epics-devel/base-7.0.7>patch -p1 < patch1
> >
> >   >
> >
> >   > patching file configure/toolchain.c
> >
> >   >
> >
> >   > corvette:local/epics-devel/base-7.0.7>git diff configure/toolchain.c
> >
> >   >
> >
> >   > diff --git a/configure/toolchain.c b/configure/toolchain.c
> >
> >   >
> >
> >   > index 2f76b47..bc37cad 100644
> >
> >   >
> >
> >   > --- a/configure/toolchain.c
> >
> >   >
> >
> >   > +++ b/configure/toolchain.c
> >
> >   >
> >
> >   > @@ -45,5 +45,7 @@ COMMANDLINE_LIBRARY ?= READLINE
> >
> >   >
> >
> >   > COMMANDLINE_LIBRARY ?= EPICS
> >
> >   >
> >
> >   > #  endif
> >
> >   >
> >
> >   > #else
> >
> >   >
> >
> >   > -COMMANDLINE_LIBRARY ?= EPICS
> >
> >   >
> >
> >   > +COMMANDLINE_LIBRARY ?= $(strip $(if $(wildcard \
> >
> >   >
> >
> >   > +    $(if $(GNU_DIR),$(GNU_DIR)/include/readline/readline.h) \
> >
> >   >
> >
> >   > +), READLINE, EPICS))
> >
> >   >
> >
> >   > #endif
> >
> >   >
> >
> >   > My diff looks just like your patch, so I think I did it correctly.
> >
> >   >
> >
> >   > However, it fails to build and I can't figure out the problem, since the parentheses do match.
> >
> >   >
> >
> >   > corvette:local/epics-devel/base-7.0.7>make
> >
> >   >
> >
> >   > make -C ./configure install
> >
> >   >
> >
> >   > make[1]: Entering directory `/usr/local/epics-devel/base-7.0.7/configure'
> >
> >   >
> >
> >   > make -C O.linux-x86_64 -f ../Makefile TOP=../.. \
> >
> >   >
> >
> >   >      T_A=linux-x86_64 install
> >
> >   >
> >
> >   > make[2]: Entering directory `/usr/local/epics-devel/base-7.0.7/configure/O.linux-x86_64'
> >
> >   >
> >
> >   > ../../cfg/TOOLCHAIN.linux-x86_64.linux-x86_64:13: *** missing separator.  Stop.
> >
> >   >
> >
> >   > make[2]: Leaving directory `/usr/local/epics-devel/base-7.0.7/configure/O.linux-x86_64'
> >
> >   >
> >
> >   > make[1]: *** [install.linux-x86_64] Error 2
> >
> >   >
> >
> >   > make[1]: Leaving directory `/usr/local/epics-devel/base-7.0.7/configure'
> >
> >   >
> >
> >   > make: *** [configure.install] Error 2
> >
> >   >
> >
> >   > Thanks,
> >
> >   >
> >
> >   > Mark
> >
> >   >
> >
> >   > -----Original Message-----
> >
> >   > From: Michael Davidsaver <mdavidsaver at gmail.com <mailto:mdavidsaver at gmail.com <mailto:mdavidsaver at gmail.com>>>
> >
> >   > Sent: Friday, September 9, 2022 6:28 PM
> >
> >   > To: Mark Rivers <rivers at cars.uchicago.edu <mailto:rivers at cars.uchicago.edu <mailto:rivers at cars.uchicago.edu>>>
> >
> >   > Cc: core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov>>
> >
> >   > Subject: Re: readline in base 7.0.7
> >
> >   >
> >
> >   > On 9/8/22 11:44, Mark Rivers wrote:
> >
> >   >
> >
> >   >  >  > You will need to manually specify COMMANDLINE_LIBRARY for the
> >
> >   > time being.  (and hopefully put an OS upgrade on your roadmap)
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > This is unfortunate, since the automatic detection worked fine in base 7.0.6.1.
> >
> >   >
> >
> >   > fyi. https://github.com/epics-base/epics-base/pull/254 <https://github.com/epics-base/epics-base/pull/254> <https://github.com/epics-base/epics-base/pull/254 <https://github.com/epics-base/epics-base/pull/254>>
> >
> >   > <https://github.com/epics-base/epics-base/pull/254 <https://github.com/epics-base/epics-base/pull/254 <https://github.com/epics-base/epics-base/pull/254>>>
> >
> >   >
> >
> >   > As always, I would encourage those interested to follow PRs.
> >
> >   >
> >
> >   > Compatibility doesn't just happen by itself.
> >
> >   >
> >
> >   > The following lightly tested patch should restore the previous
> >
> >   >
> >
> >   > $(GNU_DIR) automatic detection for older compilers.  (not just
> >
> >   >
> >
> >   > linux)
> >
> >   >
> >
> >   >  > diff --git a/configure/toolchain.c b/configure/toolchain.c
> >
> >   >
> >
> >   >  > index 2f76b475b..bc37cad64 100644
> >
> >   >
> >
> >   >  > --- a/configure/toolchain.c
> >
> >   >
> >
> >   >  > +++ b/configure/toolchain.c
> >
> >   >
> >
> >   >  > @@ -45,5 +45,7 @@ COMMANDLINE_LIBRARY ?= READLINE
> >
> >   >
> >
> >   >  >  COMMANDLINE_LIBRARY ?= EPICS
> >
> >   >
> >
> >   >  >  #  endif
> >
> >   >
> >
> >   >  >  #else
> >
> >   >
> >
> >   >  > -COMMANDLINE_LIBRARY ?= EPICS
> >
> >   >
> >
> >   >  > +COMMANDLINE_LIBRARY ?= $(strip $(if $(wildcard \
> >
> >   >
> >
> >   >  > +    $(if $(GNU_DIR),$(GNU_DIR)/include/readline/readline.h) \
> >
> >   >
> >
> >   >  > +), READLINE, EPICS))
> >
> >   >
> >
> >   >  >  #endif
> >
> >   >
> >
> >   >  > The release notes really should say that manual editing is now required for older compilers, while it previously was not.
> >
> >   >
> >
> >   > Specific suggestions on wording are welcomed.  I find it
> >
> >   >
> >
> >   > difficult to write succinct documentation which speaks
> >
> >   >
> >
> >   > to every user.
> >
> >   >
> >
> >   >  >  They should also say which version of gcc is required to have the automatic detection work.
> >
> >   >
> >
> >   > GCC >=5.0
> >
> >   >
> >
> >   > https://gcc.gnu.org/projects/cxx-status.html#cxx17 <https://gcc.gnu.org/projects/cxx-status.html#cxx17> <https://gcc.gnu.org/projects/cxx-status.html#cxx17 <https://gcc.gnu.org/projects/cxx-status.html#cxx17>>
> >
> >   > <https://gcc.gnu.org/projects/cxx-status.html#cxx17 <https://gcc.gnu.org/projects/cxx-status.html#cxx17 <https://gcc.gnu.org/projects/cxx-status.html#cxx17>>>
> >
> >   >
> >
> >   > I don't know when clang or msvc added support.
> >
> >   >
> >
> >   >  >  I now need to spend a few hours rebuilding base and synApps for a bunch of architectures.
> >
> >   >
> >
> >   > I'm sorry to have wasted your time.
> >
> >   >
> >
> >   >  >  > and hopefully put an OS upgrade on your roadmap)
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > We have a number of machines where vendor support for kernel drivers limits such upgrades.  RHEL 7 has 2 more years before EOL, and is still used on many APS beamlines.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > Mark
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > -----Original Message-----
> >
> >   >
> >
> >   >  > From: Michael Davidsaver <mdavidsaver at gmail.com
> >
> >   > <mailto:mdavidsaver at gmail.com <mailto:mdavidsaver at gmail.com <mailto:mdavidsaver at gmail.com>>>>
> >
> >   >
> >
> >   >  > Sent: Thursday, September 8, 2022 1:29 PM
> >
> >   >
> >
> >   >  > To: Mark Rivers <rivers at cars.uchicago.edu
> >
> >   > <mailto:rivers at cars.uchicago.edu <mailto:rivers at cars.uchicago.edu <mailto:rivers at cars.uchicago.edu>>>>
> >
> >   >
> >
> >   >  > Cc: core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov>> <mailto:core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov>>>
> >
> >   >
> >
> >   >  > Subject: Re: readline in base 7.0.7
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > On 9/8/22 10:40, Mark Rivers via Core-talk wrote:
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > I just built base 7.0.7 on Centos 7 with no changes to the files in configure/os.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > Right, I wasn't paying attention.  Automatic detection now depends on GCC version.  As I recall, centos7 has GCC 4.8, which is too old.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  > You will need to manually specify COMMANDLINE_LIBRARY for the time
> >
> >   > being.  (and hopefully put an OS upgrade on your roadmap)
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > My IOCs now are not built with libreadline, while in base 7.0.6.1 they were, again with no changes to the configure/os files.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > I see this change between base 7.0.6.1 and base 7.0.7
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > corvette:base-7.0.7/configure/os>git diff R7.0.6.1 R7.0.7
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > CONFIG_SITE.Common.linux-x
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > CONFIG_SITE.Common.linux-x86
> >
> >   > CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > CONFIG_SITE.Common.linux-xscale_be
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > corvette:base-7.0.7/configure/os>git diff R7.0.6.1 R7.0.7
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > diff --git a/configure/os/CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > b/configure/os/CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > index e098b76..b98385f 100644
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > --- a/configure/os/CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > +++ b/configure/os/CONFIG_SITE.Common.linux-x86_64
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > @@ -15,10 +15,6 @@
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > #    to inform the system of the shared library location.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > -# Use GNU Readline if the header file is installed
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > -COMMANDLINE_LIBRARY = $(strip $(if $(wildcard \
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > -    $(GNU_DIR)/include/readline/readline.h), READLINE, EPICS))
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > -
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > # If libreadline needs additional libraries to be linked with
> >
> >   > it, try
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > # uncommenting each of the lines below in turn, starting with
> >
> >   > the top
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > # one and working downwards, until the build succeeds. Do a 'make rebuild'
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > The release notes say:
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > *Automatic COMMANDLINE_LIBRARY w/ newer compilers*
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > When built with a compiler supporting __has_include<>, the presence of the <readline/readline.h> will be used to automatically determine a default value for COMMANDLINE_LIBRARY.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > Mingw builds with readline support now link -ltermcap instead of -lcurses.
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > This should not effect sites which set explicitly set COMMANDLINE_LIBRARY as the only definition in Base now has the form COMMANDLINE_LIBRARY ?= ....
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > I interpret the first sentence here to mean that the test for
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > COMMANDLINE_LIBRARY is done in the compiler if possible, but it
> >
> >   > does
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > not say that the previous way of doing it in the CONFIG_SITE
> >
> >   > file (for
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > older compilers) has been removed
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > Does this mean that I now need to edit CONFIG_SITE.Common.linux-x86_64 to manually set COMMANDLINE_LIBRARY?
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > Thanks,
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  > Mark
> >
> >   >
> >
> >   >  >
> >
> >   >
> >
> >   >  >  >
> >
> >   >
> >
> >   >  >
> >
> >   >