From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 51C17469719 for ; Wed, 14 Oct 2020 05:42:40 +0300 (MSK) Date: Wed, 14 Oct 2020 05:42:58 +0300 From: Alexander Turenko Message-ID: <20201014024258.pfvyh6wd6ezx5mrr@tkn_work_nb> References: <029eade3955655f76ee90125ba61726780307a82.1602198773.git.avtikhon@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <029eade3955655f76ee90125ba61726780307a82.1602198773.git.avtikhon@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1] packages: tagged commit force push to live repo List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: tarantool-patches@dev.tarantool.org In brief: The commit fixes the problem with pushing to the release repositories (no x.y.z.0 package + red CI), but introduces the problem with pushing to the live repositories (no x.y.(z-1).rrr package + red CI). I would say, it decreases urgency of the problem. However: 1. It is not good to have red CI due to infrastructure problems in the day of a release. 2. Races still affect a set of packages we deploy, that's disgusting. 3. One more commit in `git log` that I unable to undestand without diving into a lot of subtle details. There is proposal how to overcome all problems and I don't see downsides or complexities. I already described it voicely and written in the issue. It is as simple as: | [ -z "${CI_COMMIT_TAG:-}" ] && \ | git tag -d "$(git tag --points-at HEAD)" 2>/dev/null || true I would let Kirill to decide how much known to be problematic partial fixes we should create before actually fix the root problem (as shown above). Let's dive into details. ---- `git tag --points-at` is supported since git 1.7.10 (8 years ago). Aside of `git tag --points-at` there is also `git describe --exact-match` since git 1.5.5 (10 years ago). It's commit message have suggestion how to do the same on older versions ([1]). My previous suggestion about `git name-rev` was just mistake. I'm sorry. Now I found that it does not guarantee that it'll point to a tag that points to a given commit. It may be some other tag: | $ git name-rev --name-only --tags --no-undefined c2d8c03ee | 2.6.0~2 | | $ git tag --points-at c2d8c03ee | cat | 2.5.1 | | $ git describe --exact-match c2d8c03ee | 2.5.1 Okay, if you're bother about the next CI jobs, `git fetch` will fetch tags by default: I verified it. You may add explicit --tags (-t). You may also want to add --prune-tags in addition to --prune to get rid of stale tags if we'll step into the undesired situation when a tag is changed somehow. See details below. [1]: https://github.com/git/git/commit/2c33f7575452f53382dcf77fdc88a2ea5d46f09d WBR, Alexander Turenko. > packages: tagged commit force push to live repo There are no commits with such prefix, but there are 24 commits made during this year with 'gitlab-ci' prefix, which fit good here. > > If the commit tagged, then push packages to 'live' repository with force > push '-f -s' flags to be sure that if occasionaly the commit before current > commit pushed packages with the current branch naming then the current push > won't fail on it and will push the packages to the release repository after. > > Closes #3745 We discussed it as the temporary workaround for the problem. Let's be realistic, will you return back here, if we'll close the issue? > --- > > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-3745-force-push-live > Issue: https://github.com/tarantool/tarantool/issues/3745 > > .gitlab.mk | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/.gitlab.mk b/.gitlab.mk > index e1a83ac07..808288f23 100644 > --- a/.gitlab.mk > +++ b/.gitlab.mk > @@ -120,11 +120,18 @@ package: deploy_prepare > > deploy: package > echo ${GPG_SECRET_KEY} | base64 -d | gpg --batch --import || true > - ./tools/update_repo.sh -o=${OS} -d=${DIST} \ > - -b="${LIVE_REPO_S3_DIR}/${BUCKET}" build > + # If the commit tagged, then push packages to 'live' repository with force > + # push '-f -s' flags to be sure that if occasionaly the commit before current > + # commit pushed packages with the current branch naming then the current push > + # won't fail on it and will push the packages to the release repository after. -s (skip error) is to pass over a clash, -f (force) is to replace a package in the case. Those options are kinda opposite, right? I just checked the code: -s does not change anything when -f is present. So it looks for me as a good way to create a confusion for no reason at all. > if [ "${CI_COMMIT_TAG}" != "" ]; then \ > + ./tools/update_repo.sh -o=${OS} -d=${DIST} \ > + -b="${LIVE_REPO_S3_DIR}/${BUCKET}" -f -s build ; \ > ./tools/update_repo.sh -o=${OS} -d=${DIST} \ > -b="${RELEASE_REPO_S3_DIR}/${BUCKET}" build ; \ > + else \ > + ./tools/update_repo.sh -o=${OS} -d=${DIST} \ > + -b="${LIVE_REPO_S3_DIR}/${BUCKET}" build ; \ > fi How it works now (before your patch): | | branch push job | tag job | | ------------- | --------------- | ------------------------- | | x.y.(z-1).rrr | push live | not possible | | x.y.z.0 | push live [^1] | push live && release [^1] | [^1]: Those jobs will obviously clash. Everything depends on the order: as they will be scheduled. Possible variants: Scenario 1: | (branch push job) push live x.y.z.0 | (tag job) fail on push live x.y.z.0 | => no release x.y.z.0 | => red CI Scenario 2: | (tag job) push live x.y.z.0 | push release x.y.z.0 | (branch push job) fail on push live x.y.z.0 | => red CI -------- As it would work after the full resolution that I proposed (drop a current tag if "${CI_COMMIT_TAG}" is empty or not defined): | | branch push job | tag job | | ------------- | --------------- | ------------------------- | | x.y.(z-1).rrr | push live | not possible | | x.y.z.0 | not possible | push live && push release | -------- The alternative would be detection of a tagged commit on a branch job and skip the job when we found this situation. It'll give the following behaviour: | | branch push job | tag job | | ------------- | --------------- | ------------------------- | | x.y.(z-1).rrr | push live | not possible | | x.y.z.0 | skip [^2] | push live && push release | [^2]: In fact this x.y.z.0 build is some x.y.(z-1).rrr, but it was not lucky and was scheduled after arriving of the x.y.z tag to the mirror. So we'll miss live pushes and, what is weird, randomly. We can get x.y.(z-1).rrr pushed for one distro, but missed for another. This variant is not the best, but at least it will push all packages to the release repositories and will not lead to any extra fails in CI with package name clashes. -------- Now let's look at the behaviour you propose: | branch push job | tag job | | ------------- | --------------- | ------------------------------- | | x.y.(z-1).rrr | push live | not possible | | x.y.z.0 | push live [^3] | push-force live && push release | [^3]: So, we there are two possible situations: Scenario 1: | (branch push job) push live x.y.z.0 | (tab job) push-force live x.y.z.0 | push release x.y.z.0 | => missed x.y.(z-1).rrr, but | everything else is okay Scenario 2: | (tag job) push-force live x.y.z.0 | push release x.y.z.0 | (branch push job) fail on push live x.y.z.0 | => red CI | => missed x.y.(z-1).rrr I doubt GitLab CI provides any implicit guarantees about ordering of jobs. Everything is highly depend on timings, so unlikely someone may say that the scenario 2 is not possible or have a little probability. -------- Regarding `git tag --points-at` vs `git describe --exact-match` vs `git name-rev <...>`. I made some tests: | #!/bin/sh | | set -e | | revisions="" comments="" | revisions="${revisions} e7203e389" comment_0="current master" | revisions="${revisions} d5ec7e948" comment_1="just after 2.6.0" | revisions="${revisions} 47aa4e01e" comment_2="on 2.6.0" | revisions="${revisions} 7bfcf57e4" comment_3="just before 2.6.0 (after 2.5.1)" | revisions="${revisions} c2d8c03ee" comment_4="on 2.5.1" | revisions="${revisions} 807c7fa58" comment_5="just before 2.5.1" | revisions="${revisions} d1647590e" comment_6="2.5.0.82 (between 2.5.0 and 2.5.1)" | revisions="${revisions} ad13b6d57" comment_7="just after 2.5.0" | revisions="${revisions} fef6505c7" comment_8="on 2.5.0" | revisions="${revisions} ca79e1cfb" comment_9="just before 2.5.0" | | i=0 | for rev in ${revisions}; do | # Get a tag. | # | # tag='' | # version="$(git describe --long --always "${rev}")" | # case "${version}" in | # *.*.*-0-g*) | # tag="${version%%-*}" | # ;; | # esac | | # Get a tag (bashism) | # | # tag='' | # version="$(git describe --long --always "${rev}")" | # [[ $version =~ [0-9]+\.[0-9]+\.[0-9]+-0-g ]] && tag="${version%%-*}" | | comment_ref="comment_${i}" | printf '| %-5s | %-5s | %-9s | %-34s |\n' \ | "$(git tag --points-at "${rev}" 2>/dev/null)" \ | "$(git describe --exact-match "${rev}" 2>/dev/null)" \ | "$(git name-rev --name-only --tags --no-undefined "${rev}" 2>/dev/null)" \ | "${!comment_ref}" | let i=i+1 | done It gives the following output: | | | | current master | | | | | just after 2.6.0 | | 2.6.0 | 2.6.0 | 2.6.0^0 | on 2.6.0 | | | | 2.6.0~1 | just before 2.6.0 (after 2.5.1) | | 2.5.1 | 2.5.1 | 2.6.0~2 | on 2.5.1 | | | | 2.6.0~3 | just before 2.5.1 | | | | 2.6.0~199 | 2.5.0.82 (between 2.5.0 and 2.5.1) | | | | 2.6.0~280 | just after 2.5.0 | | 2.5.0 | 2.5.0 | 2.5.0^0 | on 2.5.0 | | | | 2.5.0~1 | just before 2.5.0 |