[Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Krzysztof Kanas]

environment-setup used BASH_SOURCE which is bash specific and empty
variable for zsh (and other shell's).
Use $0 which should be work across multiple shells(tcsh, dash, zsh)
It won't work if other script is sourcing environment-setup.

Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
---
 package/environment-setup/environment-setup | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/environment-setup/environment-setup b/package/environment-setup/environment-setup
index e9bc36fdd093..2b54e88d6689 100644
--- a/package/environment-setup/environment-setup
+++ b/package/environment-setup/environment-setup
@@ -16,4 +16,8 @@ Some tips:
 * To build CMake-based projects, use the "cmake" alias
 
 EOF
-SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
+if [ x"${BASH_SOURCE}" != x"" ]; then
+        SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
+else
+	SDK_PATH=$(dirname $(realpath $0))
+fi
-- 
2.33.0

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Arnout Vandecappelle]

On 30/08/2021 22:13, Krzysztof Kanas wrote:
> environment-setup used BASH_SOURCE which is bash specific and empty
> variable for zsh (and other shell's).
> Use $0 which should be work across multiple shells(tcsh, dash, zsh)
> It won't work if other script is sourcing environment-setup.

 The environment-setup is supposed to be source'd, so how is this ever going to
work?

 I think the only viable solution is to require an argument if shell is not bash.

 Regards,
 Arnout

> 
> Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
> ---
>  package/environment-setup/environment-setup | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/package/environment-setup/environment-setup b/package/environment-setup/environment-setup
> index e9bc36fdd093..2b54e88d6689 100644
> --- a/package/environment-setup/environment-setup
> +++ b/package/environment-setup/environment-setup
> @@ -16,4 +16,8 @@ Some tips:
>  * To build CMake-based projects, use the "cmake" alias
>  
>  EOF
> -SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
> +if [ x"${BASH_SOURCE}" != x"" ]; then
> +        SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
> +else
> +	SDK_PATH=$(dirname $(realpath $0))
> +fi
> 
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Krzysztof Kanas]

On 01/09/2021 15:26, Arnout Vandecappelle wrote:
> 
> 
> On 30/08/2021 22:13, Krzysztof Kanas wrote:
>> environment-setup used BASH_SOURCE which is bash specific and empty
>> variable for zsh (and other shell's).
>> Use $0 which should be work across multiple shells(tcsh, dash, zsh)
>> It won't work if other script is sourcing environment-setup.
> 
>   The environment-setup is supposed to be source'd, so how is this ever going to
> work?
This script will work when sourced interactively from bash shell due the 
test for ${BASH_SOURCE} so this behavior is not changed.

What I meant by this paragraph was how different shell's set $0 augment 
whether script is sourced interactively or sourced from another script, e.g:
cat > foo.sh
source environment-setup
^D

But I missed case when environment-setup is sourced from another 
directory, lucky this works in zsh case, but not in case of dash, ksh.
So the fix would be applicable to zsh only (actually my main purpose for 
the patch).

> 
>   I think the only viable solution is to require an argument if shell is not bash.
So you are thinking of
if [ $SHELL != bash ] ; then ?
I don't know if that will be easy/portable to do  b/c running,

zsh
bash
env|grep SHELL
SHELL=/bin/zsh
XTERM_SHELL=/bin/zsh

> 
>   Regards,
>   Arnout
> 
>>
>> Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
>> ---
>>   package/environment-setup/environment-setup | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/environment-setup/environment-setup b/package/environment-setup/environment-setup
>> index e9bc36fdd093..2b54e88d6689 100644
>> --- a/package/environment-setup/environment-setup
>> +++ b/package/environment-setup/environment-setup
>> @@ -16,4 +16,8 @@ Some tips:
>>   * To build CMake-based projects, use the "cmake" alias
>>   
>>   EOF
>> -SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>> +if [ x"${BASH_SOURCE}" != x"" ]; then
>> +        SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>> +else
>> +	SDK_PATH=$(dirname $(realpath $0))
>> +fi
>>
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Arnout Vandecappelle]

On 01/09/2021 16:48, Krzysztof Kanas wrote:
> 
> 
> On 01/09/2021 15:26, Arnout Vandecappelle wrote:
>>
>>
>> On 30/08/2021 22:13, Krzysztof Kanas wrote:
>>> environment-setup used BASH_SOURCE which is bash specific and empty
>>> variable for zsh (and other shell's).
>>> Use $0 which should be work across multiple shells(tcsh, dash, zsh)
>>> It won't work if other script is sourcing environment-setup.
>>
>>   The environment-setup is supposed to be source'd, so how is this ever going to
>> work?
> This script will work when sourced interactively from bash shell due the test
> for ${BASH_SOURCE} so this behavior is not changed.
> 
> What I meant by this paragraph was how different shell's set $0 augment whether
> script is sourced interactively or sourced from another script, e.g:
> cat > foo.sh
> source environment-setup
> ^D
> 
> But I missed case when environment-setup is sourced from another directory,
> lucky this works in zsh case, but not in case of dash, ksh.
> So the fix would be applicable to zsh only (actually my main purpose for the
> patch).
> 
>>
>>   I think the only viable solution is to require an argument if shell is not
>> bash.
> So you are thinking of
> if [ $SHELL != bash ] ; then ?

 I was thinking of your if [ x"${BASH_SOURCE}" != x"" ]

 Regards,
 Arnout

> I don't know if that will be easy/portable to do  b/c running,
> 
> zsh
> bash
> env|grep SHELL
> SHELL=/bin/zsh
> XTERM_SHELL=/bin/zsh
> 
>>
>>   Regards,
>>   Arnout
>>
>>>
>>> Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
>>> ---
>>>   package/environment-setup/environment-setup | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/package/environment-setup/environment-setup
>>> b/package/environment-setup/environment-setup
>>> index e9bc36fdd093..2b54e88d6689 100644
>>> --- a/package/environment-setup/environment-setup
>>> +++ b/package/environment-setup/environment-setup
>>> @@ -16,4 +16,8 @@ Some tips:
>>>   * To build CMake-based projects, use the "cmake" alias
>>>     EOF
>>> -SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>>> +if [ x"${BASH_SOURCE}" != x"" ]; then
>>> +        SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>>> +else
>>> +    SDK_PATH=$(dirname $(realpath $0))
>>> +fi
>>>
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/environment-setup/environment-setup: add zsh

[Krzysztof Kanas]

On 01/09/2021 16:59, Arnout Vandecappelle wrote:
> 
> 
> On 01/09/2021 16:48, Krzysztof Kanas wrote:
>>
>>
>> On 01/09/2021 15:26, Arnout Vandecappelle wrote:
>>>
>>>
>>> On 30/08/2021 22:13, Krzysztof Kanas wrote:
>>>> environment-setup used BASH_SOURCE which is bash specific and empty
>>>> variable for zsh (and other shell's).
>>>> Use $0 which should be work across multiple shells(tcsh, dash, zsh)
>>>> It won't work if other script is sourcing environment-setup.
>>>
>>>    The environment-setup is supposed to be source'd, so how is this ever going to
>>> work?
>> This script will work when sourced interactively from bash shell due the test
>> for ${BASH_SOURCE} so this behavior is not changed.
>>
>> What I meant by this paragraph was how different shell's set $0 augment whether
>> script is sourced interactively or sourced from another script, e.g:
>> cat > foo.sh
>> source environment-setup
>> ^D
>>
>> But I missed case when environment-setup is sourced from another directory,
>> lucky this works in zsh case, but not in case of dash, ksh.
>> So the fix would be applicable to zsh only (actually my main purpose for the
>> patch).
>>
>>>
>>>    I think the only viable solution is to require an argument if shell is not
>>> bash.
>> So you are thinking of
>> if [ $SHELL != bash ] ; then ?
> 
>   I was thinking of your if [ x"${BASH_SOURCE}" != x"" ]

So what about:

if [ x"$BASH_VERSION" != x"" ] ; then
         SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
elif [ x"$ZSH_VERSION" != x"" ] ; then
         SDK_PATH=$(dirname $(realpath $0))
else
         echo 'unsuported shell
fi

This should work for bash as previously add zsh support, but won't work 
for dash (as it didn't work previously ), but will give clearer error 
indication.

> 
>   Regards,
>   Arnout
> 
>> I don't know if that will be easy/portable to do  b/c running,
>>
>> zsh
>> bash
>> env|grep SHELL
>> SHELL=/bin/zsh
>> XTERM_SHELL=/bin/zsh
>>
>>>
>>>    Regards,
>>>    Arnout
>>>
>>>>
>>>> Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
>>>> ---
>>>>    package/environment-setup/environment-setup | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/package/environment-setup/environment-setup
>>>> b/package/environment-setup/environment-setup
>>>> index e9bc36fdd093..2b54e88d6689 100644
>>>> --- a/package/environment-setup/environment-setup
>>>> +++ b/package/environment-setup/environment-setup
>>>> @@ -16,4 +16,8 @@ Some tips:
>>>>    * To build CMake-based projects, use the "cmake" alias
>>>>      EOF
>>>> -SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>>>> +if [ x"${BASH_SOURCE}" != x"" ]; then
>>>> +        SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
>>>> +else
>>>> +    SDK_PATH=$(dirname $(realpath $0))
>>>> +fi
>>>>
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/environment-setup/environment-setup: add zsh

[Arnout Vandecappelle]

On 02/09/2021 13:28, Krzysztof Kanas wrote:
> environment-setup used BASH_SOURCE which is bash specific and empty
> variable for zsh (and other shell's).
> Zsh Uses $0. Unfortunately POSIX is not specifying how exactly $0
> should behave when in sourced (or using special dot utility). So other
> shell support have to be implemented in different manner.
> 
> Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>
> 
> ---
> Changes in v2:
> 	- fix shell detection logic
> 	- change commit comment
> ---
>  package/environment-setup/environment-setup | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/package/environment-setup/environment-setup b/package/environment-setup/environment-setup
> index e9bc36fdd093..6716c19a577a 100644
> --- a/package/environment-setup/environment-setup
> +++ b/package/environment-setup/environment-setup
> @@ -16,4 +16,10 @@ Some tips:
>  * To build CMake-based projects, use the "cmake" alias
>  
>  EOF
> -SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
> +if [ x"$BASH_VERSION" != x"" ] ; then
> +	SDK_PATH=$(dirname $(realpath "${BASH_SOURCE[0]}"))
> +elif [ x"$ZSH_VERSION" != x"" ] ; then
> +	SDK_PATH=$(dirname $(realpath $0))
> +else
> +	echo unsuported shell

 I fixed the spelling mistake, added quotes around it, and applied to master,
thanks.

 Regards,
 Arnout

> +fi
> 
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/environment-setup/environment-setup: add zsh

[Peter Korsgaard]

>>>>> "Krzysztof" == Krzysztof Kanas <kkanas@fastmail.com> writes:

 > environment-setup used BASH_SOURCE which is bash specific and empty
 > variable for zsh (and other shell's).
 > Zsh Uses $0. Unfortunately POSIX is not specifying how exactly $0
 > should behave when in sourced (or using special dot utility). So other
 > shell support have to be implemented in different manner.

 > Signed-off-by: Krzysztof Kanas <kkanas@fastmail.com>

 > ---
 > Changes in v2:
 > 	- fix shell detection logic
 > 	- change commit comment

Committed to 2021.02.x, 2021.05.x and 2021.08.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot