Re: [PATCH v2] isar-init-build-env: Always set ISARROOT

[vijai kumar <vijaikumar.kanagarajan@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3260 bytes --]

Ok. On a recent thought, Things would go wrong only when someone moves the 
isar-init-build-env and writes
a wrapper script in the same location. In that case, one would need to pass 
in the ISARROOT from the caller to isar-init-build-env.

Right now I dont see anyone moving this thing. The wrapper script could 
very well be in the same location
as in isar-init-build-env, unless you want the wrapper to be in the same 
name for some reason(like for kas??).

It is not a lot of code, people could modify the isar-init-build-env 
directly instead of writing a wrapper in the same name
and moving the original one.

The issue we have in hand is more critical than this one feature, which 
could very well be written in other ways.

Thanks,
Vijai Kumar K

On Monday, September 30, 2019 at 12:32:00 PM UTC+5:30, Jan Kiszka wrote:
>
> On 30.09.19 08:46, Vijai Kumar K wrote: 
> > On Mon, Sep 30, 2019 at 08:06:14AM +0200, Jan Kiszka wrote: 
> >> On 27.09.19 23:43, vijaikumar...@gmail.com <javascript:> wrote: 
> >>> From: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> >>> 
> >>> When switching between two ISAR workspaces in the same shell 
> >>> session, the ISARROOT setting of the previous workspace would be 
> >>> picked up for the new workspace resulting in an incorrect 
> >>> configuration. 
> >>> 
> >>> Always set ISARROOT irrespective of whether it is empty or not. 
> >>> 
> >>> Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
>
> >>> --- 
> >>> Changes in v2: 
> >>>       - Remove redundant code 
> >>> 
> >>>    isar-init-build-env | 7 ++----- 
> >>>    1 file changed, 2 insertions(+), 5 deletions(-) 
> >>> 
> >>> diff --git a/isar-init-build-env b/isar-init-build-env 
> >>> index b08bb59..404efcb 100755 
> >>> --- a/isar-init-build-env 
> >>> +++ b/isar-init-build-env 
> >>> @@ -42,13 +42,10 @@ if [ -z "$ZSH_NAME" ] && [ "$0" = "$THIS_SCRIPT" 
> ]; then 
> >>>        exit 1 
> >>>    fi 
> >>> -if [ -z "$ISARROOT" ]; then 
> >>> -    ISARROOT=$(dirname "$THIS_SCRIPT") 
> >>> -    ISARROOT=$(readlink -f "$ISARROOT") 
> >>> -fi 
> >>> +ISARROOT=$(dirname "$THIS_SCRIPT") 
> >>> +ISARROOT=$(readlink -f "$ISARROOT") 
> >>>    unset THIS_SCRIPT 
> >>> -ISARROOT=$(readlink -f "$ISARROOT") 
> >>>    export ISARROOT 
> >>>    . "$ISARROOT/scripts/isar-buildenv-internal" "$1" && \ 
> >>>        TEMPLATECONF="$TEMPLATECONF" 
> "$ISARROOT/scripts/isar-setup-builddir" || { 
> >>> 
> >> 
> >> OE is doing the same, i.e. does not update OEROOT on second execution. 
> This 
> >> allows to set OEROOT prior to calling the script (
> https://github.com/openembedded/openembedded-core/commit/3327e2a9222004d8ac7974cb1d9fe77c81176cfc). 
>
> > 
> > Yes. But then OE also unsets OEROOT later in that script. 
> > 
> > But with our below change we are no longer doing the same. 
> > 
> https://github.com/ilbers/isar/commit/b59cf9ef8aae682937e8a4a5cda1c6d080d329b7 
> > 
>
> But this is not changing the fact that we can current take ISARROOT from 
> the 
> caller's environment. If we remove that, in deviation from OE, it should 
> be a 
> clear decision. And it should be stated somewhere. 
>
> Jan 
>
> -- 
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE 
> Corporate Competence Center Embedded Linux 
>

[-- Attachment #1.2: Type: text/html, Size: 6138 bytes --]