Message ID | 20230512031550.1715288-1-Qi.Chen@windriver.com |
---|---|
State | Accepted, archived |
Commit | 3405a3221b8f6641a8e42b04bc7acd6e032aeab8 |
Headers | show |
Series | [V2] oe-buildenv-internal: remove path from previous project | expand |
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Chen Qi via > lists.openembedded.org > Sent: den 12 maj 2023 05:16 > To: openembedded-core@lists.openembedded.org > Subject: [OE-core][PATCH V2] oe-buildenv-internal: remove path from previous project > > From: Chen Qi <Qi.Chen@windriver.com> > > For now, only that paths added from current project are removed Change to: Until now, only paths added for the current project were removed > to avoid PATH growing unnecessarily. This is to handle the case > of sourcing the init script into different build directories. > > However, if we source the init script from different projects into > different build directories, the paths added by previous projects > are not cleaned up. > > To avoid this, we record the paths added into OE_ADDED_PATH, and > remove it in the next sourcing. > > Note that the paths, "$OEROOT/scripts:$BITBAKEDIR/bin:", are added > as a whole. A previous commit, "oe-buildenv-internal: Add paths to $PATH individually", > made the change to treat these two paths separately, the reason was > not "assuming the path to the scripts directory always is in $PATH > directly before the bitbake directory". But this is exactly the effect > of the codes. I see no reason why we should complicate things. And now I have to try to remember why I changed this seven years ago... Unfortunately, time has taken its toll on my memory and I do not remember why I changed this. It may have been as simple as to make the code more generic and possibly handle more paths being added, or to take into account that other things may modify PATH after oe-init-build-env has been sourced. I guess I will just have to wait and see if anything breaks after this change is applied... > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > scripts/oe-buildenv-internal | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal > index 485d4c52e1..598120a86d 100755 > --- a/scripts/oe-buildenv-internal > +++ b/scripts/oe-buildenv-internal > @@ -92,15 +92,13 @@ fi > PYTHONPATH=$BITBAKEDIR/lib:$PYTHONPATH > export PYTHONPATH > > +# Remove old added path > +[ -n "$OE_ADDED_PATH" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATH##") || PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") Change to: # Remove any paths added by sourcing this script before [ "$OE_ADDED_PATHS" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATHS##") || PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") and rename `OE_ADDED_PATH` to `OE_ADDED_PATHS` below. > +OE_ADDED_PATH="$OEROOT/scripts:$BITBAKEDIR/bin:" Move the above line to after the comment below. > # Make sure our paths are at the beginning of $PATH > -for newpath in "$BITBAKEDIR/bin" "$OEROOT/scripts"; do > - # Remove any existences of $newpath from $PATH > - PATH=$(echo $PATH | sed -re "s#(^|:)$newpath(:|$)#\2#g;s#^:##") > - > - # Add $newpath to $PATH > - PATH="$newpath:$PATH" > -done > -unset BITBAKEDIR newpath > +PATH="$OE_ADDED_PATH$PATH" > +export OE_ADDED_PATH Remove `export OE_ADDED_PATH`. This is a sourced script and the variable should not be needed by anything else. > +unset BITBAKEDIR All in all, this is how I would like it to look in the end: # Remove any paths added by sourcing this script before [ "$OE_ADDED_PATHS" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATHS##") || PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") # Make sure our paths are at the beginning of $PATH OE_ADDED_PATHS="$OEROOT/scripts:$BITBAKEDIR/bin:" PATH="$OE_ADDED_PATHS$PATH" # This is not needed any more unset BITBAKEDIR > > # Used by the runqemu script > export BUILDDIR You may as well remove `export PATH` on the next line too. It is not needed. > -- > 2.34.1 //Peter
Hi Peter, Thanks for your careful review. I'll send out V3 according to your suggestion. Just one thing. I think we need to export OE_ADDED_PATHS for two reason: 1. users can clearly see it in the `env' command's output. 2. if the prefixed PATH is carried into some subprocess (e.g., a subshell), so should this OE_ADDED_PATHS variable that prefixes it. Regards, Qi On 5/12/23 20:27, Peter Kjellerstedt wrote: >> -----Original Message----- >> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Chen Qi via >> lists.openembedded.org >> Sent: den 12 maj 2023 05:16 >> To: openembedded-core@lists.openembedded.org >> Subject: [OE-core][PATCH V2] oe-buildenv-internal: remove path from previous project >> >> From: Chen Qi <Qi.Chen@windriver.com> >> >> For now, only that paths added from current project are removed > Change to: > > Until now, only paths added for the current project were removed > >> to avoid PATH growing unnecessarily. This is to handle the case >> of sourcing the init script into different build directories. >> >> However, if we source the init script from different projects into >> different build directories, the paths added by previous projects >> are not cleaned up. >> >> To avoid this, we record the paths added into OE_ADDED_PATH, and >> remove it in the next sourcing. >> >> Note that the paths, "$OEROOT/scripts:$BITBAKEDIR/bin:", are added >> as a whole. A previous commit, "oe-buildenv-internal: Add paths to $PATH individually", >> made the change to treat these two paths separately, the reason was >> not "assuming the path to the scripts directory always is in $PATH >> directly before the bitbake directory". But this is exactly the effect >> of the codes. I see no reason why we should complicate things. > And now I have to try to remember why I changed this seven years ago... > Unfortunately, time has taken its toll on my memory and I do not > remember why I changed this. It may have been as simple as to make the > code more generic and possibly handle more paths being added, or to take > into account that other things may modify PATH after oe-init-build-env > has been sourced. I guess I will just have to wait and see if anything > breaks after this change is applied... > >> Signed-off-by: Chen Qi <Qi.Chen@windriver.com> >> --- >> scripts/oe-buildenv-internal | 14 ++++++-------- >> 1 file changed, 6 insertions(+), 8 deletions(-) >> >> diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal >> index 485d4c52e1..598120a86d 100755 >> --- a/scripts/oe-buildenv-internal >> +++ b/scripts/oe-buildenv-internal >> @@ -92,15 +92,13 @@ fi >> PYTHONPATH=$BITBAKEDIR/lib:$PYTHONPATH >> export PYTHONPATH >> >> +# Remove old added path >> +[ -n "$OE_ADDED_PATH" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATH##") || PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") > Change to: > > # Remove any paths added by sourcing this script before > [ "$OE_ADDED_PATHS" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATHS##") || > PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") > > and rename `OE_ADDED_PATH` to `OE_ADDED_PATHS` below. > >> +OE_ADDED_PATH="$OEROOT/scripts:$BITBAKEDIR/bin:" > Move the above line to after the comment below. > >> # Make sure our paths are at the beginning of $PATH >> -for newpath in "$BITBAKEDIR/bin" "$OEROOT/scripts"; do >> - # Remove any existences of $newpath from $PATH >> - PATH=$(echo $PATH | sed -re "s#(^|:)$newpath(:|$)#\2#g;s#^:##") >> - >> - # Add $newpath to $PATH >> - PATH="$newpath:$PATH" >> -done >> -unset BITBAKEDIR newpath >> +PATH="$OE_ADDED_PATH$PATH" >> +export OE_ADDED_PATH > Remove `export OE_ADDED_PATH`. This is a sourced script and the > variable should not be needed by anything else. > >> +unset BITBAKEDIR > All in all, this is how I would like it to look in the end: > > # Remove any paths added by sourcing this script before > [ "$OE_ADDED_PATHS" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATHS##") || > PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") > > # Make sure our paths are at the beginning of $PATH > OE_ADDED_PATHS="$OEROOT/scripts:$BITBAKEDIR/bin:" > PATH="$OE_ADDED_PATHS$PATH" > > # This is not needed any more > unset BITBAKEDIR > >> # Used by the runqemu script >> export BUILDDIR > You may as well remove `export PATH` on the next line too. It is > not needed. > >> -- >> 2.34.1 > //Peter >
> -----Original Message----- > From: ChenQi <Qi.Chen@windriver.com> > Sent: den 15 maj 2023 06:53 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org > Subject: Re: [OE-core][PATCH V2] oe-buildenv-internal: remove path from > previous project > > Hi Peter, > > Thanks for your careful review. I'll send out V3 according to your > suggestion. > > Just one thing. I think we need to export OE_ADDED_PATHS for two reason: > 1. users can clearly see it in the `env' command's output. Sure, but it should really be seen as an internal variable for this script. The only reason it has to be in the environment is for it to be available for the next run of this script. Nothing else should care about it. > 2. if the prefixed PATH is carried into some subprocess (e.g., a subshell), > so should this OE_ADDED_PATHS variable that prefixes it. That is not how variables in shell works. They are expanded immediately, so PATH will be set to whatever value $OE_ADDED_PATHS expands to and you will not see any reference to it after PATH has been modified. > > Regards, > Qi //Peter
Hi Peter, Sorry I didn't express my point clearly. I know that shell variable is expanded immediately. What I really meant is that as the paths, "$OEROOT/scripts:$BITBAKEDIR/bin:", have been added to PATH, and PATH is exported, some variable would also need to be exported to carry that information of the added paths. Otherwise, a subprocess (e.g., a subshell) will not know enough information to act accordingly. e.g., 1) . path1/to/poky/oe-init-build-env build-1 2) bash 3) . path2/to/poky/oe-init-build-env build-2 With the above three steps, the resulting PATH is not what we expect if OE_ADDED_PATHS is not exported. Regards, Qi -----Original Message----- From: Peter Kjellerstedt <peter.kjellerstedt@axis.com> Sent: Monday, May 15, 2023 7:48 PM To: Chen, Qi <Qi.Chen@windriver.com>; openembedded-core@lists.openembedded.org Subject: RE: [OE-core][PATCH V2] oe-buildenv-internal: remove path from previous project > -----Original Message----- > From: ChenQi <Qi.Chen@windriver.com> > Sent: den 15 maj 2023 06:53 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; > openembedded-core@lists.openembedded.org > Subject: Re: [OE-core][PATCH V2] oe-buildenv-internal: remove path > from previous project > > Hi Peter, > > Thanks for your careful review. I'll send out V3 according to your > suggestion. > > Just one thing. I think we need to export OE_ADDED_PATHS for two reason: > 1. users can clearly see it in the `env' command's output. Sure, but it should really be seen as an internal variable for this script. The only reason it has to be in the environment is for it to be available for the next run of this script. Nothing else should care about it. > 2. if the prefixed PATH is carried into some subprocess (e.g., a > subshell), > so should this OE_ADDED_PATHS variable that prefixes it. That is not how variables in shell works. They are expanded immediately, so PATH will be set to whatever value $OE_ADDED_PATHS expands to and you will not see any reference to it after PATH has been modified. > > Regards, > Qi //Peter
diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal index 485d4c52e1..598120a86d 100755 --- a/scripts/oe-buildenv-internal +++ b/scripts/oe-buildenv-internal @@ -92,15 +92,13 @@ fi PYTHONPATH=$BITBAKEDIR/lib:$PYTHONPATH export PYTHONPATH +# Remove old added path +[ -n "$OE_ADDED_PATH" ] && PATH=$(echo $PATH | sed -e "s#$OE_ADDED_PATH##") || PATH=$(echo $PATH | sed -e "s#$OEROOT/scripts:$BITBAKEDIR/bin:##") +OE_ADDED_PATH="$OEROOT/scripts:$BITBAKEDIR/bin:" # Make sure our paths are at the beginning of $PATH -for newpath in "$BITBAKEDIR/bin" "$OEROOT/scripts"; do - # Remove any existences of $newpath from $PATH - PATH=$(echo $PATH | sed -re "s#(^|:)$newpath(:|$)#\2#g;s#^:##") - - # Add $newpath to $PATH - PATH="$newpath:$PATH" -done -unset BITBAKEDIR newpath +PATH="$OE_ADDED_PATH$PATH" +export OE_ADDED_PATH +unset BITBAKEDIR # Used by the runqemu script export BUILDDIR