Commit 465dd47
Render generate-github-actions output via Jinja2
Summary:
`GenerateGitHubActionsCmd.write_job_for_platform` previously emitted GitHub Actions workflow YAML through ~120 inline `out.write()` calls interleaved with platform conditionals — a wall of string-building that made every CI-shape change a delicate edit. This diff replaces that with a single Jinja2 template plus a context-builder method, while preserving byte-identical output for every existing invocation.
The template lives at `getdeps/templates/workflow.yml.j2`. To keep GitHub Actions `${{ ... }}` expressions readable inside the template, the Jinja environment uses non-default delimiters: `<< >>` for variables, `<% %>` for blocks, `<# #>` for comments. `keep_trailing_newline=True` is set so the template's final newline is preserved verbatim. The literal string `generated` in the file header is constructed at render time as `<<"@" + "generated">>` so the template itself isn't flagged as a generated file. `jinja2` is imported lazily inside `_render_workflow` so OSS GHA runners executing `getdeps.py build`/`test`/`install-system-deps` are not forced to install it; only callers of `generate-github-actions` need it on their system.
The refactor also splits `getdeps.py` into a thin OSS-facing shim and a `getdeps/cli.py` module that holds all the command classes, `parse_args`, and `main`. Supporting pieces moved with it: `ProjectCmdBase`, `UsageError`, and `BUILD_TYPE_ARG` to `getdeps/cmd_base.py`; `GenerateGitHubActionsCmd` plus its rendering helpers (`_render_workflow`, `_build_render_context`, `_parse_per_package_defines`) to `getdeps/workflow_generator.py`. The shim's only job is `sys.path.insert` for OSS users running `python3 build/fbcode_builder/getdeps.py …` directly. Inside Buck, a new `python_binary` target `//opensource/fbcode_builder:getdeps` invokes `getdeps.cli:main`, brings the third-party deps along, and resolves the long-standing namespace collision between `getdeps.py` (script) and `getdeps/` (package). `update-all-github-actions.sh` now invokes via `buck run` so Meta engineers no longer need jinja2 installed in their system python.
The new `_build_render_context` method does all data preparation (manifest walk, dep ordering, command-string composition, env-var assembly) and returns a flat `dict` consumed by the template. CLI-string assembly (`build_type_arg`, `cmake_arg_for(...)`, `getdepscmd ...`) stays in Python — composing these in Jinja would be worse than what we have today.
Several small simplifications fall out of the move: the per-OS `(artifacts, runs_on, py3)` derivation moves into a `_resolve_platform(args, build_opts)` helper at module scope; the `_PLATFORMS` list becomes a module-level constant; the five repeated `manifest.get("github.actions", ..., ctx=manifest_ctx)` lookups go through a local `gh(key)` closure; and the `tests_arg`/`job_file_prefix`/`job_name` "default-then-override" blocks collapse to single `or`/ternary expressions. With those out, `write_job_for_platform` no longer needs its `noqa: C901`.
Output equivalence is verified two ways. (1) `getdeps/test/workflow_generator_test.py` is a golden test that drives the generator in-process for four representative scenarios (xxhash all-OSes, folly shared-libs linux, openr no-system-packages run-on-all-branches, rebalancer with cmake overrides) and asserts each emitted YAML file matches a checked-in fixture. When the fixture and actual output drift, the assertion failure prints how to regenerate. The test accepts a `--update-fixtures` flag (`buck run //opensource/fbcode_builder/getdeps/test:test -- --update-fixtures`) which rewrites the per-scenario fixture files from current output and skips assertions; the resulting `sl status` diff is the change record. (2) Running every invocation in `getdeps/facebook/update-all-github-actions.sh` against pre- and post-refactor builds and diffing produces zero differences across all 44 generated `.yml` files.
This change does not touch the CLI flag surface, schema, or output files of `update-all-github-actions.sh`. It is a pure refactor.
Reviewed By: bigfootjon
Differential Revision: D104495377
fbshipit-source-id: c7eb6a118c94d41fdc4525235f2319772f2598121 parent c16f85a commit 465dd47
14 files changed
Lines changed: 4387 additions & 1753 deletions
File tree
- build/fbcode_builder
- getdeps
- templates
- test
- fixtures/expected
- openr
- rebalancer_linux
- xxhash_plain
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
476 | 476 | | |
477 | 477 | | |
478 | 478 | | |
479 | | - | |
| 479 | + | |
480 | 480 | | |
481 | 481 | | |
482 | 482 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
973 | 973 | | |
974 | 974 | | |
975 | 975 | | |
976 | | - | |
| 976 | + | |
977 | 977 | | |
978 | 978 | | |
979 | 979 | | |
| |||
0 commit comments