Use jinja2 for templating #19630
Merged
Use jinja2 for templating #19630
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I guess the CI will fail with missing deps… This will need to be figured out. |
bluca
added a commit
to bluca/systemd-centos-ci
that referenced
this pull request
May 16, 2021
Pushed to Salsa for the bionic tests. PR for the CentOS tests: systemd/systemd-centos-ci#382 |
bluca
added a commit
to bluca/systemd-centos-ci
that referenced
this pull request
May 16, 2021
bluca
reviewed
May 16, 2021
mrc0mmand
pushed a commit
to systemd/systemd-centos-ci
that referenced
this pull request
May 16, 2021
|
Looks very nice! But the CI is a bit unhappy: |
bluca
added
the
ci-fails/needs-rework 🔥
|
@bluca judging by https://autopkgtest.ubuntu.com/results/autopkgtest-bionic-upstream-systemd-ci-systemd-ci/bionic/amd64/s/systemd-upstream/20210516_223707_2a255@/log.gz (from #19598) Ubuntu CI is broken |
Ah yes, jijnja is arch:all so the :native suffix is wrong - removed |
|
Updated:
Let's see if this helps. |
|
Updated:
|
DavidKorczynski
pushed a commit
to google/oss-fuzz
that referenced
this pull request
May 17, 2021
For systemd/systemd#19630: m4 is being replaced by jinja2. Let's pull in both until the dust settles.
|
I'm not sure where semaphore is getting the list of build-dependencies - it's not from the upstream-ci branch in Salsa, as I've updated it. Maybe from the uploaded package - in which case python3-jinja2 needs to be added manually to .semaphore/semaphore-runner.sh for now |
|
This is a strange one, in CentOS: |
|
One more attempt:
|
|
Another attempt:
|
We were using both in various places. To keep things simple, let rpm do the substitution.
The naming of variables is very inconsistent. I tried to use more modern style naming (UNDERSCORED_TITLE_CASE), but I didn't change existing names too much. Only SYSTEM_DATA_UNIT_PATH is renamed to SYSTEM_DATA_UNIT_DIR to match SYSTEM_CONFIG_UNIT_DIR.
One stanza had "if install_sysconfdir_samples", while the other "if install_sysconfdir", which looks like a mistake. install_sysconfdir_samples is now used for both.
We had two big 'configuration_data' objects in meson config. (There are in fact
more. On is added in this series, and there's one for efi… But those others
have a handful variables only for specific purposes and don't matter). The two
sets are 'conf' and 'substs', and were inherited from the original autotools
system. In the past there was even a third set ('m4_defines'), but @yuwata
removed it in 348b443. And those two/three
systems had very similar data, but with different variable names, because of
historical reasons. They also used subtly different quoting (.set()
vs. .set10() vs. .set_quoted()), which was required because the templating
engines were not flexible enough. This meants we had more work when changing
things, and we needed to search for different variable names, etc.
With a more flexible templating engine we can do with just one
configuration_data object.
This doesn't matter too much, but makes things a bit more consistent.
A minor advantage is that the file is not a configuration file for meson
anymore, so:
a) It is not built unless pulled in by another target. Since
we don't usually build man pages by default, this saves a tiny
amount of work.
b) When the .in file is updated, meson does not reconfigure everything,
but just rebuilds the dependent targets.
Now that the conversion is finished, time for benchmarking:
a full build with default settings (and -Dstandalonebinaries=true), yields
before this pull request: 1687 targets, 148.13s user 35.17s system 317% cpu 57.697 total
with the full pull request: 1714 targets, 143.07s user 27.87s system 314% cpu 54.369 total
The difference doesn't seem significant. Partial rebuilds might be faster as
mentioned before.
We can generate the right string in the template directly.
Let's use the same names as in the jinja2 substitutions.
The order was a complete mess. Let's make it a bit more tidy.
Recent meson versions include the directory name in the target name, so there is no conflict for files with the same name in different directories. But at least with meson-0.49.2 in buster we have conflict with sysusers.d/systemd.conf.
|
Rebased and force-pushed. CIs were already green. Merging. |
ddstreet
pushed a commit
to ddstreet/systemd
that referenced
this pull request
Jul 1, 2021
Added in place of m4 via: systemd#19630
DaanDeMeyer
pushed a commit
to DaanDeMeyer/systemd-rpm
that referenced
this pull request
Jul 7, 2022
python3-devel hasn't been needed since we split out the python module, a few years ago. Pull in jinja2 for systemd/systemd#19630.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
m4 use is dropped, and the two parallel substitution systems in meson are unified.