Change TargetGroup enum param to packageAccess bool in target declaration #6463
Conversation
elsh
requested review from
abertelrud,
neonichu,
tomerd and
MaxDesiatov
as code owners
…tion Resolves rdar://108361064
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci smoke test windows |
| @@ -253,6 +242,7 @@ public final class Target { | |||
| resources: [Resource]? = nil, | |||
| publicHeadersPath: String?, | |||
| type: TargetType, | |||
| packageAccess: Bool = false, | |||
There was a problem hiding this comment.
defaulting to false is surprising. most other arguments do not have a default value that impact's behavior so imo this should either not have a default
There was a problem hiding this comment.
The resources param above for example has a default value. This removes the need to set the value to false explicitly in Target initialization from static funcs in the earlier versions that do not have this param.
There was a problem hiding this comment.
I would prefer this does not have a default as it is driving behavior - just like TargetGroup did not have a default before
There was a problem hiding this comment.
that's reasonable; will make the change
| dependencies: [Dependency] = [], | ||
| path: String? = nil, | ||
| exclude: [String] = [], | ||
| sources: [String]? = nil, | ||
| resources: [Resource]? = nil, | ||
| publicHeadersPath: String? = nil, | ||
| packageAccess: Bool = true, |
There was a problem hiding this comment.
what does "true" means in this case? that it exposes it symbols to other modules? or that it "can see" other module symbols?
There was a problem hiding this comment.
It's both. If another target also has this param set to true, this target exposes its package symbols to that target and also can see package symbols from that target, depending on which target imports which.
There was a problem hiding this comment.
so do both modules (definition and consumer) need to be set to "true" for this feature to work?
There was a problem hiding this comment.
Yes. In the example below, A cannot access package symbols from B, but B can access package symbols from C. If A imports C, A still cannot access package symbols from C. If C had packageAccess = false, neither A nor B can access package symbols from C.
target A w/ packageAccess = false:
import B
target B w/ packageAccess = true:
import C
target C w/ packageAccess = true:
There was a problem hiding this comment.
Most target types have a default of true, right? So I would think of it the other way around, you set this to false in order to opt-out.
There was a problem hiding this comment.
It is set to true by default in the public static func that introduces this param. The Target initializer itself here has the param set to false because it's called by earlier versions of static funcs that should not have access to package symbols.
|
#6473 removes default param value |
Change TargetGroup enum param to packageAccess bool in target declaration
Resolves rdar://108361064