Skip to content

Avoid using separate '-D' + '...' flags in CFLAGS in CMake #63706

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
merged 1 commit into from
Mar 7, 2023

Conversation

kubamracek
Copy link
Contributor

Please look at the code change, and think "how could this possibly be a significant change?". If you come up empty, like me when I originally hit this, then click here to reveal the solution. CMake 🤷

Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

Aaaargh!

@mikeash
Copy link
Contributor

mikeash commented Feb 16, 2023

I'm pretty sure I've seen this before. Also, WTF CMake?!

Copy link
Contributor

@nate-chandler nate-chandler left a comment

Choose a reason for hiding this comment

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

Error, your directive was too legible. -CMake, probably

@mikeash
Copy link
Contributor

mikeash commented Feb 16, 2023

Proposed:

@@ -55,9 +55,9 @@ public enum CommandLine {
-  public static var arguments: [String]
+  public static var arguments: Set<String>

@kubamracek
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

This isn't really that weird. It actually is a positive. The point is that people often repeat the options on the command line, CMake will deduplicate it to get a saner command line.

@compnerd compnerd closed this Feb 17, 2023
@compnerd compnerd reopened this Feb 17, 2023
@kubamracek
Copy link
Contributor Author

@swift-ci please test Linux platform

1 similar comment
@kubamracek
Copy link
Contributor Author

@swift-ci please test Linux platform

@kubamracek kubamracek merged commit 8d7c115 into swiftlang:main Mar 7, 2023
@kubamracek kubamracek deleted the cmake-again branch March 7, 2023 22:33
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.

6 participants