Re: ab/ci-setup-simplify (was Re: What's cooking in git.git (Apr 2022, #05; Mon, 18))

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 21, 2022, 18:36 UTC
Message-ID: <220421.86fsm66zmz.gmgdl@evledraar.gmail.com>
In-Reply-To: <1157a463-f6c6-1df5-59cd-419d73eed1df@gmail.com>

On Tue, Apr 19 2022, Phillip Wood wrote:

Show 25 quoted lines

>> * ab/ci-setup-simplify (2022-04-14) 29 commits
>> [...]
>>   Will merge to 'next'?
>>   source: <cover-v3-00.29-00000000000-20220413T194847Z-avarab@gmail.com>
>
> I haven't had time to read all 31 patches from v4 in detail but I have
> looked at the results in seen.
>
> Looking at seen:ci/install-dependencies.sh the shebang has been
> changed to "#!/bin/sh" but it contains
> "BREW_PACKAGE=${CC_PACKAGE/-/@}" which is a bashism.
>
> Looking at seen:.github/workflows/main.yaml to skip running the tests
> one has to set "skip-tests: no" which is utterly confusing.
>
> From what I saw scanning the patches there seemed to be a lot of
> churn, both of existing code and code that gets added and then
> moved/refactored within the series.
>
> Looking at the output of a recent ci run of seen the steps to prepare
> the environment before building and testing print all the environment 
> variables rather than just the ones being set for that step which
> seems to go against the aim of "CI: narrow down variable definitions
> in --build and --test". (Also the "SKIP" prefix in the output lacks a
> ":")

Thanks. Those were all helpful. I replied to these in a re-roll CL at: https://lore.kernel.org/git/cover-v5-00.29-00000000000-20220421T181526Z-avarab@gmail.com/

> Dscho raised concerns that this removes any support for azure
> pipelines which he uses when preparing security patches.

In the v1 discussion of my series ~2 months ago I asked him how he'd prefer to proceed with that: https://lore.kernel.org/git/220222.86y2236ndp.gmgdl@evledraar.gmail.com/

There wasn't any reply to that for about a month so I submitted the v2, noting that we might want to do something different there: https://lore.kernel.org/git/cover-v2-00.25-00000000000-20220325T182534Z-avarab@gmail.com/

And then as a follow-up to that v2 there was a follow-up discussion ending (from my side) here: https://lore.kernel.org/git/220406.86bkxeecoi.gmgdl@evledraar.gmail.com/

So, in brief summary I'm still happy to accommodate any such use-case. But I don't think it's OK to say that code we don't even use in-tree in area X must be kept as-is, to the point where it blocks forward progress for things that are used in-tree.

To be clear: I'm not saying that's Johannes's position, it may or may not be: The point is that he hasn't replied to https://lore.kernel.org/git/220222.86y2236ndp.gmgdl@evledraar.gmail.com/ or any relevant follow-up discussions. So I don't know what he's expecting there, or if it can be reasonably accommodated in making changes to ci/* which by their nature must decide to support/keep/fork/remove some of that code.

> I think splitting out the build and test steps is a good idea but I'm
> less convinced by some of the other changes.

What other changes are you referring to here?