Plz move these toinputs: and host the matrix on the calling side (inbuild.yml). This is the separation I was going for when I first introduced thereusable-*.yml module pattern — no matrices in modules.

This is a convenient way to define a variable that's reused a few times in the workflow. It could be anenv, but it's not actually needed in the environment.

I know. I designed the modular approach with consistency in mind (which includes only allowing matrices on the top level). Having a matrix in reusable workflows sometimes has awkward side effects in a few places on GH, which I'd rather we avoid.

FWIW, you can use inputs with default values the same way — for var declarations — just a bit more verbosely. Although, I'd argue that being able to fill out descriptions for the inputs would be beneficial by making the purpose documented explicitly.

It could be anenv, but it's not actually needed in the environment.

I noticed you're settingOPENSSL_VER already so it's already in there.

And I know that foros, people in other PRs were unhappy to see it in the matrix 🤷‍♂️
Actually, in#136729 (comment), Hugo suggested usingenv.IMAGE_OS_VERSION /${IMAGE_OS_VERSION} in the steps, making the matrix entry redundant.

Another reason for usingenv is that it'll trip Zizmor checks — they are probably suppressed right now but once re-enabled, we'll need to deal with the interpolation on GHA vs. Bash level.

I'll need to work on migrating everything to env vars at some point and making sure Zizmor enforces that.

This will also need to interpolate new matrix factors since the job names will be confusing if there's ever more than one job.

There's the same jobs as before. The TSan ones just now compile OpenSSL with TSan

Right, so there shouldn't be a matrix, then:#143752 (comment)

We can move this conditional to the GHA step so that skipping is more prominent.

Also dropfi and the second conditional block below too.

I merged the if blocks, but you can't lift the conditional the GHA step because you still need the:

elseecho"UBSAN_OPTIONS=${SAN_LOG_OPTION}">>"$GITHUB_ENV"fi

and the unconditional

echo"CC=clang">>"$GITHUB_ENV"echo"CXX=clang++">>"$GITHUB_ENV"

@colesbury what I'd recommend is splitting these into two steps so that it'd be possible to move the conditional and it'd be prominent in the log. Alternatively, it might make sense to just moveCC andCXX into the declarative per-stepenv: since they aren't computed dynamically anyway. Though, looking more into it, it sound like they probably belong in theInstall dependencies which makesclang available and is already setting up the env to use it — just like these env vars.