google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.79k stars 292 forks source link

Swift version should be specified in the Podspec for `PromisesSwift` pod #150

Closed edudnyk closed 3 years ago

edudnyk commented 3 years ago

Getting this error when updating to PromisesSwift 1.2.9 (cocoapods 1.9.3 and 1.10.0.beta.1):

[!] Unable to determine Swift version for the following pods:

- `PromisesSwift` does not specify a Swift version and none of the targets (`Dummy`) integrating it have the `SWIFT_VERSION` attribute set. Please contact the author or set the `SWIFT_VERSION` attribute in at least one of the targets that integrate this pod.
ykjchen commented 3 years ago

Hey @edudnyk are you able to propose a fix?

edudnyk commented 3 years ago

swift_version has been removed from the Podspec in this commit https://github.com/google/promises/commit/70c337e196c81f795eedb5d7a42cf966f07cdc3d. The commit message does not specify the reason for doing that. So, the proposed fix is to revert this commit. Or revert just individual podspec file change.

ykjchen commented 3 years ago

@tristane0 do you remember the history behind removing the s.swift_version = '5' line from PromisesSwift.podspec?

ghost commented 3 years ago

I believe it was removed because we assumed that developers would set the Swift version in their xcodeproj target(s) that depend on PromisesSwift. Also, we wanted to ensure that we supported the most commonly used Swift versions and wanted to avoid having to update the Swift version(s) in the podspec each time a new version was released.

@edudnyk are you setting the Swift version for the target(s) in your xcodeproj that depend on PromisesSwift? If you are setting it, are you seeing this issue with a specific Swift version?

edudnyk commented 3 years ago

With build systems like buck and bazel, there is no xcodeproj initially, it gets generated by the build system. I can confirm that if there is xcodeproj file with the target that uses PromisesSwift, and that target's build settings have SWIFT_VERSION specified, the error does not get emitted. But, it does get emitted when target does not specify it.

Let's say I have very old codebase that works with Swift 4, and you are using Swift 5 in your pod. Why don't you want to enforce Swift 5 for your pod and, instead, inheriting Swift 4 in this situation?

ghost commented 3 years ago

For Bazel, you rely on the Swift version that's set by the current Xcode that you are using to compile your project with. You could override the "default" Swift version if you really wanted to by specifying the swift_version in the copts param of your swift_library target. But this is not really necessary anymore if compiling everything with Swift 5.1 (when module stability was introduced) or newer.

We are essentially saying the same thing with CocoaPods in that we are relying on the default Swift version for the current Xcode that our developers are using to build the PromisesSwift library. I don't think we have any limitations with the versions at the moment, so if a developer, for example, wanted to compile with Swift 4.2, I believe everything would still work.

@ykjchen and @shoumikhin may have additional thoughts.

edudnyk commented 3 years ago

@tristane0 Prior to defining swift_version in Buck or Bazel, the pod should be installed with cocoa pods. That's where the error is emitted.

ghost commented 3 years ago

@edudnyk I may be misunderstanding your current config for managing your dependencies. Are you using Bazel, Buck, or CocoaPods? It sounds like you may be using CocoaPods and Bazel or Buck. If yes, can you provide some additional details regarding your current setup for your dependencies?

edudnyk commented 3 years ago

I am using the cocoa pods to load dependencies, then build with buck. There is no initial xcodeproj to integrate the pods into. You can unzip this and run pod install inside of the unzipped folder to reproduce the issue dummy-target-pods.zip

Podfile syntax allows to specify :integrate_targets => false,, but in this case, Podspec should define the Swift version.

ghost commented 3 years ago

Thank you for the example project to repro. @shoumikhin may be able to provide more details about using Buck to avoid having to rely on Cocoapods.

edudnyk commented 3 years ago

Pods is dependency management system, there is no dependency management mechanisms in Buck, so I don't think that it is feasible for me to switch from cocoa pods to something else.

edudnyk commented 3 years ago

Given that cocoa pods allow to specify :integrate_targets => false, it is inconsistent to rely that there is the parent project around when pod install occurs.

ghost commented 3 years ago

Just to clarify, are you using Buck to create the xcodeproj and relying on CP as the dependency manager? If yes, is there an issue with setting the swift version for the targets in your xcodeproj that depend on Promises rather than relying on CocoaPods to set the version?

To fix the issue you are seeing, which Swift version(s) are you proposing that we add to the PromisesSwift podspec?

edudnyk commented 3 years ago

are you using Buck to create the xcodeproj and relying on CP as the dependency manager

Yes

If yes, is there an issue with setting the swift version for the targets in your xcodeproj that depend on Promises rather than relying on CocoaPods to set the version?

No, there is no issue with Buck setup itself, if I have the source code of PromisesSwift locally, I will be able to build it with Buck. The issue is with the installation of the PromisesSwift pod v 1.2.9 via pod install (installation of PromisesSwift 1.2.8 works fine, because PromisesSwift.podspec v 1.2.8 has s.swift_version = '4.2').

To fix the issue you are seeing, which Swift version(s) are you proposing that we add to the PromisesSwift podspec?

I would define the supported version like min( xcode-non-beta-dev-tools-highest-swift-version, promises-swift-highest-supported-swift-version).

ghost commented 3 years ago

Thank you for the clarification! I didn't realize that 1.2.8 contained the swift_version in the podspec.

Would you be able to put together a PR with your suggested fix so that we can discuss there? Can loop in @ykjchen as well.

shoumikhin commented 3 years ago

@edudnyk are you using CocoaPods to fetch the source code and then add your custom BUCK file to it and build? Can you just download the code from GithHub directly instead? Perhaps, you can leverage git submodules to manage the dependencies.

edudnyk commented 3 years ago

With submodules, there won't be a dependency management, so, as I said, it is not feasible.

ykjchen commented 3 years ago

@edudnyk your fix has been published in 1.2.10. Thanks for your contribution.