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

Do not pass inhibit warnings compiler flags for Swift source files. #9014

Merged
merged 1 commit into from Jul 19, 2019

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Jul 18, 2019

closes #9013

It will probably be for 1.7.5 release and will add a milestone.

Passing those flags in Xcode 11 Beta 4 fails to compile.

@dnkoutso dnkoutso changed the base branch from master to 1-7-stable July 18, 2019 00:09
@dnkoutso
Copy link
Contributor Author

We also pass -suppress-warnings for Swift https://github.com/CocoaPods/CocoaPods/blob/1-7-stable/lib/cocoapods/target/build_settings.rb#L785 so no need to use SWIFT_SUPPRESS_WARNINGS

@segiddins
Copy link
Member

Should file a radar for this regression

@dnkoutso
Copy link
Contributor Author

Will do but I do not expect that to be fixed anytime soon. Maybe that is my answer not to point to 1-7-stable and have it on master for 1.8.x.

@dnkoutso
Copy link
Contributor Author

@mrgrauel
Copy link

A fast release - 1.7.5 - would be awesome 👍

@dnkoutso
Copy link
Contributor Author

We are waiting a day or two to hear from Apple if this change was intended or a regression from their part.

If people are blocked please use https://bundler.io/ and point to this branch or commit to be unblocked.

We do not want to release a 1.7.5 and then release 1.7.6 again if this is a regression from Apple.

@lilyball
Copy link

This doesn't seem like an Xcode regression. It seems like an Xcode bugfix. I just compiled my project using Xcode 10.2.1 and checked, and it did not pass the per-file compiler flags to swift files. In Xcode 11 beta 4 it's now passing the per-file compiler flags to swift files.

The fact is, these flags never worked on Swift. We just didn't notice before because Xcode wasn't passing them along. Now that it is passing them along, it's obvious that the flags shouldn't be applied to swift files.

I strongly recommend publishing a 1.7.5 for this instead of waiting.

@dnkoutso dnkoutso added this to the 1.7.5 milestone Jul 19, 2019
@dnkoutso dnkoutso merged commit baea64c into CocoaPods:1-7-stable Jul 19, 2019
@dnkoutso dnkoutso deleted the swift_compiler_warnings branch July 19, 2019 16:41
@dnkoutso
Copy link
Contributor Author

Shipped with 1.7.5.

@Coeur
Copy link
Contributor

Coeur commented Jul 23, 2019

We do not want to release a 1.7.5 and then release 1.7.6 again

@dnkoutso
Well, see #9013 (comment)

@barrypress
Copy link

barrypress commented Jul 23, 2019

@Coeur is right about #9013. The attached project is a clean iOS project, then including the Down pod, and fails to compile. I believe the problem is that the PR did not address the flag no-shorten-64-to-32

DownTest.zip

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Jul 24, 2019

This happens because the podspec specifies compiler_flags https://github.com/iwasrobbed/Down/blob/master/Down.podspec#L20

We will have to re-think this through as a relatively bigger change to handle compiler flags for different source file types.

@lilyball
Copy link

In the short term we should probably just stop passing per-file compilation flags to Swift files. It apparently never worked before, so omitting them entirely restores the old behavior. Then we can explore adding a separate option for Swift compilation flags.

@amorde
Copy link
Member

amorde commented Jul 24, 2019

That's not true, there are per-file complication flags for Swift that do work, the problem was CocoaPods was including Objective-C-specific flags when it shouldn't have been.

There's the -Xcc option for passing flags down to the Objective-C/C/C++ compiler, maybe we should be using that instead?

@lilyball
Copy link

swiftc has some flags that make sense to pass (though I'm not sure which ones are appropriate as per-file flags), but it's clear that Xcode prior to Xcode 11 Beta 4 wasn't passing the per-file flags to swiftc. This is really easy to test; just create a new project in Xcode 10, add a dummy per-file compilation flag to a Swift file, and build. Not only does the build succeed despite the garbage flag, but if you inspect the build log, the flag was never passed.

But more than that, given that compilation flags are unique to each compiler, it simply doesn't make sense for CocoaPods to have a single property that it applies to all source files. It should have a property for clang flags, and a property for swiftc flags. I'm also not sure why it's even using per-file compilation flags at all instead of just passing these in the appropriate per-language build setting.

@amorde
Copy link
Member

amorde commented Jul 24, 2019

Yes that's the issue, the compiler_flags DSL was introduced before Swift was released and it was never updated. We might add a swift_compiler_flags to match but no concrete plans for that yet

@barrypress
Copy link

barrypress commented Jul 24, 2019 via email

@Coeur
Copy link
Contributor

Coeur commented Jul 25, 2019

@barrypress :
All things related to the issue are written here between your two messages.
Feel free to do a Pull Request adopting either amorde idea (add a swift_compiler_flags) or lilyball idea (stop passing per-file compilation flags to Swift files) if you need a fix urgently. Otherwise, tell your userbase to avoid Xcode 11 beta 4+ for a few weeks?

@lilyball
Copy link

@barrypress You can also submit a PR to Down that removes that flag from the podspec as per this comment. As the comment says, this will likely introduce some warnings in the C code, but you can then handle that by adding the appropriate #pragmas around the code in question to disable the warning in a targeted fashion.

@Coeur
Copy link
Contributor

Coeur commented Aug 7, 2019

@barrypress as there wasn't much visible move on that in the past two weeks, I would suggest that you open a specific GitHub issue in order to track that Xcode 11 issue with pods requiring compiler_flags that are incompatible with Swift. Then link that issue to this pull request for reference.

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.

None yet

7 participants