-
Notifications
You must be signed in to change notification settings - Fork 723
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
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. |
bde9266
to
69c3649
Compare
@pvdrz Is this CI failure related to the PR? It looks unrelated to me, but I'm not sure. |
69c3649
to
1e87fed
Compare
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. |
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.
Please rebase, CI got fixed already IIRC.
Looks reasonable to me.
bindgen/lib.rs
Outdated
.options | ||
.clang_args | ||
.iter() | ||
.filter(|arg| !arg.contains("-MMD") && !arg.contains("-MD")) |
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.
Should this be starts_with
? Also shouldn't we also skip the long-version of these? (--write-user-dependencies
/ --user-dependencies
)?
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.
Resolved.
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.
161131f
to
76920aa
Compare
Closes #3069
Closes #3070
I've tested the fixes, but would appreciate any additional feedback on this @ojeda.