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

Use libdir variable in pktsetup Makefile #15

Closed
wants to merge 1 commit into from

Conversation

kyrios123
Copy link
Contributor

I use /usr/lib64 so it would be nice to use $(libdir) instead of using a hardcoded /lib value in the Makefile

@pali
Copy link
Owner

pali commented Mar 22, 2018

Hm... better way would be to ask udev for correct directory and not to try guessing it.

Seems that pkg-config knows it and pkg-config --variable=udevdir udev returns /lib/udev.

There is also PKG_CHECK_VAR macro for autoconf to retrieve variable from pkg-config, see: https://autotools.io/pkgconfig/variables.html But it is supported only in the recent pkg-config version (>= 0.28). And I do not think that depending on such recent version just for udevdir is a good idea.

Anyway, there is a PKG_PROG_PKG_CONFIG macro for autoconf which fills path to pkg-config executable and then you can execute pkg-config with --variable parameter to retrieve needed udev path. This should work also with old pkg-config versions.

@pali
Copy link
Owner

pali commented Mar 22, 2018

Maybe something like this putting above AM_CONDITIONAL(USE_READLINE... in configure.ac?

m4_include([pkg.m4])
PKG_PROG_PKG_CONFIG
PKG_CHECK_MODULES(UDEV, [udev], [ac_cv_udevdir=`$PKG_CONFIG --variable=udevdir udev`])
AC_SUBST(UDEVDIR, $ac_cv_udevdir)

And then using UDEVDIR in Makefile.am? It is untested.

@kyrios123
Copy link
Contributor Author

kyrios123 commented Mar 23, 2018

I just shared this because I'm trying to get rid of /usr/lib and this did the job but to be honest, I don't know much about autotools. So if something better can be done, just ignore this PR !

@pali
Copy link
Owner

pali commented Mar 23, 2018

$(libdir) is not a good idea. Correct way is really to use udevdir. But becuase all my testing machines uses /lib/udev I cannot test change which I provided above...

@kyrios123
Copy link
Contributor Author

Hi,

I made a test locally on the udftools 2.0 tarball

I added this in the configure.ac (your code without the m4_include())

PKG_PROG_PKG_CONFIG
PKG_CHECK_MODULES(UDEV, [udev], [ac_cv_udevdir=`$PKG_CONFIG --variable=udevdir udev`])
AC_SUBST(UDEVDIR, $ac_cv_udevdir)

the Makefile.am looks like this

sbin_PROGRAMS = pktsetup
pktsetup_SOURCES = pktsetup.c
EXTRA_DIST = pktsetup.rules

install-data-local:
	mkdir -p "$(DESTDIR)$(UDEVDIR)/rules.d"
	$(INSTALL_DATA) "$(srcdir)/pktsetup.rules" "$(DESTDIR)$(UDEVDIR)/rules.d/80-pktsetup.rules"

uninstall-local:
	rm -f "$(DESTDIR)$(UDEVDIR)/rules.d/80-pktsetup.rules"

and

Here is how my libudev.pc looks like

#  This file is part of systemd.
#
#  systemd is free software; you can redistribute it and/or modify it
#  under the terms of the GNU Lesser General Public License as published by
#  the Free Software Foundation; either version 2.1 of the License, or
#  (at your option) any later version.

prefix=/usr
exec_prefix=/usr
libdir=/usr/lib64
includedir=/usr/include

Name: libudev
Description: Library to access udev device information
Version: 235
Libs: -L${libdir} -ludev
Cflags: -I${includedir}

There is no error but the udev rules aren't installed under /usr/lib64 !?
/usr/lib/udev/rules.d/80-pktsetup.rules

@pali
Copy link
Owner

pali commented Mar 23, 2018

... --variable=udevdir udev ...

Here is how my libudev.pc looks like

You are looking at the wrong file. pkg-config is called with udev, not with libudev. Search for udev.pc

@pali
Copy link
Owner

pali commented Apr 22, 2018

Can you recheck udev.pc?

@kyrios123
Copy link
Contributor Author

Sorry. I completely forgot about this. It is working fine locally. I close this and I'll send another PR done as you suggested.

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.

None yet

2 participants