Hi Richard,
Thanks for the review, please find my feedback in follow.
On Fri, Oct 20, 2023 at 4:57 PM Richard Purdie
<richard.pur...@linuxfoundation.org> wrote:
>
> Hi David,
>
> Thanks for revising this, it is heading in the right direction.
>
> On Fri, 2023-10-20 at 16:05 +0200, David Pierret wrote:
> > - Add setup and run script openembedded specific
> > - Add job config
> >
> > Signed-off-by: David Pierret <david.pier...@smile.fr>
> > ---
> > config.json | 6 ++++++
> > scripts/run-auh-oe | 45 ++++++++++++++++++++++++++++++++++++++++++++
> > scripts/setup-auh-oe | 34 +++++++++++++++++++++++++++++++++
> > 3 files changed, 85 insertions(+)
> > create mode 100755 scripts/run-auh-oe
> > create mode 100755 scripts/setup-auh-oe
> >
> > diff --git a/config.json b/config.json
> > index 3acb710..bbd0aaf 100644
> > --- a/config.json
> > +++ b/config.json
> > @@ -1420,6 +1420,12 @@
> > "${SCRIPTSDIR}/setup-auh ${HELPERBUILDDIR};
> > ${SCRIPTSDIR}/run-auh ${HELPERBUILDDIR} ${WEBPUBLISH_DIR}/pub/auh/"
> > ]
> > },
> > + "auh-openembedded" : {
>
> This needs to be "auh-meta-oe" since "auh" is for openembedded-core and
> this is otherwise going to be confusing.
>
> > + "NEEDREPOS" : ["poky", "meta-openembedded"],
> > + "EXTRAPLAINCMDS" : [
> > + "${SCRIPTSDIR}/setup-auh-oe ${HELPERBUILDDIR};
> > ${SCRIPTSDIR}/run-auh-oe ${HELPERBUILDDIR} ${WEBPUBLISH_DIR}/pub/auh/
> > ${HELPERBUILDDIR}/meta-openembedded
> > ${HELPERBUILDDIR}/meta-openembedded/meta-oe
> > ${HELPERBUILDDIR}/meta-openembedded/meta-python
> > ${HELPERBUILDDIR}/meta-openembedded/meta-perl
> > ${HELPERBUILDDIR}/meta-openembedded/meta-networking
> > ${HELPERBUILDDIR}/meta-openembedded/meta-multimedia
> > ${HELPERBUILDDIR}/meta-openembedded/meta-gnome
> > ${HELPERBUILDDIR}/meta-openembedded/meta-xfce
> > ${HELPERBUILDDIR}/meta-openembedded/meta-filesystems
> > ${HELPERBUILDDIR}/meta-openembedded/meta-initramfs
> > ${HELPERBUILDDIR}/meta-openembedded/meta-webserver "
> > + ]
> > + },
> > "a-quick" : {
> > "TEMPLATE" : "trigger-build"
> > },
>
> Would it be better to have one setup-auh/run-auh script and add
> ${HELPERTARGET} as a parameter?
>
> This should translate to "auh" or "auh-meta-oe" and the script can then
> have conditionals in to handle both cases rather than duplicating the
> script?
The setup-auh script is cloning its own repository instead of using
the one initialized by CI.
Any reason to not use them ? We could use the NEEDREPOS instead of the
setup script,
and add the auto-upgrade-helper repository in the config.
>
> > diff --git a/scripts/run-auh-oe b/scripts/run-auh-oe
> > new file mode 100755
> > index 0000000..588755a
> > --- /dev/null
> > +++ b/scripts/run-auh-oe
>
> Similarly, this should be run-auh-meta-oe.
>
> > @@ -0,0 +1,45 @@
> > +#!/bin/bash
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Run Auto Upgrade Helper in a directory set up by setup_auh.
> > +#
> > +# Called with $1 - the directory where the setup was created
> > +
> > +if [ -z $1 ]; then
> > + echo "Use: $0 [auh_setup_dir] [publish_dir] [meta_dir] [meta_list]"
> > + exit 1
> > +fi
> > +
> > +full_dir=$(readlink -e $1)
> > +
> > +auh_dir=$full_dir/auto-upgrade-helper
> > +poky_dir=$full_dir/poky
> > +
> > +build_dir=$full_dir/build
> > +sstate_dir=$full_dir/build/sstate-cache
> > +meta_dir=$3
> > +meta_list=${@:4}
> > +machine_list="qemux86 qemux86-64 qemuarm qemumips qemuppc qemux86_musl"
>
> Do we need to vary the machine_list per layer? At present it only seems
> to support one list anyway so I'm not sure this does anything useful?
The list is the same as the one used for poky.
>
> > +pushd $meta_dir || exit 1
> > +
> > +# Base the upgrades on meta_openembedded master
> > +git fetch origin
> > +git checkout -B tmp-auh-upgrades origin/main
> > +
> > +source $poky_dir/oe-init-build-env $build_dir
> > +
> > +# build the layer_names variable to me used in the command line
> > +layer_names=""
> > +for d in $meta_list; do
> > + layer_names+=" $(basename ${d})"
> > +done
> > +
> > +$auh_dir/upgrade-helper.py -e --layer-dir ${meta_dir} --layer-names
> > ${layer_names} --layer-machines ${machine_list} -- all
>
> Would it be simpler just to iterate, calling upgrade-helper once per
> sub-layer or meta-openembedded?
Is it prefered to set 1 step per layer ?
or keep the call to the script but iterate over a list of layers ?
>
> You may have considered that in which case I'd just like to understand
> why you ended up with this as the preferred way of doing things?
>
> In some ways a separate report/run may be useful for the way meta-oe
> maintainers might handle this?
The iteration over a list was my first approach, but aborted following
review on patch v1.
The V1 review asked to allow the AUH to accept dynamic layers as parameters.
We maybe misunderstood each other.
>
> > +
> > +if [ -n $2 ]; then
> > + cp -rf $build_dir/upgrade-helper/* $2
> > +fi
> > +
> > +popd
> > diff --git a/scripts/setup-auh-oe b/scripts/setup-auh-oe
> > new file mode 100755
> > index 0000000..ba1a7fb
> > --- /dev/null
> > +++ b/scripts/setup-auh-oe
>
> This naming also needs tweaking.
>
> > @@ -0,0 +1,34 @@
> > +#!/bin/bash
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Initialize Auto Upgrade Helper in a directory.
> > +#
> > +# Called with $1 - the directory to place the setup
> > +CONFIG_DIR=`dirname $0`/auh-config
> > +
> > +if [ -z $1 ]; then
> > + echo "Use: $0 target_dir"
> > + exit 1
> > +fi
> > +
> > +mkdir -p $1
> > +pushd $1
> > +
> > +git clone git://git.yoctoproject.org/poky
> > +pushd poky
> > +git config user.email a...@yoctoproject.org
> > +git config user.name "Auto Upgrade Helper"
> > +popd
> > +git clone git://git.openembedded.org/meta-openembedded
> > +pushd meta-openembedded
> > +git config user.email a...@yoctoproject.org
> > +git config user.name "Auto Upgrade Helper"
> > +popd
> > +git clone git://git.yoctoproject.org/auto-upgrade-helper
> > +source poky/oe-init-build-env build
> > +mkdir -p upgrade-helper
> > +popd
> > +
> > +cp $CONFIG_DIR/upgrade-helper.conf $1/build/upgrade-helper
> > +cat $CONFIG_DIR/local.conf.append >> $1/build/conf/local.conf
>
> Would a upgrade-helper.conf per target help and simplfy the script
> differences?
Do you mean 1 upgrade-helper.conf per layer ?
>
> Cheers,
>
> Richard
>
>
regards
David
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#61461): https://lists.yoctoproject.org/g/yocto/message/61461
Mute This Topic: https://lists.yoctoproject.org/mt/102081665/21656
Group Owner: yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
