Fix bugs in --clang-macro-fallback #3072
Conversation
|
I forgot to mention here that I'm doing some additional work on this so I changed it to a draft for the time being. I'll let you know when it's ready for review. |
jbaublitz
force-pushed
the
fixes-issue-3069-3070
branch
5 times, most recently
from
bde9266 to
69c3649
Compare
|
@pvdrz Is this CI failure related to the PR? It looks unrelated to me, but I'm not sure. |
jbaublitz
force-pushed
the
fixes-issue-3069-3070
branch
from
69c3649 to
1e87fed
Compare
jbaublitz
force-pushed
the
fixes-issue-3069-3070
branch
from
1e87fed to
7a675c0
Compare
|
My other bindgen PR fails in the same way as this one, so I'm going to consider it unrelated. @ojeda I think this is ready for you to take a look at it if you'd like. |
bindgen/lib.rs
Outdated
| .options | ||
| .clang_args | ||
| .iter() | ||
| .filter(|arg| !arg.contains("-MMD") && !arg.contains("-MD")) |
There was a problem hiding this comment.
Should this be starts_with? Also shouldn't we also skip the long-version of these? (--write-user-dependencies / --user-dependencies)?
jbaublitz
force-pushed
the
fixes-issue-3069-3070
branch
2 times, most recently
from
1127232 to
161131f
Compare
This commit resolves a bug where -include was not respected and a bug where -MMD was passed to the fallback translation unit which would cause the dependency file to be overwritten with useless information about temporary files.
jbaublitz
force-pushed
the
fixes-issue-3069-3070
branch
from
161131f to
76920aa
Compare
|
Would it be possible to add tests for these bugs? |
For the dependency generation, I am not sure how we would go about testing that only because I haven't seen any tests in the infrastructure for side effects, but it should be possible for the -include flag fix. |
Closes #3069
Closes #3070
I've tested the fixes, but would appreciate any additional feedback on this @ojeda.