[PATCH] autogen: fix autoopts script generation to handle shebang

[PATCH] autogen: fix autoopts script generation to handle shebang

[Awais Belal]

With deep directory hierarchy of the build dir the shenbang
generated for the autoopts scripts fail due to shebang length
crossing 128 characters.
This patch handles such scenarios for most cases by following
the symlink in HOSTTOOLS to the actual host executable so the
problem is resolved in most cases.

Signed-off-by: Awais Belal <awais_belal@mentor.com>
---
 .../autogen/autogen-native_5.18.12.bb              |  1 +
 ...oopts-mk-tpl-config.sh-fix-shebang-length.patch | 49 ++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 meta/recipes-devtools/autogen/autogen/0001-autoopts-mk-tpl-config.sh-fix-shebang-length.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..1d76810fd7 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-autoopts-mk-tpl-config.sh-fix-shebang-length.patch \
 "
 
 SRC_URI[md5sum] = "551d15ccbf5b5fc5658da375d5003389"
diff --git a/meta/recipes-devtools/autogen/autogen/0001-autoopts-mk-tpl-config.sh-fix-shebang-length.patch b/meta/recipes-devtools/autogen/autogen/0001-autoopts-mk-tpl-config.sh-fix-shebang-length.patch
new file mode 100644
index 0000000000..453ecb28d6
--- /dev/null
+++ b/meta/recipes-devtools/autogen/autogen/0001-autoopts-mk-tpl-config.sh-fix-shebang-length.patch
@@ -0,0 +1,49 @@
+From 2648a4064aff7ae82e74b8de6ade93dc9c04ce3b Mon Sep 17 00:00:00 2001
+From: Awais Belal <awais_belal@mentor.com>
+Date: Fri, 12 May 2017 18:22:02 +0500
+Subject: [PATCH] autoopts/mk-tpl-config.sh: fix shebang length
+
+The shebang length cannot be more than 128 charaters
+and the change
+http://cgit.openembedded.org/openembedded-core/commit/meta/classes/base.bbclass?id=fa764a403da34bb0ca9fa3767a9e9dba8d685965
+requires that a user needs to make sure the HOSTTOOLS
+directory is under 128 characters in order to get
+the build through otherwise this causes
+bad interpreter: Permission denied
+when the builddir path is too long.
+This change now follows the path to actual host
+executable to be put in the final scripts that
+are generated so the shebang remains correct in
+most situations.
+
+Upstream-Status: Inappropriate
+
+Signed-off-by: Awais Belal <awais_belal@mentor.com>
+---
+ autoopts/mk-tpl-config.sh | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/autoopts/mk-tpl-config.sh b/autoopts/mk-tpl-config.sh
+index c4708a2..7de546b 100755
+--- a/autoopts/mk-tpl-config.sh
++++ b/autoopts/mk-tpl-config.sh
+@@ -98,11 +98,14 @@ fix_scripts() {
+         st=`sed 1q $f`
+ 
+         case "$st" in
+-        *perl ) echo '#!/usr/bin/env perl'
++        *perl )  PERL=`which perl`
++                 PERL=`readlink -f ${PERL}`
++                 echo '#!' ${PERL}
+                  sed 1d $f
+                  ;;
+ 
+-        */sh )   echo '#!' ${POSIX_SHELL}
++        */sh )   SH=`readlink -f ${POSIX_SHELL}`
++                 echo '#!' ${SH}
+                  sed 1d $f
+                  ;;
+ 
+-- 
+2.11.1
+
-- 
2.11.1

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Alexander Kanavin]

On 05/12/2017 04:59 PM, Awais Belal wrote:
> +-        *perl ) echo '#!/usr/bin/env perl'
> ++        *perl )  PERL=`which perl`
> ++                 PERL=`readlink -f ${PERL}`
> ++                 echo '#!' ${PERL}

Isn't this backwards? And why is PERL set twice in a row?


Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Belal, Awais]

> Isn't this backwards? And why is PERL set twice in a row?

Really did not get the first question. Regarding PERL being set twice, I am just trying to manipulate it to the actual host binary, is there a better way? 'which perl' or 'env perl' will only point to the symlink under HOSTTOOLS.

BR,
Awais

________________________________________
From: openembedded-core-bounces@lists.openembedded.org <openembedded-core-bounces@lists.openembedded.org> on behalf of Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Friday, May 12, 2017 7:06 PM
To: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen: fix autoopts script generation to handle shebang

On 05/12/2017 04:59 PM, Awais Belal wrote:
> +-        *perl ) echo '#!/usr/bin/env perl'
> ++        *perl )  PERL=`which perl`
> ++                 PERL=`readlink -f ${PERL}`
> ++                 echo '#!' ${PERL}

Isn't this backwards? And why is PERL set twice in a row?


Alex

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Alexander Kanavin]

On 05/15/2017 12:01 PM, Belal, Awais wrote:

> Really did not get the first question. Regarding PERL being set
> twice, I am just trying to manipulate it to the actual host binary,
> is there a better way? 'which perl' or 'env perl' will only point to
> the symlink under HOSTTOOLS.


1. You are replacing

#!/usr/bin/env perl

with

#! <result of calling `readlink -f ${PERL}`>

The first line will guarantee a short shebang, while the second one may 
result in a very long shebang which may hit the hardcoded kernel limit. 
So you are effectively adding the problem instead of fixing it (and 
there is nothing to be fixed).

2. You do this:

PERL=`which perl`
PERL=`readlink -f ${PERL}`

There is no need for the first line, if PERL is immediately set to 
something else.


Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Belal, Awais]

Hi Alex,

> The first line will guarantee a short shebang, while the second one may
> result in a very long shebang which may hit the hardcoded kernel limit.

I do not see how the second one can be longer in any case than the first one but that's probably because of my lack of knowledge. I can confirm that the changes around perl setting are not required and I only stepped over it because I was using an older commit of core however the change for sh is still required. Should I submit a v2 with that change only or is there a concern around that as well?

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Monday, May 15, 2017 4:53 PM
To: Belal, Awais; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen: fix autoopts script generation to handle shebang

On 05/15/2017 12:01 PM, Belal, Awais wrote:

> Really did not get the first question. Regarding PERL being set
> twice, I am just trying to manipulate it to the actual host binary,
> is there a better way? 'which perl' or 'env perl' will only point to
> the symlink under HOSTTOOLS.


1. You are replacing

#!/usr/bin/env perl

with

#! <result of calling `readlink -f ${PERL}`>

The first line will guarantee a short shebang, while the second one may
result in a very long shebang which may hit the hardcoded kernel limit.
So you are effectively adding the problem instead of fixing it (and
there is nothing to be fixed).

2. You do this:

PERL=`which perl`
PERL=`readlink -f ${PERL}`

There is no need for the first line, if PERL is immediately set to
something else.


Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Alexander Kanavin]

On 05/15/2017 06:03 PM, Belal, Awais wrote:
> Hi Alex,
>
>> The first line will guarantee a short shebang, while the second one
>> may result in a very long shebang which may hit the hardcoded
>> kernel limit.
>
> I do not see how the second one can be longer in any case than the
> first one but that's probably because of my lack of knowledge. I can
> confirm that the changes around perl setting are not required and I
> only stepped over it because I was using an older commit of core
> however the change for sh is still required. Should I submit a v2
> with that change only or is there a concern around that as well?

The sh part is also using readlink, which produces full paths, and 
therefore is incorrect. You simply should not use full paths in #!, as 
depending on your setup they may be still too long.

The standard way to fix too long #! lines in oe is to patch upstream 
code to use
#!/usr/bin/env something
(where something is just the binary name).

Why not simply replace ${POSIX_SHELL} with /bin/sh? Where and how is it set?

Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Belal, Awais]

> The standard way to fix too long #! lines in oe is to patch upstream
> code to use
> #!/usr/bin/env something
> (where something is just the binary name).

> Why not simply replace ${POSIX_SHELL} with /bin/sh? Where and how is it set?

POSIX_SHELL is being set to "/usr/bin/env sh" already through the recipe but it gets resolved/expanded to <build-dir>/hosttools/<shell-bin> during the configuration process so it's not usable for shebang in deep directory hierarchy scenarios.
I guess a better way would be simply to use the same technique that we're using for perl. So for shell we'll have

#!/usr/bin/env sh

in mk-tpl-config.sh directly. I'll submit the change as v2 if you think this is okay.

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Monday, May 15, 2017 8:10 PM
To: Belal, Awais; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen: fix autoopts script generation to handle shebang

On 05/15/2017 06:03 PM, Belal, Awais wrote:
> Hi Alex,
>
>> The first line will guarantee a short shebang, while the second one
>> may result in a very long shebang which may hit the hardcoded
>> kernel limit.
>
> I do not see how the second one can be longer in any case than the
> first one but that's probably because of my lack of knowledge. I can
> confirm that the changes around perl setting are not required and I
> only stepped over it because I was using an older commit of core
> however the change for sh is still required. Should I submit a v2
> with that change only or is there a concern around that as well?

The sh part is also using readlink, which produces full paths, and
therefore is incorrect. You simply should not use full paths in #!, as
depending on your setup they may be still too long.

The standard way to fix too long #! lines in oe is to patch upstream
code to use
#!/usr/bin/env something
(where something is just the binary name).

Why not simply replace ${POSIX_SHELL} with /bin/sh? Where and how is it set?

Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Alexander Kanavin]

On 05/16/2017 08:36 AM, Belal, Awais wrote:
>> The standard way to fix too long #! lines in oe is to patch
>> upstream code to use #!/usr/bin/env something (where something is
>> just the binary name).
>
>> Why not simply replace ${POSIX_SHELL} with /bin/sh? Where and how
>> is it set?
>
> POSIX_SHELL is being set to "/usr/bin/env sh" already through the
> recipe but it gets resolved/expanded to
> <build-dir>/hosttools/<shell-bin> during the configuration process so
> it's not usable for shebang in deep directory hierarchy scenarios. I
> guess a better way would be simply to use the same technique that
> we're using for perl. So for shell we'll have
>
> #!/usr/bin/env sh
>
> in mk-tpl-config.sh directly. I'll submit the change as v2 if you
> think this is okay.

You should not patch out the use of POSIX_SHELL after the fact. Find 
where it is resolved/expanded in the source code in the first place, and 
patch it there. Also, please check why setting POSIX_SHELL in the recipe 
no longer has any effect - there is a patch called 
0001-config-libopts.m4-regenerate-it-from-config-libopts.patch which 
should do the trick, but does not.

Alex

Re: [PATCH] autogen: fix autoopts script generation to handle shebang

[Belal, Awais]

> You should not patch out the use of POSIX_SHELL after the fact. Find
> where it is resolved/expanded in the source code in the first place, and
> patch it there. Also, please check why setting POSIX_SHELL in the recipe
> no longer has any effect - there is a patch called
> 0001-config-libopts.m4-regenerate-it-from-config-libopts.patch which
> should do the trick, but does not.

I guess I see the problem now. The mechanism in libopts.m4 won't allow setting POSIX_SHELL to anything other than an actual file because it does

test -x ${POSIX_SHELL} && break

Now the recipe sets POSIX_SHELL to "/usr/bin/env sh" which would resolve to the actual shell binary when used but itself it's just a string so the condition (test -x) fails because POSIX_SHELL at that particular moment is just a string and not an executable. Moving ahead the script does

POSIX_SHELL=`which bash`

which then resolves/expands to <build-dir>/tmp/hosttools/bash and obviously breaks what we're trying to achieve with "/usr/bin/env sh". I believe the first check in libopts.m4 for POSIX_SHELL should be changed to

test -n ${POSIX_SHELL} && break

So the user can set it to whatever he desires obviously he'll need to make sure that this would work as a proper shell. Thoughts?

BR,
Awais

________________________________________
From: Alexander Kanavin <alexander.kanavin@linux.intel.com>
Sent: Tuesday, May 16, 2017 4:32 PM
To: Belal, Awais; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] autogen: fix autoopts script generation to handle shebang

On 05/16/2017 08:36 AM, Belal, Awais wrote:
>> The standard way to fix too long #! lines in oe is to patch
>> upstream code to use #!/usr/bin/env something (where something is
>> just the binary name).
>
>> Why not simply replace ${POSIX_SHELL} with /bin/sh? Where and how
>> is it set?
>
> POSIX_SHELL is being set to "/usr/bin/env sh" already through the
> recipe but it gets resolved/expanded to
> <build-dir>/hosttools/<shell-bin> during the configuration process so
> it's not usable for shebang in deep directory hierarchy scenarios. I
> guess a better way would be simply to use the same technique that
> we're using for perl. So for shell we'll have
>
> #!/usr/bin/env sh
>
> in mk-tpl-config.sh directly. I'll submit the change as v2 if you
> think this is okay.

You should not patch out the use of POSIX_SHELL after the fact. Find
where it is resolved/expanded in the source code in the first place, and
patch it there. Also, please check why setting POSIX_SHELL in the recipe
no longer has any effect - there is a patch called
0001-config-libopts.m4-regenerate-it-from-config-libopts.patch which
should do the trick, but does not.

Alex