GeezerVison

cc @freakboy3742 @mhsmith

I think this can go under the same issue, #137242?

Thanks; I'm on holiday for the next few days, but I'll look at this when I get back.

So - I can see how this change works; I guess the part I'm not understanding is why it's a net improvement.

Adding this reusable workflow adds almost 100 lines of (moderately complex) GitHub Actions configuration, so that we can re-use the workflow... in exactly 2 places. In one of those places (CPython's repo), exactly 1 line of code is saved by this refactor; in release-tools, we save 9 lines. And the duplicated code that is being replaced isn't especially complex - it's "checkout repository", "call single script", "save artefacts" - all very common (and reasonably well understood) actions; and it's being replaced by a workflow configuration that isn't exactly complex, but at the very least has it's own learning curve.

In a world where the python Android/android.py ci step was more complicated (i.e., the original form of the Android CI PR), I can see how this sort of refactoring would make sense. But given where we landed (using ::group:: markers and encoding everything in Python), I feel like I'm missing something.

Is there some other use case that I'm missing? Some other benefit of being a reusable workflow?

@freakboy3742 the idea is to stop having steps in build.yml altogether. Currently, jobs are spread across multiple layers. In the ideal world, build.yml should only declare inter-job deps, setting up the workflow run shape + job conditionals and matrixes, leading up to running alls-green; and everything else would be in "modules".

I'd treat it more as an architectural thing rather than anything else. One of the UX bits is that two related jobs are now displayed under a common category in the sidebar as follows:

Previously, they showed up as two disconnected entries. I'd like to get the CI setup into a more consistent state and nicer look. So while it doesn't deduplicate too many things right now, it improves consistency and layer separation (inputs/matrix in build.yml while the implementation is in reusable-*.ymls).

I'm hopefully going be looking into making other jobs consistent but wanted to start small, with something still fresh in memory.

@webknjaz I can see how the summary is a nice feature on the action summary page - but the "on PR" view doesn't have any of those affordances. And if a build fails, I usually find that I'm navigating directly to the broken build page from the PR page, rather than going through the Actions navigation.

I guess I don't have any particularly strong feelings about the sort of organization you're describing. I can't find any fault with the implementation; but to me, it still seems like a lot more complexity without a lot of improvement. If there were details in the workflow that benefitted from having a single point of configuration, it might make more sense - but the structure you've described here essentially turns all that into workflow inputs, so you don't get that benefit either.

So - consider me -0. I won't fight it if someone else thinks this is worth it, though.

I'm OK with the idea of breaking each platform out into its own file to keep the size of the top-level workflow under control. And the grouping in the sidebar is also an improvement.

However, the attempt to share the workflow between the cpython and release-tools repositories has generated a huge amount of extra complexity which is not present in the resuable workflow files of the other platforms. And because cross-repository workflow references can't contain variable expansions (as discussed in #137186), the release-tools PR will need to hard-code one cpython commit for the workflow, and use it to build a different commit of the same repository. That's a recipe for even more confusion.

As @freakboy3742 said, the amount that's actually shared between the two repositories doesn't justify this. The checkout step is trivial, the build and test step is a very simple command, and the upload-artifacts step doesn't need to be in the cpython repository at all (it currently isn't).

So I'd suggest abandoning the release-tools PR, and reducing this PR only to splitting the Android workflow out into a separate file.

I can see how the summary is a nice feature on the action summary page - but the "on PR" view doesn't have any of those affordances.

There's a small effect in the PR widget as well -- the check identifiers become slash-delimited which kinda provides grouping. Not as visual, of course. Still, I wouldn't look at this as the main goal of the PR. Just a nice cosmetic side effect.

And if a build fails, I usually find that I'm navigating directly to the broken build page from the PR page, rather than going through the Actions navigation.

That's right. And the "category" is unfolded in the sidebar when you go to the log page directly.

[..] but to me, it still seems like a lot more complexity without a lot of improvement. If there were details in the workflow that benefitted from having a single point of configuration, it might make more sense - but the structure you've described here essentially turns all that into workflow inputs, so you don't get that benefit either.

I've been exploring how to use reusable workflows most efficiently and concluded that having such separation is a benefit by itself. Currently, this repo has multiple reusable workflows that are slightly different. This spreads many configuration quirks across multiple places. However, if they are all consolidated on the top level, it becomes much easier to merge a lot of those "modules" into something more uniform. So my strategy isn't making the reusable workflows branchy internally with implicit self-config within. It's rather surfacing things to the top level to make the invocation modes clear to the caller.

the attempt to share the workflow between the cpython and release-tools repositories has generated a huge amount of extra complexity which is not present in the resuable workflow files of the other platforms. And because cross-repository workflow references can't contain variable expansions (as discussed in #137186), the release-tools PR will need to hard-code one cpython commit for the workflow, and use it to build a different commit of the same repository. That's a recipe for even more confusion.

I can see your point about the confusion. However, I came to realize that referencing the commit is what makes such reuse reliable in the first place. So I see it as an advantage. But yes, this confusion that made me question needing a separate repository for referencable automation back in that discussion.

I think it's fine in this case, though, because the inputs are pre-computed early and have assigned names.

the amount that's actually shared between the two repositories doesn't justify this. The checkout step is trivial, the build and test step is a very simple command, and the upload-artifacts step doesn't need to be in the cpython repository at all (it currently isn't).

So I'd suggest abandoning the release-tools PR, and reducing this PR only to splitting the Android workflow out into a separate file.

I could do this, I guess. Still, I think it'd be interesting to consider cross-project reusability in the future. Also, I've been thinking that the upload artifacts step would be useful in the upstream CPython repo too as this would allow the PR creators download the artifacts for local inspection and not just see that "something got built in the jobs, w/o a chance to verify it". I can imagine the reusable workflow being useful for third-party projects that would like to build and test against unreleased versions of CPython too.

@mhsmith I've dropped the cross-repo reusability related inputs to only have things everyone seems to have agreed on for now.

I'm still a bit uncertain about the cross-repo automation split in general but can't articulate why at the moment. This can be a separate discussion in the future, if I get to formulating a better proposal.

I'm still not clear why you've added so many inputs and outputs that aren't present in the existing resuable workflow files. And even when the inputs are present, they aren't using the same names, e.g. you have runner-vm-os, where the existing workflows simply say os.

I'm also not really a fan of using @ and emojis in job names. A plain English "building X on Y" would be clearer.

This PR is stale because it has been open for 30 days with no activity.

This PR is attached to gh-137242, which is already completed: the fix for that issue landed through other changes, and this PR's approach was not taken or has since become obsolete. Closing to cull the number of open pull requests. Do not reopen this pull request without getting a go-ahead from a triager or core developer.