[Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

[Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

[Alexander V. Tikhonov]

Changed the Vanilla paths of the packages in S3 to
the Tarantool documented on its web site, to give
the ability to Tarantool users to continue use it
w/o changes.

Also moved the RPM sources packages to the standalone
path.

Follows up #3380
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-rpm-packages-paths-full-ci
Issue: https://github.com/tarantool/tarantool/issues/3380

 tools/update_repo.sh | 73 +++++++++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 22 deletions(-)

diff --git a/tools/update_repo.sh b/tools/update_repo.sh
index 65a977187..ec6146514 100755
--- a/tools/update_repo.sh
+++ b/tools/update_repo.sh
@@ -405,42 +405,68 @@ EOF
 }
 
 # The 'pack_rpm' function especialy created for RPM packages. It works
-# with RPM packing OS like Centos, Fedora. It is based on globaly known
+# with RPM packing OS like CentOS, Fedora. It is based on globaly known
 # tool 'createrepo' from:
-#     https://linux.die.net/man/8/createrepo
+#   https://linux.die.net/man/8/createrepo
 # This tool works with single distribution of the given OS.
-# Result of the routine is the rpm package for YUM repository with
-# file structure equal to the Centos/Fedora:
-#     http://mirror.centos.org/centos/7/os/x86_64/Packages/
-#     http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
+#
+# Vanilla RPM packages structure for YUM repositories looks like:
+#   CentOS:
+#     Binaries: http://mirror.centos.org/centos/7/os/x86_64/Packages/
+#     Sources: http://vault.centos.org/8.1.1911/BaseOS/Source/SPackages/
+#  Fedora:
+#     Binaries: http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
+#     Sources: http://mirrors.kernel.org/fedora/releases/30/Everything/source/tree/Packages/t/
+# Decided to have back port capability at Tarantool links:
+#   CentOS:
+#     Tarantool: https://www.tarantool.io/en/download/os-installation/rhel-centos/
+#     Changed Vanila -> MCS S3:
+#       centos/7/os/x86_64/Packages -> centos/7/x86_64/Packages
+#       8.1.1911/BaseOS/Source/SPackages -> centos/8.1.1911/SRPMS/Packages
+#   Fedora:
+#     Tarantool: https://www.tarantool.io/en/download/os-installation/fedora/
+#     Changed Vanila -> MCS S3:
+#       fedora/releases/30/Everything/x86_64/os/Packages -> fedora/30/x86_64/Packages
+#       fedora/releases/30/Everything/source/tree/Packages -> fedora/30/SRPMS/Packages
 function pack_rpm {
-    if ! ls $repo/*.rpm >/dev/null ; then
-        echo "ERROR: Current '$repo' path doesn't have RPM packages in path"
-        usage
-        exit 1
-    fi
+    pack_subdir=$1
+    pack_patterns=$2
+
+    for pack_pattern in $pack_patterns ; do
+        if ! ls $repo/$pack_pattern >/dev/null ; then
+            if [ "$pack_pattern" == "*.noarch.rpm" ]; then
+                pack_patterns=$(echo "$pack_patterns" | sed "s#$pack_pattern##g")
+                continue
+            fi
+            echo "ERROR: Current '$repo' path doesn't have $pack_pattern packages in path"
+            usage
+            exit 1
+        fi
+    done
 
     # prepare the workspace
     prepare_ws ${os}_${option_dist}
 
     # copy the needed package binaries to the workspace
-    cp $repo/*.rpm $ws/.
+    for pack_pattern in $pack_patterns ; do
+        cp $repo/$pack_pattern $ws/.
+    done
 
     pushd $ws
 
     # set the paths
-    if [ "$os" == "el" ]; then
-        repopath=$option_dist/os/x86_64
-        rpmpath=Packages
-    elif [ "$os" == "fedora" ]; then
-        repopath=releases/$option_dist/Everything/x86_64/os
-        rpmpath=Packages/$proddir
+    repopath=$option_dist/$pack_subdir
+    rpmpath=Packages
+    if [ "$os" == "fedora" ]; then
+        rpmpath=$rpmpath/$proddir
     fi
     packpath=$repopath/$rpmpath
 
     # prepare local repository with packages
     $mk_dir $packpath
-    mv *.rpm $packpath/.
+    for pack_pattern in $pack_patterns ; do
+        mv $pack_pattern $packpath/.
+    done
     cd $repopath
 
     # copy the current metadata files from S3
@@ -547,8 +573,10 @@ EOF
     gpg --detach-sign --armor repodata/repomd.xml
 
     # copy the packages to S3
-    for file in $rpmpath/*.rpm ; do
-        $aws_cp_public $file "$bucket_path/$repopath/$file"
+    for pack_pattern in $pack_patterns ; do
+        for file in $rpmpath/$pack_pattern ; do
+            $aws_cp_public $file "$bucket_path/$repopath/$file"
+        done
     done
 
     # update the metadata at the S3
@@ -563,7 +591,8 @@ EOF
 if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then
     pack_deb
 elif [ "$os" == "el" -o "$os" == "fedora" ]; then
-    pack_rpm
+    pack_rpm x86_64 "*.x86_64.rpm *.noarch.rpm"
+    pack_rpm SRPMS "*.src.rpm"
 else
     echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"
     usage
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

[Oleg Piskunov]

[-- Attachment #1: Type: text/plain, Size: 4986 bytes --]


Sasha, could you please remove comments about Vanilla packages structure.
For the rest - LGTM
 
Regards,
Oleg
  
>Вторник, 18 февраля 2020, 10:47 +03:00 от Alexander V. Tikhonov <avtikhon@tarantool.org>:
> 
>Changed the Vanilla paths of the packages in S3 to
>the Tarantool documented on its web site, to give
>the ability to Tarantool users to continue use it
>w/o changes.
>
>Also moved the RPM sources packages to the standalone
>path.
>
>Follows up #3380
>---
>
>Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-rpm-packages-paths-full-ci
>Issue:  https://github.com/tarantool/tarantool/issues/3380
>
> tools/update_repo.sh | 73 +++++++++++++++++++++++++++++++-------------
> 1 file changed, 51 insertions(+), 22 deletions(-)
>
>diff --git a/tools/update_repo.sh b/tools/update_repo.sh
>index 65a977187..ec6146514 100755
>--- a/tools/update_repo.sh
>+++ b/tools/update_repo.sh
>@@ -405,42 +405,68 @@ EOF
> }
> 
> # The 'pack_rpm' function especialy created for RPM packages. It works
>-# with RPM packing OS like Centos, Fedora. It is based on globaly known
>+# with RPM packing OS like CentOS, Fedora. It is based on globaly known
> # tool 'createrepo' from:
>-#  https://linux.die.net/man/8/createrepo
>+#  https://linux.die.net/man/8/createrepo
> # This tool works with single distribution of the given OS.
>-# Result of the routine is the rpm package for YUM repository with
>-# file structure equal to the Centos/Fedora:
>-#  http://mirror.centos.org/centos/7/os/x86_64/Packages/
>-#  http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
>+#
>+# Vanilla RPM packages structure for YUM repositories looks like:
>+# CentOS:
>+# Binaries:  http://mirror.centos.org/centos/7/os/x86_64/Packages/
>+# Sources:  http://vault.centos.org/8.1.1911/BaseOS/Source/SPackages/
>+# Fedora:
>+# Binaries:  http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
>+# Sources:  http://mirrors.kernel.org/fedora/releases/30/Everything/source/tree/Packages/t/
>+# Decided to have back port capability at Tarantool links:
>+# CentOS:
>+# Tarantool:  https://www.tarantool.io/en/download/os-installation/rhel-centos/
>+# Changed Vanila -> MCS S3:
>+# centos/7/os/x86_64/Packages -> centos/7/x86_64/Packages
>+# 8.1.1911/BaseOS/Source/SPackages -> centos/8.1.1911/SRPMS/Packages
>+# Fedora:
>+# Tarantool:  https://www.tarantool.io/en/download/os-installation/fedora/
>+# Changed Vanila -> MCS S3:
>+# fedora/releases/30/Everything/x86_64/os/Packages -> fedora/30/x86_64/Packages
>+# fedora/releases/30/Everything/source/tree/Packages -> fedora/30/SRPMS/Packages
> function pack_rpm {
>- if ! ls $repo/*.rpm >/dev/null ; then
>- echo "ERROR: Current '$repo' path doesn't have RPM packages in path"
>- usage
>- exit 1
>- fi
>+ pack_subdir=$1
>+ pack_patterns=$2
>+
>+ for pack_pattern in $pack_patterns ; do
>+ if ! ls $repo/$pack_pattern >/dev/null ; then
>+ if [ "$pack_pattern" == "*.noarch.rpm" ]; then
>+ pack_patterns=$(echo "$pack_patterns" | sed "s#$pack_pattern##g")
>+ continue
>+ fi
>+ echo "ERROR: Current '$repo' path doesn't have $pack_pattern packages in path"
>+ usage
>+ exit 1
>+ fi
>+ done
> 
>     # prepare the workspace
>     prepare_ws ${os}_${option_dist}
> 
>     # copy the needed package binaries to the workspace
>- cp $repo/*.rpm $ws/.
>+ for pack_pattern in $pack_patterns ; do
>+ cp $repo/$pack_pattern $ws/.
>+ done
> 
>     pushd $ws
> 
>     # set the paths
>- if [ "$os" == "el" ]; then
>- repopath=$option_dist/os/x86_64
>- rpmpath=Packages
>- elif [ "$os" == "fedora" ]; then
>- repopath=releases/$option_dist/Everything/x86_64/os
>- rpmpath=Packages/$proddir
>+ repopath=$option_dist/$pack_subdir
>+ rpmpath=Packages
>+ if [ "$os" == "fedora" ]; then
>+ rpmpath=$rpmpath/$proddir
>     fi
>     packpath=$repopath/$rpmpath
> 
>     # prepare local repository with packages
>     $mk_dir $packpath
>- mv *.rpm $packpath/.
>+ for pack_pattern in $pack_patterns ; do
>+ mv $pack_pattern $packpath/.
>+ done
>     cd $repopath
> 
>     # copy the current metadata files from S3
>@@ -547,8 +573,10 @@ EOF
>     gpg --detach-sign --armor repodata/repomd.xml
> 
>     # copy the packages to S3
>- for file in $rpmpath/*.rpm ; do
>- $aws_cp_public $file "$bucket_path/$repopath/$file"
>+ for pack_pattern in $pack_patterns ; do
>+ for file in $rpmpath/$pack_pattern ; do
>+ $aws_cp_public $file "$bucket_path/$repopath/$file"
>+ done
>     done
> 
>     # update the metadata at the S3
>@@ -563,7 +591,8 @@ EOF
> if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then
>     pack_deb
> elif [ "$os" == "el" -o "$os" == "fedora" ]; then
>- pack_rpm
>+ pack_rpm x86_64 "*.x86_64.rpm *.noarch.rpm"
>+ pack_rpm SRPMS "*.src.rpm"
> else
>     echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"
>     usage
>--
>2.17.1
>  
 
 
--
Oleg Piskunov
 

[-- Attachment #2: Type: text/html, Size: 7295 bytes --]

Re: [Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

[Alexander Turenko]

> Make S3 paths the same to Tarantool documented

Please, add a prefix: gitlab-ci, infra or so. Before we use 'gitlab-ci'.

'S3 paths' does not say anything about change you doing. Is it about
tarballs? Or packages? Or some other part of infrastructure?

> Changed the Vanilla paths of the packages in S3 to
> the Tarantool documented on its web site, to give
> the ability to Tarantool users to continue use it
> w/o changes.

Here you introduce the term 'the Vanilla paths' (which is used as
'Vanila' below), but it is neither commonly known nor explained. In fact
you meant base URL of main CentOS/Fedora repository. Since we providing
our own repository it obviously will have its own base URL.

> 
> Also moved the RPM sources packages to the standalone
> path.
> 
> Follows up #3380

Maybe like so:

 | gitlab-ci: adjust base URL of RPM/Deb repositories
 |
 | Our S3 based repositories now reflect packagecloud.io repositories
 | structure.
 |
 | It will allow us to migrate from packagecloud.io w/o much complicating
 | redirection rules on a web server serving download.tarantool.org.
 |
 | Deploy source packages (*.src.rpm) into separate 'SRPM' repository
 | like packagecloud.io does.
 |
 | <everything else you change>
 | 
 | Follows up #3380

See, I didn't introduced any new term, but described why and how the
behaviour of the script was changed.

> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-rpm-packages-paths-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/3380
> 
>  tools/update_repo.sh | 73 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/update_repo.sh b/tools/update_repo.sh
> index 65a977187..ec6146514 100755
> --- a/tools/update_repo.sh
> +++ b/tools/update_repo.sh
> @@ -405,42 +405,68 @@ EOF
>  }
>  
>  # The 'pack_rpm' function especialy created for RPM packages. It works
> -# with RPM packing OS like Centos, Fedora. It is based on globaly known
> +# with RPM packing OS like CentOS, Fedora. It is based on globaly known

If you want to fix typos, then fix also 'globaly'.

>  # tool 'createrepo' from:
> -#     https://linux.die.net/man/8/createrepo
> +#   https://linux.die.net/man/8/createrepo
>  # This tool works with single distribution of the given OS.
> -# Result of the routine is the rpm package for YUM repository with
> -# file structure equal to the Centos/Fedora:
> -#     http://mirror.centos.org/centos/7/os/x86_64/Packages/
> -#     http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
> +#
> +# Vanilla RPM packages structure for YUM repositories looks like:
> +#   CentOS:
> +#     Binaries: http://mirror.centos.org/centos/7/os/x86_64/Packages/
> +#     Sources: http://vault.centos.org/8.1.1911/BaseOS/Source/SPackages/
> +#  Fedora:
> +#     Binaries: http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
> +#     Sources: http://mirrors.kernel.org/fedora/releases/30/Everything/source/tree/Packages/t/
> +# Decided to have back port capability at Tarantool links:
> +#   CentOS:
> +#     Tarantool: https://www.tarantool.io/en/download/os-installation/rhel-centos/
> +#     Changed Vanila -> MCS S3:
> +#       centos/7/os/x86_64/Packages -> centos/7/x86_64/Packages
> +#       8.1.1911/BaseOS/Source/SPackages -> centos/8.1.1911/SRPMS/Packages
> +#   Fedora:
> +#     Tarantool: https://www.tarantool.io/en/download/os-installation/fedora/
> +#     Changed Vanila -> MCS S3:
> +#       fedora/releases/30/Everything/x86_64/os/Packages -> fedora/30/x86_64/Packages
> +#       fedora/releases/30/Everything/source/tree/Packages -> fedora/30/SRPMS/Packages

If we agree to don't bother anybody to imagine what 'Vanilla/Vanila' is
(and what is meant under 'MCS S3', which is just file storage), then
let's just show base URLs.

However it is okay to give links to intructions, which will allow to
verify that the URLs are right.

>  function pack_rpm {
> -    if ! ls $repo/*.rpm >/dev/null ; then
> -        echo "ERROR: Current '$repo' path doesn't have RPM packages in path"
> -        usage
> -        exit 1
> -    fi
> +    pack_subdir=$1
> +    pack_patterns=$2
> +
> +    for pack_pattern in $pack_patterns ; do
> +        if ! ls $repo/$pack_pattern >/dev/null ; then
> +            if [ "$pack_pattern" == "*.noarch.rpm" ]; then
> +                pack_patterns=$(echo "$pack_patterns" | sed "s#$pack_pattern##g")
> +                continue
> +            fi
> +            echo "ERROR: Current '$repo' path doesn't have $pack_pattern packages in path"
> +            usage
> +            exit 1
> +        fi
> +    done

This logic is flawed (say, it will not work with vshard). Okay, let's
give an error if there are no files, but it should not require files to
be present for a certain architecture.

I would collect files using globs. Then check whether we found at least
one. Then use this list of files. A kind of this:

 | # Collect RPM files matching provided glob.
 | rpms=''
 | for glog in ${globs}; do
 |     for file in ${repo}/${glob}; do
 |         rpms="${rpms} ${file}"
 |     done
 | done
 |
 | # Verify that at least one RPM file is found.
 | if [ -z "${rpms}" ]; then
 |     <...>
 | fi

But feel free to pick up any way, I'll not push you for mine one.

>  
>      # prepare the workspace
>      prepare_ws ${os}_${option_dist}
>  
>      # copy the needed package binaries to the workspace
> -    cp $repo/*.rpm $ws/.
> +    for pack_pattern in $pack_patterns ; do
> +        cp $repo/$pack_pattern $ws/.
> +    done

After the change above it can be simplified to:

 | cp ${rpms} "$ws/"

Note: no quotes around ${rpms} to use file glob expansion.

>  
>      pushd $ws
>  
>      # set the paths
> -    if [ "$os" == "el" ]; then
> -        repopath=$option_dist/os/x86_64
> -        rpmpath=Packages
> -    elif [ "$os" == "fedora" ]; then
> -        repopath=releases/$option_dist/Everything/x86_64/os
> -        rpmpath=Packages/$proddir
> +    repopath=$option_dist/$pack_subdir
> +    rpmpath=Packages
> +    if [ "$os" == "fedora" ]; then
> +        rpmpath=$rpmpath/$proddir
>      fi

I don't think that we need different layout for CentOS and Fedora.
Packagecloud.io has the same flat layout for both.

>      packpath=$repopath/$rpmpath
>  
>      # prepare local repository with packages
>      $mk_dir $packpath
> -    mv *.rpm $packpath/.
> +    for pack_pattern in $pack_patterns ; do
> +        mv $pack_pattern $packpath/.
> +    done

Same as above.

>      cd $repopath
>  
>      # copy the current metadata files from S3
> @@ -547,8 +573,10 @@ EOF
>      gpg --detach-sign --armor repodata/repomd.xml
>  
>      # copy the packages to S3
> -    for file in $rpmpath/*.rpm ; do
> -        $aws_cp_public $file "$bucket_path/$repopath/$file"
> +    for pack_pattern in $pack_patterns ; do
> +        for file in $rpmpath/$pack_pattern ; do
> +            $aws_cp_public $file "$bucket_path/$repopath/$file"
> +        done

Same as above.

>      done
>  
>      # update the metadata at the S3
> @@ -563,7 +591,8 @@ EOF
>  if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then
>      pack_deb
>  elif [ "$os" == "el" -o "$os" == "fedora" ]; then
> -    pack_rpm
> +    pack_rpm x86_64 "*.x86_64.rpm *.noarch.rpm"
> +    pack_rpm SRPMS "*.src.rpm"

It deserves a comment.

>  else
>      echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"
>      usage
> -- 
> 2.17.1
> 

And while we're here: I would extract explicit keyid to an environment
variable (it is a least in 'SignWith:').

I would also use `gpg -u <keyid> <...>` when signing, because it is
possible that there is more then one key for a user/machine.

Re: [Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented

[Alexander Tikhonov]

[-- Attachment #1: Type: text/plain, Size: 8827 bytes --]

Alexander, thanks a lot for a deep review, I've answered the questions below.


>Вторник, 18 февраля 2020, 20:08 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>> Make S3 paths the same to Tarantool documented
>
>Please, add a prefix: gitlab-ci, infra or so. Before we use 'gitlab-ci'.
>
>'S3 paths' does not say anything about change you doing. Is it about
>tarballs? Or packages? Or some other part of infrastructure?
>
>> Changed the Vanilla paths of the packages in S3 to
>> the Tarantool documented on its web site, to give
>> the ability to Tarantool users to continue use it
>> w/o changes.
>
>Here you introduce the term 'the Vanilla paths' (which is used as
>'Vanila' below), but it is neither commonly known nor explained. In fact
>you meant base URL of main CentOS/Fedora repository. Since we providing
>our own repository it obviously will have its own base URL.
>
>> 
>> Also moved the RPM sources packages to the standalone
>> path.
>> 
>> Follows up #3380
>
>Maybe like so:
>
> | gitlab-ci: adjust base URL of RPM/Deb repositories
> |
> | Our S3 based repositories now reflect packagecloud.io repositories
> | structure.
> |
> | It will allow us to migrate from packagecloud.io w/o much complicating
> | redirection rules on a web server serving download.tarantool.org.
> |
> | Deploy source packages (*.src.rpm) into separate 'SRPM' repository
> | like packagecloud.io does.
> |
> | <everything else you change>
> | 
> | Follows up #3380
>
>See, I didn't introduced any new term, but described why and how the
>behaviour of the script was changed.
Thank for the complete commit message, I've used it completely and
added comment about changed signing key.
>
>> ---
>> 
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-3380-rpm-packages-paths-full-ci
>> Issue:  https://github.com/tarantool/tarantool/issues/3380
>> 
>>  tools/update_repo.sh | 73 +++++++++++++++++++++++++++++++-------------
>>  1 file changed, 51 insertions(+), 22 deletions(-)
>> 
>> diff --git a/tools/update_repo.sh b/tools/update_repo.sh
>> index 65a977187..ec6146514 100755
>> --- a/tools/update_repo.sh
>> +++ b/tools/update_repo.sh
>> @@ -405,42 +405,68 @@ EOF
>>  }
>> 
>>  # The 'pack_rpm' function especialy created for RPM packages. It works
>> -# with RPM packing OS like Centos, Fedora. It is based on globaly known
>> +# with RPM packing OS like CentOS, Fedora. It is based on globaly known
>
>If you want to fix typos, then fix also 'globaly'. 
Right, corrected.
>
>
>>  # tool 'createrepo' from:
>> -#  https://linux.die.net/man/8/createrepo
>> +#  https://linux.die.net/man/8/createrepo
>>  # This tool works with single distribution of the given OS.
>> -# Result of the routine is the rpm package for YUM repository with
>> -# file structure equal to the Centos/Fedora:
>> -#  http://mirror.centos.org/centos/7/os/x86_64/Packages/
>> -#  http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
>> +#
>> +# Vanilla RPM packages structure for YUM repositories looks like:
>> +#   CentOS:
>> +#     Binaries:  http://mirror.centos.org/centos/7/os/x86_64/Packages/
>> +#     Sources:  http://vault.centos.org/8.1.1911/BaseOS/Source/SPackages/
>> +#  Fedora:
>> +#     Binaries:  http://mirrors.kernel.org/fedora/releases/30/Everything/x86_64/os/Packages/t/
>> +#     Sources:  http://mirrors.kernel.org/fedora/releases/30/Everything/source/tree/Packages/t/
>> +# Decided to have back port capability at Tarantool links:
>> +#   CentOS:
>> +#     Tarantool:  https://www.tarantool.io/en/download/os-installation/rhel-centos/
>> +#     Changed Vanila -> MCS S3:
>> +#       centos/7/os/x86_64/Packages -> centos/7/x86_64/Packages
>> +#       8.1.1911/BaseOS/Source/SPackages -> centos/8.1.1911/SRPMS/Packages
>> +#   Fedora:
>> +#     Tarantool:  https://www.tarantool.io/en/download/os-installation/fedora/
>> +#     Changed Vanila -> MCS S3:
>> +#       fedora/releases/30/Everything/x86_64/os/Packages -> fedora/30/x86_64/Packages
>> +#       fedora/releases/30/Everything/source/tree/Packages -> fedora/30/SRPMS/Packages
>
>If we agree to don't bother anybody to imagine what 'Vanilla/Vanila' is
>(and what is meant under 'MCS S3', which is just file storage), then
>let's just show base URLs.
>
>However it is okay to give links to intructions, which will allow to
>verify that the URLs are right. 
Ok, I've removed all links and information about OS's structures and wrote
links on the Tarantool instructions.
>
>
>>  function pack_rpm {
>> -    if ! ls $repo/*.rpm >/dev/null ; then
>> -        echo "ERROR: Current '$repo' path doesn't have RPM packages in path"
>> -        usage
>> -        exit 1
>> -    fi
>> +    pack_subdir=$1
>> +    pack_patterns=$2
>> +
>> +    for pack_pattern in $pack_patterns ; do
>> +        if ! ls $repo/$pack_pattern >/dev/null ; then
>> +            if [ "$pack_pattern" == "*.noarch.rpm" ]; then
>> +                pack_patterns=$(echo "$pack_patterns" | sed "s#$pack_pattern##g")
>> +                continue
>> +            fi
>> +            echo "ERROR: Current '$repo' path doesn't have $pack_pattern packages in path"
>> +            usage
>> +            exit 1
>> +        fi
>> +    done
>
>This logic is flawed (say, it will not work with vshard). Okay, let's
>give an error if there are no files, but it should not require files to
>be present for a certain architecture. 
Ok, I see, I've fixed it for all the patterns and set checker that some
packages found to be registered.
>
>
>I would collect files using globs. Then check whether we found at least
>one. Then use this list of files. A kind of this:
>
> | # Collect RPM files matching provided glob.
> | rpms=''
> | for glog in ${globs}; do
> |     for file in ${repo}/${glob}; do
> |         rpms="${rpms} ${file}"
> |     done
> | done
> |
> | # Verify that at least one RPM file is found.
> | if [ -z "${rpms}" ]; then
> |     <...>
> | fi
>
>But feel free to pick up any way, I'll not push you for mine one. 
Ok, sure, I've integrated your way with some minor corrections.
>
>
>> 
>>      # prepare the workspace
>>      prepare_ws ${os}_${option_dist}
>> 
>>      # copy the needed package binaries to the workspace
>> -    cp $repo/*.rpm $ws/.
>> +    for pack_pattern in $pack_patterns ; do
>> +        cp $repo/$pack_pattern $ws/.
>> +    done
>
>After the change above it can be simplified to:
>
> | cp ${rpms} "$ws/"
>
>Note: no quotes around ${rpms} to use file glob expansion.
Ok, done.
>
>> 
>>      pushd $ws
>> 
>>      # set the paths
>> -    if [ "$os" == "el" ]; then
>> -        repopath=$option_dist/os/x86_64
>> -        rpmpath=Packages
>> -    elif [ "$os" == "fedora" ]; then
>> -        repopath=releases/$option_dist/Everything/x86_64/os
>> -        rpmpath=Packages/$proddir
>> +    repopath=$option_dist/$pack_subdir
>> +    rpmpath=Packages
>> +    if [ "$os" == "fedora" ]; then
>> +        rpmpath=$rpmpath/$proddir
>>      fi
>
>I don't think that we need different layout for CentOS and Fedora.
>Packagecloud.io has the same flat layout for both. 
Ok, I've removed extra subdirs and set common paths setup for
Fedora and CentOS.
>
>
>>      packpath=$repopath/$rpmpath
>> 
>>      # prepare local repository with packages
>>      $mk_dir $packpath
>> -    mv *.rpm $packpath/.
>> +    for pack_pattern in $pack_patterns ; do
>> +        mv $pack_pattern $packpath/.
>> +    done
>
>Same as above. 
Ok, done.
>
>
>>      cd $repopath
>> 
>>      # copy the current metadata files from S3
>> @@ -547,8 +573,10 @@ EOF
>>      gpg --detach-sign --armor repodata/repomd.xml
>> 
>>      # copy the packages to S3
>> -    for file in $rpmpath/*.rpm ; do
>> -        $aws_cp_public $file "$bucket_path/$repopath/$file"
>> +    for pack_pattern in $pack_patterns ; do
>> +        for file in $rpmpath/$pack_pattern ; do
>> +            $aws_cp_public $file "$bucket_path/$repopath/$file"
>> +        done
>
>Same as above. 
Ok, done.
>
>
>>      done
>> 
>>      # update the metadata at the S3
>> @@ -563,7 +591,8 @@ EOF
>>  if [ "$os" == "ubuntu" -o "$os" == "debian" ]; then
>>      pack_deb
>>  elif [ "$os" == "el" -o "$os" == "fedora" ]; then
>> -    pack_rpm
>> +    pack_rpm x86_64 "*.x86_64.rpm *.noarch.rpm"
>> +    pack_rpm SRPMS "*.src.rpm"
>
>It deserves a comment. 
Added the comment.
>
>
>>  else
>>      echo "USAGE: given OS '$os' is not supported, use any single from the list: $alloss"
>>      usage
>> -- 
>> 2.17.1
>> 
>
>And while we're here: I would extract explicit keyid to an environment
>variable (it is a least in 'SignWith:'). 
Ok, I've moved it to gitlab-ci variable "GPG_SIGN_KEY"
>
>
>I would also use `gpg -u <keyid> <...>` when signing, because it is
>possible that there is more then one key for a user/machine. 
Ok, sure, thanks.
>


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 14232 bytes --]