diff mbox

dependencies.sh: cleanup dependencies.sh

Message ID 1391459047-23665-1-git-send-email-maxime.hadjinlian@gmail.com
State Superseded
Headers show

Commit Message

Maxime Hadjinlian Feb. 3, 2014, 8:24 p.m. UTC 
Cleanup mixed indents and remove commented lines.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 support/dependencies/dependencies.sh | 214 +++++++++++++++++------------------
 1 file changed, 107 insertions(+), 107 deletions(-)

Comments

Luca Ceresoli Feb. 4, 2014, 4:19 p.m. UTC | #1 
Hi Maxime,

Maxime Hadjinlian wrote:
> Cleanup mixed indents and remove commented lines.
>
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> ---
>   support/dependencies/dependencies.sh | 214 +++++++++++++++++------------------
>   1 file changed, 107 insertions(+), 107 deletions(-)
>
> diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
> index 47d4d10..06386e6 100755
> --- a/support/dependencies/dependencies.sh
> +++ b/support/dependencies/dependencies.sh
> @@ -1,199 +1,199 @@
>   #!/bin/sh
>   # vi: set sw=4 ts=4:
> -#set -x
>
>   export LC_ALL=C
>
>   # Verify that grep works
>   echo "WORKS" | grep "WORKS" >/dev/null 2>&1
>   if test $? != 0 ; then
> -	echo
> -	echo "grep doesn't work"
> -	exit 1
> +echo
> +echo "grep doesn't work"
> +exit 1
>   fi

Are you really suggesting we should remove all nesting? Well, I guess
there's been a problem with the e-mail or something...
Maxime Hadjinlian Feb. 4, 2014, 10:35 p.m. UTC | #2 
On 4 Feb 2014 17:20, "Luca Ceresoli" <luca@lucaceresoli.net> wrote:
>
> Hi Maxime,
Hi Luca,
>
>
> Maxime Hadjinlian wrote:
>>
>> Cleanup mixed indents and remove commented lines.
>>
>> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
>> ---
>>   support/dependencies/dependencies.sh | 214
+++++++++++++++++------------------
>>   1 file changed, 107 insertions(+), 107 deletions(-)
>>
>> diff --git a/support/dependencies/dependencies.sh
b/support/dependencies/dependencies.sh
>> index 47d4d10..06386e6 100755
>> --- a/support/dependencies/dependencies.sh
>> +++ b/support/dependencies/dependencies.sh
>> @@ -1,199 +1,199 @@
>>   #!/bin/sh
>>   # vi: set sw=4 ts=4:
>> -#set -x
>>
>>   export LC_ALL=C
>>
>>   # Verify that grep works
>>   echo "WORKS" | grep "WORKS" >/dev/null 2>&1
>>   if test $? != 0 ; then
>> -       echo
>> -       echo "grep doesn't work"
>> -       exit 1
>> +echo
>> +echo "grep doesn't work"
>> +exit 1
>>   fi
>
>
> Are you really suggesting we should remove all nesting? Well, I guess
> there's been a problem with the e-mail or something...
Well, I asked Peter before doing this, the file contained mixed indents. We
thought it would be nice to have a consistent indent with the Mk files.

But if we find we don't want that because it removes clarity in the code. I
can perfectly resend with only the space/tab fixed.

I really don't have a strong opinion on this one. Thomas Petazzoni and Yann
E. Morin also found it disturbing.
Peter what is your decision ?
>
> --
> Luca
Thomas De Schampheleire Feb. 5, 2014, 11:47 a.m. UTC | #3 
Hi,

On Tue, Feb 4, 2014 at 11:35 PM, Maxime Hadjinlian
<maxime.hadjinlian@gmail.com> wrote:
[..]
>>
>> Are you really suggesting we should remove all nesting? Well, I guess
>> there's been a problem with the e-mail or something...
> Well, I asked Peter before doing this, the file contained mixed indents. We
> thought it would be nice to have a consistent indent with the Mk files.
>
> But if we find we don't want that because it removes clarity in the code. I
> can perfectly resend with only the space/tab fixed.
>
> I really don't have a strong opinion on this one. Thomas Petazzoni and Yann
> E. Morin also found it disturbing.
> Peter what is your decision ?
>>

My preference would be to keep indenting nested levels, but
consistently with either tabs or spaces. As there are sometimes 4
nested levels, not indenting makes it hard to understand at first
sight.

Best regards,
Thomas
Arnout Vandecappelle Feb. 5, 2014, 9:03 p.m. UTC | #4 
On 04/02/14 23:35, Maxime Hadjinlian wrote:
> 
> On 4 Feb 2014 17:20, "Luca Ceresoli" <luca@lucaceresoli.net
> <mailto:luca@lucaceresoli.net>> wrote:
>>
>> Hi Maxime,
> Hi Luca,
>>
>>
>> Maxime Hadjinlian wrote:
>>>
>>> Cleanup mixed indents and remove commented lines.
>>>
>>> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com
> <mailto:maxime.hadjinlian@gmail.com>>
>>> ---
>>>   support/dependencies/dependencies.sh | 214
> +++++++++++++++++------------------
>>>   1 file changed, 107 insertions(+), 107 deletions(-)
>>>
>>> diff --git a/support/dependencies/dependencies.sh
> b/support/dependencies/dependencies.sh
>>> index 47d4d10..06386e6 100755
>>> --- a/support/dependencies/dependencies.sh
>>> +++ b/support/dependencies/dependencies.sh
>>> @@ -1,199 +1,199 @@
>>>   #!/bin/sh
>>>   # vi: set sw=4 ts=4:
>>> -#set -x
>>>
>>>   export LC_ALL=C
>>>
>>>   # Verify that grep works
>>>   echo "WORKS" | grep "WORKS" >/dev/null 2>&1
>>>   if test $? != 0 ; then
>>> -       echo
>>> -       echo "grep doesn't work"
>>> -       exit 1
>>> +echo
>>> +echo "grep doesn't work"
>>> +exit 1
>>>   fi
>>
>>
>> Are you really suggesting we should remove all nesting? Well, I guess
>> there's been a problem with the e-mail or something...
> Well, I asked Peter before doing this, the file contained mixed indents.
> We thought it would be nice to have a consistent indent with the Mk files.

 dependencies.sh is not a .mk file...

 For .mk files it makes some sense not to indent:

- conditions are very rarely nested;

- indentation is already used to indicate commands within a rule;

- it's convenient to have all rules and variable names aligned at column 0.

 For scripts, we do have indentation, but not consistently (sometimes
tab, sometimes 4 spaces). IMHO it _should_ be tab.

 Regards,
 Arnout

> 
> But if we find we don't want that because it removes clarity in the code.
> I can perfectly resend with only the space/tab fixed.
> 
> I really don't have a strong opinion on this one. Thomas Petazzoni and
> Yann E. Morin also found it disturbing.
> Peter what is your decision ?
>>
>> --
>> Luca
> 
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Peter Korsgaard Feb. 5, 2014, 11 p.m. UTC | #5 
>>>>> "Maxime" == Maxime Hadjinlian <maxime.hadjinlian@gmail.com> writes:

 >> Are you really suggesting we should remove all nesting? Well, I guess
 >> there's been a problem with the e-mail or something...

 > Well, I asked Peter before doing this, the file contained mixed indents. We
 > thought it would be nice to have a consistent indent with the Mk files.

 > But if we find we don't want that because it removes clarity in the code. I can
 > perfectly resend with only the space/tab fixed.

 > I really don't have a strong opinion on this one. Thomas Petazzoni and Yann E.
 > Morin also found it disturbing.
 > Peter what is your decision ?

Sorry, I think I misunderstood you when you asked. I thought you
referred to indentation with a tab like make, not to drop all
indentation.

I would prefer to keep it like it is now, just with consistent indentation.
diff mbox

Patch

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index 47d4d10..06386e6 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -1,199 +1,199 @@ 
 #!/bin/sh
 # vi: set sw=4 ts=4:
-#set -x
 
 export LC_ALL=C
 
 # Verify that grep works
 echo "WORKS" | grep "WORKS" >/dev/null 2>&1
 if test $? != 0 ; then
-	echo
-	echo "grep doesn't work"
-	exit 1
+echo
+echo "grep doesn't work"
+exit 1
 fi
 
 # sanity check for CWD in LD_LIBRARY_PATH
 # try not to rely on egrep..
 if test -n "$LD_LIBRARY_PATH" ; then
-	echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep ':\.:' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep 'TRiGGER_start\.:' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep ':\.TRiGGER_end' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep 'TRiGGER_start\.TRiGGER_end' >/dev/null 2>&1
-	if test $? = 0; then
-		echo
-		echo "You seem to have the current working directory in your"
-		echo "LD_LIBRARY_PATH environment variable. This doesn't work."
-		exit 1;
-	fi
+echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep ':\.:' >/dev/null 2>&1 ||
+echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep 'TRiGGER_start\.:' >/dev/null 2>&1 ||
+echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep ':\.TRiGGER_end' >/dev/null 2>&1 ||
+echo TRiGGER_start"$LD_LIBRARY_PATH"TRiGGER_end | grep 'TRiGGER_start\.TRiGGER_end' >/dev/null 2>&1
+if test $? = 0; then
+echo
+echo "You seem to have the current working directory in your"
+echo "LD_LIBRARY_PATH environment variable. This doesn't work."
+exit 1;
+fi
 fi;
 
 # sanity check for CWD in PATH. Having the current working directory
 # in the PATH makes the toolchain build process break.
 # try not to rely on egrep..
 if test -n "$PATH" ; then
-	echo TRiGGER_start"$PATH"TRiGGER_end | grep ':\.:' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$PATH"TRiGGER_end | grep 'TRiGGER_start\.:' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$PATH"TRiGGER_end | grep ':\.TRiGGER_end' >/dev/null 2>&1 ||
-	echo TRiGGER_start"$PATH"TRiGGER_end | grep 'TRiGGER_start\.TRiGGER_end' >/dev/null 2>&1
-	if test $? = 0; then
-		echo
-		echo "You seem to have the current working directory in your"
-		echo "PATH environment variable. This doesn't work."
-		exit 1;
-	fi
+echo TRiGGER_start"$PATH"TRiGGER_end | grep ':\.:' >/dev/null 2>&1 ||
+echo TRiGGER_start"$PATH"TRiGGER_end | grep 'TRiGGER_start\.:' >/dev/null 2>&1 ||
+echo TRiGGER_start"$PATH"TRiGGER_end | grep ':\.TRiGGER_end' >/dev/null 2>&1 ||
+echo TRiGGER_start"$PATH"TRiGGER_end | grep 'TRiGGER_start\.TRiGGER_end' >/dev/null 2>&1
+if test $? = 0; then
+echo
+echo "You seem to have the current working directory in your"
+echo "PATH environment variable. This doesn't work."
+exit 1;
+fi
 fi;
 
 if test -n "$PERL_MM_OPT" ; then
-    echo
-    echo "You have PERL_MM_OPT defined because Perl local::lib"
-    echo "is installed on your system. Please unset this variable"
-    echo "before starting Buildroot, otherwise the compilation of"
-    echo "Perl related packages will fail"
-    exit 1
+echo
+echo "You have PERL_MM_OPT defined because Perl local::lib"
+echo "is installed on your system. Please unset this variable"
+echo "before starting Buildroot, otherwise the compilation of"
+echo "Perl related packages will fail"
+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;
+echo
+echo "You must install 'which' on your build machine";
+exit 1;
 fi;
 
 if ! which sed > /dev/null ; then
-	echo
-	echo "You must install 'sed' on your build machine"
-	exit 1
+echo
+echo "You must install 'sed' on your build machine"
+exit 1
 fi
 
 # Check make
 MAKE=$(which make 2> /dev/null)
 if [ -z "$MAKE" ] ; then
-	echo
-	echo "You must install 'make' on your build machine";
-	exit 1;
+echo
+echo "You must install 'make' on your build machine";
+exit 1;
 fi;
 MAKE_VERSION=$($MAKE --version 2>&1 | sed -e 's/^.* \([0-9\.]\)/\1/g' -e 's/[-\ ].*//g' -e '1q')
 if [ -z "$MAKE_VERSION" ] ; then
-	echo
-	echo "You must install 'make' on your build machine";
-	exit 1;
+echo
+echo "You must install 'make' on your build machine";
+exit 1;
 fi;
 MAKE_MAJOR=$(echo $MAKE_VERSION | sed -e "s/\..*//g")
 MAKE_MINOR=$(echo $MAKE_VERSION | sed -e "s/^$MAKE_MAJOR\.//g" -e "s/\..*//g" -e "s/[a-zA-Z].*//g")
 if [ $MAKE_MAJOR -lt 3 ] || [ $MAKE_MAJOR -eq 3 -a $MAKE_MINOR -lt 81 ] ; then
-	echo
-	echo "You have make '$MAKE_VERSION' installed.  GNU make >=3.81 is required"
-	exit 1;
+echo
+echo "You have make '$MAKE_VERSION' installed.  GNU make >=3.81 is required"
+exit 1;
 fi;
 
 # Check host gcc
 COMPILER=$(which $HOSTCC_NOCCACHE 2> /dev/null)
 if [ -z "$COMPILER" ] ; then
-	COMPILER=$(which cc 2> /dev/null)
+COMPILER=$(which cc 2> /dev/null)
 fi;
 if [ -z "$COMPILER" ] ; then
-	echo
-	echo "You must install 'gcc' on your build machine";
-	exit 1;
+echo
+echo "You must install 'gcc' on your build machine";
+exit 1;
 fi;
 
 COMPILER_VERSION=$($COMPILER -v 2>&1 | sed -n '/^gcc version/p' |
 	sed -e 's/^gcc version \([0-9\.]\)/\1/g' -e 's/[-\ ].*//g' -e '1q')
 if [ -z "$COMPILER_VERSION" ] ; then
-	echo
-	echo "You must install 'gcc' on your build machine";
-	exit 1;
+echo
+echo "You must install 'gcc' on your build machine";
+exit 1;
 fi;
 COMPILER_MAJOR=$(echo $COMPILER_VERSION | sed -e "s/\..*//g")
 COMPILER_MINOR=$(echo $COMPILER_VERSION | sed -e "s/^$COMPILER_MAJOR\.//g" -e "s/\..*//g")
 if [ $COMPILER_MAJOR -lt 3 -o $COMPILER_MAJOR -eq 2 -a $COMPILER_MINOR -lt 95 ] ; then
-	echo
-	echo "You have gcc '$COMPILER_VERSION' installed.  gcc >= 2.95 is required"
-	exit 1;
+echo
+echo "You have gcc '$COMPILER_VERSION' installed.  gcc >= 2.95 is required"
+exit 1;
 fi;
 
 # check for host CXX
 CXXCOMPILER=$(which $HOSTCXX_NOCCACHE 2> /dev/null)
 if [ -z "$CXXCOMPILER" ] ; then
-	CXXCOMPILER=$(which c++ 2> /dev/null)
+CXXCOMPILER=$(which c++ 2> /dev/null)
 fi
+
 if [ -z "$CXXCOMPILER" ] ; then
-	echo
-	echo "You may have to install 'g++' on your build machine"
-	#exit 1
+echo
+echo "You may have to install 'g++' on your build machine"
 fi
+
 if [ ! -z "$CXXCOMPILER" ] ; then
-	CXXCOMPILER_VERSION=$($CXXCOMPILER -v 2>&1 | sed -n '/^gcc version/p' |
-		sed -e 's/^gcc version \([0-9\.]\)/\1/g' -e 's/[-\ ].*//g' -e '1q')
-	if [ -z "$CXXCOMPILER_VERSION" ] ; then
-		echo
-		echo "You may have to install 'g++' on your build machine"
-	fi
+CXXCOMPILER_VERSION=$($CXXCOMPILER -v 2>&1 | sed -n '/^gcc version/p' |
+	sed -e 's/^gcc version \([0-9\.]\)/\1/g' -e 's/[-\ ].*//g' -e '1q')
+if [ -z "$CXXCOMPILER_VERSION" ] ; then
+echo
+echo "You may have to install 'g++' on your build machine"
+fi
 
-	CXXCOMPILER_MAJOR=$(echo $CXXCOMPILER_VERSION | sed -e "s/\..*//g")
-	CXXCOMPILER_MINOR=$(echo $CXXCOMPILER_VERSION | sed -e "s/^$CXXCOMPILER_MAJOR\.//g" -e "s/\..*//g")
-	if [ $CXXCOMPILER_MAJOR -lt 3 -o $CXXCOMPILER_MAJOR -eq 2 -a $CXXCOMPILER_MINOR -lt 95 ] ; then
-		echo
-		echo "You have g++ '$CXXCOMPILER_VERSION' installed.  g++ >= 2.95 is required"
-		exit 1
-	fi
+CXXCOMPILER_MAJOR=$(echo $CXXCOMPILER_VERSION | sed -e "s/\..*//g")
+CXXCOMPILER_MINOR=$(echo $CXXCOMPILER_VERSION | sed -e "s/^$CXXCOMPILER_MAJOR\.//g" -e "s/\..*//g")
+if [ $CXXCOMPILER_MAJOR -lt 3 -o $CXXCOMPILER_MAJOR -eq 2 -a $CXXCOMPILER_MINOR -lt 95 ] ; then
+echo
+echo "You have g++ '$CXXCOMPILER_VERSION' installed.  g++ >= 2.95 is required"
+exit 1
+fi
 fi
 
 # Check bash
 if ! $SHELL --version 2>&1 | grep -q '^GNU bash'; then
-	echo
-	echo "You must install 'bash' on your build machine";
-	exit 1;
+echo
+echo "You must install 'bash' on your build machine";
+exit 1;
 fi;
 
 # 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
+	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"
+	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"
+	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"
+	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"
+	echo "  bzcat is usually part of the bzip2 package in your distribution"
+	fi
 	fi
-    fi
 done
 
 if test "${missing_progs}" = "yes" ; then
-    exit 1
+exit 1
 fi
 
 if grep ^BR2_TOOLCHAIN_BUILDROOT=y $BUILDROOT_CONFIG > /dev/null && \
-   grep ^BR2_ENABLE_LOCALE=y       $BUILDROOT_CONFIG > /dev/null ; then
-   if ! which locale > /dev/null ; then
-       echo
-       echo "You need locale support on your build machine to build a toolchain supporting locales"
-       exit 1 ;
-   fi
-   if ! locale -a | grep -q -i utf8$ ; then
-       echo
-       echo "You need at least one UTF8 locale to build a toolchain supporting locales"
-       exit 1 ;
-   fi
+	grep ^BR2_ENABLE_LOCALE=y       $BUILDROOT_CONFIG > /dev/null ; then
+	if ! which locale > /dev/null ; then
+	echo
+	echo "You need locale support on your build machine to build a toolchain supporting locales"
+	exit 1 ;
+	fi
+	if ! locale -a | grep -q -i utf8$ ; then
+	echo
+	echo "You need at least one UTF8 locale to build a toolchain supporting locales"
+	exit 1 ;
+	fi
 fi
 
 if grep -q ^BR2_PACKAGE_CLASSPATH=y $BUILDROOT_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
+	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_HOSTARCH_NEEDS_IA32_LIBS=y $BUILDROOT_CONFIG ; then
-    if test ! -f /lib/ld-linux.so.2 ; then
+	if test ! -f /lib/ld-linux.so.2 ; then
 	echo
 	echo "Your Buildroot configuration uses pre-built tools for the x86 architecture,"
 	echo "but your build machine uses the x86-64 architecture without the 32 bits compatibility"
@@ -203,23 +203,23 @@  if grep -q ^BR2_HOSTARCH_NEEDS_IA32_LIBS=y $BUILDROOT_CONFIG ; then
 	echo "For other distributions, refer to the documentation on how to install the 32 bits"
 	echo "compatibility libraries."
 	exit 1
-    fi
+	fi
 fi
 
 if grep -q ^BR2_HOSTARCH_NEEDS_IA32_COMPILER=y $BUILDROOT_CONFIG ; then
-    if ! echo "int main(void) {}" | gcc -m32 -x c - ; then
+	if ! echo "int main(void) {}" | gcc -m32 -x c - ; then
 	echo
 	echo "Your Buildroot configuration needs a compiler capable of building 32 bits binaries."
 	echo "If you're running a Debian/Ubuntu distribution, install the gcc-multilib package."
 	echo "For other distributions, refer to their documentation."
 	exit 1
-    fi
+	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."
-    exit 1
+echo "Your Perl installation is not complete enough, at least Data::Dumper is missing."
+echo "On Debian/Ubuntu distributions, install the 'perl' package."
+exit 1
 fi