ninjabackend: bring back historical "include_directories(is_system: true)" ordering#15578
Merged
bonzini merged 1 commit intomesonbuild:masterfrom Mar 6, 2026
Merged
ninjabackend: bring back historical "include_directories(is_system: true)" ordering#15578bonzini merged 1 commit intomesonbuild:masterfrom
bonzini merged 1 commit intomesonbuild:masterfrom
Conversation
Contributor
Author
|
See #15577 for another attempt at a fix that didn't work well. Maybe it could be revisited once the "object model for compiler arguments" idea pans out. |
…rue)" ordering
Prior to commit a6ddf2534 ("backend/ninja: Extend IncludeDirs.rel_string_list
for backend use"), when include_directories() was called with
`is_system: true` and multiple directories, the resulting `-isystem`
flags appeared on the command line in the opposite order from what was
specified: the last-listed directory was searched first by the compiler.
This was the opposite convention from non-system -I includes, where the
first-listed directory is searched first.
The asymmetry was not intentional; it was a side effect of how
CompilerArgs.__iadd__ handles flags that are not in prepend_prefixes:
- `-I` is in CLikeCompilerArgs.prepend_prefixes, so `__iadd__`
prepends it. The ninja backend iterates directories in reversed
order and adds them one at a time; the prepend-on-each-add
counteracts the reversal, producing the original order.
- `-isystem` was not in prepend_prefixes, so __iadd__ appended it.
The same reversed one-at-a-time iteration then produced a reversed
sequence — the last-listed directory ended up first on the command line.
Commit a6ddf25 changed generate_inc_dir to add all directories
in a single bulk commands += call instead of one at a time. This
preserved the correct order for -I (the double reversal in
__iadd__ still works for a bulk add) but silently broke -isystem,
because appending a list at once preserves input order rather than
reversing it.
Undo the changes a6ddf25 to preserve the previous behavior.
Adding -isystem to prepend_prefixes would also make the ordering
consistent, but projects that relied on the old reversed
behaviour would silently break.
This file contains hidden or 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
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.
Prior to commit a6ddf2534 ("backend/ninja: Extend IncludeDirs.rel_string_list for backend use"), when include_directories() was called with
is_system: trueand multiple directories, the resulting-isystemflags appeared on the command line in the opposite order from what was specified: the last-listed directory was searched first by the compiler. This was the opposite convention from non-system -I includes, where the first-listed directory is searched first.The asymmetry was not intentional; it was a side effect of how
CompilerArgs.__iadd__handles flags that are not in prepend_prefixes:-Iis in CLikeCompilerArgs.prepend_prefixes, so__iadd__prepends it. The ninja backend iterates directories in reversed order and adds them one at a time; the prepend-on-each-add counteracts the reversal, producing the original order.-isystemwas not in prepend_prefixes, so__iadd__appended it. The same reversed one-at-a-time iteration then produced a reversed sequence — the last-listed directory ended up first on the command line.Commit a6ddf25 changed generate_inc_dir to add all directories in a single bulk commands += call instead of one at a time. This preserved the correct order for -I (the double reversal in
__iadd__still works for a bulk add) but silently broke -isystem, because appending a list at once preserves input order rather than reversing it.Undo the changes a6ddf25 to preserve the previous behavior. Adding -isystem to prepend_prefixes would also make the ordering consistent, but projects that relied on the old reversed behaviour would silently break.