mdavidsaver / pvxs

PVA protocol client/server library and utilities.
https://mdavidsaver.github.io/pvxs/
Other
19 stars 25 forks source link

Build fails against Base 7.0.1 #7

Closed ralphlange closed 4 years ago

ralphlange commented 4 years ago

PVXS does not build against Base 7.0.1 Building against 7.0.1 fails with

/usr/bin/g++  -D_GNU_SOURCE -D_DEFAULT_SOURCE         -DUSE_TYPED_RSET -I../../src   -D_X86_64_  -DUNIX  -Dlinux     -O3 -g   -Wall     -std=c++11  -mtune=generic     -m64  -I. -I../O.Common -I. -I. -I.. -I../../include/compiler/gcc -I../../include/os/Linux -I../../include -I/opt/codac-6.0/epics/include/compiler/gcc -I/opt/codac-6.0/epics/include/os/Linux -I/opt/codac-6.0/epics/include       -I/opt/codac-6.0/include   -c ../testget.cpp
/usr/bin/g++  -D_GNU_SOURCE -D_DEFAULT_SOURCE         -DUSE_TYPED_RSET -I../../src   -D_X86_64_  -DUNIX  -Dlinux     -O3 -g   -Wall     -std=c++11  -mtune=generic     -m64  -I. -I../O.Common -I. -I. -I.. -I../../include/compiler/gcc -I../../include/os/Linux -I../../include -I/opt/codac-6.0/epics/include/compiler/gcc -I/opt/codac-6.0/epics/include/os/Linux -I/opt/codac-6.0/epics/include       -I/opt/codac-6.0/include   -c ../testmon.cpp
[WARN] ../testconfig.cpp: In function 'void {anonymous}::testParse()':
[ERROR] ../testconfig.cpp:43:40: error: 'epicsEnvUnset' was not declared in this scope
[WARN] epicsEnvUnset("EPICS_PVA_ADDR_LIST");
[WARN] ^
[WARN] make[2]: *** [testconfig.o] Error 1
[WARN] make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/home/DCS/langer/work/CODAC/m-epics-pvxs/target/pvxs-0.0.git20200522/test/O.linux-x86_64'
[WARN] make[1]: *** [install.linux-x86_64] Error 2
make[1]: Leaving directory `/home/DCS/langer/work/CODAC/m-epics-pvxs/target/pvxs-0.0.git20200522/test'
[WARN] make: *** [test.install] Error 2

To Reproduce Steps to reproduce the behavior:

  1. make
  2. oops

Expected behavior A working build.

Information:

ralphlange commented 4 years ago

Actually, I don't mind. So: Fixing the documentation is a valid solution for me.

mdavidsaver commented 4 years ago

Introducing new features on the stable branch is such a bother... Any idea how this version dependency should be expressed?

https://github.com/mdavidsaver/pvxs/blob/92ea351ebf1c04107a4c0a03f42d9855baf7cd65/test/testconfig.cpp#L42-L46

Maybe?

#if (EPICS_VERSION_INT>=VERSION_INT(3,15,6,0) && EPICS_VERSION_INT>=VERSION_INT(7,0,0,0)) || EPICS_VERSION_INT>=VERSION_INT(7,0,2,0))
ralphlange commented 4 years ago

Should be (V >= 3.15.6 && V < 7) || V >=7.0.2 with a less-than as second operand, right?!

New features really should have macros to test for them programmatically. The ugly but very useful HAS_EPICS_ENV_UNSET kind of things.

mdavidsaver commented 4 years ago

May be fixed by 64d0505610ab013ee02d07dff44b85fc3d1e5f39

ralphlange commented 4 years ago

Is fixed by 64d0505

What about a header file epicsFeatures.h where such HAS_* definitions are added with their features? (And using ugly version math for features in the past, as needed?) Keeps the regular code clean and anyone who needs them can do

#include <epicsFeatures.h>
...
#ifdef HAS_ENV_UNSET
...

@anjohnson - Opinions?

ralphlange commented 4 years ago

That was incomplete... Of course, in order to work for past Base versions, the epicsFeatures.h header must be from a separate module. That could be a script that checks the header files from the base version it is being used with, to generate the correct definitions in epicsFeatures.h.

The end user would have to add that 'EPICS Features' module to the RELEASE dependencies to be able to use the HAS_* macros. Isn't that reinventing autotools? Well,...

mdavidsaver commented 4 years ago

header must be from a separate module

I've thought about doing something like this in the past. Part of the reason I didn't was the complexity of dealing with three series. Partly because I could see the inevitable inclusion of wrapper/compat implementations adding yet more complexity. And partly because I see this as something a build tool can do more generally, and with less effort. So comparisons to autotools and especially gnulib seem appropriate to me.

mdavidsaver commented 4 years ago

Also unrelated to the reported issue. I've been experimenting with toolchain inspection w/ PVXS. So far not on master branch.

I'm starting with detecting whether the header sys/sdt.h for systemtap is available. In configure/Makefile I have

$(TOP)/configure/CONFIG_SDT.Common.$(T_A): maybe_sdt.h ../Makefile
        $(CPP) $(CPPFLAGS) $(INCLUDES) $< > maybe_sdt.c \
         && echo "USR_CPPFLAGS+=-DUSE_SDT" > $@ \
         || echo "# No SDT" > $@

The generated CONFIG_SDT.Common.$(T_A) is included by Makefiles in other directories.

It works fine with SHELL=sh builds. Based on tests with WINE I think it will work with SHELL=cmd as well.

anjohnson commented 4 years ago

If you're introducing new features to Base and want to be able to test for them, put a #define HAS_<feature-name> in the header file of the Base API header that declares it, e.g:

woz$ git grep 'define HAS_'
documentation/RELEASE_NOTES.md:#define HAS_aidset
modules/database/src/std/rec/aaiRecord.dbd.pod:    %#define HAS_aaidset
modules/database/src/std/rec/aaoRecord.dbd.pod:    %#define HAS_aaodset
modules/database/src/std/rec/aiRecord.dbd.pod:    %#define HAS_aidset
modules/database/src/std/rec/aoRecord.dbd.pod:    %#define HAS_aodset
ralphlange commented 4 years ago

Sure. But how to deal with new features that didn't do that? Like epicsEnvUnset() (the root of this ticket)?

anjohnson commented 4 years ago

The simplest way is probably to develop and publish a common epicsFeatures.h header out-of-tree, which modules that need it would include a private copy of (but should not install/publish). That would have to use EPICS_VERSION_INT comparisons so it could be fairly ugly inside, but a module would only need to update its copy when it starts using a new feature (kind of like updating the version of the .ci module they incorporate). There are probably several examples of these kinds of tests in github.com/epics-modules already.