fix bug 487

PATCH In review Dexuan Cui 2 participants 7 messages Nov 1, 2011 – Nov 2, 2011

lore

Share & tools

lore

Dexuan Cui

author

2011-11-01 08:11 UTC #0 lore raw

cover fix bug 487

The following changes since commit 4ce1f57e723cb6ccfd67ed1570228602547bd112:

distro_tracking: polkit, libsndfile, etc (2011-10-31 23:03:11 +0000)

are available in the git repository at:
git://git.pokylinux.org/poky-contrib dcui/bug487
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dcui/bug487

Dexuan Cui (1):
bash: make job control really work

 meta/recipes-extended/bash/bash.inc    |    1 +
 meta/recipes-extended/bash/bash_4.2.bb |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

Signature · 1 line

1.7.6

Dexuan Cui

author

2011-11-01 08:11 UTC #1 lore raw

PATCH 1/1 bash: make job control really work

It turns out 9393ff833f44570fd5f500bc9de6c72db94b0296 didn't really fix the bug.

This patch is made and tested after I read the link below
http://www.mail-archive.com/[email protected]/msg03107.html

[YOCTO #487]

Signed-off-by Dexuan Cui <[email protected]>

---
 meta/recipes-extended/bash/bash.inc    |    1 +
 meta/recipes-extended/bash/bash_4.2.bb |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

meta/recipes-extended/bash/bash.inc +1 -0

index d55e517..d495538 100644
@@ -23,6 +23,7 @@ ALTERNATIVE_LINK = "${base_bindir}/sh"
 ALTERNATIVE_PRIORITY = "100"
 
 do_configure () {
+	export bash_cv_job_control_missing=present
 	gnu-configize
 	oe_runconf
 }

meta/recipes-extended/bash/bash_4.2.bb +1 -1

index a0f5e4e..8070918 100644
@@ -1,6 +1,6 @@
 require bash.inc
 
-PR = "r0"
+PR = "r1"
 
 SRC_URI = "${GNU_MIRROR}/bash/${BPN}-${PV}.tar.gz;name=tarball \
            ${GNU_MIRROR}/bash/bash-4.2-patches/bash42-001;apply=yes;striplevel=0;name=patch001 \

Signature · 1 line

1.7.6

Richard Purdie

2011-11-01 10:27 UTC #2 lore raw

Re: [PATCH 1/1] bash: make job control really work

Quoted 19 lines · Dexuan Cui

It turns out 9393ff833f44570fd5f500bc9de6c72db94b0296 didn't really fix the bug.
This patch is made and tested after I read the link below
http://www.mail-archive.com/[email protected]/msg03107.html
[YOCTO #487]
Signed-off-by Dexuan Cui <[email protected]>
---
 meta/recipes-extended/bash/bash.inc    |    1 +
 meta/recipes-extended/bash/bash_4.2.bb |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)
meta/recipes-extended/bash/bash.inc +0 -0
index d55e517..d495538 100644

meta/recipes-extended/bash/bash.inc ALTERNATIVE_LINK = "${base_bindir}/sh" · line 29

Quoted 7 lines

meta/recipes-extended/bash/bash.inc +1 -0

@@ -23,6 +23,7 @@ ALTERNATIVE_LINK = "${base_bindir}/sh"
 ALTERNATIVE_PRIORITY = "100"
 
 do_configure () {
+	export bash_cv_job_control_missing=present
 	gnu-configize
 	oe_runconf
 }

This really should go into the common site files...

Cheers,

Richard

Cui, Dexuan

author

2011-11-02 07:17 UTC #3 lore raw

Re: [PATCH 1/1] bash: make job control really work

Richard Purdie wrote on 2011-11-01:

Quoted 22 lines · Dexuan Cui

Quoted 21 lines
It turns out 9393ff833f44570fd5f500bc9de6c72db94b0296 didn't really fix the bug.
This patch is made and tested after I read the link below
http://www.mail-archive.com/[email protected]/msg03107.html
[YOCTO #487]
Signed-off-by Dexuan Cui <[email protected]>
---
 meta/recipes-extended/bash/bash.inc    |    1 +
 meta/recipes-extended/bash/bash_4.2.bb |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)
meta/recipes-extended/bash/bash.inc +0 -0
index d55e517..d495538 100644
@@ -23,6 +23,7 @@ ALTERNATIVE_LINK = "${base_bindir}/sh"
 ALTERNATIVE_PRIORITY = "100"
 

Quoted 4 lines

meta/recipes-extended/bash/bash.inc ALTERNATIVE_LINK = "${base_bindir}/sh" · line 26
Quoted 2 lines
meta/recipes-extended/bash/bash.inc +0 -0
 do_configure () { +	export bash_cv_job_control_missing=present
 	gnu-configize 	oe_runconf }
This really should go into the common site files...

Hi Richard,
I found we do define the variable:
meta/site/common-linux:33:bash_cv_job_control_missing=${bash_cv_job_control_missing=present} but looks autoconf doesn't realize the variable has been assigned the value 'present'? I think this is because of the below do_configure in bash.inc -- looks autoreconf is skipped? do_configure () {
gnu-configize
oe_runconf
}
Why do we need a customized do_configure to replace autotools_do_configure?

Later, after I added
do_configure_prepend () {
autoreconf -f -i -s
}
The generated config.log does show bash_cv_job_control_missing is assigned with 'present'.
(BTW, common-linux also introduces many other variables -- would this be safe? Actually here I only need to introduce bash_cv_job_control_missing.)

However, finally, do_compile got a strange failure:

| shell.c: In function 'shell_reinitialize':
| shell.c:1742:20: error: 'PPROMPT' undeclared (first use in this function)
| shell.c:1742:20: note: each undeclared identifier is reported only once for each function it appears in
| shell.c:1743:22: error: 'SPROMPT' undeclared (first use in this function)

Could you please give some suggestions?

Thanks,
-- Dexuan

Richard Purdie

2011-11-02 10:41 UTC #4 lore raw

Re: [PATCH 1/1] bash: make job control really work

Quoted 45 lines · Dexuan

Richard Purdie wrote on 2011-11-01:
Quoted 22 lines · Dexuan Cui
Quoted 21 lines
It turns out 9393ff833f44570fd5f500bc9de6c72db94b0296 didn't really fix the bug.
This patch is made and tested after I read the link below
http://www.mail-archive.com/[email protected]/msg03107.html
[YOCTO #487]
Signed-off-by Dexuan Cui <[email protected]>
---
 meta/recipes-extended/bash/bash.inc    |    1 +
 meta/recipes-extended/bash/bash_4.2.bb |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)
meta/recipes-extended/bash/bash.inc +0 -0
index d55e517..d495538 100644
@@ -23,6 +23,7 @@ ALTERNATIVE_LINK = "${base_bindir}/sh"
 ALTERNATIVE_PRIORITY = "100"
 
Quoted 4 lines
meta/recipes-extended/bash/bash.inc ALTERNATIVE_LINK = "${base_bindir}/sh" · line 26
Quoted 2 lines
meta/recipes-extended/bash/bash.inc +0 -0
 do_configure () { +	export bash_cv_job_control_missing=present
 	gnu-configize 	oe_runconf }
This really should go into the common site files...
Hi Richard,
I found we do define the variable:
meta/site/common-linux:33:bash_cv_job_control_missing=${bash_cv_job_control_missing=present} but looks autoconf doesn't realize the variable has been assigned the value 'present'? I think this is because of the below do_configure in bash.inc -- looks autoreconf is skipped? do_configure () {
gnu-configize
oe_runconf
}
Why do we need a customized do_configure to replace autotools_do_configure?
Later, after I added
do_configure_prepend () {
autoreconf -f -i -s
}
The generated config.log does show bash_cv_job_control_missing is assigned with 'present'.
(BTW, common-linux also introduces many other variables -- would this be safe? Actually here I only need to introduce bash_cv_job_control_missing.)

Quoted 7 lines

However, finally, do_compile got a strange failure:

| shell.c: In function 'shell_reinitialize':
| shell.c:1742:20: error: 'PPROMPT' undeclared (first use in this function)
| shell.c:1742:20: note: each undeclared identifier is reported only once for each function it appears in
| shell.c:1743:22: error: 'SPROMPT' undeclared (first use in this function)

Could you please give some suggestions?

This is why its a really bad idea for recipes to have their own configure rather than using our core one :/.

I had a go at this problem myself and it took a bit of figuring out. The problem is that bash ships "config.h.in" but autoheader overwrites it. This removes the start/end includes from config.h or config-bot.h and config-top.h. We can do something like this:

export AUTOHEADER = "true"

do_configure_prepend () {
if [ ! -e acinclude.m4 ]; then
cat aclocal.m4 > acinclude.m4
fi
}

instead of the current do_configure override. The _prepend ensures the bash specific macros are preserved and the export AUTOHEADER stops autoheader from running at all.

Could you test those changes against bash 4.x and bash 3.x (replacing the current do_configure in bash.inc) and then if that works send a patch please?

Cheers,

Richard

Richard Purdie

2011-11-02 11:47 UTC #5 lore raw

PATCH 1/1 bash: Ensure we fully reautoconf the recipes so site data is used

This ensures bug 487 (missing job control functionality) really gets fixed.

[YOCTO #487]

Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash.inc +6 -3

index d55e517..876be1e 100644
@@ -22,9 +22,12 @@ ALTERNATIVE_PATH = "${base_bindir}/bash"
 ALTERNATIVE_LINK = "${base_bindir}/sh"
 ALTERNATIVE_PRIORITY = "100"
 
-do_configure () {
-	gnu-configize
-	oe_runconf
+export AUTOHEADER = "true"
+
+do_configure_prepend () {
+	if [ ! -e acinclude.m4 ]; then
+		cat aclocal.m4 > acinclude.m4
+	fi
 }
 
 pkg_postinst_${PN} () {

meta/recipes-extended/bash/bash_3.2.48.bb +7 -4

index 1520c4e..f2ba572 100644
@@ -6,7 +6,7 @@ LICENSE = "GPLv2+"
 LIC_FILES_CHKSUM = "file://COPYING;md5=fd5d9bcabd8ed5a54a01ce8d183d592a"
 DEPENDS = "ncurses"
 
-PR = "r7"
+PR = "r8"
 
 SRC_URI = "${GNU_MIRROR}/bash/bash-${PV}.tar.gz \
            ${GNU_MIRROR}/bash/bash-3.2-patches/bash32-049;apply=yes;striplevel=0 \

@@ -23,9 +23,12 @@ sbindir = "/sbin"
 EXTRA_OECONF = "--with-ncurses"
 export CC_FOR_BUILD = "${BUILD_CC}"
 
-do_configure () {
-	gnu-configize
-	oe_runconf
+export AUTOHEADER = "true"
+
+do_configure_prepend () {
+	if [ ! -e acinclude.m4 ]; then
+		cat aclocal.m4 > acinclude.m4
+	fi
 }
 
 pkg_postinst_${PN} () {

meta/recipes-extended/bash/bash_4.2.bb +1 -1

index a0f5e4e..8070918 100644
@@ -1,6 +1,6 @@
 require bash.inc
 
-PR = "r0"
+PR = "r1"
 
 SRC_URI = "${GNU_MIRROR}/bash/${BPN}-${PV}.tar.gz;name=tarball \
            ${GNU_MIRROR}/bash/bash-4.2-patches/bash42-001;apply=yes;striplevel=0;name=patch001 \

Cui, Dexuan

author

2011-11-02 15:54 UTC #6 lore raw

Re: [PATCH 1/1] bash: make job control really work

In reply to Richard Purdie

Richard Purdie wrote on 2011-11-02:

Quoted 16 lines · Dexuan

Quoted 2 lines
Richard Purdie wrote on 2011-11-01:
Quoted 1 line · Dexuan Cui
I had a go at this problem myself and it took a bit of figuring out. The problem is that bash ships "config.h.in" but autoheader overwrites it. This removes the start/end includes from config.h or config-bot.h and config-top.h. We can do something like this:
export AUTOHEADER = "true"
do_configure_prepend () {
if [ ! -e acinclude.m4 ]; then
cat aclocal.m4 > acinclude.m4
fi
}

Quoted 6 lines

instead of the current do_configure override. The _prepend ensures the bash specific macros are preserved and the export AUTOHEADER stops autoheader from running at all.
Could you test those changes against bash 4.x and bash 3.x (replacing the current do_configure in bash.inc) and then if that works send a patch please?

Looks bug 487 only happens to bash 4.x and bash 3.x doesn't have such a bug (the recipe of bash 3.x doesn't use bash.inc, either)

RP, thanks a lot for the help! Here is the new patch:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=dcui/bug487&id=b524337ed5670de2c0d294c3f6970e24f23847eb

Thanks,
-- Dexuan