All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Belal, Awais" <Awais_Belal@mentor.com>
To: Alexander Kanavin <alexander.kanavin@linux.intel.com>,
	"openembedded-core@lists.openembedded.org"
	<openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] autogen: fix autoopts script generation to handle shebang
Date: Wed, 17 May 2017 08:50:07 +0000	[thread overview]
Message-ID: <1495011007241.56954@mentor.com> (raw)
In-Reply-To: <51bb3192-0566-418f-9448-4db95e29b567@linux.intel.com>

> 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



      reply	other threads:[~2017-05-17  8:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-12 13:59 [PATCH] autogen: fix autoopts script generation to handle shebang Awais Belal
2017-05-12 14:06 ` Alexander Kanavin
2017-05-15  9:01   ` Belal, Awais
2017-05-15 11:53     ` Alexander Kanavin
2017-05-15 15:03       ` Belal, Awais
2017-05-15 15:10         ` Alexander Kanavin
2017-05-16  5:36           ` Belal, Awais
2017-05-16 11:32             ` Alexander Kanavin
2017-05-17  8:50               ` Belal, Awais [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1495011007241.56954@mentor.com \
    --to=awais_belal@mentor.com \
    --cc=alexander.kanavin@linux.intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.