On Tue, Feb 22 2022, Johannes Schindelin wrote: > Hi, > > > On Mon, 21 Feb 2022, Ævar Arnfjörð Bjarmason wrote: > >> Remove Azure-specific code that's been unused since 6081d3898fe (ci: >> retire the Azure Pipelines definition, 2020-04-11). As noted in a >> larger removal of all of the Azure-supporting code in [1] (although >> that missed some of this) there is more of it in-tree, but let's focus >> on only the ci/* code for now. >> >> This is needed because in subsequent commits this unused code would >> either need to be changed to accommodate changes in ci/*, or be >> removed. >> >> As we'll see in those subsequent commits the end-state we're heading >> towards will actually make it easier to add new CI types in the >> future, even though the only one we're left with now is the GitHub >> CI. I.e. the "ci/*" framework will be made to do less, not more. We'll >> be offloading more of what it does to our generic build and test >> system. >> >> While I'm at it (since the line needs to be touched anyway) change an >> odd 'if test true == "$GITHUB_ACTIONS"' to the more usual style used >> in other places of 'if test "$GITHUB_ACTIONS" = "true"'. >> >> 1. https://lore.kernel.org/git/patch-1.1-eec0a8c3164-20211217T000418Z-avarab@xxxxxxxxx/ > > This has been discussed before, and I already gave my NAK. > > It is sad that I have to repeat myself: it is a good thing to have the > Azure Pipelines definition as a fall-back. In the past, this has served us > very well especially when we had to run a barrage of security fixes, for a > slew of backports to previous release trains. > > You seem to have fun to just remove this code, under some assumption that > it is not needed, despite me pointing out that it is needed. I previously submitted a stand-alone patch to remove it[1] after the removal of (most of) the Travis CI was merged in f9b889dd67b (Merge branch 'ab/ci-updates', 2021-12-15). I submitted that not for "fun", but because I had other CI/test-lib.sh changes depending on that. I.e. I needed to either patch code unused in-tree I couldn't test without reverting your 6081d3898fe (ci: retire the Azure Pipelines definition, 2020-04-11) (and presumably set up some Azure CI account etc.), or remove it. It's still unclear to me how Azure CI is being used as a "fall-back", can you explain that? I.e. we don't have the azure-pipelines.yml anymore (and neither does git-for-windows/git). Is this in-tree code being used by some out-of-tree fork of git.git, or do you mean you'd like to keep it in case we find a reason to revert your 6081d3898fe? In this case (and as I think this series makes clear), we can't easily keep certain Travis + Azure assumptions around *and* simplify how ci/lib.sh works in the same way. At the tip of this series it's turned into a very dumb variable setting helper with the CI recipe itself driving the test. Whereas Travis and Azure were using a feature where a "job" once running would attempt to skip the rest of the CI run. And in your [1] you mentioned the reasons for keeping it around being: The reason is that there are still some things that Azure Pipelines can do that GitHub workflows cannot, for example: - present the logs of failed tests in an intuitive manner, - re-run _only_ failed jobs. It seems to me that in your parallel series you're working on 1/2 of those, and with/without those changes we could otherwise improve the presentation of the failed test runs enough with the "grouping" etc. feature. And as for 2/2 that any such support would be mostly orthogonal to keeping Azure code that's currently unused around. I.e. a goal of this series is also to make the CI less of a special snowflake, because I'm not only interested in running the CI code on GitHub CI, but also locally. Which means that for any future port to Azure, GitLab CI etc. we'd presumably just have a thin recipe that ran "make" followed by "make test", and for a re-run of only that job it would just use the CI itself to do that, without us needing to carry any special-cases in ci/*. I'm not at all opposed to keeping Azure support in-tree. I specifically have a problem with unused code holding other improvements hostage. I.e. neither I nor anyone else can easily change anything surrounding it while being assured that we haven't broken that unused code. As noted in 04/25 I think you've also had that issue, i.e. your recent 0e7696c64db (it seems to me) introduced what would have been a bug if it were combined with the azure-pipelines.yml code removed in your 6081d3898fe. 1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2112201834050.347@xxxxxxxxxxxxxxxxx/
