Patchwork [meta-webserver] webmin: fix failures in do_install task

login
register
mail settings
Submitter Qi.Chen@windriver.com
Date Nov. 22, 2013, 2:37 a.m.
Message ID <1385087849-30272-1-git-send-email-Qi.Chen@windriver.com>
Download mbox | patch
Permalink /patch/62189/
State New, archived
Headers show

Comments

Qi.Chen@windriver.com - Nov. 22, 2013, 2:37 a.m.
From: Chen Qi <Qi.Chen@windriver.com>

Previously, even if something went wrong in setup.sh script, the do_install
task would still exit successfully. As a result, failures in do_install
task were hidden.

Unlike the setup.sh script shipped with webmin source code, some commands
in our own setup.sh script are possible to fail because we are not installing
webmin on target.

If we examine the log file of the do_install, we can see the following error.

	  Script was not run with full path (failed to find run-postinstalls.pl
	  under /usr/lib64/webmin/webmin)

This patch fixes the above problem. It also adds checks to the return
value of some commands in the setup.sh script so that if the commands
fail, the do_install task will fail too.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 .../recipes-webadmin/webmin/files/setup.sh         |   42 ++++++++++++--------
 .../recipes-webadmin/webmin/webmin_1.620.bb        |    2 +-
 2 files changed, 26 insertions(+), 18 deletions(-)
Paul Eggleton - Dec. 3, 2013, 10:19 a.m.
Hi Qi,

On Friday 22 November 2013 10:37:29 Qi.Chen@windriver.com wrote:
> -    export perl=perl
> +    export perl=${OECMAKE_PERLNATIVE_DIR}/perl

I'm afraid I don't like the look of this - OECMAKE_PERLNATIVE_DIR is set for 
use by cmake, and perlnative.bbclass should be putting our perl binary into 
PATH so it should be run anyway. Is this change actually necessary?

Cheers,
Paul
Qi.Chen@windriver.com - Dec. 3, 2013, 10:45 a.m.
On 12/03/2013 06:19 PM, Paul Eggleton wrote:
> Hi Qi,
>
> On Friday 22 November 2013 10:37:29 Qi.Chen@windriver.com wrote:
>> -    export perl=perl
>> +    export perl=${OECMAKE_PERLNATIVE_DIR}/perl
> I'm afraid I don't like the look of this - OECMAKE_PERLNATIVE_DIR is set for
> use by cmake, and perlnative.bbclass should be putting our perl binary into
> PATH so it should be run anyway. Is this change actually necessary?
>
> Cheers,
> Paul
>

Please drop this patch. I found that the do_install task in webmin runs 
some perl scripts (e.g. postinstall.pl and enable-collections.pl) that 
are supposed to be run on target. I suspect webmin has never worked 
correctly on target as a result. I'm gonna propose another fix. The goal 
is to make it work on target.

Best Regards,
Chen Qi
Paul Eggleton - Dec. 3, 2013, 11:02 a.m.
On Tuesday 03 December 2013 18:45:37 ChenQi wrote:
> On 12/03/2013 06:19 PM, Paul Eggleton wrote:
> > On Friday 22 November 2013 10:37:29 Qi.Chen@windriver.com wrote:
> >> -    export perl=perl
> >> +    export perl=${OECMAKE_PERLNATIVE_DIR}/perl
> > 
> > I'm afraid I don't like the look of this - OECMAKE_PERLNATIVE_DIR is set
> > for use by cmake, and perlnative.bbclass should be putting our perl
> > binary into PATH so it should be run anyway. Is this change actually
> > necessary?
> 
> Please drop this patch. I found that the do_install task in webmin runs
> some perl scripts (e.g. postinstall.pl and enable-collections.pl) that
> are supposed to be run on target. I suspect webmin has never worked
> correctly on target as a result. I'm gonna propose another fix. The goal
> is to make it work on target.

I can assure you that it has worked on the target; at least it did about 6 
months ago (perhaps more by chance rather than design though).

Cheers,
Paul
Qi.Chen@windriver.com - Dec. 5, 2013, 6:51 a.m.
On 12/03/2013 07:02 PM, Paul Eggleton wrote:
> On Tuesday 03 December 2013 18:45:37 ChenQi wrote:
>> On 12/03/2013 06:19 PM, Paul Eggleton wrote:
>>> On Friday 22 November 2013 10:37:29 Qi.Chen@windriver.com wrote:
>>>> -    export perl=perl
>>>> +    export perl=${OECMAKE_PERLNATIVE_DIR}/perl
>>> I'm afraid I don't like the look of this - OECMAKE_PERLNATIVE_DIR is set
>>> for use by cmake, and perlnative.bbclass should be putting our perl
>>> binary into PATH so it should be run anyway. Is this change actually
>>> necessary?
>> Please drop this patch. I found that the do_install task in webmin runs
>> some perl scripts (e.g. postinstall.pl and enable-collections.pl) that
>> are supposed to be run on target. I suspect webmin has never worked
>> correctly on target as a result. I'm gonna propose another fix. The goal
>> is to make it work on target.
> I can assure you that it has worked on the target; at least it did about 6
> months ago (perhaps more by chance rather than design though).
>
> Cheers,
> Paul
>

Hi Paul,

Thanks for your information :)

I just tried it out on sato image with 'webmin' and 'midori' installed.
1) https://localhost:10000
Cannot connect
2) http://localhost:10000
Login page appeared, but I cannot login even if I provided the correct 
username and password.

I did this test on qemu, but I think the result should have nothing to 
do with qemu.

Best Regards,
Chen Qi

Patch

diff --git a/meta-webserver/recipes-webadmin/webmin/files/setup.sh b/meta-webserver/recipes-webadmin/webmin/files/setup.sh
index 8d24f92..75e5214 100755
--- a/meta-webserver/recipes-webadmin/webmin/files/setup.sh
+++ b/meta-webserver/recipes-webadmin/webmin/files/setup.sh
@@ -1,6 +1,16 @@ 
 #!/bin/sh
 # Modified version of setup.sh distributed with webmin
 
+done_or_error () {
+	if [ "$?" = "0" ]; then
+	    echo "..done"
+	    echo ""
+	else
+	    echo "Error out: $@"
+	    exit 1
+	fi
+}
+
 if [ "$wadir" = "" ]; then
 	echo "ERROR: wadir not specified"
 	echo ""
@@ -41,7 +51,7 @@  echo $var_dir > $config_dir/var-path
 echo "Creating web server config files.."
 cfile=$config_dir/miniserv.conf
 echo "port=$port" >> $cfile
-echo "root=$wadir_runtime" >> $cfile
+echo "root=$wadir" >> $cfile
 echo "mimetypes=$wadir_runtime/mime.types" >> $cfile
 echo "addtype_cgi=internal/cgi" >> $cfile
 echo "realm=Webmin Server" >> $cfile
@@ -124,8 +134,7 @@  chmod 600 $kfile
 echo "keyfile=$config_dir_runtime/miniserv.pem" >> $cfile
 
 chmod 600 $cfile
-echo "..done"
-echo ""
+done_or_error "chmod 600 $cfile"
 
 echo "Creating access control file.."
 afile=$config_dir/webmin.acl
@@ -136,8 +145,7 @@  else
 	echo "$login: $defaultmods" >> $afile
 fi
 chmod 600 $afile
-echo "..done"
-echo ""
+done_or_error "chmod 600 $afile"
 
 if [ "$login" != "root" -a "$login" != "admin" ]; then
 	# Allow use of RPC by this user
@@ -147,8 +155,7 @@  fi
 if [ "$noperlpath" = "" ]; then
 	echo "Inserting path to perl into scripts.."
 	(find "$wadir" -name '*.cgi' -print ; find "$wadir" -name '*.pl' -print) | $perl "$wadir/perlpath.pl" $perl_runtime -
-	echo "..done"
-	echo ""
+	done_or_error
 fi
 
 echo "Creating start and stop scripts.."
@@ -184,8 +191,7 @@  echo "pidfile=\`grep \"^pidfile=\" $config_dir_runtime/miniserv.conf | sed -e 's
 echo "kill -USR1 \`cat \$pidfile\`" >>$config_dir/reload
 
 chmod 755 $config_dir/start $config_dir/stop $config_dir/restart $config_dir/reload
-echo "..done"
-echo ""
+done_or_error "chmod 755 $config_dir/start $config_dir/stop $config_dir/restart $config_dir/reload"
 
 if [ "$upgrading" = 1 ]; then
 	echo "Updating config files.."
@@ -223,8 +229,7 @@  fi
 # Disallow unknown referers by default
 echo "referers_none=1" >>$config_dir/config
 echo $ver > $config_dir/version
-echo "..done"
-echo ""
+done_or_error "echo $ver > $config_dir/version"
 
 # Set passwd_ fields in miniserv.conf from global config
 for field in passwd_file passwd_uindex passwd_pindex passwd_cindex passwd_mindex; do
@@ -276,8 +281,7 @@  fi
 if [ "$makeboot" = "1" ]; then
 	echo "Configuring Webmin to start at boot time.."
 	(cd "$wadir/init" ; WEBMIN_CONFIG=$config_dir WEBMIN_VAR=$var_dir "$wadir/init/atboot.pl" $bootscript)
-	echo "..done"
-	echo ""
+	done_or_error
 fi
 
 # If password delays are not specifically disabled, enable them
@@ -326,15 +330,19 @@  done
 if [ "$nopostinstall" = "" ]; then
 	echo "Running postinstall scripts .."
 	(cd "$wadir" ; WEBMIN_CONFIG=$config_dir WEBMIN_VAR=$var_dir "$wadir/run-postinstalls.pl")
-	echo "..done"
-	echo ""
+	done_or_error
 fi
 
 # Enable background collection
 if [ "$upgrading" != 1 -a -r $config_dir/system-status/enable-collection.pl ]; then
 	echo "Enabling background status collection .."
 	$config_dir/system-status/enable-collection.pl 5
-	echo "..done"
-	echo ""
+	done_or_error
 fi
 
+# Substitude some values to finalize the file to be used on target
+echo "Finalizing web server files.."
+find $prefix -type f | xargs sed -i "s#$prefix##g"
+done_or_error "$prefix cleanup"
+find $prefix -type f | xargs sed -i "s#$perl#$perl_runtime#g"
+done_or_error "$perl cleanup"
diff --git a/meta-webserver/recipes-webadmin/webmin/webmin_1.620.bb b/meta-webserver/recipes-webadmin/webmin/webmin_1.620.bb
index 8822b92..3cdcc73 100644
--- a/meta-webserver/recipes-webadmin/webmin/webmin_1.620.bb
+++ b/meta-webserver/recipes-webadmin/webmin/webmin_1.620.bb
@@ -80,7 +80,7 @@  do_install() {
     rm -rf ${D}${libexecdir}/webmin/patches
 
     # Run setup script
-    export perl=perl
+    export perl=${OECMAKE_PERLNATIVE_DIR}/perl
     export perl_runtime=${bindir}/perl
     export prefix=${D}
     export tempdir=${S}/install_tmp