[swift-evolution] [swift-build-dev] Proposal: SwiftPM Target Access Control

[Ankit Agarwal]

On Wed, Jul 13, 2016 at 9:27 PM, Anders Bertelrud <anders at apple.com> wrote:

> Thanks for taking the initiative for this, Ankit.  It's a very welcome
> improvement.
>
> Comments inline.
>
> On 2016-07-12, at 11.15, Ankit Agarwal via swift-build-dev <
> swift-build-dev at swift.org> wrote:
>
> To mark a target as exported/public I propose PackageDescription's Target gains
> a flags property which would be a Set of the following Flag enum declared
> inside Target class:
>
> public enum Flag {
>     /// Makes the target public or "exported" for other packages to use.
>     case public}
>
> The Flag enum will be flexible in case we need to add more attributes in
> future as opposed to a boolean property to mark the public nature of the
> target.
>
> I would prefer that this be a boolean parameter rather than a generic
> `flags` parameter, since it makes the manifest read more naturally, and,
> importantly, is no less extensible than an enum.  Additional parameters
> with default values can as easily be added as more enum cases can, and in
> either case, a manifest written to assume the existence of `public` will be
> equally incompatible with older versions of the package manager.
>
> So, for example:
>
> let package = Package(
>    name: "FooLibrary",
>    targets: [
>        Target(name: "FooLibrary", public: true),
>        Target(name: "SampleCLI", dependencies: ["FooLibrary"]),
>    ])
>
> We can keep some obvious defaults for targets which can be implicitly
> public for e.g.
>
>    1. Package has only one target.
>    2. Target with same name as package.
>
> I'm a bit wary of magic here.  I think it would be clearer to have the
> manifest declare what is public and what is not.  With magic naming
> conventions it's too easy to accidentally change semantics just by renaming
> a target.
>

I agree that we should avoid too much magic, I think you missed the
sentence below those examples in the proposal where I mentioned that we
should avoid those instead. However as Daniel mentioned we don't want to
overcomplicate the manifest for simple packages and for beginners, keeping
that in mind we updated the proposed solution which you can find on the
proposal link (
https://github.com/aciidb0mb3r/swift-evolution/blob/swiftpm-module-access-control/proposals/xxxx-swiftpm-target-access-control.md
)


> I propose that enum Target.Dependency gains a new case External(package:
> String, target: String) to declare dependency on an external package's
> target.
>
> Since it's the same fundamental kind of dependency in either case, would
> it be better to have `package` be an optional parameter to Target?
>
> So that `Target(name: "Foo")` is local but `Target(name: "Foo", package:
> "Bar")` external?  That would seem more logical.
>
>
I like `Target(name: "Foo", package: "Bar")` but I prefer package name is
stated before the target name `Target(package: "Foo", name: "Bar")` but
then this gets weird and `Target(package:target)` is also weird so I chose
`External(package:target)` instead. However if people prefer
`Target(name:package)` more then I am fine with it.

-- 
Ankit
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160713/6049abc2/attachment-0001.html>