-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: auto-generate syntax files #5627
Merged
Merged
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
See commit 9bf5e45 ("ci: sort items on paths-ignore lists", 2022-07-12) / PR netblue30#5481.
And also add an "error: " prefix, to make the output clearer. Before: $ rm -f config.mk; make config.mk printf 'run ./configure to generate %s\n' "config.mk" >&2 run ./configure to generate config.mk false make: *** No rule to make target 'config.mk'. Stop. After: $ rm -f config.mk; make config.mk error: run ./configure to generate config.mk make: *** No rule to make target 'config.mk'. Stop. This amends commit e21637c ("makefiles: add generated files as dependencies", 2022-06-23) / PR netblue30#5219.
The seccomp filters are used by firejail itself at runtime (and are installed to `$(libdir)`), while the man pages are used by an external program (and installing them is optional; see `HAVE_MAN`), so reorder them. Misc: The seccomp filter targets were apparently added on commit 64431c7 ("seccomp work 1", 2016-11-20).
Having all of syntax files in the same directory makes it easier to reference all of them at once on a makefile (such as with `contrib/syntax/files/*.in`). Also, this makes the path to the gtksourceview language-spec shorter. Current path/new path: * contrib/gtksourceview-5/language-specs/firejail-profile.lang * contrib/syntax/files/firejail-profile.lang Currently, adding a rule to the root Makefile to generate the language-spec in the same directory as an input file would take at least 95 characters (with only a single dependency): contrib/gtksourceview-5/language-specs/%.lang: contrib/gtksourceview-5/language-specs/%.lang.in With this commit, the above shortened to 59 characters: contrib/syntax/files/%.lang: contrib/syntax/files/%.lang.in Which should make it more readable. Relates to netblue30#2679 netblue30#5502.
Changes: * Use the commands from contrib/vim/syntax/firejail.vim to create makefile targets to generate syntax lists in contrib/syntax/lists * Add contrib/syntax/files/example.in as an example of how to generate syntax files * Generate and add the syntax lists, to make it easier to spot if they are properly updated when a new command is added or if their recipes also need changes * Add "syntax" and "contrib" makefile targets Note: The generation commands are executed mostly silently to avoid generating too much noise when also making other targets. Note2: In some generation commands, a `$$` escape is used to pass `$` to the shell, to avoid being interpreted by make as the start of a macro. Note3: `@make_input@` is used in example.in to make it clear that the file is generated (and that it is generated by make rather than configure), similarly to how `@configure_input@` is used in configure input files. See also apparmor.vim: $ head -n 2 /usr/share/vim/vimfiles/syntax/apparmor.vim " generated from apparmor.vim.in by create-apparmor.vim.py " do not edit this file - edit apparmor.vim.in or create-apparmor.vim.py instead Environment: apparmor 3.1.2-1 on Artix Linux. Relates to netblue30#2679 netblue30#5502 netblue30#5577 netblue30#5612.
Changes: * Generate firejail.vim from firejail.vim.in * Generate firejail-profile.lang from firejail-profile.lang.in * Update the manual syntax file steps on the new command checklist on CONTRIBUTING.md to use `make syntax` instead Relates to netblue30#2679 netblue30#5502 netblue30#5577 netblue30#5612.
Make the syntax target by default to make it harder to forget to update the syntax files. Note that the syntax files are built mostly silently and that they are generated relatively fast (they only take 40~70ms to build on a not exactly recent machine with `make clean; time make -j 4 syntax`), so they should not add much noise nor time noise when just trying to build firejail, for example.
glitsj16
approved these changes
Jan 28, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Very nice work!
To ensure a consistent order. Misc: This might also make it a bit faster.
Thanks! |
merged! |
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 12, 2023
Make the non-phony targets that are defined in the root Makefile depend on it, to ensure that they get re-generated if their recipes change. Note that these targets are generated nearly instantly, so this should not noticeably affect rebuild times. Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 12, 2023
Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 12, 2023
Escape `.` only when generating the syntax files rather than directly in the syntax lists, so that the latter contain the command names as is. Also, make sure to escape it for the arg1 syntax list as well. Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 13, 2023
Make the non-phony targets that are defined in the root Makefile depend on it, to ensure that they get re-generated if their recipes change. Note that these targets are generated nearly instantly, so this should not noticeably affect rebuild times. Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 13, 2023
Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 13, 2023
Escape `.` only when generating the syntax files rather than directly in the syntax lists, so that the latter contain the command names as is. Also, make sure to escape it for the arg1 syntax list as well. Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 14, 2023
Escape `.` only when generating the syntax files rather than directly in the syntax lists, so that the latter contain the command names as is. This also makes the escaping apply to the arg1 syntax list as well. Note: Double escaping (`\\\\.`) is used in `regex_fromlf` because its output is used in another sed replacement (where it needs to be `\\.`). Relates to netblue30#5627.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 14, 2023
Escape `.` only when generating the syntax files rather than directly in the syntax lists, so that the latter contain the command names as is. This also makes the escaping apply to the arg1 syntax list as well. Note: Double escaping (`\\\\.`) is used in `regex_fromlf` because its output is used in another sed replacement (where it needs to be `\\.`). Relates to netblue30#5627.
kmk3
added a commit
that referenced
this pull request
Sep 6, 2023
Fix the list generation and run `make syntax`. Relates to #5627.
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.
Main changes:
makefile targets to generate syntax lists in contrib/syntax/lists
syntax files
are properly updated when a new command is added or if their recipes
also need changes
CONTRIBUTING.md to use
make syntax
insteadRelates to #2679 #5502 #5577 #5612.