Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH] alsa-utils: Install and delete from the same udev-rules-dir
@ 2017-09-21 14:56 Ola x Nilsson
  2017-09-21 15:54 ` Tanu Kaskinen
  0 siblings, 1 reply; 2+ messages in thread
From: Ola x Nilsson @ 2017-09-21 14:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: olani

The --with-udev-rules-dir option used with udev is exactly what the
configure script uses, so there is no need for it.
On the other hand, if we do not have an udev.pc file we should tell
alsa-utils where to install the rules so we know where they should be
deleted from.

Signed-off-by: Ola x Nilsson <olani@axis.com>
---
 meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb b/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
index c8f4b861bd..7fc6c29a61 100644
--- a/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
+++ b/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
@@ -9,14 +9,17 @@ DEPENDS = "alsa-lib ncurses libsamplerate0"
 
 PACKAGECONFIG ??= "udev"
 
+udevdir = "${nonarch_base_libdir}/udev"
+
 # alsabat can be built also without fftw support (with reduced functionality).
 # It would be better to always enable alsabat, but provide an option for
 # enabling/disabling fftw. The configure script doesn't support that, however
 # (at least in any obvious way), so for now we only support alsabat with fftw
 # or no alsabat at all.
 PACKAGECONFIG[bat] = "--enable-bat,--disable-bat,fftwf"
-
-PACKAGECONFIG[udev] = "--with-udev-rules-dir=`pkg-config --variable=udevdir udev`/rules.d,,udev"
+# The udev-rules dir is taken from udev.pc.  Set the dir explicitly
+# when udev is not in our DEPENDS.
+PACKAGECONFIG[udev] = ",--with-udev-rules-dir=${udevdir}/rules.d,udev"
 PACKAGECONFIG[manpages] = "--enable-xmlto, --disable-xmlto, xmlto-native docbook-xml-dtd4-native docbook-xsl-stylesheets-native"
 
 SRC_URI = "ftp://ftp.alsa-project.org/pub/utils/alsa-utils-${PV}.tar.bz2 \
@@ -101,9 +104,13 @@ do_install() {
 	rm ${D}${sbindir}/alsa-info.sh
 	rm -f ${D}${sbindir}/alsabat-test.sh
 
-	if ${@bb.utils.contains('PACKAGECONFIG', 'udev', 'false', 'true', d)}; then
-		# This is where alsa-utils will install its rules if we don't tell it anything else.
-		rm -rf ${D}${nonarch_base_libdir}/udev
-		rmdir --ignore-fail-on-non-empty ${D}${nonarch_base_libdir}
+	if [ "${@bb.utils.filter('PACKAGECONFIG', 'udev', d)}" ]; then
+		# This is where alsa-utils installs if there is an udev.pc file
+		udevdir=$(pkg-config --variable=udevdir udev)
+	else
+		# This is where we told alsa-utils to install the rules
+		udevdir=${udevdir}
 	fi
+	rm -rf ${D}$udevdir
+	rmdir --ignore-fail-on-non-empty ${D}${udevdir%/*}
 }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] alsa-utils: Install and delete from the same udev-rules-dir
  2017-09-21 14:56 [PATCH] alsa-utils: Install and delete from the same udev-rules-dir Ola x Nilsson
@ 2017-09-21 15:54 ` Tanu Kaskinen
  0 siblings, 0 replies; 2+ messages in thread
From: Tanu Kaskinen @ 2017-09-21 15:54 UTC (permalink / raw)
  To: Ola x Nilsson, openembedded-core; +Cc: olani

On Thu, 2017-09-21 at 16:56 +0200, Ola x Nilsson wrote:
> The --with-udev-rules-dir option used with udev is exactly what the
> configure script uses, so there is no need for it.
> On the other hand, if we do not have an udev.pc file we should tell
> alsa-utils where to install the rules so we know where they should be
> deleted from.
> 
> Signed-off-by: Ola x Nilsson <olani@axis.com>
> ---
>  meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb b/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
> index c8f4b861bd..7fc6c29a61 100644
> --- a/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
> +++ b/meta/recipes-multimedia/alsa/alsa-utils_1.1.4.bb
> @@ -9,14 +9,17 @@ DEPENDS = "alsa-lib ncurses libsamplerate0"
>  
>  PACKAGECONFIG ??= "udev"
>  
> +udevdir = "${nonarch_base_libdir}/udev"
> +
>  # alsabat can be built also without fftw support (with reduced functionality).
>  # It would be better to always enable alsabat, but provide an option for
>  # enabling/disabling fftw. The configure script doesn't support that, however
>  # (at least in any obvious way), so for now we only support alsabat with fftw
>  # or no alsabat at all.
>  PACKAGECONFIG[bat] = "--enable-bat,--disable-bat,fftwf"
> -
> -PACKAGECONFIG[udev] = "--with-udev-rules-dir=`pkg-config --variable=udevdir udev`/rules.d,,udev"
> +# The udev-rules dir is taken from udev.pc.  Set the dir explicitly
> +# when udev is not in our DEPENDS.
> +PACKAGECONFIG[udev] = ",--with-udev-rules-dir=${udevdir}/rules.d,udev"

I'll be reading this recipe every once in a while, and I feel this
comment won't be easy to understand after some time. My suggestion for
an alternative:

# If udev is not in PACKAGECONFIG, we will leave the udev rules
# unpackaged and remove them in do_install(). When removing the rules,
# we need to know where they were installed, which is why we set the
# udev-rules-dir variable here.

>  PACKAGECONFIG[manpages] = "--enable-xmlto, --disable-xmlto, xmlto-native docbook-xml-dtd4-native docbook-xsl-stylesheets-native"
>  
>  SRC_URI = "ftp://ftp.alsa-project.org/pub/utils/alsa-utils-${PV}.tar.bz2 \
> @@ -101,9 +104,13 @@ do_install() {
>  	rm ${D}${sbindir}/alsa-info.sh
>  	rm -f ${D}${sbindir}/alsabat-test.sh
>  
> -	if ${@bb.utils.contains('PACKAGECONFIG', 'udev', 'false', 'true', d)}; then
> -		# This is where alsa-utils will install its rules if we don't tell it anything else.
> -		rm -rf ${D}${nonarch_base_libdir}/udev
> -		rmdir --ignore-fail-on-non-empty ${D}${nonarch_base_libdir}
> +	if [ "${@bb.utils.filter('PACKAGECONFIG', 'udev', d)}" ]; then
> +		# This is where alsa-utils installs if there is an udev.pc file
> +		udevdir=$(pkg-config --variable=udevdir udev)
> +	else
> +		# This is where we told alsa-utils to install the rules
> +		udevdir=${udevdir}
>  	fi
> +	rm -rf ${D}$udevdir
> +	rmdir --ignore-fail-on-non-empty ${D}${udevdir%/*}

This will remove the udev rules also when udev is enabled. Previously
they were removed only when udev was disabled. I suppose this is a
mistake, since you didn't indicate this kind of change in the commit
message.

Some comment about what you're doing would be nice when using
${udevdir%/*}. At least I didn't understand what is happening here
without reading the bash manual.

-- 
Tanu

https://www.patreon.com/tanuk


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-09-21 16:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-21 14:56 [PATCH] alsa-utils: Install and delete from the same udev-rules-dir Ola x Nilsson
2017-09-21 15:54 ` Tanu Kaskinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox