Skip to content
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

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

jbaublitz
Copy link
Contributor

Closes #3069
Closes #3070

I've tested the fixes, but would appreciate any additional feedback on this @ojeda.

@jbaublitz jbaublitz marked this pull request as draft January 8, 2025 16:53
@ojeda ojeda added the rust-for-linux Issues relevant to the Rust for Linux project label Jan 9, 2025
@jbaublitz
Copy link
Contributor Author

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 jbaublitz force-pushed the fixes-issue-3069-3070 branch 5 times, most recently from bde9266 to 69c3649 Compare January 10, 2025 16:31
@jbaublitz
Copy link
Contributor Author

@pvdrz Is this CI failure related to the PR? It looks unrelated to me, but I'm not sure.

@jbaublitz jbaublitz force-pushed the fixes-issue-3069-3070 branch from 69c3649 to 1e87fed Compare January 10, 2025 19:02
@jbaublitz jbaublitz marked this pull request as ready for review January 15, 2025 15:58
@jbaublitz jbaublitz force-pushed the fixes-issue-3069-3070 branch from 1e87fed to 7a675c0 Compare January 20, 2025 21:08
@jbaublitz
Copy link
Contributor Author

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.

Copy link
Contributor

@emilio emilio left a 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"))
Copy link
Contributor

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)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

@jbaublitz jbaublitz force-pushed the fixes-issue-3069-3070 branch 2 times, most recently from 1127232 to 161131f Compare February 10, 2025 16:01
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 jbaublitz force-pushed the fixes-issue-3069-3070 branch from 161131f to 76920aa Compare February 10, 2025 16:05
@emilio emilio added this pull request to the merge queue Feb 10, 2025
Merged via the queue into rust-lang:main with commit 76920aa Feb 10, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust-for-linux Issues relevant to the Rust for Linux project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--clang-macro-fallback interaction with -Wp,-MMD,file.d --clang-macro-fallback interaction with -include
3 participants