[PATCH] classes: Replace "if test" file tests with POSIX file tests

[PATCH] classes: Replace "if test" file tests with POSIX file tests

[Robert P. J. Day]

In entire meta/classes/ directory, replace shell tests of the form
"if test -? ..." with POSIX tests of the form "if [ -? ...

Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

  as i cannot abide those silly "if test" constructs, i zipped through
and changed them to POSIX form. is there any objection to this kind of
cleanup? next up ... all those "x${VAR} = x" tests ...


diff --git a/meta/classes/distutils-tools.bbclass b/meta/classes/distutils-tools.bbclass
index 3ef9cc5..4243681 100644
--- a/meta/classes/distutils-tools.bbclass
+++ b/meta/classes/distutils-tools.bbclass
@@ -44,14 +44,14 @@ distutils_do_install() {
         done

         echo "Step 4 of ${PN} Install ..."
-        if test -e ${D}${bindir} ; then
+        if [ -e ${D}${bindir} ]; then
             for i in ${D}${bindir}/* ; do \
                 sed -i -e s:${STAGING_BINDIR_NATIVE}:${bindir}:g $i
             done
         fi

         echo "Step 4 of ${PN} Install ..."
-        if test -e ${D}${sbindir}; then
+        if [ -e ${D}${sbindir} ]; then
             for i in ${D}${sbindir}/* ; do \
                 sed -i -e s:${STAGING_BINDIR_NATIVE}:${bindir}:g $i
             done
@@ -63,7 +63,7 @@ distutils_do_install() {
         #
         # FIXME: Bandaid against wrong datadir computation
         #
-        if test -e ${D}${datadir}/share; then
+        if [ -e ${D}${datadir}/share ]; then
             mv -f ${D}${datadir}/share/* ${D}${datadir}/
         fi
 }
diff --git a/meta/classes/distutils.bbclass b/meta/classes/distutils.bbclass
index 857572d..3340d1e 100644
--- a/meta/classes/distutils.bbclass
+++ b/meta/classes/distutils.bbclass
@@ -41,7 +41,7 @@ distutils_do_install() {
         # only modify file if it contains path  and recompile it
         find ${D} -name "*.py" -exec grep -q ${D} {} \; -exec sed -i -e s:${D}::g {} \; -exec ${STAGING_BINDIR_NATIVE}/python-native/python -mcompileall {} \;

-        if test -e ${D}${bindir} ; then
+        if [ -e ${D}${bindir} ]; then
             for i in ${D}${bindir}/* ; do \
                 if [ ${PN} != "${BPN}-native" ]; then
                 	sed -i -e s:${STAGING_BINDIR_NATIVE}/python-native/python:${bindir}/env\ python:g $i
@@ -50,7 +50,7 @@ distutils_do_install() {
             done
         fi

-        if test -e ${D}${sbindir}; then
+        if [ -e ${D}${sbindir} ]; then
             for i in ${D}${sbindir}/* ; do \
                 if [ ${PN} != "${BPN}-native" ]; then
                 	sed -i -e s:${STAGING_BINDIR_NATIVE}/python-native/python:${bindir}/env\ python:g $i
@@ -65,13 +65,13 @@ distutils_do_install() {
         #
         # FIXME: Bandaid against wrong datadir computation
         #
-        if test -e ${D}${datadir}/share; then
+        if [ -e ${D}${datadir}/share ]; then
             mv -f ${D}${datadir}/share/* ${D}${datadir}/
             rmdir ${D}${datadir}/share
         fi

 	# Fix backport modules
-	if test -e ${STAGING_LIBDIR}/${PYTHON_DIR}/site-packages/backports/__init__.py && test -e ${D}${PYTHON_SITEPACKAGES_DIR}/backports/__init__.py; then
+	if [ -e ${STAGING_LIBDIR}/${PYTHON_DIR}/site-packages/backports/__init__.py ] && [ -e ${D}${PYTHON_SITEPACKAGES_DIR}/backports/__init__.py ]; then
 	   rm ${D}${PYTHON_SITEPACKAGES_DIR}/backports/__init__.py;
 	   rm ${D}${PYTHON_SITEPACKAGES_DIR}/backports/__init__.pyc;
 	fi
diff --git a/meta/classes/distutils3.bbclass b/meta/classes/distutils3.bbclass
index a6720c5..d306b88 100644
--- a/meta/classes/distutils3.bbclass
+++ b/meta/classes/distutils3.bbclass
@@ -45,14 +45,14 @@ distutils3_do_install() {
         # support filenames with *spaces*
         find ${D} -name "*.py" -exec grep -q ${D} {} \; -exec sed -i -e s:${D}::g {} \;

-        if test -e ${D}${bindir} ; then
+        if [ -e ${D}${bindir} ]; then
             for i in ${D}${bindir}/* ; do \
                 sed -i -e s:${STAGING_BINDIR_NATIVE}/${PYTHON_PN}-native/${PYTHON_PN}:${bindir}/env\ ${PYTHON_PN}:g $i
                 sed -i -e s:${STAGING_BINDIR_NATIVE}:${bindir}:g $i
             done
         fi

-        if test -e ${D}${sbindir}; then
+        if [ -e ${D}${sbindir} ]; then
             for i in ${D}${sbindir}/* ; do \
                 sed -i -e s:${STAGING_BINDIR_NATIVE}/python-${PYTHON_PN}/${PYTHON_PN}:${bindir}/env\ ${PYTHON_PN}:g $i
                 sed -i -e s:${STAGING_BINDIR_NATIVE}:${bindir}:g $i
@@ -64,7 +64,7 @@ distutils3_do_install() {
         #
         # FIXME: Bandaid against wrong datadir computation
         #
-        if test -e ${D}${datadir}/share; then
+        if [ -e ${D}${datadir}/share ]; then
             mv -f ${D}${datadir}/share/* ${D}${datadir}/
             rmdir ${D}${datadir}/share
         fi
diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
index 42bbd73..85f71a2 100644
--- a/meta/classes/go.bbclass
+++ b/meta/classes/go.bbclass
@@ -36,7 +36,7 @@ GO_INSTALL ?= "${GO_IMPORT}/..."

 do_go_compile() {
 	GOPATH=${S}:${STAGING_LIBDIR}/${TARGET_SYS}/go go env
-	if test -n "${GO_INSTALL}" ; then
+	if [ -n "${GO_INSTALL}" ]; then
 		GOPATH=${S}:${STAGING_LIBDIR}/${TARGET_SYS}/go go install -v ${GO_INSTALL}
 	fi
 }
@@ -61,7 +61,7 @@ do_go_install() {

 	chown -R root:root "${D}${GOROOT_FINAL}"

-	if test -e "${D}${GOBIN_FINAL}" ; then
+	if [ -e "${D}${GOBIN_FINAL}" ]; then
 		install -d -m 0755 "${D}${bindir}"
 		find "${D}${GOBIN_FINAL}" ! -type d -print0 | xargs -r0 mv --target-directory="${D}${bindir}"
 		rmdir -p "${D}${GOBIN_FINAL}" || true
diff --git a/meta/classes/gtk-doc.bbclass b/meta/classes/gtk-doc.bbclass
index 297eac6..0ae2729 100644
--- a/meta/classes/gtk-doc.bbclass
+++ b/meta/classes/gtk-doc.bbclass
@@ -50,7 +50,7 @@ export GIO_MODULE_DIR=${STAGING_LIBDIR}/gio/modules-dummy

 GIR_EXTRA_LIBS_PATH=\`find ${B} -name .libs| tr '\n' ':'\`\$GIR_EXTRA_LIBS_PATH

-if test -d ".libs"; then
+if [ -d ".libs" ]; then
     $qemu_binary ".libs/\$@"
 else
     $qemu_binary "\$@"
diff --git a/meta/classes/kernel-fitimage.bbclass b/meta/classes/kernel-fitimage.bbclass
index 2d344b5..8ae2eb6 100644
--- a/meta/classes/kernel-fitimage.bbclass
+++ b/meta/classes/kernel-fitimage.bbclass
@@ -96,7 +96,7 @@ fitimage_emit_section_kernel() {
 	kernel_csum="sha1"

 	ENTRYPOINT=${UBOOT_ENTRYPOINT}
-	if test -n "${UBOOT_ENTRYSYMBOL}"; then
+	if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
 		ENTRYPOINT=`${HOST_PREFIX}nm ${S}/vmlinux | \
 			awk '$4=="${UBOOT_ENTRYSYMBOL}" {print $2}'`
 	fi
@@ -339,7 +339,7 @@ fitimage_assemble() {
 	#
 	# Step 2: Prepare a DTB image section
 	#
-	if test -n "${KERNEL_DEVICETREE}"; then
+	if [ -n "${KERNEL_DEVICETREE}" ]; then
 		dtbcount=1
 		for DTB in ${KERNEL_DEVICETREE}; do
 			if echo ${DTB} | grep -q '/dts/'; then
@@ -359,7 +359,7 @@ fitimage_assemble() {
 	#
 	# Step 3: Prepare a setup section. (For x86)
 	#
-	if test -e arch/${ARCH}/boot/setup.bin ; then
+	if [ -e arch/${ARCH}/boot/setup.bin ]; then
 		setupcount=1
 		fitimage_emit_section_setup ${1} "${setupcount}" arch/${ARCH}/boot/setup.bin
 	fi
@@ -383,7 +383,7 @@ fitimage_assemble() {

 	# Force the first Kernel and DTB in the default config
 	kernelcount=1
-	if test -n "${dtbcount}"; then
+	if [ -n "${dtbcount}" ]; then
 		dtbcount=1
 	fi

@@ -392,7 +392,7 @@ fitimage_assemble() {
 	#
 	fitimage_emit_section_maint ${1} confstart

-	if test -n "${DTBS}"; then
+	if [ -n "${DTBS}" ]; then
 		i=1
 		for DTB in ${DTBS}; do
 			fitimage_emit_section_config ${1} "${kernelcount}" "${DTB}" "${ramdiskcount}" "${setupcount}" "`expr ${i} = ${dtbcount}`"
diff --git a/meta/classes/kernel-uboot.bbclass b/meta/classes/kernel-uboot.bbclass
index 345e7f5..868e97d 100644
--- a/meta/classes/kernel-uboot.bbclass
+++ b/meta/classes/kernel-uboot.bbclass
@@ -1,5 +1,5 @@
 uboot_prep_kimage() {
-	if test -e arch/${ARCH}/boot/compressed/vmlinux ; then
+	if [ -e arch/${ARCH}/boot/compressed/vmlinux ]; then
 		vmlinux_path="arch/${ARCH}/boot/compressed/vmlinux"
 		linux_suffix=""
 		linux_comp="none"
diff --git a/meta/classes/kernel-uimage.bbclass b/meta/classes/kernel-uimage.bbclass
index 7e7185f..e2e9b63 100644
--- a/meta/classes/kernel-uimage.bbclass
+++ b/meta/classes/kernel-uimage.bbclass
@@ -23,7 +23,7 @@ do_uboot_mkimage() {
 			uboot_prep_kimage

 			ENTRYPOINT=${UBOOT_ENTRYPOINT}
-			if test -n "${UBOOT_ENTRYSYMBOL}"; then
+			if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
 				ENTRYPOINT=`${HOST_PREFIX}nm ${S}/vmlinux | \
 					awk '$3=="${UBOOT_ENTRYSYMBOL}" {print $1}'`
 			fi
diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
index dd3cac5..563582e 100644
--- a/meta/classes/populate_sdk_base.bbclass
+++ b/meta/classes/populate_sdk_base.bbclass
@@ -246,8 +246,7 @@ populate_sdk_log_check() {

 		echo "log_check: Using $lf_path as logfile"

-		if test -e "$lf_path"
-		then
+		if [ -e "$lf_path" ]; then
 			${IMAGE_PKGTYPE}_log_check $target $lf_path
 		else
 			echo "Cannot find logfile [$lf_path]"

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

✗ patchtest: failure for classes: Replace "if test" file tests with POSIX file tests

[Patchwork]

== Series Details ==

Series: classes: Replace "if test" file tests with POSIX file tests
Revision: 1
URL   : https://patchwork.openembedded.org/series/5977/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at 552bd782d9)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Richard Purdie]

On Sat, 2017-03-25 at 14:03 -0400, Robert P. J. Day wrote:
> In entire meta/classes/ directory, replace shell tests of the form
> "if test -? ..." with POSIX tests of the form "if [ -? ...
> 
> Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> 
> ---
> 
>   as i cannot abide those silly "if test" constructs, i zipped
> through
> and changed them to POSIX form. is there any objection to this kind
> of
> cleanup? next up ... all those "x${VAR} = x" tests ...

I've merged most of this. There were pieces of the diff that simply
didn't apply so I skipped them.

The reason I've taken what applied is that this is very expensive for
us to test right now and I made the most of the testing time I had on
the weekend. The issue is changes to the core classes rebuild
everything so we can't reuse sstate and the tests are much slower.

We're running late with M3 rc2 but just about to build it and I thought
it was worth getting some of this in rather none at all. Please do
figure out the remaining pieces and send them and we may or may not
tweak the remaining bits in M4.

Cheers,

Richard

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Robert P. J. Day]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On Mon, 27 Mar 2017, Richard Purdie wrote:

> On Sat, 2017-03-25 at 14:03 -0400, Robert P. J. Day wrote:
> > In entire meta/classes/ directory, replace shell tests of the form
> > "if test -? ..." with POSIX tests of the form "if [ -? ...
> >
> > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> >
> > ---
> >
> >   as i cannot abide those silly "if test" constructs, i zipped
> > through
> > and changed them to POSIX form. is there any objection to this kind
> > of
> > cleanup? next up ... all those "x${VAR} = x" tests ...
>
> I've merged most of this. There were pieces of the diff that simply
> didn't apply so I skipped them.
>
> The reason I've taken what applied is that this is very expensive
> for us to test right now and I made the most of the testing time I
> had on the weekend. The issue is changes to the core classes rebuild
> everything so we can't reuse sstate and the tests are much slower.
>
> We're running late with M3 rc2 but just about to build it and I
> thought it was worth getting some of this in rather none at all.
> Please do figure out the remaining pieces and send them and we may
> or may not tweak the remaining bits in M4.

  my next cleanup was going to be (as mentioned above) replace all
those silly "x${VAR} = x" string tests with either of:

  [ -z "${VAR}" ]
  [ -n "${VAR}" ]

is there time to get that in before the next freeze? if so, i can do
it quickly for testing; if not, i guess it can just wait.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Robert P. J. Day
> Sent: den 28 mars 2017 11:57
> To: Richard Purdie
> Cc: OE Core mailing list
> Subject: Re: [OE-core] [PATCH] classes: Replace "if test" file tests
> with POSIX file tests
> 
> On Mon, 27 Mar 2017, Richard Purdie wrote:
> 
> > On Sat, 2017-03-25 at 14:03 -0400, Robert P. J. Day wrote:
> > > In entire meta/classes/ directory, replace shell tests of the form
> > > "if test -? ..." with POSIX tests of the form "if [ -? ...
> > >
> > > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> > >
> > > ---
> > >
> > >   as i cannot abide those silly "if test" constructs, i zipped
> > > through
> > > and changed them to POSIX form. is there any objection to this kind
> > > of
> > > cleanup? next up ... all those "x${VAR} = x" tests ...
> >
> > I've merged most of this. There were pieces of the diff that simply
> > didn't apply so I skipped them.
> >
> > The reason I've taken what applied is that this is very expensive
> > for us to test right now and I made the most of the testing time I
> > had on the weekend. The issue is changes to the core classes rebuild
> > everything so we can't reuse sstate and the tests are much slower.
> >
> > We're running late with M3 rc2 but just about to build it and I
> > thought it was worth getting some of this in rather none at all.
> > Please do figure out the remaining pieces and send them and we may
> > or may not tweak the remaining bits in M4.
> 
>   my next cleanup was going to be (as mentioned above) replace all
> those silly "x${VAR} = x" string tests with either of:
> 
>   [ -z "${VAR}" ]
>   [ -n "${VAR}" ]

There is no reason to have the -n in there. This is better:

  [ -z "${VAR}" ]
  [ "${VAR}" ]

> is there time to get that in before the next freeze? if so, i can do
> it quickly for testing; if not, i guess it can just wait.
> 
> rday

//Peter

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Richard Purdie]

On Tue, 2017-03-28 at 05:56 -0400, Robert P. J. Day wrote:
> On Mon, 27 Mar 2017, Richard Purdie wrote:
> 
> > 
> > On Sat, 2017-03-25 at 14:03 -0400, Robert P. J. Day wrote:
> > > 
> > > In entire meta/classes/ directory, replace shell tests of the
> > > form
> > > "if test -? ..." with POSIX tests of the form "if [ -? ...
> > > 
> > > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> > > 
> > > ---
> > > 
> > >   as i cannot abide those silly "if test" constructs, i zipped
> > > through
> > > and changed them to POSIX form. is there any objection to this
> > > kind
> > > of
> > > cleanup? next up ... all those "x${VAR} = x" tests ...
> > I've merged most of this. There were pieces of the diff that simply
> > didn't apply so I skipped them.
> > 
> > The reason I've taken what applied is that this is very expensive
> > for us to test right now and I made the most of the testing time I
> > had on the weekend. The issue is changes to the core classes
> > rebuild
> > everything so we can't reuse sstate and the tests are much slower.
> > 
> > We're running late with M3 rc2 but just about to build it and I
> > thought it was worth getting some of this in rather none at all.
> > Please do figure out the remaining pieces and send them and we may
> > or may not tweak the remaining bits in M4.
>   my next cleanup was going to be (as mentioned above) replace all
> those silly "x${VAR} = x" string tests with either of:
> 
>   [ -z "${VAR}" ]
>   [ -n "${VAR}" ]
> 
> is there time to get that in before the next freeze? if so, i can do
> it quickly for testing; if not, i guess it can just wait.

I think that needs to wait for 2.4 but I agree its worth cleaning that
up.

Cheers,

Richard

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Robert P. J. Day]

[-- Attachment #1: Type: text/plain, Size: 2725 bytes --]

On Tue, 28 Mar 2017, Peter Kjellerstedt wrote:

> > -----Original Message-----
> > From: openembedded-core-bounces@lists.openembedded.org
> > [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> > Robert P. J. Day
> > Sent: den 28 mars 2017 11:57
> > To: Richard Purdie
> > Cc: OE Core mailing list
> > Subject: Re: [OE-core] [PATCH] classes: Replace "if test" file tests
> > with POSIX file tests
> >
> > On Mon, 27 Mar 2017, Richard Purdie wrote:
> >
> > > On Sat, 2017-03-25 at 14:03 -0400, Robert P. J. Day wrote:
> > > > In entire meta/classes/ directory, replace shell tests of the form
> > > > "if test -? ..." with POSIX tests of the form "if [ -? ...
> > > >
> > > > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> > > >
> > > > ---
> > > >
> > > >   as i cannot abide those silly "if test" constructs, i zipped
> > > > through
> > > > and changed them to POSIX form. is there any objection to this kind
> > > > of
> > > > cleanup? next up ... all those "x${VAR} = x" tests ...
> > >
> > > I've merged most of this. There were pieces of the diff that simply
> > > didn't apply so I skipped them.
> > >
> > > The reason I've taken what applied is that this is very expensive
> > > for us to test right now and I made the most of the testing time I
> > > had on the weekend. The issue is changes to the core classes rebuild
> > > everything so we can't reuse sstate and the tests are much slower.
> > >
> > > We're running late with M3 rc2 but just about to build it and I
> > > thought it was worth getting some of this in rather none at all.
> > > Please do figure out the remaining pieces and send them and we may
> > > or may not tweak the remaining bits in M4.
> >
> >   my next cleanup was going to be (as mentioned above) replace all
> > those silly "x${VAR} = x" string tests with either of:
> >
> >   [ -z "${VAR}" ]
> >   [ -n "${VAR}" ]
>
> There is no reason to have the -n in there. This is better:
>
>   [ -z "${VAR}" ]
>   [ "${VAR}" ]

  i'm fine with that, as long as it's officially POSIX. that also
suggests the alternate tests:

  [ "${VAR}" ]
  [ ! "${VAR}" ]

i am happy to defer to the will of the masses, it's just nice to have
a style standard one way or the other.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: [PATCH] classes: Replace "if test" file tests with POSIX file tests

[Patrick Ohly]

On Tue, 2017-03-28 at 06:54 -0400, Robert P. J. Day wrote:
>   i'm fine with that, as long as it's officially POSIX. that also
> suggests the alternate tests:
> 
>   [ "${VAR}" ]
>   [ ! "${VAR}" ]

I personally prefer this. ! instead of -z feels more consistent (when
not using the special "-n", let's also avoid the special "-z" and use
the more general "!" instead.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.