[Buildroot] [PATCH v6] dependencies.sh: improve the missing perl modules detection

[Buildroot] [PATCH v6] dependencies.sh: improve the missing perl modules detection

[Vicente Olivert Riera]

Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
---
Changes v5 -> v6:
  - Remove the check for perl itself, as is already done at line #161.

Changes v4 -> v5:
  - Use the string concatenation operator (+=) to assing the values to
    the required_perl_modules. That way the comments about why each
    module is needed can be in the same line where the module is added
    to the variable.
    Use also += for the missing_perl_modules variable.

Changes v3 -> v4:
  - Thread::Queue module is actually needed because host-automake uses
    it. Comment modified accordingly. (Highlighted by Thomas Petazzoni)

Changes v2 -> v3:
  - Add comments about why each module is needed. (Suggested by Baruch
    Siach)

Changes v1 -> v2:
  - Check first for perl itself, and then the modules. Also keep the
    comment which says that perl is needed to build host-autoconf.
    (Suggested by Baruch Siach)

 support/dependencies/dependencies.sh |   27 ++++++++++++++++++++++-----
 1 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index 01ad828..8ff667c 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -236,10 +236,27 @@ if grep -q ^BR2_HOSTARCH_NEEDS_IA32_COMPILER=y $BR2_CONFIG ; then
 	fi
 fi
 
-# Check that the Perl installation is complete enough to build
-# host-autoconf.
-if ! perl  -e "require Data::Dumper" > /dev/null 2>&1 ; then
-	echo "Your Perl installation is not complete enough, at least Data::Dumper is missing."
-	echo "On Debian/Ubuntu distributions, install the 'perl' package."
+# Check that the Perl installation is complete enough for Buildroot.
+# Here is the space-separated list of the required modules.
+required_perl_modules="Data::Dumper" # Needed to build host-autoconf
+required_perl_modules+=" Thread:Queue" # Used by host-automake
+
+# This variable will keep the modules that are missing in your system.
+missing_perl_modules=""
+
+for pm in $required_perl_modules ; do
+	if ! perl  -e "require $pm" > /dev/null 2>&1 ; then
+		missing_perl_modules+=" $pm"
+	fi
+done
+
+if [ -n "$missing_perl_modules" ] ; then
+	echo "Your Perl installation is not complete enough; at least the following"
+	echo "modules are missing:"
+	echo ""
+	for pm in $missing_perl_modules ; do
+		echo -e "\t $pm"
+	done
+	echo ""
 	exit 1
 fi
-- 
1.7.1

[Buildroot] [PATCH v6] dependencies.sh: improve the missing perl modules detection

[Thomas Petazzoni]

Vicente,

I think the patch is great, but there are some implementation issues,
see below.

On Fri, 25 Sep 2015 11:06:25 +0100, Vicente Olivert Riera wrote:

> -# Check that the Perl installation is complete enough to build
> -# host-autoconf.
> -if ! perl  -e "require Data::Dumper" > /dev/null 2>&1 ; then
> -	echo "Your Perl installation is not complete enough, at least Data::Dumper is missing."
> -	echo "On Debian/Ubuntu distributions, install the 'perl' package."
> +# Check that the Perl installation is complete enough for Buildroot.
> +# Here is the space-separated list of the required modules.

The space-separated comment doesn't make much sense IMO.

> +required_perl_modules="Data::Dumper" # Needed to build host-autoconf
> +required_perl_modules+=" Thread:Queue" # Used by host-automake

The += operator is bash specific, and not POSIX compliant. Since this
dependencies.sh script specifies #!/bin/sh, you're not guaranteed to be
executed with bash.

> +
> +# This variable will keep the modules that are missing in your system.
> +missing_perl_modules=""
> +
> +for pm in $required_perl_modules ; do
> +	if ! perl  -e "require $pm" > /dev/null 2>&1 ; then
> +		missing_perl_modules+=" $pm"

Ditto.

> +	fi
> +done
> +
> +if [ -n "$missing_perl_modules" ] ; then
> +	echo "Your Perl installation is not complete enough; at least the following"
> +	echo "modules are missing:"
> +	echo ""

Quotes not needed here, just "echo" is sufficient.

> +	for pm in $missing_perl_modules ; do
> +		echo -e "\t $pm"
> +	done
> +	echo ""

Ditto.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v6] dependencies.sh: improve the missing perl modules detection

[Vicente Olivert Riera]

Hi Thomas,

thanks a lot for your review. See comments below, please.

On 09/28/2015 09:56 PM, Thomas Petazzoni wrote:
> Vicente,
> 
> I think the patch is great, but there are some implementation issues,
> see below.
> 
> On Fri, 25 Sep 2015 11:06:25 +0100, Vicente Olivert Riera wrote:
> 
>> -# Check that the Perl installation is complete enough to build
>> -# host-autoconf.
>> -if ! perl  -e "require Data::Dumper" > /dev/null 2>&1 ; then
>> -	echo "Your Perl installation is not complete enough, at least Data::Dumper is missing."
>> -	echo "On Debian/Ubuntu distributions, install the 'perl' package."
>> +# Check that the Perl installation is complete enough for Buildroot.
>> +# Here is the space-separated list of the required modules.
> 
> The space-separated comment doesn't make much sense IMO.

Uhm..., ok. I think having two examples (Data::Dumper and Thread:Queue)
is enough to understand how to add more modules to the list. I will
remove the comment.

>> +required_perl_modules="Data::Dumper" # Needed to build host-autoconf
>> +required_perl_modules+=" Thread:Queue" # Used by host-automake
> 
> The += operator is bash specific, and not POSIX compliant. Since this
> dependencies.sh script specifies #!/bin/sh, you're not guaranteed to be
> executed with bash.

Totally true, I will fix that. Thanks!

>> +
>> +# This variable will keep the modules that are missing in your system.
>> +missing_perl_modules=""
>> +
>> +for pm in $required_perl_modules ; do
>> +	if ! perl  -e "require $pm" > /dev/null 2>&1 ; then
>> +		missing_perl_modules+=" $pm"
> 
> Ditto.

Noted.

>> +	fi
>> +done
>> +
>> +if [ -n "$missing_perl_modules" ] ; then
>> +	echo "Your Perl installation is not complete enough; at least the following"
>> +	echo "modules are missing:"
>> +	echo ""
> 
> Quotes not needed here, just "echo" is sufficient.

Yeah, that's right.

>> +	for pm in $missing_perl_modules ; do
>> +		echo -e "\t $pm"
>> +	done
>> +	echo ""
> 
> Ditto.

Noted.

I will send a new version today.

Regards,

Vincent.


> Thomas
>