[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Maxime Hadjinlian]

Avoid copy/pasting the same block of code to check if a program is
available on the host machine.

Also, introduce, BR2_NEEDS_HOST_JAVAC and BR2_NEEDS_HOST_JAR.
In a following patch, we will remove the specific check done for
classpath, and the classpath package will use these generic variant.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
Changes v1 -> v2:
  - Uses check_prog_host for which and sed too (Thomas Petazzoni)
---
 Config.in                            | 10 ++++++++++
 support/dependencies/dependencies.sh | 38 +++++++++++++++++++++---------------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/Config.in b/Config.in
index 55f5fd8..a24421f 100644
--- a/Config.in
+++ b/Config.in
@@ -23,6 +23,16 @@ config BR2_EXTERNAL
 config BR2_NEEDS_HOST_JAVA
 	bool
 
+# Hidden boolean selected by packages in need of javac in order to build
+# (example: classpath)
+config BR2_NEEDS_HOST_JAVAC
+	bool
+
+# Hidden boolean selected by packages in need of jar in order to build
+# (example: classpath)
+config BR2_NEEDS_HOST_JAR
+	bool
+
 # Hidden boolean selected by pre-built packages for x86, when they
 # need to run on x86-64 machines (example: pre-built external
 # toolchains, binary tools like SAM-BA, etc.).
diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index a8261b3..b18a888 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -51,18 +51,20 @@ if test -n "$PERL_MM_OPT" ; then
 	exit 1
 fi
 
-# Verify that which is installed
-if ! which which > /dev/null ; then
-	echo
-	echo "You must install 'which' on your build machine";
-	exit 1;
-fi;
+check_prog_host()
+{
+	prog="$1"
+	if ! which $prog > /dev/null ; then
+		echo >&2
+		echo "You must install '$prog' on your build machine" >&2
+		exit 1
+	fi
+}
 
-if ! which sed > /dev/null ; then
-	echo
-	echo "You must install 'sed' on your build machine"
-	exit 1
-fi
+# Verify that which is installed
+check_prog_host "which"
+# Verify that sed is installed
+check_prog_host "sed"
 
 # Check make
 MAKE=$(which make 2> /dev/null)
@@ -192,11 +194,15 @@ if grep -q ^BR2_PACKAGE_CLASSPATH=y $BR2_CONFIG ; then
 fi
 
 if grep -q ^BR2_NEEDS_HOST_JAVA=y $BR2_CONFIG ; then
-	if ! which java > /dev/null ; then
-		echo >&2
-		echo "You must install 'java' on your build machine" >&2
-		exit 1
-	fi
+	check_prog_host "java"
+fi
+
+if grep -q ^BR2_NEEDS_HOST_JAVAC=y $BR2_CONFIG ; then
+	check_prog_host "javac"
+fi
+
+if grep -q ^BR2_NEEDS_HOST_JAR=y $BR2_CONFIG ; then
+	check_prog_host "jar"
 fi
 
 if grep -q ^BR2_HOSTARCH_NEEDS_IA32_LIBS=y $BR2_CONFIG ; then
-- 
1.8.5.3

[Buildroot] [PATCH v2 2/2] classpath: Use generic check for host program

[Maxime Hadjinlian]

Remove the specific check that was done in dependencies.sh to use the
generic one that were introduced by the previous patch.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
Changes v1 -> v2:
  - None
---
 package/classpath/Config.in          |  2 ++
 support/dependencies/dependencies.sh | 10 ----------
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/package/classpath/Config.in b/package/classpath/Config.in
index 0153bca..6eabacd 100644
--- a/package/classpath/Config.in
+++ b/package/classpath/Config.in
@@ -1,5 +1,7 @@
 config BR2_PACKAGE_CLASSPATH
 	bool "classpath"
+	select BR2_HOST_NEEDS_JAVAC
+	select BR2_HOST_NEEDS_JAR
 	depends on BR2_PACKAGE_JAMVM
 	depends on BR2_INET_IPV6
 	help
diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index b18a888..7d69cce 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -183,16 +183,6 @@ if grep ^BR2_TOOLCHAIN_BUILDROOT=y $BR2_CONFIG > /dev/null && \
 	fi
 fi
 
-if grep -q ^BR2_PACKAGE_CLASSPATH=y $BR2_CONFIG ; then
-	for prog in javac jar; do
-		if ! which $prog > /dev/null ; then
-			echo >&2
-			echo "You must install '$prog' on your build machine" >&2
-			exit 1
-		fi
-	done
-fi
-
 if grep -q ^BR2_NEEDS_HOST_JAVA=y $BR2_CONFIG ; then
 	check_prog_host "java"
 fi
-- 
1.8.5.3

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Sun, 23 Feb 2014 17:17:40 +0100, Maxime Hadjinlian wrote:

> +# Verify that which is installed
> +check_prog_host "which"

Since your function check_prog_host uses which to determine if the
program exists, isn't it weird to use which to verify it?

Your check_host_prog function could also be used in:

# Check that a few mandatory programs are installed
missing_progs="no"
for prog in patch perl tar wget cpio python unzip rsync bc ${DL_TOOLS} ; do
        if ! which $prog > /dev/null ; then
                echo "You must install '$prog' on your build machine";
                missing_progs="yes"
                if test $prog = "svn" ; then
                        echo "  svn is usually part of the subversion package in your distribution"
                elif test $prog = "hg" ; then
                        echo "  hg is usually part of the mercurial package in your distribution"
                elif test $prog = "zcat" ; then
                        echo "  zcat is usually part of the gzip package in your distribution"
                elif test $prog = "bzcat" ; then
                        echo "  bzcat is usually part of the bzip2 package in your distribution"
                fi
        fi
done

if test "${missing_progs}" = "yes" ; then
        exit 1
fi

Though you see that this loop has this missing_progs variable that
allows to list all the missing programs, and only abort at the end.

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

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Maxime Hadjinlian]

On Sun, Feb 23, 2014 at 6:40 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Maxime Hadjinlian,
>
> On Sun, 23 Feb 2014 17:17:40 +0100, Maxime Hadjinlian wrote:
>
>> +# Verify that which is installed
>> +check_prog_host "which"
>
> Since your function check_prog_host uses which to determine if the
> program exists, isn't it weird to use which to verify it?
Agreed but that was the original check also which I found a little in
the first place. But since this patch is here only to introduce and
use a function, I didn't see how to change that here.
>
> Your check_host_prog function could also be used in:
>
> # Check that a few mandatory programs are installed
> missing_progs="no"
> for prog in patch perl tar wget cpio python unzip rsync bc ${DL_TOOLS} ; do
>         if ! which $prog > /dev/null ; then
>                 echo "You must install '$prog' on your build machine";
>                 missing_progs="yes"
>                 if test $prog = "svn" ; then
>                         echo "  svn is usually part of the subversion package in your distribution"
>                 elif test $prog = "hg" ; then
>                         echo "  hg is usually part of the mercurial package in your distribution"
>                 elif test $prog = "zcat" ; then
>                         echo "  zcat is usually part of the gzip package in your distribution"
>                 elif test $prog = "bzcat" ; then
>                         echo "  bzcat is usually part of the bzip2 package in your distribution"
>                 fi
>         fi
> done
>
> if test "${missing_progs}" = "yes" ; then
>         exit 1
> fi
>
> Though you see that this loop has this missing_progs variable that
> allows to list all the missing programs, and only abort at the end.
I wanted to keep this one as is for the moment.
As you point out, it uses the exact mechanisms we want for all the
dependencies.sh, and it would be part of a follow up patch later one,
which would uses the same mechanisms but all over the dependencies.sh.
Which is want we want in the end.
This first patch is mostly a follow up of the previous one, which
added the BR2_NEEDS_HOST_JAVA
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Maxime Hadjinlian]

Hi all,

On Sun, Feb 23, 2014 at 7:12 PM, Maxime Hadjinlian
<maxime.hadjinlian@gmail.com> wrote:
> On Sun, Feb 23, 2014 at 6:40 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Dear Maxime Hadjinlian,
>>
>> On Sun, 23 Feb 2014 17:17:40 +0100, Maxime Hadjinlian wrote:
>>
>>> +# Verify that which is installed
>>> +check_prog_host "which"
>>
>> Since your function check_prog_host uses which to determine if the
>> program exists, isn't it weird to use which to verify it?
> Agreed but that was the original check also which I found a little in
> the first place. But since this patch is here only to introduce and
> use a function, I didn't see how to change that here.
>>
>> Your check_host_prog function could also be used in:
>>
>> # Check that a few mandatory programs are installed
>> missing_progs="no"
>> for prog in patch perl tar wget cpio python unzip rsync bc ${DL_TOOLS} ; do
>>         if ! which $prog > /dev/null ; then
>>                 echo "You must install '$prog' on your build machine";
>>                 missing_progs="yes"
>>                 if test $prog = "svn" ; then
>>                         echo "  svn is usually part of the subversion package in your distribution"
>>                 elif test $prog = "hg" ; then
>>                         echo "  hg is usually part of the mercurial package in your distribution"
>>                 elif test $prog = "zcat" ; then
>>                         echo "  zcat is usually part of the gzip package in your distribution"
>>                 elif test $prog = "bzcat" ; then
>>                         echo "  bzcat is usually part of the bzip2 package in your distribution"
>>                 fi
>>         fi
>> done
>>
>> if test "${missing_progs}" = "yes" ; then
>>         exit 1
>> fi
>>
>> Though you see that this loop has this missing_progs variable that
>> allows to list all the missing programs, and only abort at the end.
> I wanted to keep this one as is for the moment.
> As you point out, it uses the exact mechanisms we want for all the
> dependencies.sh, and it would be part of a follow up patch later one,
> which would uses the same mechanisms but all over the dependencies.sh.
> Which is want we want in the end.
> This first patch is mostly a follow up of the previous one, which
> added the BR2_NEEDS_HOST_JAVA
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux, Kernel and Android engineering
>> http://free-electrons.com

Just a bump.
Thomas, if I remember correctly, you were not opposed to this patch ?
As explained, it has been found that dependencies.sh needs a few other
rounds of patch so it would be better shaped, but that can be done
later.

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Thomas De Schampheleire]

Hi Maxime,

On Fri, Mar 28, 2014 at 7:41 PM, Maxime Hadjinlian
<maxime.hadjinlian@gmail.com> wrote:
> Hi all,
>
> On Sun, Feb 23, 2014 at 7:12 PM, Maxime Hadjinlian
> <maxime.hadjinlian@gmail.com> wrote:
>> On Sun, Feb 23, 2014 at 6:40 PM, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>>> Dear Maxime Hadjinlian,
>>>
>>> On Sun, 23 Feb 2014 17:17:40 +0100, Maxime Hadjinlian wrote:
>>>
>>>> +# Verify that which is installed
>>>> +check_prog_host "which"
>>>
>>> Since your function check_prog_host uses which to determine if the
>>> program exists, isn't it weird to use which to verify it?
>> Agreed but that was the original check also which I found a little in
>> the first place. But since this patch is here only to introduce and
>> use a function, I didn't see how to change that here.
>>>
>>> Your check_host_prog function could also be used in:
>>>
>>> # Check that a few mandatory programs are installed
>>> missing_progs="no"
>>> for prog in patch perl tar wget cpio python unzip rsync bc ${DL_TOOLS} ; do
>>>         if ! which $prog > /dev/null ; then
>>>                 echo "You must install '$prog' on your build machine";
>>>                 missing_progs="yes"
>>>                 if test $prog = "svn" ; then
>>>                         echo "  svn is usually part of the subversion package in your distribution"
>>>                 elif test $prog = "hg" ; then
>>>                         echo "  hg is usually part of the mercurial package in your distribution"
>>>                 elif test $prog = "zcat" ; then
>>>                         echo "  zcat is usually part of the gzip package in your distribution"
>>>                 elif test $prog = "bzcat" ; then
>>>                         echo "  bzcat is usually part of the bzip2 package in your distribution"
>>>                 fi
>>>         fi
>>> done
>>>
>>> if test "${missing_progs}" = "yes" ; then
>>>         exit 1
>>> fi
>>>
>>> Though you see that this loop has this missing_progs variable that
>>> allows to list all the missing programs, and only abort at the end.
>> I wanted to keep this one as is for the moment.
>> As you point out, it uses the exact mechanisms we want for all the
>> dependencies.sh, and it would be part of a follow up patch later one,
>> which would uses the same mechanisms but all over the dependencies.sh.
>> Which is want we want in the end.
>> This first patch is mostly a follow up of the previous one, which
>> added the BR2_NEEDS_HOST_JAVA
>>>
>>> Thomas
>>> --
>>> Thomas Petazzoni, CTO, Free Electrons
>>> Embedded Linux, Kernel and Android engineering
>>> http://free-electrons.com
>
> Just a bump.
> Thomas, if I remember correctly, you were not opposed to this patch ?
> As explained, it has been found that dependencies.sh needs a few other
> rounds of patch so it would be better shaped, but that can be done
> later.

Is there a particular reason why the first part of the make check is
not using check_prog_host ?

Another comment is that I find the split between this and the second
patch (classpath) strange: I would move the addition of
BR2_NEEDS_HOST_JAVAC / JAR to that second patch.

Best regards,
Thomas

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Maxime Hadjinlian]

Hi Thomas, all

On Fri, May 2, 2014 at 2:20 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Hi Maxime,
>
> On Fri, Mar 28, 2014 at 7:41 PM, Maxime Hadjinlian
> <maxime.hadjinlian@gmail.com> wrote:
>> Hi all,
>>
>> On Sun, Feb 23, 2014 at 7:12 PM, Maxime Hadjinlian
>> <maxime.hadjinlian@gmail.com> wrote:
>>> On Sun, Feb 23, 2014 at 6:40 PM, Thomas Petazzoni
>>> <thomas.petazzoni@free-electrons.com> wrote:
>>>> Dear Maxime Hadjinlian,
>>>>
>>>> On Sun, 23 Feb 2014 17:17:40 +0100, Maxime Hadjinlian wrote:
>>>>
>>>>> +# Verify that which is installed
>>>>> +check_prog_host "which"
>>>>
>>>> Since your function check_prog_host uses which to determine if the
>>>> program exists, isn't it weird to use which to verify it?
>>> Agreed but that was the original check also which I found a little in
>>> the first place. But since this patch is here only to introduce and
>>> use a function, I didn't see how to change that here.
>>>>
>>>> Your check_host_prog function could also be used in:
>>>>
>>>> # Check that a few mandatory programs are installed
>>>> missing_progs="no"
>>>> for prog in patch perl tar wget cpio python unzip rsync bc ${DL_TOOLS} ; do
>>>>         if ! which $prog > /dev/null ; then
>>>>                 echo "You must install '$prog' on your build machine";
>>>>                 missing_progs="yes"
>>>>                 if test $prog = "svn" ; then
>>>>                         echo "  svn is usually part of the subversion package in your distribution"
>>>>                 elif test $prog = "hg" ; then
>>>>                         echo "  hg is usually part of the mercurial package in your distribution"
>>>>                 elif test $prog = "zcat" ; then
>>>>                         echo "  zcat is usually part of the gzip package in your distribution"
>>>>                 elif test $prog = "bzcat" ; then
>>>>                         echo "  bzcat is usually part of the bzip2 package in your distribution"
>>>>                 fi
>>>>         fi
>>>> done
>>>>
>>>> if test "${missing_progs}" = "yes" ; then
>>>>         exit 1
>>>> fi
>>>>
>>>> Though you see that this loop has this missing_progs variable that
>>>> allows to list all the missing programs, and only abort at the end.
>>> I wanted to keep this one as is for the moment.
>>> As you point out, it uses the exact mechanisms we want for all the
>>> dependencies.sh, and it would be part of a follow up patch later one,
>>> which would uses the same mechanisms but all over the dependencies.sh.
>>> Which is want we want in the end.
>>> This first patch is mostly a follow up of the previous one, which
>>> added the BR2_NEEDS_HOST_JAVA
>>>>
>>>> Thomas
>>>> --
>>>> Thomas Petazzoni, CTO, Free Electrons
>>>> Embedded Linux, Kernel and Android engineering
>>>> http://free-electrons.com
>>
>> Just a bump.
>> Thomas, if I remember correctly, you were not opposed to this patch ?
>> As explained, it has been found that dependencies.sh needs a few other
>> rounds of patch so it would be better shaped, but that can be done
>> later.
>
> Is there a particular reason why the first part of the make check is
> not using check_prog_host ?
Not really no, but as was said, there's a lot to be done for this
script to be good, this patch doesn't aim to rework it fully. Simply
refactor some code that was redundant.
Other patch needs to be done.
>
> Another comment is that I find the split between this and the second
> patch (classpath) strange: I would move the addition of
> BR2_NEEDS_HOST_JAVAC / JAR to that second patch.
Agreed, but only for the HOST_JAVAC, because if I remember correctly,
classpath doesn't need JAR. But, you are absolutely right, it would
make more sense.

I'll try to cover that during the week end and resend. (famous last words...)
>
> Best regards,
> Thomas

[Buildroot] [PATCH v2 1/2] infra: Add generic check_prog_host function

[Maxime Hadjinlian]

Avoid copy/pasting the same block of code to check if a program is
available on the host machine.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
Changes v1 -> v2:
    - Remove the introduction of BR2_NEEDS_HOST_JAVAC &
      BR2_NEEDS_HOST_JAR to add them in the classpath patch. (Thomas De
      Schampheleire)
---
 support/dependencies/dependencies.sh | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index 0566814..655b4c6 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -51,18 +51,20 @@ if test -n "$PERL_MM_OPT" ; then
 	exit 1
 fi
 
-# Verify that which is installed
-if ! which which > /dev/null ; then
-	echo
-	echo "You must install 'which' on your build machine";
-	exit 1;
-fi;
+check_prog_host()
+{
+	prog="$1"
+	if ! which $prog > /dev/null ; then
+		echo >&2
+		echo "You must install '$prog' on your build machine" >&2
+		exit 1
+	fi
+}
 
-if ! which sed > /dev/null ; then
-	echo
-	echo "You must install 'sed' on your build machine"
-	exit 1
-fi
+# Verify that which is installed
+check_prog_host "which"
+# Verify that sed is installed
+check_prog_host "sed"
 
 # Check make
 MAKE=$(which make 2> /dev/null)
@@ -196,11 +198,7 @@ if grep -q ^BR2_PACKAGE_CLASSPATH=y $BR2_CONFIG ; then
 fi
 
 if grep -q ^BR2_NEEDS_HOST_JAVA=y $BR2_CONFIG ; then
-	if ! which java > /dev/null ; then
-		echo >&2
-		echo "You must install 'java' on your build machine" >&2
-		exit 1
-	fi
+	check_prog_host "java"
 fi
 
 if grep -q ^BR2_HOSTARCH_NEEDS_IA32_LIBS=y $BR2_CONFIG ; then
-- 
1.9.2