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

set rpath for external libsemigroups #954

Merged
merged 2 commits into from
Sep 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions GNUmakefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ HPCOMBI_CONSTEXPR_FUN_ARGS = @HPCOMBI_CONSTEXPR_FUN_ARGS@
LIBSEMIGROUPS_HPCOMBI_ENABLED = @LIBSEMIGROUPS_HPCOMBI_ENABLED@
WITH_INCLUDED_LIBSEMIGROUPS = @WITH_INCLUDED_LIBSEMIGROUPS@
SYS_IS_CYGWIN = @SYS_IS_CYGWIN@
LIBSEMIGROUPS_RPATH = @LIBSEMIGROUPS_RPATH@

# sources
KEXT_SOURCES = src/bipart.cpp
Expand Down
2 changes: 1 addition & 1 deletion Makefile.gappkg
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ gen/%.$(GAP_OBJEXT): %.s GNUmakefile
# build rule for linking all object files together into a kernel extension
$(KEXT_SO): $(KEXT_OBJS)
@mkdir -p $(@D)
$(QUIET_GAC)$(GAP_CXX) -o $@ $(GAP_LDFLAGS) $(GAC_LDFLAGS) $(KEXT_OBJS) $(KEXT_LDFLAGS)
$(QUIET_GAC)$(GAP_CXX) -o $@ $(GAP_LDFLAGS) $(GAC_LDFLAGS) $(KEXT_OBJS) $(KEXT_LDFLAGS) $(LIBSEMIGROUPS_RPATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say this should rather be added to KEXT_LDFLAGS, that'll be better in the long run. But for now, either works shrug


# hook into `make clean`
clean: clean-kext
Expand Down
10 changes: 8 additions & 2 deletions m4/ax_check_libsemigroup.m4
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ AC_DEFUN([AX_CHECK_LIBSEMIGROUPS], [
[libsemigroups >= $REQUI_LIBSEMIGROUPS_VERSION],
[need_included_libsemigroups=no],
[need_included_libsemigroups=yes])],
[AC_MSG_NOTICE([ignoring flag --with-external-libsemigroups, the Semigroups configure file was created on a system without m4 macros for pkg-config available...])])
[AC_MSG_NOTICE(m4_normalize([
ignoring flag --with-external-libsemigroups, the Semigroups configure file
was created on a system without m4 macros for pkg-config available...]))])
fi
if test "$need_included_libsemigroups" = yes; then
AC_MSG_NOTICE([using included libsemigroups...])
Expand All @@ -27,7 +29,9 @@ AC_DEFUN([AX_CHECK_LIBSEMIGROUPS], [
[AS_IF(
[test -f libsemigroups/include/libsemigroups.hpp],
[],
[AC_MSG_ERROR([libsemigroups is required, clone or download the repo from https://github.com/libsemigroups/libsemigroups into this directory])])])
[AC_MSG_ERROR(m4_normalize([
libsemigroups is required, clone or download the repo from
https://github.com/libsemigroups/libsemigroups into this directory]))])])

dnl Temporary workaround for compatibility with dev version of
dnl libsemigroups which doesn't contain .VERSION file by default
Expand All @@ -53,6 +57,8 @@ AC_DEFUN([AX_CHECK_LIBSEMIGROUPS], [
else
LIBSEMIGROUPS_VERSION="$(pkg-config --modversion libsemigroups)"
AC_MSG_NOTICE([using external libsemigroups $LIBSEMIGROUPS_VERSION])
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation looks odd here (maybe tabs versus spaces), but that was of course already there before this PR, so not an objection to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Indeed, this whole file mixes tabs and spaces liberally. Probably best to ignore this for this PR)

PKG_CHECK_VAR([LIBSEMIGROUPS_RPATH], [libsemigroups], [libdir],
[AC_SUBST([LIBSEMIGROUPS_RPATH],[-Wl,-rpath,${LIBSEMIGROUPS_RPATH}])])
Comment on lines +60 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the core of the change. Looks OK to me.

fi

AS_IF([test "x$need_included_libsemigroups" = xyes],
Expand Down