Re: [gentoo-releng] [PATCH 1/2] catalyst-auto-amd64*.conf: explicitly list files to be transferred

[Matt Turner <mattst88@gentoo.org>]

On Fri, May 15, 2020 at 6:07 AM Ben Kohler <bkohler@gentoo.org> wrote:
>
> Previously when we were copying locally there was no (major) problem
> with wildcarding a long list of files to be transferred after every
> single spec finishes.  Now that we are transferring to a remote host,
> this results in a lot of wasted transfer time & bandwidth.
>
> I have adjusted the post_build function to only transfer the new files
> known to be produced by each spec (which has files meant to be
> published).

Thanks Ben. Both patches look good to me. Thanks for working on this.

Just a couple small comments inline.

> Signed-off-by: Ben Kohler <bkohler@gentoo.org>
> ---
>  tools/catalyst-auto-amd64-experimental.conf | 87 ++++++++++++++-------

Do we actually use this file in production? It looks like the main
difference is TYPE="experimental" vs TYPE="auto" and I don't see TYPE
used anywhere. Can we just remove this file?

>  tools/catalyst-auto-amd64.conf              | 86 +++++++++++++-------
>  2 files changed, 113 insertions(+), 60 deletions(-)
>
> diff --git a/tools/catalyst-auto-amd64-experimental.conf b/tools/catalyst-auto-amd64-experimental.conf
> index e7b1ea79..2aa91a03 100644
> --- a/tools/catalyst-auto-amd64-experimental.conf
> +++ b/tools/catalyst-auto-amd64-experimental.conf
> @@ -26,6 +26,7 @@ SET_minimal_nomultilib_OPTIONAL_SPECS="stage4-nomultilib-minimal.spec"
>
>  SET_x32_SPECS="stage1-x32.spec stage2-x32.spec stage3-x32.spec"
>
> +

Stray newline, but I see that this is to reduce the differences
between the experimental and non-experimental versions of the file. I
might remove the newline from the other instead, but I don't feel
strongly.

>  SET_hardened_multilib_SPECS="hardened/stage1.spec hardened/stage2.spec hardened/stage3.spec"
>  SET_hardened_multilib_OPTIONAL_SPECS="hardened/admincd-stage1.spec hardened/admincd-stage2.spec"
>  SET_minimal_hardened_multilib_OPTIONAL_SPECS="hardened/stage4-minimal.spec"
> @@ -43,7 +44,7 @@ EXTENSIONS="[.tar.xz,.tar.bz2,.tar.gz,.tar,.sfs]"
>  update_symlinks() {
>         # Symlink the latest stages3 to build from
>         for d in ${BUILD_SRCDIR_BASE}/{default,hardened} ; do
> -               pushd $d >/dev/null
> +               pushd "${d}" >/dev/null || exit
>                 for f in $(ls stage3*${EXTENSIONS} | grep -v latest | give_latest_from_dates ) ; do
>                         # 20yymmddThhmmssZ
>                         # 20yymmddhhmmss
> @@ -57,11 +58,13 @@ update_symlinks() {
>                                 <<<"$f")
>                         ln -svf "$f" "$of"
>                 done
> -               popd >/dev/null
> +               popd >/dev/null || exit
>         done
>  }
>
>  post_build() {
> +       local set=$1 spec=$2
> +
>         cmd=(
>                 rsync

This would be a good opportunity to create and use an upload()
function like the other conf files have.