Patchwork [2/4] autotools.bbclass: Report the missing configure path

login
mail settings
Submitter Darren Hart
Date Nov. 30, 2011, 9:49 p.m.
Message ID <6e118d2846f5606561b4ae1f3c3ae93fd69bc360.1322689519.git.darren@dvhart.com>
Download mbox | patch
Permalink /patch/15883/
State Accepted
Commit 8cdee4c9b8ffcba69134258eff72eede61acd12f
Headers show

Comments

Darren Hart - Nov. 30, 2011, 9:49 p.m.
From: Darren Hart <dvhart@linux.intel.com>

If the configure script isn't found, report the explicit path tried.
This can help debug subtle errors where the ${S} sourcedir may not
be exactly what is expected.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
---
 meta/classes/autotools.bbclass |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Khem Raj - Nov. 30, 2011, 10:31 p.m.
On Wed, Nov 30, 2011 at 1:49 PM, Darren Hart <darren@dvhart.com> wrote:
> From: Darren Hart <dvhart@linux.intel.com>
>
> If the configure script isn't found, report the explicit path tried.
> This can help debug subtle errors where the ${S} sourcedir may not
> be exactly what is expected.
>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> ---
>  meta/classes/autotools.bbclass |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
> index 37e7d4b..7536bac 100644
> --- a/meta/classes/autotools.bbclass
> +++ b/meta/classes/autotools.bbclass
> @@ -70,11 +70,12 @@ CONFIGUREOPT_DEPTRACK = "--disable-dependency-tracking"
>
>
>  oe_runconf () {
> -       if [ -x ${S}/configure ] ; then
> -               bbnote "Running ${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
> -               ${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
> +       cfgscript="${S}/configure"
> +       if [ -x "$cfgscript" ] ; then
> +               bbnote "Running $cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
> +               $cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
>        else
> -               bbfatal "no configure script found"
> +               bbfatal "no configure script found at $cfgscript"

why not just do one line change something like

bbfatal "configure script ${S}/configure not found"

>        fi
>  }
>
> --
> 1.7.6.4
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Darren Hart - Nov. 30, 2011, 10:37 p.m.
On 11/30/2011 02:31 PM, Khem Raj wrote:
> On Wed, Nov 30, 2011 at 1:49 PM, Darren Hart <darren@dvhart.com> wrote:
>> From: Darren Hart <dvhart@linux.intel.com>
>>
>> If the configure script isn't found, report the explicit path tried.
>> This can help debug subtle errors where the ${S} sourcedir may not
>> be exactly what is expected.
>>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> ---
>>  meta/classes/autotools.bbclass |    9 +++++----
>>  1 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
>> index 37e7d4b..7536bac 100644
>> --- a/meta/classes/autotools.bbclass
>> +++ b/meta/classes/autotools.bbclass
>> @@ -70,11 +70,12 @@ CONFIGUREOPT_DEPTRACK = "--disable-dependency-tracking"
>>
>>
>>  oe_runconf () {
>> -       if [ -x ${S}/configure ] ; then
>> -               bbnote "Running ${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
>> -               ${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
>> +       cfgscript="${S}/configure"
>> +       if [ -x "$cfgscript" ] ; then
>> +               bbnote "Running $cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
>> +               $cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
>>        else
>> -               bbfatal "no configure script found"
>> +               bbfatal "no configure script found at $cfgscript"
> 
> why not just do one line change something like
> 
> bbfatal "configure script ${S}/configure not found"

I prefer to use a variable when I need a value more than once. It avoids
future bugs where one is changed and the others were missed. As this
string is used 4 times, it seemed like the right call.

--
Darren

> 
>>        fi
>>  }
>>
>> --
>> 1.7.6.4
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Khem Raj - Nov. 30, 2011, 10:48 p.m.
>>> -               bbfatal "no configure script found"
>>> +               bbfatal "no configure script found at $cfgscript"
>>
>> why not just do one line change something like
>>
>> bbfatal "configure script ${S}/configure not found"
>
> I prefer to use a variable when I need a value more than once. It avoids
> future bugs where one is changed and the others were missed. As this
> string is used 4 times, it seemed like the right call.

ok with variable still the cfgscript itself is the script and the
message "no configure script found at $cfgscript"
seems like $cfgscript is a location. you may want to make it
"$cfgscript configure script not found" or something
Darren Hart - Nov. 30, 2011, 10:57 p.m.
On 11/30/2011 02:48 PM, Khem Raj wrote:
>>>> -               bbfatal "no configure script found"
>>>> +               bbfatal "no configure script found at $cfgscript"
>>>
>>> why not just do one line change something like
>>>
>>> bbfatal "configure script ${S}/configure not found"
>>
>> I prefer to use a variable when I need a value more than once. It avoids
>> future bugs where one is changed and the others were missed. As this
>> string is used 4 times, it seemed like the right call.
> 
> ok with variable still the cfgscript itself is the script and the
> message "no configure script found at $cfgscript"
> seems like $cfgscript is a location. you may want to make it
> "$cfgscript configure script not found" or something

I can see that (although I think can be argued this is correct as well -
a path with a filename is still a location). I was copying the language
used elsewhere... but from where is eluding my search at the moment. RP
has pulled these already I believe, so if the language here truly
bothers you, we can address it in a follow-on. Personally I don't think
it's really worth the trouble.
Khem Raj - Nov. 30, 2011, 11:26 p.m.
> I can see that (although I think can be argued this is correct as well -
> a path with a filename is still a location). I was copying the language
> used elsewhere... but from where is eluding my search at the moment. RP
> has pulled these already I believe, so if the language here truly
> bothers you, we can address it in a follow-on. Personally I don't think
> it's really worth the trouble.

yeah its fine

Patch

diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
index 37e7d4b..7536bac 100644
--- a/meta/classes/autotools.bbclass
+++ b/meta/classes/autotools.bbclass
@@ -70,11 +70,12 @@  CONFIGUREOPT_DEPTRACK = "--disable-dependency-tracking"
 
 
 oe_runconf () {
-	if [ -x ${S}/configure ] ; then
-		bbnote "Running ${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
-		${S}/configure ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
+	cfgscript="${S}/configure"
+	if [ -x "$cfgscript" ] ; then
+		bbnote "Running $cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} $@"
+		$cfgscript ${CONFIGUREOPTS} ${EXTRA_OECONF} "$@" || bbfatal "oe_runconf failed"
 	else
-		bbfatal "no configure script found"
+		bbfatal "no configure script found at $cfgscript"
 	fi
 }