Patchwork [v2] image-prelink: remove hardcoded path assumptions

login
register
mail settings
Submitter Phil Blundell
Date May 17, 2011, 2:17 p.m.
Message ID <1305641835.2429.228.camel@phil-desktop>
Download mbox | patch
Permalink /patch/4257/
State New, archived
Headers show

Comments

Phil Blundell - May 17, 2011, 2:17 p.m.
Signed-off-by: Phil Blundell <philb@gnu.org>
---
 meta/classes/image-prelink.bbclass |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
Richard Purdie - May 17, 2011, 2:35 p.m.
On Tue, 2011-05-17 at 15:17 +0100, Phil Blundell wrote:
> Signed-off-by: Phil Blundell <philb@gnu.org>
> ---
>  meta/classes/image-prelink.bbclass |   13 +++++--------
>  1 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/classes/image-prelink.bbclass b/meta/classes/image-prelink.bbclass
> index ee0951c..350c29d 100644
> --- a/meta/classes/image-prelink.bbclass
> +++ b/meta/classes/image-prelink.bbclass
> @@ -12,25 +12,22 @@ prelink_image () {
>  	echo "Size before prelinking $pre_prelink_size."
>  
>  	# We need a prelink conf on the filesystem, add one if it's missing
> -	if [ ! -e ${IMAGE_ROOTFS}/etc/prelink.conf ]; then
> -		cp ${STAGING_DIR_NATIVE}/etc/prelink.conf \
> -			${IMAGE_ROOTFS}/etc/prelink.conf
> +	if [ ! -e ${IMAGE_ROOTFS}${sysconfdir}/prelink.conf ]; then
> +		cp ${STAGING_DIR_NATIVE}${sysconfdir_native}/prelink.conf \
> +			${IMAGE_ROOTFS}${sysconfdir}/prelink.conf
>  		dummy_prelink_conf=true;
>  	else
>  		dummy_prelink_conf=false;
>  	fi
>  
>  	# prelink!
> -	${STAGING_DIR_NATIVE}/usr/sbin/prelink --root ${IMAGE_ROOTFS} -amR
> +	${STAGING_DIR_NATIVE}${sbindir_native}/prelink --root ${IMAGE_ROOTFS} -amR -N -c ${sysconfdir}/prelink.conf
>  
>  	# Remove the prelink.conf if we had to add it.
>  	if [ "$dummy_prelink_conf" == "true" ]; then
> -		rm -f ${IMAGE_ROOTFS}/etc/prelink.conf
> +		rm -f ${IMAGE_ROOTFS}${sysconfdir}/prelink.conf
>  	fi
>  
> -	# Cleanup temporary file, it's not needed...
> -	rm -f ${IMAGE_ROOTFS}/etc/prelink.cache
> -

The patch description and what it does don't quite match (the rm is
removed and options are added to prelink).

Cheers,

Richard
Mark Hatle - May 17, 2011, 2:45 p.m.
On 5/17/11 9:35 AM, Richard Purdie wrote:
> On Tue, 2011-05-17 at 15:17 +0100, Phil Blundell wrote:
>> Signed-off-by: Phil Blundell <philb@gnu.org>
>> ---
>>  meta/classes/image-prelink.bbclass |   13 +++++--------
>>  1 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/meta/classes/image-prelink.bbclass b/meta/classes/image-prelink.bbclass
>> index ee0951c..350c29d 100644
>> --- a/meta/classes/image-prelink.bbclass
>> +++ b/meta/classes/image-prelink.bbclass

...

>>  	# prelink!
>> -	${STAGING_DIR_NATIVE}/usr/sbin/prelink --root ${IMAGE_ROOTFS} -amR
>> +	${STAGING_DIR_NATIVE}${sbindir_native}/prelink --root ${IMAGE_ROOTFS} -amR -N -c ${sysconfdir}/prelink.conf

...

>>  
>> -	# Cleanup temporary file, it's not needed...
>> -	rm -f ${IMAGE_ROOTFS}/etc/prelink.cache
>> -
> 
> The patch description and what it does don't quite match (the rm is
> removed and options are added to prelink).

I agree the commit message needs some more description.  Just as an FYI.  The -N
option causes the prelinker to not generate/save the cache file.  While the -c
tells it to read a specific .conf file.

So the actual code changes are good, the description is the only thing that
needs to be improved, so someone not familiar with the prelinker can understand
the change.

--Mark

> Cheers,
> 
> Richard
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Phil Blundell - May 17, 2011, 2:47 p.m.
On Tue, 2011-05-17 at 15:35 +0100, Richard Purdie wrote:
> The patch description and what it does don't quite match (the rm is
> removed and options are added to prelink).

The added "-c ..." option is necessary in case ${sysconfdir_native} and
${sysconfdir} are not the same thing.

The "-N" tells prelink to not generate a cache file, which seems a
better plan than having it generate one which we then have to delete.
But I can drop that change, and/or send it as a separate patch if you
prefer.

p.
Richard Purdie - May 17, 2011, 3:17 p.m.
On Tue, 2011-05-17 at 15:47 +0100, Phil Blundell wrote:
> On Tue, 2011-05-17 at 15:35 +0100, Richard Purdie wrote:
> > The patch description and what it does don't quite match (the rm is
> > removed and options are added to prelink).
> 
> The added "-c ..." option is necessary in case ${sysconfdir_native} and
> ${sysconfdir} are not the same thing.
> 
> The "-N" tells prelink to not generate a cache file, which seems a
> better plan than having it generate one which we then have to delete.
> But I can drop that change, and/or send it as a separate patch if you
> prefer.

I'm fine with the change, I'd just like the commit message to match :)

Cheers,

Richard

Patch

diff --git a/meta/classes/image-prelink.bbclass b/meta/classes/image-prelink.bbclass
index ee0951c..350c29d 100644
--- a/meta/classes/image-prelink.bbclass
+++ b/meta/classes/image-prelink.bbclass
@@ -12,25 +12,22 @@  prelink_image () {
 	echo "Size before prelinking $pre_prelink_size."
 
 	# We need a prelink conf on the filesystem, add one if it's missing
-	if [ ! -e ${IMAGE_ROOTFS}/etc/prelink.conf ]; then
-		cp ${STAGING_DIR_NATIVE}/etc/prelink.conf \
-			${IMAGE_ROOTFS}/etc/prelink.conf
+	if [ ! -e ${IMAGE_ROOTFS}${sysconfdir}/prelink.conf ]; then
+		cp ${STAGING_DIR_NATIVE}${sysconfdir_native}/prelink.conf \
+			${IMAGE_ROOTFS}${sysconfdir}/prelink.conf
 		dummy_prelink_conf=true;
 	else
 		dummy_prelink_conf=false;
 	fi
 
 	# prelink!
-	${STAGING_DIR_NATIVE}/usr/sbin/prelink --root ${IMAGE_ROOTFS} -amR
+	${STAGING_DIR_NATIVE}${sbindir_native}/prelink --root ${IMAGE_ROOTFS} -amR -N -c ${sysconfdir}/prelink.conf
 
 	# Remove the prelink.conf if we had to add it.
 	if [ "$dummy_prelink_conf" == "true" ]; then
-		rm -f ${IMAGE_ROOTFS}/etc/prelink.conf
+		rm -f ${IMAGE_ROOTFS}${sysconfdir}/prelink.conf
 	fi
 
-	# Cleanup temporary file, it's not needed...
-	rm -f ${IMAGE_ROOTFS}/etc/prelink.cache
-
 	pre_prelink_size=`du -ks ${IMAGE_ROOTFS} | awk '{size = $1 ; print size }'`
 	echo "Size after prelinking $pre_prelink_size."
 }