From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C4C06469719 for ; Tue, 18 Feb 2020 20:08:30 +0300 (MSK) Date: Tue, 18 Feb 2020 20:08:50 +0300 From: Alexander Turenko Message-ID: <20200218170850.ay2qicnk5kldaenf@tkn_work_nb> References: <36105cdadaa8c8a99da901a4bb0fd0d44a936ae6.1582011996.git.avtikhon@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <36105cdadaa8c8a99da901a4bb0fd0d44a936ae6.1582011996.git.avtikhon@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1] Make S3 paths the same to Tarantool documented List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: Oleg Piskunov , tarantool-patches@dev.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. | | | | 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 <...>` when signing, because it is possible that there is more then one key for a user/machine.