Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Linux support for external tirpc library #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Linux support for external tirpc library #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions asyn/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@ asyn_SYS_LIBS_WIN32 = ws2_32 winmm
asyn_SYS_LIBS_cygwin32 = $(CYGWIN_RPC_LIB)

# Some linux systems moved RPC related symbols to libtirpc
# Define TIRPC in configure/CONFIG_SITE in this case
# Set TIRPC* variables in configure/CONFIG_SITE in this case
ifeq ($(TIRPC),YES)
USR_INCLUDES_Linux += -I/usr/include/tirpc
asyn_SYS_LIBS_Linux += tirpc
USR_INCLUDES_Linux += -I$(TIRPC_INCLUDE)
ifeq ($(TIRPC_EXTERNAL),YES)
tirpc_DIR = $(TIRPC_LIB)
asyn_LIBS_Linux += tirpc
else
asyn_SYS_LIBS_Linux += tirpc
endif
endif

SRC_DIRS += $(ASYN)/asynDriver
Expand Down
10 changes: 8 additions & 2 deletions configure/CONFIG_SITE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,14 @@ DRV_FTDI_USE_LIBFTDI1=NO
#EPICS_LIBCOM_ONLY=YES

# Some linux systems moved RPC related symbols to libtirpc
# To enable linking against this library, uncomment the following line
# TIRPC=YES
# To enable linking against this library, set TIRPC=YES
TIRPC=NO
# If TIRPC_EXTERNAL=NO, libtirpc is a system library; otherwise, it is not
TIRPC_EXTERNAL=NO
# Path to the include files for libtirpc
TIRPC_INCLUDE=/usr/include/tirpc
# If TIRPC_EXTERNAL=YES, path to the library files for libtirpc
TIRPC_LIB=
Comment on lines +58 to +65
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in most cases in AD for example, ***_EXTERNAL specifies whether to use a version of the library built as a part of the EPICS build system or not, rather than a system version vs. source install. Maybe we could automatically use the system version if TIRPC_INCLUDE and TIRPC_LIB are not set, and we can comment them out by default? Then we wouldn't need the TIRPC_EXTERNAL.

Something like: https://github.com/areaDetector/ADCore/blob/0ebde8129e652876b7fd71b459275725f70b2da6/ADApp/commonDriverMakefile#L230

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was basically trying to be consistent with areaDetector's configure/EXAMPLE_CONFIG_SITE.local, which contains the following comment:

# Configure which 3rd party libraries to use and where to find them.
# For each library XXX the following definitions are used:
# WITH_XXX      Build the plugins and drivers that require this library.
#               Build the source code for this library in ADSupport if XXX_EXTERNAL=NO.
# XXX_EXTERNAL  If NO then build the source code for this library in ADSupport.
# XXX_INCLUDE   If XXX_EXTERNAL=YES then this is the path to the include files for XXX.
#               However, if XXX is a system library whose include files are in a 
#               standard include search path then do not define XXX_INCLUDE.
# XXX_LIB       If XXX_EXTERNAL=YES then this is the path to the library files for XXX.
#               However, if XXX is a system library whose library files in a 
#               standard library search path then do not define XXX_LIB.

But I'd be happy with the approach you proposed too. Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also see an approach like what you're describing in measComp/measCompApp/src/Makefile. So, do you want to make the change, or do you want me to change this PR?


-include $(SUPPORT)/configure/CONFIG_SITE

Expand Down
9 changes: 7 additions & 2 deletions testGpibApp/src/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ testGpibVx_SRCS_vxWorks += testGpibVx_registerRecordDeviceDriver.cpp
testGpib_LIBS += devTestGpib
testGpib_LIBS += testSupport asyn
ifeq ($(TIRPC),YES)
USR_INCLUDES += -I/usr/include/tirpc
testGpib_SYS_LIBS += tirpc
USR_INCLUDES += -I$(TIRPC_INCLUDE)
ifeq ($(TIRPC_EXTERNAL),YES)
tirpc_DIR = $(TIRPC_LIB)
testGpib_LIBS += tirpc
else
testGpib_SYS_LIBS += tirpc
endif
endif
SYS_PROD_LIBS_cygwin32 += $(CYGWIN_RPC_LIB)
testGpib_LIBS += $(EPICS_BASE_IOC_LIBS)
Expand Down