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

Conditional target dependencies proposal implementation #2428

Merged

Conversation

hartbit
Copy link
Contributor

@hartbit hartbit commented Nov 29, 2019

This is the implementation of the target dependency proposal that I plan to write a proposal for. I implemented it so that it only affects the build plan, but not the dependency resolver. This allows the Package.resolved file to stay platform and configuration independent:

  • I modified TargetDescription.Dependency, Target.Dependency, ResolvedTarget.Dependency to store the conditions configured in the manifest for each case.
  • I merged Target.dependencies and Target.targetDeps into one Target.dependencies property containing all dependencies, and adapted code using those properties in consequence.
  • I modified ResolvedTarget.recursiveDependencies to get all dependencies (target + product) and added a recursiveTargetDependencies function to do what the previous function did. I also added a buildDependencies function to filter dependencies that satisfy an certain build environment. Similarly, I defined recursiveBuildDependencies and recursiveBuildTargetDependencies that do the same thing but recursively.
  • I created reachableBuildTargets and reachableBuildProducts functions on PackageGraph that mirror the reachableTargets and reachableProducts properties but filter dependencies that don't satisfy the input environment.
  • Using the above new properties and functions, I modified the implementation of LLBuildManifestBuilder and BuildPlan to only take into account dependencies that satisfy the build environment.

But I'm facing one issue: how to generate an Xcode project with targets that have conditional dependencies on configuration? I don't want to add target dependencies universally. I'm not even sure if this is supported by Xcode. Any ideas?

@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch 9 times, most recently from 6446f27 to f919bb7 Compare December 3, 2019 08:24
@hartbit hartbit changed the title Conditional target dependencies proposal implementation WIP: Conditional target dependencies proposal implementation Dec 3, 2019
@hartbit
Copy link
Contributor Author

hartbit commented Dec 3, 2019

@swift-ci please smoke test

@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch from f919bb7 to 3c76cd8 Compare December 3, 2019 19:25
@hartbit
Copy link
Contributor Author

hartbit commented Dec 3, 2019

@swift-ci please smoke test

@jakepetroules
Copy link
Contributor

But I'm facing one issue: how to generate an Xcode project with targets that have conditional dependencies on configuration? I don't want to add target dependencies universally. I'm not even sure if this is supported by Xcode. Any ideas?

For linkage you can use EXCLUDED_SOURCE_FILE_NAMES tricks, but for target dependencies, there's no way to conditionalize them.

@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch 2 times, most recently from da66345 to 42495f2 Compare December 11, 2019 09:56
@hartbit
Copy link
Contributor Author

hartbit commented Dec 11, 2019

@jakepetroules Thanks for the answer! As it won't be able to officially support conditional target dependencies, @aciidb0mb3r suggested outputting a warning in those situations, which I have done. The implementation is now complete from my side.

@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch from 42495f2 to 6ab5ef1 Compare December 11, 2019 22:33
@hartbit hartbit changed the title WIP: Conditional target dependencies proposal implementation Conditional target dependencies proposal implementation Jan 6, 2020
@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch 3 times, most recently from 40aefdc to f76c789 Compare January 10, 2020 20:25
@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch from f76c789 to 6cd80ed Compare February 2, 2020 13:58
@@ -15,26 +15,56 @@ public final class ResolvedTarget: CustomStringConvertible, ObjectIdentifierProt

/// Represents dependency of a resolved target.
public enum Dependency: Hashable {
public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not possible to let compiler auto synthesize this?

Copy link
Contributor Author

@hartbit hartbit Feb 4, 2020

Choose a reason for hiding this comment

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

No, because we don't want to have equality on conditions.

Copy link
Contributor

@aciidgh aciidgh left a comment

Choose a reason for hiding this comment

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

I have minor nitpicky comments but it looks great! Thanks for working on it!

@hartbit hartbit force-pushed the conditional-target-dependencies-impl branch from 6cd80ed to 85a38c1 Compare February 4, 2020 18:55
@hartbit
Copy link
Contributor Author

hartbit commented Feb 4, 2020

@swift-ci please smoke test

@aciidgh aciidgh merged commit 16c2d07 into swiftlang:master Feb 4, 2020
@aciidgh aciidgh deleted the conditional-target-dependencies-impl branch February 4, 2020 20:17
aciidgh added a commit to hartbit/swift-package-manager that referenced this pull request Feb 4, 2020
…endencies-impl

Conditional target dependencies proposal implementation
@rvenable
Copy link

@jakepetroules Thanks for the answer! As it won't be able to officially support conditional target dependencies, @aciidb0mb3r suggested outputting a warning in those situations, which I have done. The implementation is now complete from my side.

So it sounds like .when(configuration: .debug) won't work at all for generating Xcode projects. Is that true? What about when just opening the Package.swift in Xcode without generating a project? Will Xcode be able to handle this? If so, will Xcode actually put flags into the build so that #if DEBUG conditions can be used in source code?

@E13Lau
Copy link

E13Lau commented Dec 8, 2020

@jakepetroules Thanks for the answer! As it won't be able to officially support conditional target dependencies, @aciidb0mb3r suggested outputting a warning in those situations, which I have done. The implementation is now complete from my side.

So it sounds like .when(configuration: .debug) won't work at all for generating Xcode projects. Is that true? What about when just opening the Package.swift in Xcode without generating a project? Will Xcode be able to handle this? If so, will Xcode actually put flags into the build so that #if DEBUG conditions can be used in source code?

Same question.How can i using .when(configuration: .debug) with iOS project? That's no way to edit the Package.swift.

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

5 participants