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.

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 non-system tirpc library #216

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

Conversation

jlmuir
Copy link

@jlmuir jlmuir commented Jan 27, 2025

Add support on Linux for using a tirpc library that is not a system library.

@AppVeyorBot
Copy link

Build asyn 1.0.290 failed (commit 8cacd75dde by @jlmuir)

Comment on lines 58 to 65
# 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=
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?

Copy link
Author

Choose a reason for hiding this comment

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

Didn't hear from you, so went ahead and made the change that I think you're wishing for and force-pushed. Thanks!

Add support on Linux for using a tirpc library that is not a system
library.
@jlmuir jlmuir force-pushed the external-tirpc-support branch from 5e8cf7e to 044951b Compare February 8, 2025 23:15
@AppVeyorBot
Copy link

Build asyn 1.0.293 failed (commit a804e432b1 by @jlmuir)

@jlmuir jlmuir changed the title Add Linux support for external tirpc library Add Linux support for non-system tirpc library Feb 9, 2025
@MarkRivers
Copy link
Member

I think this is a slippery slope. These are the SYS_LIBS referenced in asyn/asyn/Makefile:

  asyn_SYS_LIBS_Linux += tirpc
  asyn_SYS_LIBS += usb-1.0
  asyn_SYS_LIBS += ftdi1
  asyn_SYS_LIBS += ftdi
  asyn_SYS_LIBS += gpib

If we add comments for non-system locations of tirpc, then don't we also need to add them for usb-1.0, ftdi1, ftdi, and gpib?

Can you explain why you are not installing tirpc as a system library?

@jlmuir
Copy link
Author

jlmuir commented Feb 12, 2025

Re a slippery slope, I would say that supporting all library dependencies being non-system libraries would be good. For example, I think ADSupport mostly (fully?) does this. Thinking of libraries and programs that I've built from source that are not an EPICS module, it seems that most, if not all, of them support specifying the location of all dependent libraries, includes, and programs, and do not assume system locations. That's part of writing portable software.

The reason I'm not installing tirpc as a system library is that I'm planning to package all EPICS modules that I use as pkgsrc packages. I want to use pkgsrc because it's a cross-platform package management system. This means I can have consistent builds and installs of packages on macOS and Linux and possibly others. To make the binary packages that I build of these pkgsrc packages as portable as possible, I want to build them on a minimal system platform and with dependencies satisfied by other pkgsrc packages, not system packages. This way, dependencies can be cleanly specified and satisfied for each pkgsrc package, and I don't have library dependencies that can change out from under me (e.g., the user can't remove or upgrade a system package that a pkgsrc package depends on that then breaks a pkgsrc package). And anyone who uses these packages can install a package and know that it will work. If I didn't do it this way, then there'd be no guarantee that installing a binary pkgsrc package would even work because it could depend on a set of system packages that have to be installed separately on the system in order for it to work. And then I'd have to maintain a separate list of system packages per platform (and per package if I wanted to allow the user to only install what was really needed) that the user would need to install separately in order for a given pkgsrc package to actually work. Bad business.

@jlmuir
Copy link
Author

jlmuir commented Feb 12, 2025

Another reason that's not mine but could be someone else's is, if a user wants to build and install asyn and dependencies in their home directory, for example, or some other location that does not require administrator privileges to install to. By supporting a non-system tirpc, the user could install tirpc in their home directory and then be able to build asyn against it. This situation could arise on a system where the user doesn't have administrator privileges, and they either can't ask for other system packages to be installed, or the system administrator does not want to install other system packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants