[PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Awais Belal]

Using 'test -x' is only viable in situations where
POSIX_SHELL is directly set to an executable whereas
there are scenarios where a user might want to set
it to "env sh" so that it can be used in places
where there's a problem with shebang lengths etc. This
is exactly how it is being used in OE so build failures
are seen in cases where deep directory hierarchies are
used for the builddir.
We now use 'test -n' just to make sure if POSIX_SHELL
is set by the user or not and then move forward to
other tests in case it is unset.

Signed-off-by: Awais Belal <awais_belal@mentor.com>
---
 .../autogen/autogen-native_5.18.12.bb              |  1 +
 ...pts-allow-user-to-set-POSIX_SHELL-as-desi.patch | 37 ++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch

diff --git a/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb b/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
index 853477cf7c..37571e3358 100644
--- a/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
+++ b/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
@@ -14,6 +14,7 @@ SRC_URI = "${GNU_MIRROR}/autogen/rel${PV}/autogen-${PV}.tar.gz \
            file://fix-script-err-when-processing-libguile.patch \
            file://0001-config-libopts.m4-regenerate-it-from-config-libopts..patch \
            file://0002-autoopts-mk-tpl-config.sh-fix-perl-path.patch \
+           file://0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch \
 "
 
 SRC_URI[md5sum] = "551d15ccbf5b5fc5658da375d5003389"
diff --git a/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch b/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch
new file mode 100644
index 0000000000..bfbf6621e1
--- /dev/null
+++ b/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch
@@ -0,0 +1,37 @@
+From dfb30e438a051993c69357c5069170ec779e91e4 Mon Sep 17 00:00:00 2001
+From: Awais Belal <awais_belal@mentor.com>
+Date: Wed, 17 May 2017 15:06:48 +0500
+Subject: [PATCH] config/libopts: allow user to set POSIX_SHELL as desired
+
+Using 'test -x' is only viable in situations where
+POSIX_SHELL is directly set to an executable whereas
+there are scenarios where a user might want to set
+it to "env sh" so that it can be used in places
+where there's a problem with shebang lengths etc.
+We now use 'test -n' just to make sure if POSIX_SHELL
+is set by the user or not and then move forward to
+other tests in case it is unset.
+
+Upstream-Status: Pending
+
+Signed-off-by: Awais Belal <awais_belal@mentor.com>
+---
+ config/libopts.m4 | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/config/libopts.m4 b/config/libopts.m4
+index 51e6a39..efabc7f 100644
+--- a/config/libopts.m4
++++ b/config/libopts.m4
+@@ -114,7 +114,7 @@ AC_DEFUN([INVOKE_LIBOPTS_MACROS_FIRST],[
+   AC_PROG_SED
+   [while :
+   do
+-      test -x "$POSIX_SHELL" && break
++      test -n "$POSIX_SHELL" && break
+       POSIX_SHELL=`which bash`
+       test -x "$POSIX_SHELL" && break
+       POSIX_SHELL=`which dash`
+-- 
+2.11.1
+
-- 
2.11.1

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Robert Yang]

Hi Awais,

On 05/18/2017 02:50 PM, Awais Belal wrote:
> Using 'test -x' is only viable in situations where
> POSIX_SHELL is directly set to an executable whereas
> there are scenarios where a user might want to set
> it to "env sh" so that it can be used in places
> where there's a problem with shebang lengths etc. This
> is exactly how it is being used in OE so build failures
> are seen in cases where deep directory hierarchies are
> used for the builddir.
> We now use 'test -n' just to make sure if POSIX_SHELL
> is set by the user or not and then move forward to
> other tests in case it is unset.
>
> Signed-off-by: Awais Belal <awais_belal@mentor.com>
> ---
>  .../autogen/autogen-native_5.18.12.bb              |  1 +
>  ...pts-allow-user-to-set-POSIX_SHELL-as-desi.patch | 37 ++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch
>
> diff --git a/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb b/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
> index 853477cf7c..37571e3358 100644
> --- a/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
> +++ b/meta/recipes-devtools/autogen/autogen-native_5.18.12.bb
> @@ -14,6 +14,7 @@ SRC_URI = "${GNU_MIRROR}/autogen/rel${PV}/autogen-${PV}.tar.gz \
>             file://fix-script-err-when-processing-libguile.patch \
>             file://0001-config-libopts.m4-regenerate-it-from-config-libopts..patch \
>             file://0002-autoopts-mk-tpl-config.sh-fix-perl-path.patch \
> +           file://0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch \
>  "
>
>  SRC_URI[md5sum] = "551d15ccbf5b5fc5658da375d5003389"
> diff --git a/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch b/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch
> new file mode 100644
> index 0000000000..bfbf6621e1
> --- /dev/null
> +++ b/meta/recipes-devtools/autogen/autogen/0001-config-libopts-allow-user-to-set-POSIX_SHELL-as-desi.patch
> @@ -0,0 +1,37 @@
> +From dfb30e438a051993c69357c5069170ec779e91e4 Mon Sep 17 00:00:00 2001
> +From: Awais Belal <awais_belal@mentor.com>
> +Date: Wed, 17 May 2017 15:06:48 +0500
> +Subject: [PATCH] config/libopts: allow user to set POSIX_SHELL as desired
> +
> +Using 'test -x' is only viable in situations where
> +POSIX_SHELL is directly set to an executable whereas
> +there are scenarios where a user might want to set
> +it to "env sh" so that it can be used in places
> +where there's a problem with shebang lengths etc.
> +We now use 'test -n' just to make sure if POSIX_SHELL
> +is set by the user or not and then move forward to
> +other tests in case it is unset.
> +
> +Upstream-Status: Pending
> +
> +Signed-off-by: Awais Belal <awais_belal@mentor.com>
> +---
> + config/libopts.m4 | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/config/libopts.m4 b/config/libopts.m4
> +index 51e6a39..efabc7f 100644
> +--- a/config/libopts.m4
> ++++ b/config/libopts.m4
> +@@ -114,7 +114,7 @@ AC_DEFUN([INVOKE_LIBOPTS_MACROS_FIRST],[
> +   AC_PROG_SED
> +   [while :
> +   do
> +-      test -x "$POSIX_SHELL" && break
> ++      test -n "$POSIX_SHELL" && break

The problem is libopts.m4 is auto generated, so it may override
when upgrade autogen-native.

// Robert

> +       POSIX_SHELL=`which bash`
> +       test -x "$POSIX_SHELL" && break
> +       POSIX_SHELL=`which dash`
> +--
> +2.11.1
> +
>

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Alexander Kanavin]

On 05/18/2017 10:23 AM, Robert Yang wrote:
>> +-      test -x "$POSIX_SHELL" && break
>> ++      test -n "$POSIX_SHELL" && break
>
> The problem is libopts.m4 is auto generated, so it may override
> when upgrade autogen-native.

That's right. The file is generated from libopts.def in the same 
directory. I now looked at the code finally :) and I think it's better 
to leave the POSIX_SHELL variable alone (and not set it from the recipe 
either because it has no effect), and go back to the original idea of 
patching the place where it's used to make a shebang line - I think 
there's only one such place in the source code.

But fixing libopts.def is okay too, I just think it's more trouble.

Alex

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Belal, Awais]

> That's right. The file is generated from libopts.def in the same
> directory. I now looked at the code finally :) and I think it's better
> to leave the POSIX_SHELL variable alone (and not set it from the recipe
> either because it has no effect), and go back to the original idea of
> patching the place where it's used to make a shebang line - I think
> there's only one such place in the source code.

> But fixing libopts.def is okay too, I just think it's more trouble.

The usage of libopts.def has been deprecated upstream: git://git.sv.gnu.org/autogen.git # 5cbe233387d7f7b36752736338d1cd4f71287daa but they've still kept libopts.m4 so I think this patch will do for future use as well and shouldn't have a problem.

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Thursday, May 18, 2017 1:36 PM
To: Robert Yang; Belal, Awais; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

On 05/18/2017 10:23 AM, Robert Yang wrote:
>> +-      test -x "$POSIX_SHELL" && break
>> ++      test -n "$POSIX_SHELL" && break
>
> The problem is libopts.m4 is auto generated, so it may override
> when upgrade autogen-native.

That's right. The file is generated from libopts.def in the same
directory. I now looked at the code finally :) and I think it's better
to leave the POSIX_SHELL variable alone (and not set it from the recipe
either because it has no effect), and go back to the original idea of
patching the place where it's used to make a shebang line - I think
there's only one such place in the source code.

But fixing libopts.def is okay too, I just think it's more trouble.

Alex

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Alexander Kanavin]

On 05/18/2017 02:23 PM, Belal, Awais wrote:
>> That's right. The file is generated from libopts.def in the same
>> directory. I now looked at the code finally :) and I think it's
>> better to leave the POSIX_SHELL variable alone (and not set it from
>> the recipe either because it has no effect), and go back to the
>> original idea of patching the place where it's used to make a
>> shebang line - I think there's only one such place in the source
>> code.
>
>> But fixing libopts.def is okay too, I just think it's more
>> trouble.
>
> The usage of libopts.def has been deprecated upstream:
> git://git.sv.gnu.org/autogen.git #
> 5cbe233387d7f7b36752736338d1cd4f71287daa but they've still kept
> libopts.m4 so I think this patch will do for future use as well and
> shouldn't have a problem.

I just checked why is autogen in oe-core in the first place. The only 
thing that needs it now is grub, and it will no longer be necessary when 
Khem's grub 2.02 patch shows up in master. So then we can remove the 
whole thing. But thanks for fixing! :)

Alex

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Belal, Awais]

> I just checked why is autogen in oe-core in the first place. The only
> thing that needs it now is grub, and it will no longer be necessary when
> Khem's grub 2.02 patch shows up in master. So then we can remove the
> whole thing. But thanks for fixing! :)

I went through Khem's grub2 changes: http://cgit.openembedded.org/openembedded-core-contrib/commit/?h=kraj/master&id=0af89270aa83d6ad0a4859e18643b53191567c8b and in those changes the grub2.inc still reads

DEPENDS = "flex-native bison-native autogen-native"

So I think this can be valuable :)

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Thursday, May 18, 2017 5:32 PM
To: Belal, Awais; Robert Yang; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

On 05/18/2017 02:23 PM, Belal, Awais wrote:
>> That's right. The file is generated from libopts.def in the same
>> directory. I now looked at the code finally :) and I think it's
>> better to leave the POSIX_SHELL variable alone (and not set it from
>> the recipe either because it has no effect), and go back to the
>> original idea of patching the place where it's used to make a
>> shebang line - I think there's only one such place in the source
>> code.
>
>> But fixing libopts.def is okay too, I just think it's more
>> trouble.
>
> The usage of libopts.def has been deprecated upstream:
> git://git.sv.gnu.org/autogen.git #
> 5cbe233387d7f7b36752736338d1cd4f71287daa but they've still kept
> libopts.m4 so I think this patch will do for future use as well and
> shouldn't have a problem.

I just checked why is autogen in oe-core in the first place. The only
thing that needs it now is grub, and it will no longer be necessary when
Khem's grub 2.02 patch shows up in master. So then we can remove the
whole thing. But thanks for fixing! :)

Alex

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Alexander Kanavin]

On 05/18/2017 04:20 PM, Belal, Awais wrote:
>> I just checked why is autogen in oe-core in the first place. The
>> only thing that needs it now is grub, and it will no longer be
>> necessary when Khem's grub 2.02 patch shows up in master. So then
>> we can remove the whole thing. But thanks for fixing! :)
>
> I went through Khem's grub2 changes:
> http://cgit.openembedded.org/openembedded-core-contrib/commit/?h=kraj/master&id=0af89270aa83d6ad0a4859e18643b53191567c8b
> and in those changes the grub2.inc still reads
>
> DEPENDS = "flex-native bison-native autogen-native"
>
> So I think this can be valuable :)

Don't take what DEPENDS says as authoritative on the subject. :) The 
only way to tell if some dependency is no longer needed is careful 
inspection of upstream code.

In this case:
http://git.savannah.gnu.org/cgit/grub.git/tree/NEWS#n159

Alex

Re: [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

[Belal, Awais]

> In this case:
> http://git.savannah.gnu.org/cgit/grub.git/tree/NEWS#n159

Ooooh, thanks :)

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Thursday, May 18, 2017 6:24 PM
To: Belal, Awais; Robert Yang; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen-native: allow user to set POSIX_SHELL as desired

On 05/18/2017 04:20 PM, Belal, Awais wrote:
>> I just checked why is autogen in oe-core in the first place. The
>> only thing that needs it now is grub, and it will no longer be
>> necessary when Khem's grub 2.02 patch shows up in master. So then
>> we can remove the whole thing. But thanks for fixing! :)
>
> I went through Khem's grub2 changes:
> http://cgit.openembedded.org/openembedded-core-contrib/commit/?h=kraj/master&id=0af89270aa83d6ad0a4859e18643b53191567c8b
> and in those changes the grub2.inc still reads
>
> DEPENDS = "flex-native bison-native autogen-native"
>
> So I think this can be valuable :)

Don't take what DEPENDS says as authoritative on the subject. :) The
only way to tell if some dependency is no longer needed is careful
inspection of upstream code.

In this case:
http://git.savannah.gnu.org/cgit/grub.git/tree/NEWS#n159

Alex