Re: [PATCH 2/2] travis-ci: record and skip successfully built trees

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Thu, Dec 28, 2017 at 12:16 PM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
>> On 28 Dec 2017, at 00:00, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>>
>> On Wed, Dec 27, 2017 at 8:15 PM, Lars Schneider
>> <larsxschneider@gmail.com> wrote:
>>>
>>>> On 27 Dec 2017, at 17:49, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>>>> +# Skip the build job if the same tree has already been built and tested
>>>> +# successfully before (e.g. because the branch got rebased, changing only
>>>> +# the commit messages).
>>>> +skip_good_tree () {
>>>> +     if ! good_tree_info="$(grep "^$(git rev-parse $TRAVIS_COMMIT^{tree}) " "$good_trees_file")"
>>>> +     then
>>>> +             # haven't seen this tree yet; continue the build job
>>>> +             return
>>>> +     fi
>>>> +
>>>> +     echo "$good_tree_info" | {
>>>> +             read tree prev_good_commit prev_good_job_number prev_good_job_id
>>>> +
>>>> +             if test "$TRAVIS_JOB_ID" =  "$prev_good_job_id"
>>>
>>> Under what circumstances would that be true?
>>
>> When the user hits 'Restart job' on the Travis CI web interface,
>> $TRAVI_JOB_NUMBER and _ID remain the same in the restarted build job as
>> they were in the original.
>> So the condition is true when the user hits 'Restart job' on a build job
>> that was the first to successfully build and test the current tree.
>
> I think I would prefer it if Travis would rerun all jobs if I hit the
> "refresh" button. What is your intention here?

I considered that and don't think it's worth the effort.

First, I think it's a rather rare use case.  I don't know what others
are doing, but I only hit 'Restart job' to restart timeouted OSX build
jobs (and to test this patch :), and that already works with this patch.
I don't really see any reason to restart old successful build jobs,
except perhaps when a new version of one of the dependencies becomes
available (e.g. P4 and Git LFS versions are not hardcoded on OSX, we use
whatever homebrew delivers), to see that an older version still works
with the new dependencies.  Has anyone ever done something like that? :)

Second, we need to know when a build job is run after the user hit
'Restart job'.  Unless I overlooked something, Travis CI doesn't
indicate this.  I'm not sure this is documented explicitly, but it seems
that a restarted build job gets the same $TRAVIS_JOB_{NUMBER,ID}
variables as the original.  We could use this to identify restarted
build jobs, but to do that we would have to save this information at the
end of every successful build, too, and add additional checks to this
function, of course.

>>>> +             then
>>>> +                     cat <<-EOF
>>>> +                     Skipping build job for commit $TRAVIS_COMMIT.
>>>> +                     This commit has already been built and tested successfully by this build job.
>>>> +                     To force a re-build delete the branch's cache and then hit 'Restart job'.
>>>> +                     EOF
>>>> +             else
>>>> +                     cat <<-EOF
>>>> +                     Skipping build job for commit $TRAVIS_COMMIT.
>>>> +                     This commit's tree has already been built and tested successfully in build job $prev_good_job_number for commit $prev_good_commit.
>>>> +                     The log of that build job is available at https://travis-ci.org/$TRAVIS_REPO_SLUG/jobs/$prev_good_job_id
>>>> +                     To force a re-build delete the branch's cache and then hit 'Restart job'.
>>>> +                     EOF
>>>
>>> Maybe add a few newlines before and after EOF to make the text more stand out?
>>> Or print it in a different color? Maybe red?
>>>
>>> See: https://travis-ci.org/szeder/git/jobs/322247836#L622-L625
>>
>> I considered using color for the first line, but then didn't do it,
>> because I didn't want to decide the color :)
>> Anyway, red is the general error/failure color, but this is neither.  It
>> could either be green, to match the color of the build job's result, or
>> something neutral like blue or yellow.
>
> You are right about red. I think I like yellow to express "warning".
> But this is just a nit.

OK, yellow it will be then.


> "skip_branch_tip_with_tag" could print its output yellow, too.
>
> - Lars