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

Generate and install libgap.pc for use with pkg-config #5080

Merged
merged 21 commits into from
Oct 14, 2022
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
5 changes: 3 additions & 2 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ jobs:
- uses: gap-actions/setup-cygwin@v1
if: ${{ runner.os == 'Windows' }}
with:
PKGS_TO_INSTALL: 'wget,git,gcc-g++,gcc-core,m4,libgmp-devel,make,automake,libtool,autoconf,autoconf2.5,zlib-devel,libreadline-devel,libmpc-devel,libmpfr-devel,xdg-utils'
PKGS_TO_INSTALL: 'wget,git,gcc-g++,gcc-core,m4,libgmp-devel,make,automake,libtool,autoconf,autoconf2.5,zlib-devel,libreadline-devel,libmpc-devel,libmpfr-devel,xdg-utils,pkg-config'

# There are two cygwin installs on github actions (ours,
# and a preinstalled one which we can't use as not enough packages are installed.
Expand Down Expand Up @@ -228,8 +228,9 @@ jobs:
fi
sudo apt-get update
sudo apt-get install --no-install-recommends "${packages[@]}"
sudo apt-get install pkg-config
elif [ "$RUNNER_OS" == "macOS" ]; then
brew install gmp zlib
brew install gmp zlib pkg-config
fi
python -m pip install gcovr

Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/confdefs.h
/conftest*
/libtool
/libgap.pc
Copy link
Member

Choose a reason for hiding this comment

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

BTW, why libgap.pc and not gap.pc? (I don't want to bikeshed, and I am fine with keeping it as-is! I am merely curious and wondering if there is a deeper rationale for these names that you are aware of. Looking at the pkgconfig dir of one of my machines, I see gap.pc, mpfr.pc, zlib.pc yet libcurl.pc and libzmq.pc -- so it seems pretty much random?

Copy link
Member Author

Choose a reason for hiding this comment

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

To me, libgap.pc is pretty much what's needed to include libgap in the project. But gap.pc is a bit unclear.

/src/config.h.in
/src/config.h.in~

Expand Down
31 changes: 30 additions & 1 deletion Makefile.rules
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ clean:
rm -f bin/gap.sh sysinfo.gap*
rm -f gap$(EXEEXT) gac ffgen
rm -f libgap.la
rm -f libgap.pc
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
rm -rf .libs
rm -f doc/wsp.g
rm -f cnf/GAP-*
Expand Down Expand Up @@ -656,10 +657,12 @@ install-headers: $(FFDATA_H) build/version.h
# Create a symlink to support packages using `#include "src/compiled.h"`
ln -sfn . $(DESTDIR)$(includedir)/gap/src

install-libgap: libgap.la
install-libgap: libgap.la libgap.pc
@echo "Warning, 'make install-libgap' is incomplete"
$(INSTALL) -d -m 0755 $(DESTDIR)$(libdir)
$(LTINSTALL) -s libgap.la $(DESTDIR)$(libdir)
$(INSTALL) -d -m 0755 $(DESTDIR)$(libdir)/pkgconfig
$(INSTALL) -m 0644 libgap.pc $(DESTDIR)$(libdir)/pkgconfig


.PHONY: install install-bin install-doc install-gaproot install-headers install-libgap
Expand Down Expand Up @@ -1258,6 +1261,28 @@ testlibgap: ${LIBGAPTESTS}
$(v) -A -l $(top_srcdir) -q -T --nointeract >$(v).out && \
diff $(top_srcdir)/$(v).expect $(v).out && ) :

# test libgap's pkg-config's libgap version reporting
testpkgconfigversion: install-libgap
ifeq ($(shell PKG_CONFIG_PATH=$(DESTDIR)$(libdir)/pkgconfig pkg-config --modversion libgap), $(PACKAGE_VERSION))
@echo "pkg-config reports correct version of libgap: "$(PACKAGE_VERSION)
else
@echo "pkg-config does not report version of libgap, or is not installed"
dimpase marked this conversation as resolved.
Show resolved Hide resolved
exit 1
endif

# test libgap's pkg-config's libgap flags fetching
testpkgconfigbuild: install-libgap install-headers
$(eval PKG_REPORTED_CFLAGS := $(shell PKG_CONFIG_PATH=$(DESTDIR)$(libdir)/pkgconfig pkg-config --cflags libgap | cut -d' ' -f 1)/gap)
Copy link
Member

Choose a reason for hiding this comment

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

Why the cut?

Ahh, I think it's to append /gap to the first of include paths returned by--cflags, right? But why to the first, and why is it OK to discard any other? Are there other?

Copy link
Member Author

Choose a reason for hiding this comment

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

we need cut, as pkg-config --cflags adds a trailing space. For it's designed for convenience in the normal setups, not when you all of a sudden must add a nontrivial suffix for the include path. As I mentioned in a comment somewhere here, I'd rather have all the test code for libgap use headers with explicit gap/ prefix - then we'd need this weird hackery.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have to add gap/ here as libgap testing code uses #include <foo.h> rather than #include <gap/foo.h>.

$(eval PKG_REPORTED_LDFLAGS := $(shell PKG_CONFIG_PATH=$(DESTDIR)$(libdir)/pkgconfig pkg-config --libs libgap))
$(eval v := "tst/testlibgap/basic")
$(eval com := "tst/testlibgap/common")
$(CC) -c $(v).c -o $(v).o $(PKG_REPORTED_CFLAGS)
$(CC) -c $(com).c -o $(com).o $(PKG_REPORTED_CFLAGS)
$(QUIET_LINK)$(LINK) $(v).o -o $(v) $(com).o $(PKG_REPORTED_LDFLAGS)
$(v) -A -l $(top_srcdir) -q -T --nointeract >$(v).out && \
diff $(top_srcdir)/$(v).expect $(v).out
dimpase marked this conversation as resolved.
Show resolved Hide resolved


KERNELTESTS := $(addprefix tst/testkernel/,dstruct)

# run a test in tst/testkernel
Expand All @@ -1279,6 +1304,7 @@ testkernel: ${KERNELTESTS}

.PHONY: testinstall testmanuals testobsoletes teststandard testbugfix
.PHONY: testpackage testpackages testpackagesload testpackagesvars
.PHONY: testpkgconfigbuild testpkgconfigversion
.PHONY: check

########################################################################
Expand Down Expand Up @@ -1469,6 +1495,9 @@ build/stamp-h: $(srcdir)/src/config.h.in config.status
$(SHELL) ./config.status build/config.h
echo > $@

libgap.pc: $(srcdir)/libgap.pc.in config.status
@$(SHELL) ./config.status $@

ifneq ($(MAINTAINER_MODE),no)
$(srcdir)/src/config.h.in: $(configure_deps)
@if command -v autoheader >/dev/null 2>&1 ; then \
Expand Down
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,7 @@ AC_SUBST([GAP_DEFINES], [$GAP_DEFINES])

AC_CONFIG_FILES([GNUmakefile])
AC_CONFIG_FILES([CITATION])
AC_CONFIG_FILES([libgap.pc])
dimpase marked this conversation as resolved.
Show resolved Hide resolved
AC_CONFIG_FILES([doc/make_doc], [chmod +x doc/make_doc])
AC_CONFIG_FILES([doc/versiondata])
AC_OUTPUT
7 changes: 7 additions & 0 deletions dev/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ GAPInput
test -f $GAPPREFIX/include/gap/version.h
test -f $GAPPREFIX/include/gap/src/compiled.h # for backwards compatibility
test -f $GAPPREFIX/lib/gap/sysinfo.gap
test -f $GAPPREFIX/lib/pkgconfig/libgap.pc
test -f $GAPPREFIX/share/gap/doc/ref/chap0_mj.html
test -f $GAPPREFIX/share/gap/grp/basic.gd
test -f $GAPPREFIX/share/gap/hpcgap/lib/hpc/tasks.g
Expand All @@ -257,6 +258,12 @@ GAPInput
# TODO: should we install the GAP test suite???
cp -R $SRCDIR/tst $GAPPREFIX/share/gap/
$GAPPREFIX/bin/gap $GAPPREFIX/share/gap/tst/testinstall.g

# test integration with pkg-config
cd "$SRCDIR"
# make install # might be actually needed for testpkgconfigbuild
make testpkgconfigversion
make testpkgconfigbuild
;;

testmanuals)
Expand Down
11 changes: 11 additions & 0 deletions libgap.pc.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
prefix=@prefix@
exec_prefix=@exec_prefix@
libdir=@libdir@
includedir=@prefix@/include

Name: GAP
Description: Library containing the kernel of GAP, a computer algebra system
dimpase marked this conversation as resolved.
Show resolved Hide resolved
URL: https://www.gap-system.org
Version: @GAP_VERSION@
Libs: -L${libdir} -lgap
Cflags: -I${includedir}