<HTML><BODY>Igor,<br><br>Thanks a lot for the review, I've made all the changes as you suggested. Please recheck the changes.<br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
        Понедельник, 13 января 2020, 13:31 +03:00 от Igor Munkin <imun@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY">Sasha,<br><br>
Thanks for the patch! It LGTM to me, but I left some polish-aimed<br>
comments below, please consider them.<br><br>
Furthermore, I Cced Sasha Tu. to take a look on it since the patch is a<br>
large one an I can overlooked something vital.<br><br>
On 16.12.19, Alexander V. Tikhonov wrote:<br>
                                 > ---<br>
> <br>
> Github: <a href="https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-push-packages-s3-full-ci" target="_blank">https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-push-packages-s3-full-ci</a><br>
> Issue: <a href="https://github.com/tarantool/tarantool/issues/3380" target="_blank">https://github.com/tarantool/tarantool/issues/3380</a><br>
> <br>
> v1: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012021.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012021.html</a><br>
> v2: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012352.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012352.html</a><br>
> <br>
> Changes v2:<br>
> - made changes in script from draft to pre-release stages<br>
> <br>
> Changes v3:<br>
> - corrected code style, minor updates<br>
> - script is ready for release<br><br>
Please consider adding the changelog in the reverse order.<br><br>
> <br>
>  tools/add_pack_s3_repo.sh | 381 +++++++++++++++++++++-----------------<br>
>  1 file changed, 209 insertions(+), 172 deletions(-)<br>
> <br><br>
General comments:<br>
* Please choose a single code style for function definition. Currently<br>
  you mix two following approaches:<br>
|  usage()<br>
|  {<br>
  and<br>
|  function pack_rpm {<br></div></div></div></div></blockquote>Corrected.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br>
> diff --git a/tools/add_pack_s3_repo.sh b/tools/add_pack_s3_repo.sh<br>
> index 2316b9015..280a07ab0 100755<br>
> --- a/tools/add_pack_s3_repo.sh<br>
> +++ b/tools/add_pack_s3_repo.sh<br>
> @@ -4,8 +4,12 @@ set -e<br>
>  rm_file='rm -f'<br>
>  rm_dir='rm -rf'<br>
>  mk_dir='mkdir -p'<br>
> +ws_prefix=/tmp/tarantool_repo_s3<br>
>  <br>
> -alloss='ubuntu debian centos el fedora'<br>
> +alloss='ubuntu debian el fedora'<br>
> +product=tarantool<br>
> +# the path with binaries either repository<br>
> +repo=.<br>
>  <br>
>  get_os_dists()<br>
>  {<br>
> @@ -16,7 +20,7 @@ get_os_dists()<br>
>          alldists='trusty xenial cosmic disco bionic eoan'<br><br>
Minor: IMHO it's more convenient to sort distros in the release date<br>
order.</div></div></div></div></blockquote>Corrected.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>      elif [ "$os" == "debian" ]; then<br>
>          alldists='jessie stretch buster'<br>
> -    elif [ "$os" == "centos" -o "$os" == "el" ]; then<br>
> +    elif [ "$os" == "el" ]; then<br>
>          alldists='6 7 8'<br>
>      elif [ "$os" == "fedora" ]; then<br>
>          alldists='27 28 29 30 31'<br>
> @@ -25,16 +29,45 @@ get_os_dists()<br>
>      echo "$alldists"<br>
>  }<br>
>  <br>
> -ws_prefix=/tmp/tarantool_repo_s3<br>
> -create_lockfile()<br>
> +prepare_ws()<br>
>  {<br>
> -    lockfile -l 1000 $1<br>
> +    # temporary lock the publication to the repository<br>
> +    ws=${ws_prefix}_${branch}_${os}<br>
> +    if [ "$os" != "ubuntu" -a "$os" != "debian" ]; then<br>
> +        ws=${ws}_${option_dist}<br>
> +    fi<br><br>
I guess the suffix can be constructed within pack_{rpm,deb} subroutine<br>
(i.e. in prepare_ws caller) and passed as an argument to prepare_ws.<br>
Consider the following:<br>
| prepare_ws ${os}_${option_dist} # for deb based distros<br>
| prepare_ws ${os}                # for rpm based distros<br></div></div></div></div></blockquote><p>Changed as you suggested, just:<br>prepare_ws ${os} # for deb based distros<br>prepare_ws ${os}_${option_dist} # for rpm based distros</p><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br>
> +    ws_lockfile=${ws}.lock<br>
> +    if [ -f $ws_lockfile ]; then<br>
> +        old_proc=$(cat $ws_lockfile)<br>
> +    fi<br>
> +    lockfile -l 60 $ws_lockfile<br>
> +    chmod u+w $ws_lockfile && echo $$ >$ws_lockfile && chmod u-w $ws_lockfile<br>
> +    if [ "$old_proc" != ""  -a "$old_proc" != "0" ]; then<br>
> +        kill -9 $old_proc >/dev/null 2>&1 || true<br>
> +    fi<br>
> +<br>
> +    # create temporary workspace with repository copy<br>
> +    $rm_dir $ws<br>
> +    $mk_dir $ws<br>
>  }<br>
>  <br>
>  usage()<br>
>  {<br>
>      cat <<EOF<br>
> -Usage: $0 -b <S3 bucket> -o <OS name> -d <OS distribuition> [-p <product>] <path><br>
> +Usage for store package binaries from the given path:<br>
> +    $0 -b=<S3 bucket> -o=<OS name> -d=<OS distribuition> [-p=<product>] <path to package binaries><br>
> +<br>
> +Usage for mirroring Debian|Ubuntu OS repositories:<br>
> +    $0 -b=<S3 bucket> -o=<OS name> [-p=<product>] <path to 'pool' subdirectory with packages repository><br>
> +<br>
> +Arguments:<br>
> +    <path><br>
> +         Path points to the directory with deb/prm packages to be used.<br>
> +         Script can be used in one of 2 modes:<br>
> +          - path with binaries packages for a single distribution<br>
> +          - path with 'pool' subdirectory with APT repository (only: debian|ubuntu), like:<br>
> +                /var/spool/apt-mirror/mirror/packagecloud.io/tarantool/$branch/$os<br>
> +<br>
>  Options:<br>
>      -b|--bucket<br>
>          MCS S3 bucket which will be used for storing the packages.<br>
> @@ -44,24 +77,19 @@ Options:<br>
>              2.2: 2_2<br>
>              1.10: 1_10<br>
>      -o|--os<br>
> -        OS to be checked, one of the list (NOTE: centos == el):<br>
> +        OS to be checked, one of the list:<br>
>              $alloss<br>
>      -d|--distribution<br>
>          Distribution appropriate to the given OS:<br>
>  EOF<br>
>      for os in $alloss ; do<br>
> -        echo "            $os: <"`get_os_dists $os`">"<br>
> +        echo "            $os: <"$(get_os_dists $os)">"<br>
>      done<br>
>      cat <<EOF<br>
>      -p|--product<br>
>           Product name to be packed with, default name is 'tarantool'<br>
>      -h|--help<br>
>           Usage help message<br>
> -    <path><br>
> -         Path points to the directory with deb/prm packages to be used.<br>
> -         Script can be used in one of 2 modes:<br>
> -          - path with binaries packages for a single distribution<br>
> -    - path with 'pool' directory with APT repository (only: debian|ubuntu)<br>
>  EOF<br>
>  }<br>
>  <br>
> @@ -70,7 +98,7 @@ do<br>
>  case $i in<br>
>      -b=*|--bucket=*)<br>
>      branch="${i#*=}"<br>
> -    if [ "$branch" != "2x" -a "$branch" != "2_3" -a "$branch" != "2_2" -a "$branch" != "1_10" ]; then<br>
> +    if echo "$branch" | grep -qvP '^(1_10|2(x|_[2-4]))$' ; then<br>
>          echo "ERROR: bucket '$branch' is not supported"<br>
>          usage<br>
>          exit 1<br>
> @@ -79,9 +107,6 @@ case $i in<br>
>      ;;<br>
>      -o=*|--os=*)<br>
>      os="${i#*=}"<br>
> -    if [ "$os" == "el" ]; then<br>
> -        os=centos<br>
> -    fi<br>
>      if ! echo $alloss | grep -F -q -w $os ; then<br>
>          echo "ERROR: OS '$os' is not supported"<br>
>          usage<br>
> @@ -90,7 +115,7 @@ case $i in<br>
>      shift # past argument=value<br>
>      ;;<br>
>      -d=*|--distribution=*)<br>
> -    DIST="${i#*=}"<br>
> +    option_dist="${i#*=}"<br>
>      shift # past argument=value<br>
>      ;;<br>
>      -p=*|--product=*)<br>
> @@ -120,27 +145,68 @@ if [ "$os" == "" ]; then<br>
>      usage<br>
>      exit 1<br>
>  fi<br>
> -alldists=`get_os_dists $os`<br>
> -if [ -n "$DIST" ] && ! echo $alldists | grep -F -q -w $DIST ; then<br>
> -    echo "ERROR: set distribution at options '$DIST' not found at supported list '$alldists'"<br>
> +alldists=$(get_os_dists $os)<br>
> +if [ -n "$option_dist" ] && ! echo $alldists | grep -F -q -w $option_dist ; then<br>
> +    echo "ERROR: set distribution at options '$option_dist' not found at supported list '$alldists'"<br>
>      usage<br>
>      exit 1<br>
>  fi<br>
>  <br>
> -# set the path with binaries<br>
> -product=$product<br>
> -if [ "$product" == "" ]; then<br>
> -    product=tarantool<br>
> -fi<br>
> -proddir=`echo $product | head -c 1`<br>
> +# set the subpath with binaries based on literal character of the product name<br>
> +proddir=$(echo $product | head -c 1)<br>
>  <br>
> -# set the path with binaries<br>
> -if [ "$repo" == "" ]; then<br>
> -    repo=.<br>
> -fi<br>
> -<br>
> -aws='aws --endpoint-url <a href="https://hb.bizmrg.com" target="_blank">https://hb.bizmrg.com</a>'<br>
> +# AWS defines<br>
> +aws='aws --endpoint-url <a href="https://hb.bizmrg.com" target="_blank">https://hb.bizmrg.com</a> s3'<br>
>  s3="s3://tarantool_repo/$branch/$os"<br><br>
Minor: s3 variable name seems to be an ambiguous one here. IMHO, bucket<br>
looks the way better.<br></div></div></div></div></blockquote>Renamed 's3' variable to 'bucket_path'<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br>
> +aws_cp_to_s3="$aws cp --acl public-read"<br>
> +aws_sync_to_s3="$aws sync --acl public-read"<br><br>
Nice change, but other commands also interact with S3. I propose to<br>
change the naming to the following one:<br>
| <aws_<action>_public<br></div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br>
> +<br>
> +update_packfile()<br>
> +{<br>
> +    packfile=$1<br>
> +    packtype=$2<br>
> +<br>
> +    locpackfile=$(echo $packfile | sed "s#^$ws/##g")<br>
> +    # register DEB/DSC pack file to Packages/Sources file<br>
> +    reprepro -Vb . include$packtype $loop_dist $packfile<br><br>
Well, it's really bad idea to use external loop variable within<br>
function. You can pass it as an explicit argument, please do it.<br></div></div></div></div></blockquote>Corrected.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br>
> +    # reprepro copied DEB/DSC file to component which is not needed<br>
> +    $rm_dir $debdir/$component<br>
> +    # to have all sources avoid reprepro set DEB/DSC file to its own registry<br>
> +    $rm_dir db<br>
> +}<br>
> +<br><br>
Minor: Since update_{packfile,metadata} are used only for deb packages,<br>
it can be showed in naming: update_deb_{packfile,metadata}.</div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
> +update_metadata()<br>
> +{<br>
> +    packpath=$1<br>
> +    packtype=$2<br>
> +<br>
> +    if [ ! -f $packpath.saved ] ; then<br>
> +        # get the latest Sources file from S3<br>
> +        $aws ls "$s3/$packpath" 2>/dev/null && \<br>
> +            $aws cp "$s3/$packpath" $packpath.saved || \<br>
> +            touch $packpath.saved<br>
> +    fi<br>
> +<br>
> +    if [ "$packtype" == "dsc" ]; then<br>
> +        # WORKAROUND: unknown why, but reprepro doesn`t save the Sources file<br>
> +        gunzip -c $packpath.gz >$packpath<br>
> +        # check if the DSC file already exists in Sources from S3<br>
> +        hash=$(grep '^Checksums-Sha256:' -A3 $packpath | \<br>
> +            tail -n 1 | awk '{print $1}')<br>
> +        if grep " $hash .*\.dsc$" $packpath.saved ; then<br>
> +            echo "WARNING: DSC file already registered in S3!"<br>
> +            return 1<br>
> +        fi<br>
> +    elif [ "$packtype" == "deb" ]; then<br>
> +        # check if the DEB file already exists in Packages from S3<br>
> +        if grep "^$(grep '^SHA256: ' $packages)$" $packages.saved ; then<br>
> +            echo "WARNING: DEB file already registered in S3!"<br>
> +            return 1<br>
> +        fi<br>
> +    fi<br>
> +    # store the new DEB entry<br>
> +    cat $packpath >>$packpath.saved<br>
> +}<br>
>  <br>
>  # The 'pack_deb' function especialy created for DEB packages. It works<br>
>  # with DEB packing OS like Ubuntu, Debian. It is based on globaly known<br>
> @@ -158,32 +224,27 @@ function pack_deb {<br>
>      # debian has special directory 'pool' for packages<br>
>      debdir=pool<br>
>  <br>
> -    # get packages from pointed location either mirror path<br>
> -    if [ "$repo" == "" ] ; then<br>
> -        repo=/var/spool/apt-mirror/mirror/packagecloud.io/tarantool/$branch/$os<br>
> -    fi<br>
> -    if [ ! -d $repo/$debdir ] && ( [ "$DIST" == "" ] || ! ls $repo/*.deb $repo/*.dsc $repo/*.tar.*z >/dev/null 2>&1 ) ; then<br>
> +    # get packages from pointed location<br>
> +    if [ ! -d $repo/$debdir ] && \<br>
> +        ( [ "$option_dist" == "" ] || \<br>
> +            ! ls $repo/*.deb $repo/*.dsc $repo/*.tar.*z >/dev/null 2>&1 ) ; then<br>
>          echo "ERROR: Current '$repo' path doesn't have any of the following:"<br>
> -        echo "Usage with set distribuition with option '-d' and packages: $0 [path with *.deb *.dsc *.tar.*z files]"<br>
> -        echo "Usage with repositories: $0 [path to repository with '$debdir' subdirectory]"<br>
> +        echo " - $0 run option '-d' and DEB packages in path"<br>
> +        echo " - 'pool' subdirectory with APT repositories"<br>
> +        usage<br>
>          exit 1<br>
>      fi<br>
>  <br>
> -    # temporary lock the publication to the repository<br>
> -    ws=${ws_prefix}_${branch}_${os}<br>
> -    ws_lockfile=${ws}.lock<br>
> -    create_lockfile $ws_lockfile<br>
> -<br>
> -    # create temporary workspace with repository copy<br>
> -    $rm_dir $ws<br>
> -    $mk_dir $ws<br>
> +    # prepare the workspace<br>
> +    prepare_ws<br>
>  <br>
>      # script works in one of 2 modes:<br>
>      # - path with binaries packages for a single distribution<br>
>      # - path with 'pool' directory with APT repository<br>
> -    if [ "$DIST" != "" ] && ls $repo/*.deb $repo/*.dsc $repo/*.tar.*z >/dev/null 2>&1 ; then<br>
> +    if [ "$option_dist" != "" ] && \<br>
> +            ls $repo/*.deb $repo/*.dsc $repo/*.tar.*z >/dev/null 2>&1 ; then<br>
>          # copy single distribution with binaries packages<br>
> -        repopath=$ws/pool/${DIST}/main/$proddir/$product<br>
> +        repopath=$ws/pool/${option_dist}/$component/$proddir/$product<br>
>          $mk_dir ${repopath}<br>
>          cp $repo/*.deb $repo/*.dsc $repo/*.tar.*z $repopath/.<br>
>      elif [ -d $repo/$debdir ]; then<br>
> @@ -191,10 +252,12 @@ function pack_deb {<br>
>          cp -rf $repo/$debdir $ws/.<br>
>      else<br>
>          echo "ERROR: neither distribution option '-d' with files $repo/*.deb $repo/*.dsc $repo/*.tar.*z set nor '$repo/$debdir' path found"<br>
> -  usage<br>
> -  $rm_file $wslock<br>
> -  exit 1<br>
> +        usage<br>
> +        $rm_file $wslock<br>
> +        exit 1<br>
>      fi<br>
> +<br>
> +    swd=$PWD<br>
>      cd $ws<br><br>
Minor: I see no reason to avoid using more fitting pushd/popd here.</div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>  <br>
>      # create the configuration file for 'reprepro' tool<br>
> @@ -202,14 +265,14 @@ function pack_deb {<br>
>      $rm_dir $confpath<br>
>      $mk_dir $confpath<br>
>  <br>
> -    for dist in $alldists ; do<br>
> +    for loop_dist in $alldists ; do<br>
>          cat <<EOF >>$confpath/distributions<br>
>  Origin: Tarantool<br>
>  Label: tarantool.org<br>
>  Suite: stable<br>
> -Codename: $dist<br>
> +Codename: $loop_dist<br>
>  Architectures: amd64 source<br>
> -Components: main<br>
> +Components: $component<br>
>  Description: Unofficial Ubuntu Packages maintained by Tarantool<br>
>  SignWith: 91B625E5<br>
>  DebIndices: Packages Release . .gz .bz2<br>
> @@ -217,84 +280,49 @@ UDebIndices: Packages . .gz .bz2<br>
>  DscIndices: Sources Release .gz .bz2<br>
>  <br>
>  EOF<br>
> -done<br>
> +    done<br>
>  <br>
>      # create standalone repository with separate components<br>
> -    for dist in $alldists ; do<br>
> -        echo =================== DISTRIBUTION: $dist =========================<br>
> +    for loop_dist in $alldists ; do<br>
> +        echo ================ DISTRIBUTION: $loop_dist ====================<br>
>          updated_deb=0<br>
>          updated_dsc=0<br>
>  <br>
>          # 1(binaries). use reprepro tool to generate Packages file<br>
> -        for deb in $ws/$debdir/$dist/$component/*/*/*.deb ; do<br>
> +        for deb in $ws/$debdir/$loop_dist/$component/*/*/*.deb ; do<br>
>              [ -f $deb ] || continue<br>
> -            locdeb=`echo $deb | sed "s#^$ws\/##g"`<br>
> -            echo "DEB: $deb"<br>
> -            # register DEB file to Packages file<br>
> -            reprepro -Vb . includedeb $dist $deb<br>
> -            # reprepro copied DEB file to local component which is not needed<br>
> -            $rm_dir $debdir/$component<br>
> -            # to have all packages avoid reprepro set DEB file to its own registry<br>
> -            $rm_dir db<br>
> -            # copy Packages file to avoid of removing by the new DEB version<br>
> -            for packages in dists/$dist/$component/binary-*/Packages ; do<br>
> -                if [ ! -f $packages.saved ] ; then<br>
> -                    # get the latest Packages file from S3<br>
> -                    $aws s3 ls "$s3/$packages" 2>/dev/null && \<br>
> -                        $aws s3 cp --acl public-read \<br>
> -                        "$s3/$packages" $packages.saved || \<br>
> -                        touch $packages.saved<br>
> -                fi<br>
> -                # check if the DEB file already exists in Packages from S3<br>
> -                if grep "^`grep "^SHA256: " $packages`$" $packages.saved ; then<br>
> -                    echo "WARNING: DEB file already registered in S3!"<br>
> -                    continue<br>
> -                fi<br>
> -                # store the new DEB entry<br>
> -                cat $packages >>$packages.saved<br>
> -                # save the registered DEB file to S3<br>
> -                $aws s3 cp --acl public-read $deb $s3/$locdeb<br>
> +            # regenerate DEB pack<br>
> +            update_packfile $deb deb<br>
> +            echo "Regenerated DEB file: $locpackfile"<br>
> +            for packages in dists/$loop_dist/$component/binary-*/Packages ; do<br>
> +                # copy Packages file to avoid of removing by the new DEB version<br>
> +                # update metadata 'Packages' files<br>
> +                if ! update_metadata $packages deb ; then continue ; fi<br><br>
I see the following approach to be more convenient for oneline if:<br>
| update_metadata $packages deb || continue</div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>                  updated_deb=1<br>
>              done<br>
> +            # save the registered DEB file to S3<br>
> +            if [ "$updated_deb" == 1 ]; then<br>
> +                $aws_cp_to_s3 $deb $s3/$locpackfile<br>
> +            fi<br>
>          done<br>
>  <br>
>          # 1(sources). use reprepro tool to generate Sources file<br>
> -        for dsc in $ws/$debdir/$dist/$component/*/*/*.dsc ; do<br>
> +        for dsc in $ws/$debdir/$loop_dist/$component/*/*/*.dsc ; do<br>
>              [ -f $dsc ] || continue<br>
> -            locdsc=`echo $dsc | sed "s#^$ws\/##g"`<br>
> -            echo "DSC: $dsc"<br>
> -            # register DSC file to Sources file<br>
> -            reprepro -Vb . includedsc $dist $dsc<br>
> -            # reprepro copied DSC file to component which is not needed<br>
> -            $rm_dir $debdir/$component<br>
> -            # to have all sources avoid reprepro set DSC file to its own registry<br>
> -            $rm_dir db<br>
> +            # regenerate DSC pack<br>
> +            update_packfile $dsc dsc<br>
> +            echo "Regenerated DSC file: $locpackfile"<br>
>              # copy Sources file to avoid of removing by the new DSC version<br>
> -            sources=dists/$dist/$component/source/Sources<br>
> -            if [ ! -f $sources.saved ] ; then<br>
> -                # get the latest Sources file from S3<br>
> -                $aws s3 ls "$s3/$sources" && \<br>
> -                    $aws s3 cp --acl public-read "$s3/$sources" $sources.saved || \<br>
> -                    touch $sources.saved<br>
> -            fi<br>
> -            # WORKAROUND: unknown why, but reprepro doesn`t save the Sources file<br>
> -            gunzip -c $sources.gz >$sources<br>
> -            # check if the DSC file already exists in Sources from S3<br>
> -            hash=`grep '^Checksums-Sha256:' -A3 $sources | \<br>
> -                tail -n 1 | awk '{print $1}'`<br>
> -            if grep " $hash .*\.dsc$" $sources.saved ; then<br>
> -                echo "WARNING: DSC file already registered in S3!"<br>
> -                continue<br>
> -            fi<br>
> -            # store the new DSC entry<br>
> -            cat $sources >>$sources.saved<br>
> -            # save the registered DSC file to S3<br>
> -            $aws s3 cp --acl public-read $dsc $s3/$locdsc<br>
> -            tarxz=`echo $locdsc | sed 's#\.dsc$#.debian.tar.xz#g'`<br>
> -            $aws s3 cp --acl public-read $ws/$tarxz "$s3/$tarxz"<br>
> -            orig=`echo $locdsc | sed 's#-1\.dsc$#.orig.tar.xz#g'`<br>
> -            $aws s3 cp --acl public-read $ws/$orig "$s3/$orig"<br>
> +            # update metadata 'Sources' file<br>
> +            if ! update_metadata dists/$loop_dist/$component/source/Sources dsc \<br>
> +                ; then continue ; fi<br><br>
I see the following approach to be more convenient for oneline if:<br>
| update_metadata $dist/$loop_dist/$component/source/Sources dsc || continue</div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>              updated_dsc=1<br>
> +            # save the registered DSC file to S3<br>
> +            $aws_cp_to_s3 $dsc $s3/$locpackfile<br>
> +            tarxz=$(echo $locpackfile | sed 's#\.dsc$#.debian.tar.xz#g')<br>
> +            $aws_cp_to_s3 $ws/$tarxz "$s3/$tarxz"<br>
> +            orig=$(echo $locpackfile | sed 's#-1\.dsc$#.orig.tar.xz#g')<br>
> +            $aws_cp_to_s3 $ws/$orig "$s3/$orig"<br>
>          done<br>
>  <br>
>          # check if any DEB/DSC files were newly registered<br>
> @@ -302,30 +330,30 @@ done<br>
>              continue || echo "Updating dists"<br>
>  <br>
>          # finalize the Packages file<br>
> -        for packages in dists/$dist/$component/binary-*/Packages ; do<br>
> +        for packages in dists/$loop_dist/$component/binary-*/Packages ; do<br>
>              mv $packages.saved $packages<br>
>          done<br>
>  <br>
>          # 2(binaries). update Packages file archives<br>
> -        for packpath in dists/$dist/$component/binary-* ; do<br>
> +        for packpath in dists/$loop_dist/$component/binary-* ; do<br>
>              pushd $packpath<br>
> -            sed "s#Filename: $debdir/$component/#Filename: $debdir/$dist/$component/#g" -i Packages<br>
> +            sed "s#Filename: $debdir/$component/#Filename: $debdir/$loop_dist/$component/#g" -i Packages<br>
>              bzip2 -c Packages >Packages.bz2<br>
>              gzip -c Packages >Packages.gz<br>
>              popd<br>
>          done<br>
>  <br>
>          # 2(sources). update Sources file archives<br>
> -        pushd dists/$dist/$component/source<br>
> -        sed "s#Directory: $debdir/$component/#Directory: $debdir/$dist/$component/#g" -i Sources<br>
> +        pushd dists/$loop_dist/$component/source<br>
> +        sed "s#Directory: $debdir/$component/#Directory: $debdir/$loop_dist/$component/#g" -i Sources<br>
>          bzip2 -c Sources >Sources.bz2<br>
>          gzip -c Sources >Sources.gz<br>
>          popd<br>
>  <br>
>          # 3. update checksums entries of the Packages* files in *Release files<br>
> -  # NOTE: it is stable structure of the *Release files when the checksum<br>
> -  #       entries in it in the following way:<br>
> -  # MD5Sum:<br>
> +        # NOTE: it is stable structure of the *Release files when the checksum<br>
> +        #       entries in it in the following way:<br>
> +        # MD5Sum:<br>
>          #  <checksum> <size> <file orig><br>
>          #  <checksum> <size> <file debian><br>
>          # SHA1:<br>
> @@ -334,14 +362,14 @@ done<br>
>          # SHA256:<br>
>          #  <checksum> <size> <file orig><br>
>          #  <checksum> <size> <file debian><br>
> -  #       The script bellow puts 'md5' value at the 1st found file entry,<br>
> -  #       'sha1' - at the 2nd and 'sha256' at the 3rd<br>
> -        pushd dists/$dist<br>
> -        for file in `grep " $component/" Release | awk '{print $3}' | sort -u` ; do<br>
> -            sz=`stat -c "%s" $file`<br>
> -            md5=`md5sum $file | awk '{print $1}'`<br>
> -            sha1=`sha1sum $file | awk '{print $1}'`<br>
> -            sha256=`sha256sum $file | awk '{print $1}'`<br>
> +        #       The script bellow puts 'md5' value at the 1st found file entry,<br>
> +        #       'sha1' - at the 2nd and 'sha256' at the 3rd<br>
> +        pushd dists/$loop_dist<br>
> +        for file in $(grep " $component/" Release | awk '{print $3}' | sort -u) ; do<br>
> +            sz=$(stat -c "%s" $file)<br>
> +            md5=$(md5sum $file | awk '{print $1}')<br>
> +            sha1=$(sha1sum $file | awk '{print $1}')<br>
> +            sha256=$(sha256sum $file | awk '{print $1}')<br>
>              awk 'BEGIN{c = 0} ; {<br>
>                  if ($3 == p) {<br>
>                      c = c + 1<br>
> @@ -350,7 +378,7 @@ done<br>
>                      if (c == 3) {print " " sh2 " " s " " p}<br>
>                  } else {print $0}<br>
>              }' p="$file" s="$sz" md="$md5" sh1="$sha1" sh2="$sha256" \<br>
> -            Release >Release.new<br>
> +                    Release >Release.new<br>
>              mv Release.new Release<br>
>          done<br>
>          # resign the selfsigned InRelease file<br>
> @@ -362,11 +390,13 @@ done<br>
>          popd<br>
>  <br>
>          # 4. sync the latest distribution path changes to S3<br>
> -        $aws s3 sync --acl public-read dists/$dist "$s3/dists/$dist"<br>
> +        $aws_sync_to_s3 dists/$loop_dist "$s3/dists/$loop_dist"<br>
>      done<br>
>  <br>
>      # unlock the publishing<br>
>      $rm_file $ws_lockfile<br>
> +<br>
> +    cd $swd<br>
>  }<br>
>  <br>
>  # The 'pack_rpm' function especialy created for RPM packages. It works<br>
> @@ -380,29 +410,26 @@ done<br>
>  #     <a href="http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/" target="_blank">http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/</a><br>
>  function pack_rpm {<br>
>      if ! ls $repo/*.rpm >/dev/null 2>&1 ; then<br>
> -        echo "ERROR: Current '$repo' has:"<br>
> -        ls -al $repo<br>
> -        echo "Usage: $0 [path with *.rpm files]"<br>
> +        echo "ERROR: Current '$repo' path doesn't have RPM packages in path"<br>
> +        usage<br>
>          exit 1<br>
>      fi<br>
>  <br>
> -    # temporary lock the publication to the repository<br>
> -    ws=${prefix_lockfile}_${branch}_${os}_${DIST}<br>
> -    ws_lockfile=${ws}.lock<br>
> -    create_lockfile $ws_lockfile<br>
> +    # prepare the workspace<br>
> +    prepare_ws<br>
>  <br>
> -    # create temporary workspace with packages copies<br>
> -    $rm_dir $ws<br>
> -    $mk_dir $ws<br>
> +    # copy the needed package binaries to the workspace<br>
>      cp $repo/*.rpm $ws/.<br>
> +<br>
> +    swd=$PWD<br>
>      cd $ws<br><br>
Minor: I see no reason to avoid using more fitting pushd/popd here.</div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>  <br>
>      # set the paths<br>
> -    if [ "$os" == "centos" ]; then<br>
> -        repopath=$DIST/os/x86_64<br>
> +    if [ "$os" == "el" ]; then<br>
> +        repopath=$option_dist/os/x86_64<br>
>          rpmpath=Packages<br>
>      elif [ "$os" == "fedora" ]; then<br>
> -        repopath=releases/$DIST/Everything/x86_64/os<br>
> +        repopath=releases/$option_dist/Everything/x86_64/os<br>
>          rpmpath=Packages/$proddir<br>
>      fi<br>
>      packpath=$repopath/$rpmpath<br>
> @@ -410,53 +437,61 @@ function pack_rpm {<br>
>      # prepare local repository with packages<br>
>      $mk_dir $packpath<br>
>      mv *.rpm $packpath/.<br>
> -    cd $repopath<br>
> +    cd $ws/$repopath<br><br>
Minor: As discussed offline $ws is always an absolute path, but the<br>
command above looks confusing to me considering the prior cd command.<br>
Feel free to ignore.</div></div></div></div></blockquote>Removed '$ws' as suggested.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15789115050368447648_BODY"><br><br>
>  <br>
>      # copy the current metadata files from S3<br>
>      mkdir repodata.base<br>
> -    for file in `$aws s3 ls $s3/$repopath/repodata/ | awk '{print $NF}'` ; do<br>
> -        $aws s3 ls $s3/$repopath/repodata/$file || continue<br>
> -        $aws s3 cp $s3/$repopath/repodata/$file repodata.base/$file<br>
> +    for file in $($aws ls $s3/$repopath/repodata/ | awk '{print $NF}') ; do<br>
> +        $aws ls $s3/$repopath/repodata/$file || continue<br>
> +        $aws cp $s3/$repopath/repodata/$file repodata.base/$file<br>
>      done<br>
>  <br>
>      # create the new repository metadata files<br>
> -    createrepo --no-database --update --workers=2 --compress-type=gz --simple-md-filenames .<br>
> +    createrepo --no-database --update --workers=2 \<br>
> +        --compress-type=gz --simple-md-filenames .<br>
>      mv repodata repodata.adding<br>
>  <br>
>      # merge metadata files<br>
>      mkdir repodata<br>
>      head -n 2 repodata.adding/repomd.xml >repodata/repomd.xml<br>
>      for file in filelists.xml other.xml primary.xml ; do<br>
> -        # 1. take the 1st line only - to skip the line with number of packages which is not needed<br>
> +        # 1. take the 1st line only - to skip the line with<br>
> +        #    number of packages which is not needed<br>
>          zcat repodata.adding/$file.gz | head -n 1 >repodata/$file<br>
> -        # 2. take 2nd line with metadata tag and update the packages number in it<br>
> +        # 2. take 2nd line with metadata tag and update<br>
> +        #    the packages number in it<br>
>          packsold=0<br>
>          if [ -f repodata.base/$file.gz ] ; then<br>
> -        packsold=`zcat repodata.base/$file.gz | head -n 2 | tail -n 1 | sed 's#.*packages="\(.*\)".*#\1#g'`<br>
> +            packsold=$(zcat repodata.base/$file.gz | head -n 2 | \<br>
> +                tail -n 1 | sed 's#.*packages="\(.*\)".*#\1#g')<br>
>          fi<br>
> -        packsnew=`zcat repodata.adding/$file.gz | head -n 2 | tail -n 1 | sed 's#.*packages="\(.*\)".*#\1#g'`<br>
> +        packsnew=$(zcat repodata.adding/$file.gz | head -n 2 | \<br>
> +            tail -n 1 | sed 's#.*packages="\(.*\)".*#\1#g')<br>
>          packs=$(($packsold+$packsnew))<br>
> -        zcat repodata.adding/$file.gz | head -n 2 | tail -n 1 | sed "s#packages=\".*\"#packages=\"$packs\"#g" >>repodata/$file<br>
> +        zcat repodata.adding/$file.gz | head -n 2 | tail -n 1 | \<br>
> +            sed "s#packages=\".*\"#packages=\"$packs\"#g" >>repodata/$file<br>
>          # 3. take only 'package' tags from new file<br>
> -        zcat repodata.adding/$file.gz | tail -n +3 | head -n -1 >>repodata/$file<br>
> +        zcat repodata.adding/$file.gz | tail -n +3 | head -n -1 \<br>
> +            >>repodata/$file<br>
>          # 4. take only 'package' tags from old file if exists<br>
>          if [ -f repodata.base/$file.gz ] ; then<br>
> -            zcat repodata.base/$file.gz | tail -n +3 | head -n -1 >>repodata/$file<br>
> +            zcat repodata.base/$file.gz | tail -n +3 | head -n -1 \<br>
> +                >>repodata/$file<br>
>          fi<br>
>          # 5. take the last closing line with metadata tag<br>
>          zcat repodata.adding/$file.gz | tail -n 1 >>repodata/$file<br>
>  <br>
>          # get the new data<br>
> -        chsnew=`sha256sum repodata/$file | awk '{print $1}'`<br>
> -        sz=`stat --printf="%s" repodata/$file`<br>
> +        chsnew=$(sha256sum repodata/$file | awk '{print $1}')<br>
> +        sz=$(stat --printf="%s" repodata/$file)<br>
>          gzip repodata/$file<br>
> -        chsgznew=`sha256sum repodata/$file.gz | awk '{print $1}'`<br>
> -        szgz=`stat --printf="%s" repodata/$file.gz`<br>
> -        timestamp=`date +%s -r repodata/$file.gz`<br>
> +        chsgznew=$(sha256sum repodata/$file.gz | awk '{print $1}')<br>
> +        szgz=$(stat --printf="%s" repodata/$file.gz)<br>
> +        timestamp=$(date +%s -r repodata/$file.gz)<br>
>  <br>
>          # add info to repomd.xml file<br>
> -        name=`echo $file | sed 's#\.xml$##g'`<br>
> -  cat <<EOF >>repodata/repomd.xml<br>
> +        name=$(echo $file | sed 's#\.xml$##g')<br>
> +        cat <<EOF >>repodata/repomd.xml<br>
>  <data type="$name"><br>
>    <checksum type="sha256">$chsgznew</checksum><br>
>    <open-checksum type="sha256">$chsnew</open-checksum><br>
> @@ -472,19 +507,21 @@ EOF<br>
>  <br>
>      # copy the packages to S3<br>
>      for file in $rpmpath/*.rpm ; do<br>
> -        $aws s3 cp --acl public-read $file "$s3/$repopath/$file"<br>
> +        $aws_cp_to_s3 $file "$s3/$repopath/$file"<br>
>      done<br>
>  <br>
>      # update the metadata at the S3<br>
> -    $aws s3 sync --acl public-read repodata "$s3/$repopath/repodata"<br>
> +    $aws_sync_to_s3 repodata "$s3/$repopath/repodata"<br>
>  <br>
>      # unlock the publishing<br>
>      $rm_file $ws_lockfile<br>
> +<br>
> +    cd $swd<br>
>  }<br>
>  <br>
>  if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then<br>
>      pack_deb<br>
> -elif [ "$os" == "centos" -o "$os" == "fedora" ]; then<br>
> +elif [ "$os" == "el" -o "$os" == "fedora" ]; then<br>
>      pack_rpm<br>
>  else<br>
>      echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"<br>
> -- <br>
> 2.17.1<br>
> <br><br>
-- <br>
Best regards,<br>
IM<br></div></div></div></div></blockquote>
<br>
<br>-- <br>Alexander Tikhonov<br></BODY></HTML>