Skip to content

Add flags method to cc::Build for adding multiple flags #1466

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

Merged

Conversation

fiveseven-lambda
Copy link
Contributor

Closes #1465. Thank you @NobodyXu.

My concerns:

  • Should we have some tests?
  • Should the flags method also handle the case where a single string of flags (e.g., "flag1 flag2") is passed and split it into individual flags? Alternatively, should we delegate this responsibility to an external crate like shell-words to handle more complex cases like quoted strings with spaces?

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks!

Should we have some tests?

IMO having a no_run example is enough

Should the flags method also handle the case where a single string of flags (e.g., "flag1 flag2") is passed and split it into individual flags?

No that should not be part of this function.

Having another function for this is acceptable, as we already do that kinds of parsing for environment variables.

fiveseven-lambda and others added 2 commits May 7, 2025 21:16
Co-authored-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>
@fiveseven-lambda fiveseven-lambda requested a review from NobodyXu May 7, 2025 12:28
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

A new release is scheduled for this Friday/Saturday.

@NobodyXu NobodyXu merged commit f50ff26 into rust-lang:main May 7, 2025
73 checks passed
@fiveseven-lambda fiveseven-lambda deleted the add-flags-method-to-cc-build branch May 7, 2025 12:40
@github-actions github-actions bot mentioned this pull request May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add .flags() to cc::Build for passing multiple compiler flags
2 participants