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

Attempt to auto-detect TIRPC=YES #187

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mdavidsaver
Copy link
Contributor

Attempts to detect when TIRPC=YES is needed. Currently only in the limited case where the host architecture is Linux.

Also, remove SNCSEQ from the CI builds as the BESSY site is still offline.

@AppVeyorBot
Copy link

Build asyn 1.0.240 completed (commit 3539991644 by @mdavidsaver)

@anjohnson
Copy link
Member

The Asyn build currently assumes that TIRPC is only needed on Linux, although that doesn't mean other OSs won't ever switch to that in the future. I approve of adding and installing a CONFIG_ASYN_MODULE file like this, it will automatically be included by any downstream support and IOC applications that use Asyn.

However static builds of those downstream applications won't necessarily know about tirpc if they don't call it directly (the synApps SSCAN module does, I've had to add a TIRPC flag to that for our RHEL-8 builds), but they will need to know whether to link their executables with the tirpc library (if the module's author never uses the VXI-11 transport or doesn't use RHEL-8 or similar they might not know about that need). It would be nice to put that library name in a variable when it is needed so the downstream Makefile just has to add that to PROD_SYS_LIBS or equivalent, or maybe Asyn could just add it to PROD_SYS_LIBS directly.

@mdavidsaver
Copy link
Contributor Author

mdavidsaver commented Jun 23, 2023

However static builds of those downstream applications won't necessarily know about tirpc ...

Yup, I know. I see this PR as a first small step towards dealing with a complication which has perhaps become the number one new user obstacle reported on tech-talk. (maybe a close second to CA timeout due to firewall)

fyi. I have to confront a similar situation with static linking wrt. libevent in the PVXS build. cf. CONFIG_PVXS_MODULE and
RULES_PVXS_MODULE. This logic is based on what I did earlier to automatically inject libpcre as a dependency to streamDevice. Although the situation with tirpc is different in some respects, such as needing to conditionally adjust the include search path, which requires knowing the installation prefix. It isn't clear to me if $(GNU_DIR) can be relied on historically.

@mdavidsaver
Copy link
Contributor Author

I am also thinking about ways to give a better error when rpc/rpc.h is missing. Something which might avoid some of the tech-talk traffic. eg. something like

#ifdef __has_include
#  if !__has_include(<rpc/rpc.h>)
#    error Missing rpc/rpc.h.  Try "apt-get install libtirpc-dev" or "dnf install tirpc-devel"
#  endif
#endif

@mdavidsaver
Copy link
Contributor Author

Also, I feel bound to point out that this PR, and any which may follow, are in effect re-inventing functionality which is present in all ~modern C/C++ build tools. Anything I do here will almost certainly be less comprehensive than eg. cmake find_path(), which extracts the compiler default search path to avoid needing to hard code /usr.

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

Successfully merging this pull request may close these issues.

None yet

3 participants