diff mbox series

ci: fix GitHub workflow when on a tagged revision

Message ID pull.619.git.1587748660308.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series ci: fix GitHub workflow when on a tagged revision | expand

Commit Message

John Passaro via GitGitGadget April 24, 2020, 5:17 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

When `master` is tagged, and then both `master` and the tag are pushed,
Travis CI will happily build both. That is a waste of energy, which is
why we skip the build for `master` in that case.

However, our GitHub workflow does not trigger on tags, therefore, this
logic results in a missing build for that revision.

Even worse: the run would _fail_ because we would skip the Windows
build, there are no artifacts to publish, and therefore no artifacts to
download in the Windows test jobs.

Let's just change the GitHub workflow to skip the logic to skip
revisions that are tagged.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    Do not skip tagged revisions in the GitHub workflow runs
    
    I noticed a failure in the run for Git for Windows v2.26.2 which was
    caused by this.
    
    This patch is based on dd/ci-swap-azure-pipelines-with-github-actions.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-619%2Fdscho%2Fgithub-workflows-and-tags-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-619/dscho/github-workflows-and-tags-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/619

 ci/lib.sh | 2 ++
 1 file changed, 2 insertions(+)


base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c

Comments

Junio C Hamano April 24, 2020, 8:50 p.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> However, our GitHub workflow does not trigger on tags, therefore, this
> logic results in a missing build for that revision.

Thanks for noticing.  The arrangement we had, which essentially said
"we know we will always build tagged version, so a push of a branch
that happens to have a tagged version at the tip can be ignored",
served us wonderfully.  Now the maintainers of projects (not just
this one) are forbidden from tagging the tip of master, queue
something else on top and push the result out, as it won't build or
test the tagged version, which is a bit sad.

>  ci/lib.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index dac36886e37..f151e2f0ecb 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -1,6 +1,7 @@
>  # Library of functions shared by all CI scripts
>  
>  skip_branch_tip_with_tag () {
> +	test -z "$DONT_SKIP_TAGS" || return 0
>  	# Sometimes, a branch is pushed at the same time the tag that points
>  	# at the same commit as the tip of the branch is pushed, and building
>  	# both at the same time is a waste.
> @@ -151,6 +152,7 @@ then
>  	CC="${CC:-gcc}"
>  
>  	cache_dir="$HOME/none"
> +	DONT_SKIP_TAGS=t

Quite straight-forward.  Thanks, will queue.

>  	export GIT_PROVE_OPTS="--timer --jobs 10"
>  	export GIT_TEST_OPTS="--verbose-log -x"
>
> base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c
Johannes Schindelin April 24, 2020, 9:12 p.m. UTC | #2
Hi Junio,

On Fri, 24 Apr 2020, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > However, our GitHub workflow does not trigger on tags, therefore, this
> > logic results in a missing build for that revision.
>
> Thanks for noticing.  The arrangement we had, which essentially said
> "we know we will always build tagged version, so a push of a branch
> that happens to have a tagged version at the tip can be ignored",
> served us wonderfully.  Now the maintainers of projects (not just
> this one) are forbidden from tagging the tip of master, queue
> something else on top and push the result out, as it won't build or
> test the tagged version, which is a bit sad.

Nobody forbids this... ;-)

And I was wrong, our current GitHub workflow _is_ triggered by tags. See
e.g. https://github.com/git-for-windows/git/actions/runs/83103626 which
was triggered by v2.26.2.windows.1.

However, you also see that it failed due to the reason I described in the
commit message.

I guess that we should either go with this patch (which would trigger full
runs also on tags), or we could alternatively trigger only on branch
pushes (and not tag pushes)?

Ciao,
Dscho

>
> >  ci/lib.sh | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index dac36886e37..f151e2f0ecb 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -1,6 +1,7 @@
> >  # Library of functions shared by all CI scripts
> >
> >  skip_branch_tip_with_tag () {
> > +	test -z "$DONT_SKIP_TAGS" || return 0
> >  	# Sometimes, a branch is pushed at the same time the tag that points
> >  	# at the same commit as the tip of the branch is pushed, and building
> >  	# both at the same time is a waste.
> > @@ -151,6 +152,7 @@ then
> >  	CC="${CC:-gcc}"
> >
> >  	cache_dir="$HOME/none"
> > +	DONT_SKIP_TAGS=t
>
> Quite straight-forward.  Thanks, will queue.
>
> >  	export GIT_PROVE_OPTS="--timer --jobs 10"
> >  	export GIT_TEST_OPTS="--verbose-log -x"
> >
> > base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c
>
>
Junio C Hamano April 24, 2020, 9:24 p.m. UTC | #3
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Fri, 24 Apr 2020, Junio C Hamano wrote:
>
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> writes:
>>
>> > However, our GitHub workflow does not trigger on tags, therefore, this
>> > logic results in a missing build for that revision.
>>
>> Thanks for noticing.  The arrangement we had, which essentially said
>> "we know we will always build tagged version, so a push of a branch
>> that happens to have a tagged version at the tip can be ignored",
>> served us wonderfully.  Now the maintainers of projects (not just
>> this one) are forbidden from tagging the tip of master, queue
>> something else on top and push the result out, as it won't build or
>> test the tagged version, which is a bit sad.
>
> Nobody forbids this... ;-)
>
> And I was wrong, our current GitHub workflow _is_ triggered by tags. See
> e.g. https://github.com/git-for-windows/git/actions/runs/83103626 which
> was triggered by v2.26.2.windows.1.
>
> However, you also see that it failed due to the reason I described in the
> commit message.
>
> I guess that we should either go with this patch (which would trigger full
> runs also on tags), or we could alternatively trigger only on branch
> pushes (and not tag pushes)?

Hmph, even if we were to go with this patch (which should be safer),
we'd definitely need an updated log message, then.

Thanks for double-checking.
diff mbox series

Patch

diff --git a/ci/lib.sh b/ci/lib.sh
index dac36886e37..f151e2f0ecb 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -1,6 +1,7 @@ 
 # Library of functions shared by all CI scripts
 
 skip_branch_tip_with_tag () {
+	test -z "$DONT_SKIP_TAGS" || return 0
 	# Sometimes, a branch is pushed at the same time the tag that points
 	# at the same commit as the tip of the branch is pushed, and building
 	# both at the same time is a waste.
@@ -151,6 +152,7 @@  then
 	CC="${CC:-gcc}"
 
 	cache_dir="$HOME/none"
+	DONT_SKIP_TAGS=t
 
 	export GIT_PROVE_OPTS="--timer --jobs 10"
 	export GIT_TEST_OPTS="--verbose-log -x"