Are there any more review comments on this?

Thanks,
Vijai Kumar K

On Wednesday, January 29, 2020 at 11:52:50 AM UTC+5:30, vijai kumar wrote:
>
> On Tue, Jan 28, 2020 at 10:43:47PM +0100, Henning Schild wrote: 
> > The unspoken rule (not yet in Isar) would be to use kas and kas-docker 
> > to get an always reproducible build with a known environment. 
> > 
>
> Yes. I agree that kas effectively handles these scenarios. But, we 
> should take into consideration the non-kas based builds, as we do now, 
> and its repercussions. Atlast kas is just another tool outside ISAR and 
> people might use it or skip it :) 
>
> > Looking at the patch you do replace the env variable ISARROOT with 
> > another "global" SCRIPTSDIR. Could you go into detail who unsets that 
> > to solve the issue with global env variables? 
>
> Sorry. I am not sure what is the issue you are referring to. I don't 
> think it needs any unset. It is derived from ISARROOT and set each time 
> the setup script is sourced. Somewhat like your BUILDDIR. If you see, 
> the problem with ISARROOT is it is set only when it is empty. This 
> behaviour derived from OE, as pointed out by Jan in [1], is to have 
> provision to have a caller/wrapper script to pass in ISARROOT to the 
> isar-init-build-env script. 
>
> Considering all these it made sense either to duplicate ISARROOT 
> (something like ISARHOME) and unset the former. But, on going through 
> the history, the ISARROOT was orignally exported to locate the most 
> commonly used scripts directory, for wic and other purposes. So it made 
> sense to address that and derive anything which might need the actual 
> ISARROOT equivalent from it. 
>
> [1] 
> https://groups.google.com/forum/#!msg/isar-users/jklvMqAT_uA/L5BQY8xXBwAJ 
>
> Thanks, 
> Vijai Kumar K 
>
> > I did not see an "unset" so i assume it must be in contrib (oe or 
> > bitbake) code. 
> > 
> > Henning 
> > 
> > On Wed, 29 Jan 2020 01:05:20 +0530 
> > Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote: 
> > 
> > > 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. The user had to manually unset ISARROOT to avoid 
> > > any issues. 
> > > 
> > > As like OEROOT in OE, let us unset ISARROOT at the end of the 
> > > initialization script. This helps us to avoid the above issue 
> > > and also preserves support for passing ISARROOT from a caller 
> > > script as in OE. 
> > > 
> > > ISARROOT is mostly used with wic implemetation for the scripts 
> > > directory. Introduce SCRIPTSDIR to satisfy wic. 
> > > 
> > > When ISARROOT equivalent is needed, derive it from SCRIPTSDIR. 
> > > 
> > > Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com> 
> > > --- 
> > >  isar-init-build-env                      |  1 + 
> > >  meta/classes/image-sdk-extension.bbclass |  2 +- 
> > >  meta/classes/wic-img.bbclass             |  9 +++++---- 
> > >  scripts/ci_build.sh                      | 12 +++++++----- 
> > >  scripts/isar-buildenv-internal           |  5 ++++- 
> > >  5 files changed, 18 insertions(+), 11 deletions(-) 
> > > 
> > > diff --git a/isar-init-build-env b/isar-init-build-env 
> > > index b08bb59..85fb82f 100755 
> > > --- a/isar-init-build-env 
> > > +++ b/isar-init-build-env 
> > > @@ -55,5 +55,6 @@ export ISARROOT 
> > >      unset ISARROOT 
> > >      return 1 
> > >  } 
> > > +unset ISARROOT 
> > >   
> > >  [ -z "$BUILDDIR" ] || cd "$BUILDDIR" 
> > > diff --git a/meta/classes/image-sdk-extension.bbclass 
> > > b/meta/classes/image-sdk-extension.bbclass index b9f2cf4..b91fdd6 
> > > 100644 --- a/meta/classes/image-sdk-extension.bbclass 
> > > +++ b/meta/classes/image-sdk-extension.bbclass 
> > > @@ -19,7 +19,7 @@ do_populate_sdk() { 
> > >      sudo rm -f ${SDKCHROOT_DIR}/chroot-setup.sh 
> > > ${SDKCHROOT_DIR}/configscript.sh 
> > >      # Copy mount_chroot.sh for convenience 
> > > -    sudo cp ${ISARROOT}/scripts/mount_chroot.sh ${SDKCHROOT_DIR} 
> > > +    sudo cp ${SCRIPTSDIR}/mount_chroot.sh ${SDKCHROOT_DIR} 
> > >   
> > >      # Create SDK archive 
> > >      cd -P ${SDKCHROOT_DIR}/.. 
> > > diff --git a/meta/classes/wic-img.bbclass 
> > > b/meta/classes/wic-img.bbclass index 7d30e45..e715f56 100644 
> > > --- a/meta/classes/wic-img.bbclass 
> > > +++ b/meta/classes/wic-img.bbclass 
> > > @@ -86,7 +86,7 @@ STAGING_DATADIR ?= "/usr/lib/" 
> > >  STAGING_LIBDIR ?= "/usr/lib/" 
> > >  STAGING_DIR ?= "${TMPDIR}" 
> > >  IMAGE_BASENAME ?= "${PN}-${DISTRO}" 
> > > -FAKEROOTCMD ?= "${ISARROOT}/scripts/wic_fakeroot" 
> > > +FAKEROOTCMD ?= "${SCRIPTSDIR}/wic_fakeroot" 
> > >  RECIPE_SYSROOT_NATIVE ?= "/" 
> > >  BUILDCHROOT_DIR = "${BUILDCHROOT_TARGET_DIR}" 
> > >   
> > > @@ -132,7 +132,7 @@ do_wic_image() { 
> > >      buildchroot_do_mounts 
> > >      sudo -s <<'EOSUDO' 
> > >          ( flock 9 
> > > -        for dir in ${BBLAYERS} ${STAGING_DIR} ${ISARROOT}/scripts; do 
> > > +        for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR}; do 
> > >              mkdir -p ${BUILDCHROOT_DIR}/$dir 
> > >              if ! mountpoint ${BUILDCHROOT_DIR}/$dir >/dev/null 2>&1; 
> > > then mount --bind --make-private $dir ${BUILDCHROOT_DIR}/$dir 
> > > @@ -148,12 +148,13 @@ EOSUDO 
> > >      WICTMP=$(cd ${BUILDCHROOT_DIR}; mktemp -d -p tmp) 
> > >   
> > >      sudo -E chroot ${BUILDCHROOT_DIR} \ 
> > > -        ${ISARROOT}/scripts/wic create ${WKS_FULL_PATH} \ 
> > > +        ${SCRIPTSDIR}/wic create ${WKS_FULL_PATH} \ 
> > >              --vars "${STAGING_DIR}/${MACHINE}/imgdata/" \ 
> > >              -o /$WICTMP/${IMAGE_FULLNAME}.wic/ \ 
> > >              --bmap \ 
> > >              -e ${IMAGE_BASENAME} ${WIC_CREATE_EXTRA_ARGS} 
> > > -    sudo chown -R $(stat -c "%U" ${ISARROOT}) ${ISARROOT}/meta 
> > > ${ISARROOT}/meta-isar ${ISARROOT}/scripts || true 
> > > +    ROOTDIR=`dirname ${SCRIPTSDIR}` 
> > > +    sudo chown -R $(stat -c "%U" ${ROOTDIR}) ${ROOTDIR}/meta 
> > > ${ROOTDIR}/meta-isar ${SCRIPTSDIR} || true WIC_DIRECT=$(ls -t -1 
> > > ${BUILDCHROOT_DIR}/$WICTMP/${IMAGE_FULLNAME}.wic/*.direct | head -1) 
> > > sudo chown -R $(id -u):$(id -g) ${BUILDCHROOT_DIR}/${WICTMP} mv -f 
> > > ${WIC_DIRECT} ${WIC_IMAGE_FILE} diff --git a/scripts/ci_build.sh 
> > > b/scripts/ci_build.sh index 7cd5a2f..727b4a9 100755 
> > > --- a/scripts/ci_build.sh 
> > > +++ b/scripts/ci_build.sh 
> > > @@ -133,13 +133,15 @@ if [ ! -d "$BUILD_DIR" ]; then 
> > >  fi 
> > >  source isar-init-build-env "$BUILD_DIR" 
> > >   
> > > +ROOTDIR=`dirname ${SCRIPTSDIR}` 
> > > + 
> > >  if [ -n "$CROSS_BUILD" ]; then 
> > >      sed -i -e 's/ISAR_CROSS_COMPILE ?= "0"/ISAR_CROSS_COMPILE ?= 
> > > "1"/g' conf/local.conf fi 
> > >   
> > >  if [ -n "$REPRO_BUILD" ]; then 
> > > - 
> > > 
> ISAR_TESTSUITE_GPG_PUB_KEY_FILE="$ISARROOT/testsuite/base-apt/test_pub.key" 
> > > - 
> > > 
> ISAR_TESTSUITE_GPG_PRIV_KEY_FILE="$ISARROOT/testsuite/base-apt/test_priv.key" 
>
> > > + 
> > > 
> ISAR_TESTSUITE_GPG_PUB_KEY_FILE="$ROOTDIR/testsuite/base-apt/test_pub.key" 
> > > + 
> > > 
> ISAR_TESTSUITE_GPG_PRIV_KEY_FILE="$ROOTDIR/testsuite/base-apt/test_priv.key" 
>
> > > export GNUPGHOME=$(mktemp -d) gpg --import 
> > > $ISAR_TESTSUITE_GPG_PUB_KEY_FILE $ISAR_TESTSUITE_GPG_PRIV_KEY_FILE 
> > > @@ -188,9 +190,9 @@ if [ -z "$FAST_BUILD" ]; then 
> > >      bitbake $BB_ARGS $TARGETS_SET 
> > >  fi 
> > >   
> > > -cp -a "${ISARROOT}/meta/classes/dpkg-base.bbclass" 
> > > "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" -echo -e 
> > > "do_fetch_append() {\n\n}" >> 
> > > "${ISARROOT}/meta/classes/dpkg-base.bbclass" +cp -a 
> > > "${ROOTDIR}/meta/classes/dpkg-base.bbclass" 
> > > "${ROOTDIR}/meta/classes/dpkg-base.bbclass.ci-backup" +echo -e 
> > > "do_fetch_append() {\n\n}" >> 
> > > "${ROOTDIR}/meta/classes/dpkg-base.bbclass" bitbake $BB_ARGS 
> > > mc:qemuamd64-stretch:isar-image-base -mv 
> > > "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" 
> > > "${ISARROOT}/meta/classes/dpkg-base.bbclass" +mv 
> > > "${ROOTDIR}/meta/classes/dpkg-base.bbclass.ci-backup" 
> > > "${ROOTDIR}/meta/classes/dpkg-base.bbclass" diff --git 
> > > a/scripts/isar-buildenv-internal b/scripts/isar-buildenv-internal 
> > > index 72a91ab..a91ad7a 100755 --- a/scripts/isar-buildenv-internal 
> > > +++ b/scripts/isar-buildenv-internal @@ -74,5 +74,8 @@ export PATH 
> > > BBPATH="${BUILDDIR}" export BBPATH 
> > > -BB_ENV_EXTRAWHITE="BUILDDIR ISARROOT http_proxy https_proxy 
> > > ftp_proxy no_proxy GNUPGHOME" +SCRIPTSDIR="${ISARROOT}/scripts" 
> > > +export SCRIPTSDIR 
> > > + 
> > > +BB_ENV_EXTRAWHITE="BUILDDIR SCRIPTSDIR http_proxy https_proxy 
> > > ftp_proxy no_proxy GNUPGHOME" export BB_ENV_EXTRAWHITE 
> > 
>