Test-only dependencies


(Paul Cantrell) #1

I’ve been working on getting Siesta up and running with SwiftPM, and have hit snags running the tests. At Ankit Aggarwal’s encouragement, I’m opening a discussion thread here.

By “test-only dependencies,” I mean external packages required by a package’s tests, but not by the package's exported targets. For example, Siesta’s tests depend on Quick, Nimble, and Nocilla, but apps that use Siesta should not have to build those dependencies and would very much not want them baked into the resulting binary.

Proper support for test-only dependencies would have three essential features:

They are not downloaded or built by “swift build,” but are downloaded and built by “swift test.”
Any modules they define do not end up in the compiled binary, and are not available for the package’s non-test targets to import.
A package ignores the test-only dependencies of its dependencies.

Those are the essentials. Also nice to have:

The ability to limit test-only dependencies to specific test modules (e.g. an integration test module might pull in some heavyweight context that isn’t necessary or desirable for basic regression tests).
The ability to import / run tests defined in an external module. (Probably a separate discussion.)

To open the discussion, I see three general approaches:

Approach 1: Special testDependencies attribute

It looks like SwiftPM flirted with this, then abandoned it?

Pros: Clear.

Cons: Creates (another) special exception for tests. Doesn’t obviously generalize to handle the “specific test modules” nice-to-have above.

Approach 2: Per-target external dependencies

SwiftPM could allow individual targets to specify external dependencies, something like this:

    targets: [
      Target(
        name: "SiestaTests",
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ])
    ],

Pros: Covers the “essential features” above in a way that generalizes nicely to other interesting use cases beside testing.

Cons: Requires redundant dependency declarations across test targets.

Approach 3: Target groups

Bundler provides arbitrary logical gem groupings, which Rails uses to isolate dependencies to development, test, and/or production environments. For example:

    group :development, :test do
      gem 'byebug'
      gem 'spring'
    end

    group :test do
      gem 'minitest-spec-rails'
      gem 'factory_girl_rails'
    end

    group :development
      gem 'letter_opener'
    end

    group :production
      gem 'puma'
    end

Apps can then decide which groups to include depending on their runtime environment.

Pros: Gives the clarity & conciseness of #1 with the generality benefits of #2.

Cons: Lots of additional structure here, both explicit and implicit. Not clear how useful this would be in SwiftPM: because Ruby is so dynamic, all dependencies are runtime dependencies, so code can decide whether it’s “in test mode” or “in production” before looking for modules. In SwiftPM, all dependencies are compile-time dependencies, so arbitrary logical groups lose the benefits that come with runtime resolution.

···

________

I’m throwing all of this out there in a state of general ignorance about SwiftPM’s direction and status. Hopefully it spurs some useful discussion.

Cheers,

Paul


(Ankit Aggarwal) #2

Hi Paul,

Thanks for opening this discussion.

I’ve been working on getting Siesta up and running with SwiftPM, and have hit snags running the tests. At Ankit Aggarwal’s encouragement, I’m opening a discussion thread here.

By “test-only dependencies,” I mean external packages required by a package’s tests, but not by the package's exported targets. For example, Siesta’s tests depend on Quick, Nimble, and Nocilla, but apps that use Siesta should not have to build those dependencies and would very much not want them baked into the resulting binary.

I agree that building test specific dependencies is not useful for packages which just uses the some package, however I think that should be controlled by some other general mechanism like specifying exact external package/target the target in question depends on. That way all the unused targets in an external package will not build. See thread “Proposal: SwiftPM Target Access Control“ for some more discussion on this idea.

Proper support for test-only dependencies would have three essential features:

They are not downloaded or built by “swift build,” but are downloaded and built by “swift test.”
Any modules they define do not end up in the compiled binary, and are not available for the package’s non-test targets to import.
A package ignores the test-only dependencies of its dependencies.

I think instead of it being test-only dependency this could be a feature which lets you define dependencies to be used only by the current package (including test targets) which will be helpful for developers to try or play with some external dependency. And in that case it should probably be called something else like for e.g.: `localDependencies`

What do you think?

···

On 29-Aug-2016, at 1:36 AM, Paul Cantrell via swift-build-dev <swift-build-dev@swift.org> wrote:

Ankit


(Paul Cantrell) #3

I agree that building test specific dependencies is not useful for packages which just uses the some package, however I think that should be controlled by some other general mechanism like specifying exact external package/target the target in question depends on.

That’s what I had in mind with “Approach 2” in my original message. Approach 3 is a related but different alternative.

Please see remarks in the original message for my take on the pros and cons.

That way all the unused targets in an external package will not build. See thread “Proposal: SwiftPM Target Access Control“ for some more discussion on this idea.

That proposal is getting warm — especially if test packages are _not_ exposed by default, something it seems to me that proposal should specify.

The new “External” in that proposal has the right semantics, but seems to me to add yet more complexity to a package DSL whose readability is already a bit strained. In particular, this seems likely to cause confusion:

An external package dependency declaration implicitly becomes dependency of each target in the package. We propose this behaviour should be retained but if a target dependency contains an External declaration then all other targets which wants to use that external dependency should explicitly state their dependency on that external package using External.

Cheers,

Paul

···

On Aug 28, 2016, at 9:23 PM, Ankit Aggarwal <ankit_aggarwal@apple.com> wrote:


(Daniel Dunbar) #4

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

On the other hand, that has proven more complicated than approach #1, and is likely to take longer to get right. I wonder if there are reasons we might end up wanting #1 even if we had a working #2 (one reason might be if we couldn't solve the duplicate declaration problem).

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us, but I could probably be persuaded that the problem is pressing enough we should consider a more targeted fix sooner.

- Daniel

···

On Aug 28, 2016, at 7:48 PM, Paul Cantrell via swift-build-dev <swift-build-dev@swift.org> wrote:

On Aug 28, 2016, at 9:23 PM, Ankit Aggarwal <ankit_aggarwal@apple.com> wrote:

I agree that building test specific dependencies is not useful for packages which just uses the some package, however I think that should be controlled by some other general mechanism like specifying exact external package/target the target in question depends on.

That’s what I had in mind with “Approach 2” in my original message. Approach 3 is a related but different alternative.

Please see remarks in the original message for my take on the pros and cons.

That way all the unused targets in an external package will not build. See thread “Proposal: SwiftPM Target Access Control“ for some more discussion on this idea.

That proposal is getting warm — especially if test packages are _not_ exposed by default, something it seems to me that proposal should specify.

The new “External” in that proposal has the right semantics, but seems to me to add yet more complexity to a package DSL whose readability is already a bit strained. In particular, this seems likely to cause confusion:

An external package dependency declaration implicitly becomes dependency of each target in the package. We propose this behaviour should be retained but if a target dependency contains an External declaration then all other targets which wants to use that external dependency should explicitly state their dependency on that external package using External.

Cheers,

Paul

_______________________________________________
swift-build-dev mailing list
swift-build-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-build-dev


(Honza Dvorsky) #5

Not sure if already mentioned, but the issue of duplicated dependencies in
targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...",
majorVersion: 1)

let package = Package(
    name: "MyPkg",
    targets: [
        .Target(name: "First"),
        .Target(name: "Second", dependencies: ["First", sharedDependency]),
        .Target(name: "Third", dependencies: [sharedDependency])
    ]
)

That's assuming we'll be able to add external deps to targets.

- Honza

···

On Mon, Aug 29, 2016 at 7:07 AM Daniel Dunbar via swift-build-dev < swift-build-dev@swift.org> wrote:

I completely agree with your original email, and agree the target-access
control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one
that included some kind of solution for managing the duplicate declaration,
that would solve the con you list with #2. That might suggest that one
approach here is to add it the list of reasons we should do a per-target
dependency proposal.

On the other hand, that has proven more complicated than approach #1, and
is likely to take longer to get right. I wonder if there are reasons we
might end up wanting #1 even if we had a working #2 (one reason might be if
we couldn't solve the duplicate declaration problem).

My leaning here is towards trying to figure out a good approach for #2
first, and see where that leaves us, but I could probably be persuaded that
the problem is pressing enough we should consider a more targeted fix
sooner.

- Daniel

> On Aug 28, 2016, at 7:48 PM, Paul Cantrell via swift-build-dev < > swift-build-dev@swift.org> wrote:
>
>
>> On Aug 28, 2016, at 9:23 PM, Ankit Aggarwal <ankit_aggarwal@apple.com> > wrote:
>>
>> I agree that building test specific dependencies is not useful for
packages which just uses the some package, however I think that should be
controlled by some other general mechanism like specifying exact external
package/target the target in question depends on.
>
> That’s what I had in mind with “Approach 2” in my original message.
Approach 3 is a related but different alternative.
>
> Please see remarks in the original message for my take on the pros and
cons.
>
>> That way all the unused targets in an external package will not build.
See thread “Proposal: SwiftPM Target Access Control“ for some more
discussion on this idea.
>
> That proposal is getting warm — especially if test packages are _not_
exposed by default, something it seems to me that proposal should specify.
>
> The new “External” in that proposal has the right semantics, but seems
to me to add yet more complexity to a package DSL whose readability is
already a bit strained. In particular, this seems likely to cause confusion:
>
>> An external package dependency declaration implicitly becomes
dependency of each target in the package. We propose this behaviour should
be retained but if a target dependency contains an External declaration
then all other targets which wants to use that external dependency should
explicitly state their dependency on that external package using External.
>
> Cheers,
>
> Paul
>
> _______________________________________________
> swift-build-dev mailing list
> swift-build-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-build-dev

_______________________________________________
swift-build-dev mailing list
swift-build-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-build-dev


(Daniel Dunbar) #6

Not sure if already mentioned, but the issue of duplicated dependencies in targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...", majorVersion: 1)

let package = Package(
    name: "MyPkg",
    targets: [
        .Target(name: "First"),
        .Target(name: "Second", dependencies: ["First", sharedDependency]),
        .Target(name: "Third", dependencies: [sharedDependency])
    ]
)

That's assuming we'll be able to add external deps to targets.

I'm not sure that is a syntax we want to support per the long term plans to split the manifest into a "leading package specification" which, while Swift, is a restricted subset we know we can parse into a machine editable form.

I would like it if the syntax for all common things was representable as a readable declarative initializer.

- Daniel

···

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com> wrote:

- Honza

On Mon, Aug 29, 2016 at 7:07 AM Daniel Dunbar via swift-build-dev <swift-build-dev@swift.org <mailto:swift-build-dev@swift.org>> wrote:
I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

On the other hand, that has proven more complicated than approach #1, and is likely to take longer to get right. I wonder if there are reasons we might end up wanting #1 even if we had a working #2 (one reason might be if we couldn't solve the duplicate declaration problem).

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us, but I could probably be persuaded that the problem is pressing enough we should consider a more targeted fix sooner.

- Daniel

> On Aug 28, 2016, at 7:48 PM, Paul Cantrell via swift-build-dev <swift-build-dev@swift.org <mailto:swift-build-dev@swift.org>> wrote:
>
>
>> On Aug 28, 2016, at 9:23 PM, Ankit Aggarwal <ankit_aggarwal@apple.com <mailto:ankit_aggarwal@apple.com>> wrote:
>>
>> I agree that building test specific dependencies is not useful for packages which just uses the some package, however I think that should be controlled by some other general mechanism like specifying exact external package/target the target in question depends on.
>
> That’s what I had in mind with “Approach 2” in my original message. Approach 3 is a related but different alternative.
>
> Please see remarks in the original message for my take on the pros and cons.
>
>> That way all the unused targets in an external package will not build. See thread “Proposal: SwiftPM Target Access Control“ for some more discussion on this idea.
>
> That proposal is getting warm — especially if test packages are _not_ exposed by default, something it seems to me that proposal should specify.
>
> The new “External” in that proposal has the right semantics, but seems to me to add yet more complexity to a package DSL whose readability is already a bit strained. In particular, this seems likely to cause confusion:
>
>> An external package dependency declaration implicitly becomes dependency of each target in the package. We propose this behaviour should be retained but if a target dependency contains an External declaration then all other targets which wants to use that external dependency should explicitly state their dependency on that external package using External.
>
> Cheers,
>
> Paul
>
> _______________________________________________
> swift-build-dev mailing list
> swift-build-dev@swift.org <mailto:swift-build-dev@swift.org>
> https://lists.swift.org/mailman/listinfo/swift-build-dev

_______________________________________________
swift-build-dev mailing list
swift-build-dev@swift.org <mailto:swift-build-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-build-dev


(Paul Cantrell) #7

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us

I’m not up on all the discussion, but that certainly seems to me like the right direction.

···

On Aug 29, 2016, at 12:07 AM, Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Aug 29, 2016, at 10:36 AM, Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com <mailto:jan.dvorsky@me.com>> wrote:

Not sure if already mentioned, but the issue of duplicated dependencies in targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...", majorVersion: 1)

        .Target(name: "Second", dependencies: ["First", sharedDependency]),

I'm not sure that is a syntax we want to support per the long term plans to split the manifest into a "leading package specification" which, while Swift, is a restricted subset we know we can parse into a machine editable form.

It’s a tangent, but … doesn’t that defeat the advantage of having the syntax be Swift, namely being able to execute code within the package declaration to eliminate redundancy and conditionally declare things?

The package syntax is already a bit hard on the eyes, and quite finicky. (When do you use “.”? What is the correct order for all those named args?) If it’s going to be a bespoke syntax anyway, I’d vote for abandoning Swift syntax altogether and focusing on readability.

(Apologies if I’m reopening a well-trodden discussion.)

Cheers,

Paul


(Honza Dvorsky) #8

Good point, I forgot about that.

···

On Mon, Aug 29, 2016 at 5:36 PM Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com> wrote:

Not sure if already mentioned, but the issue of duplicated dependencies in
targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...",
majorVersion: 1)

let package = Package(
    name: "MyPkg",
    targets: [
        .Target(name: "First"),
        .Target(name: "Second", dependencies: ["First", sharedDependency]),
        .Target(name: "Third", dependencies: [sharedDependency])
    ]
)

That's assuming we'll be able to add external deps to targets.

I'm not sure that is a syntax we want to support per the long term plans
to split the manifest into a "leading package specification" which, while
Swift, is a restricted subset we know we can parse into a machine editable
form.

I would like it if the syntax for all common things was representable as a
readable declarative initializer.

- Daniel

- Honza

On Mon, Aug 29, 2016 at 7:07 AM Daniel Dunbar via swift-build-dev < > swift-build-dev@swift.org> wrote:

I completely agree with your original email, and agree the target-access
control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one
that included some kind of solution for managing the duplicate declaration,
that would solve the con you list with #2. That might suggest that one
approach here is to add it the list of reasons we should do a per-target
dependency proposal.

On the other hand, that has proven more complicated than approach #1, and
is likely to take longer to get right. I wonder if there are reasons we
might end up wanting #1 even if we had a working #2 (one reason might be if
we couldn't solve the duplicate declaration problem).

My leaning here is towards trying to figure out a good approach for #2
first, and see where that leaves us, but I could probably be persuaded that
the problem is pressing enough we should consider a more targeted fix
sooner.

- Daniel

> On Aug 28, 2016, at 7:48 PM, Paul Cantrell via swift-build-dev < >> swift-build-dev@swift.org> wrote:
>
>
>> On Aug 28, 2016, at 9:23 PM, Ankit Aggarwal <ankit_aggarwal@apple.com> >> wrote:
>>
>> I agree that building test specific dependencies is not useful for
packages which just uses the some package, however I think that should be
controlled by some other general mechanism like specifying exact external
package/target the target in question depends on.
>
> That’s what I had in mind with “Approach 2” in my original message.
Approach 3 is a related but different alternative.
>
> Please see remarks in the original message for my take on the pros and
cons.
>
>> That way all the unused targets in an external package will not build.
See thread “Proposal: SwiftPM Target Access Control“ for some more
discussion on this idea.
>
> That proposal is getting warm — especially if test packages are _not_
exposed by default, something it seems to me that proposal should specify.
>
> The new “External” in that proposal has the right semantics, but seems
to me to add yet more complexity to a package DSL whose readability is
already a bit strained. In particular, this seems likely to cause confusion:
>
>> An external package dependency declaration implicitly becomes
dependency of each target in the package. We propose this behaviour should
be retained but if a target dependency contains an External declaration
then all other targets which wants to use that external dependency should
explicitly state their dependency on that external package using External.
>
> Cheers,
>
> Paul
>
> _______________________________________________
> swift-build-dev mailing list
> swift-build-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-build-dev

_______________________________________________
swift-build-dev mailing list
swift-build-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-build-dev


(Daniel Dunbar) #9

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us

I’m not up on all the discussion, but that certainly seems to me like the right direction.

Not sure if already mentioned, but the issue of duplicated dependencies in targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...", majorVersion: 1)

        .Target(name: "Second", dependencies: ["First", sharedDependency]),

I'm not sure that is a syntax we want to support per the long term plans to split the manifest into a "leading package specification" which, while Swift, is a restricted subset we know we can parse into a machine editable form.

It’s a tangent, but … doesn’t that defeat the advantage of having the syntax be Swift, namely being able to execute code within the package declaration to eliminate redundancy and conditionally declare things?

No, the intent was to support both cases, but to prioritize the leading package specification being able to describe most scenarios. See:
  https://github.com/apple/swift-package-manager/blob/master/Documentation/Internals/SwiftBasedManifestFormat.md

The package syntax is already a bit hard on the eyes, and quite finicky. (When do you use “.”? What is the correct order for all those named args?) If it’s going to be a bespoke syntax anyway, I’d vote for abandoning Swift syntax altogether and focusing on readability.

I am pretty happy with the decision to use Swift, and don't see a reason to abandon it here.

What I think is true is that we need to revisit exactly how the APIs are spelled to be more consistent with Swift3. That is part of the things we know we need to tackle next. We also hope to eventually be able to automatically help manage the manifest, which I believe will mitigate this problem.

If you have concrete examples of things you think are finicky or spelled badly, that is worth raising separately and feeding into a discussion of cleaning up the manifest API.

- Daniel

···

On Aug 29, 2016, at 9:00 AM, Paul Cantrell <cantrell@pobox.com> wrote:

On Aug 29, 2016, at 12:07 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:
On Aug 29, 2016, at 10:36 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com <mailto:jan.dvorsky@me.com>> wrote:

(Apologies if I’m reopening a well-trodden discussion.)

Cheers,

Paul


(Paul Cantrell) #10

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us

I’m not up on all the discussion, but that certainly seems to me like the right direction.

One way to remove the redundant declarations from #2:

(1) Allow virtual targets that have no corresponding source directory, but declare external dependencies.

(2) Allow other targets to depend on virtual targets.

So, for this directory structure:

    Source/
        ...
    Tests/
        FooTests/
        BarTests/

…a manifest could declare:

    targets: [
      Target(
        name: "Tests", // Note that /Tests/Tests does not exist!
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ]),
      Target(
        name: "FooTests",
        dependencies: ["Tests"])
      Target(
        name: "BarTests",
        dependencies: ["Tests"]),
    ],

To that, you could also add:

(3) All test targets implicitly depend on a virtual target named “Tests.”

…which would allow this simplification:

    targets: [
      Target(
        name: "Tests”,
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ])
    ],

Note that there’s no longer any need to override convention-based defaults for FooTests and BarTests.

What do you think?

Not sure if already mentioned, but the issue of duplicated dependencies in targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...", majorVersion: 1)

        .Target(name: "Second", dependencies: ["First", sharedDependency]),

I'm not sure that is a syntax we want to support per the long term plans to split the manifest into a "leading package specification" which, while Swift, is a restricted subset we know we can parse into a machine editable form.

It’s a tangent, but … doesn’t that defeat the advantage of having the syntax be Swift, namely being able to execute code within the package declaration to eliminate redundancy and conditionally declare things?

No, the intent was to support both cases, but to prioritize the leading package specification being able to describe most scenarios. See:
  https://github.com/apple/swift-package-manager/blob/master/Documentation/Internals/SwiftBasedManifestFormat.md

Thanks for the link. The “Editor Support” section did clarify things quite a bit.

If you have concrete examples of things you think are finicky or spelled badly, that is worth raising separately and feeding into a discussion of cleaning up the manifest API.

The obvious one is knowing which things are enums and which are structs. It’s hard to know when to include the leading dots.

I’ll give this some separate thought in another thread, as you suggest.

Cheers,

Paul

···

On Aug 29, 2016, at 11:10 AM, Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Aug 29, 2016, at 9:00 AM, Paul Cantrell <cantrell@pobox.com <mailto:cantrell@pobox.com>> wrote:

On Aug 29, 2016, at 12:07 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:
On Aug 29, 2016, at 10:36 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com <mailto:jan.dvorsky@me.com>> wrote:


(Daniel Dunbar) #11

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us

I’m not up on all the discussion, but that certainly seems to me like the right direction.

One way to remove the redundant declarations from #2:

(1) Allow virtual targets that have no corresponding source directory, but declare external dependencies.

(2) Allow other targets to depend on virtual targets.

This works, but would it be obvious to newcomers? It feels more like a hack rather than something we would explicitly want to be the definitive model for solving the sharing problem.

I do think that allowing such kinds of targets is something that may make sense, I'm just not sure it is the right answer to the redundant declaration.

So, for this directory structure:

    Source/
        ...
    Tests/
        FooTests/
        BarTests/

…a manifest could declare:

    targets: [
      Target(
        name: "Tests", // Note that /Tests/Tests does not exist!
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ]),
      Target(
        name: "FooTests",
        dependencies: ["Tests"])
      Target(
        name: "BarTests",
        dependencies: ["Tests"]),
    ],

To that, you could also add:

(3) All test targets implicitly depend on a virtual target named “Tests.”

…which would allow this simplification:

    targets: [
      Target(
        name: "Tests”,
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ])
    ],

Note that there’s no longer any need to override convention-based defaults for FooTests and BarTests.

What do you think?

Not sure if already mentioned, but the issue of duplicated dependencies in targets can be easily handled like this:

import PackageDescription

let sharedDependency: Package.Dependency = .Package(url: "...", majorVersion: 1)

        .Target(name: "Second", dependencies: ["First", sharedDependency]),

I'm not sure that is a syntax we want to support per the long term plans to split the manifest into a "leading package specification" which, while Swift, is a restricted subset we know we can parse into a machine editable form.

It’s a tangent, but … doesn’t that defeat the advantage of having the syntax be Swift, namely being able to execute code within the package declaration to eliminate redundancy and conditionally declare things?

No, the intent was to support both cases, but to prioritize the leading package specification being able to describe most scenarios. See:
  https://github.com/apple/swift-package-manager/blob/master/Documentation/Internals/SwiftBasedManifestFormat.md

Thanks for the link. The “Editor Support” section did clarify things quite a bit.

If you have concrete examples of things you think are finicky or spelled badly, that is worth raising separately and feeding into a discussion of cleaning up the manifest API.

The obvious one is knowing which things are enums and which are structs. It’s hard to know when to include the leading dots.

Yes, that is an example of something we know is broken (especially post Swift3 rename) but haven't had time to clean up yet.

- Daniel

···

On Sep 6, 2016, at 1:36 PM, Paul Cantrell <cantrell@pobox.com> wrote:

On Aug 29, 2016, at 11:10 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Aug 29, 2016, at 9:00 AM, Paul Cantrell <cantrell@pobox.com <mailto:cantrell@pobox.com>> wrote:

On Aug 29, 2016, at 12:07 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:
On Aug 29, 2016, at 10:36 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Aug 29, 2016, at 2:35 AM, Honza Dvorsky <jan.dvorsky@me.com <mailto:jan.dvorsky@me.com>> wrote:

I’ll give this some separate thought in another thread, as you suggest.

Cheers,

Paul


(Paul Cantrell) #12

[resend; forgot to copy list]

I completely agree with your original email, and agree the target-access control proposal amounts to a variant of #2.

We definitely need a per-target dependency solution, and if we got one that included some kind of solution for managing the duplicate declaration, that would solve the con you list with #2. That might suggest that one approach here is to add it the list of reasons we should do a per-target dependency proposal.

My leaning here is towards trying to figure out a good approach for #2 first, and see where that leaves us

I’m not up on all the discussion, but that certainly seems to me like the right direction.

One way to remove the redundant declarations from #2:

(1) Allow virtual targets that have no corresponding source directory, but declare external dependencies.

(2) Allow other targets to depend on virtual targets.

This works, but would it be obvious to newcomers? It feels more like a hack rather than something we would explicitly want to be the definitive model for solving the sharing problem.

I do think that allowing such kinds of targets is something that may make sense, I'm just not sure it is the right answer to the redundant declaration.

It feels like the a natural solution to me. I mean, it is the first one that came to mind….

In particular, this syntax, likely to be copied around and posted on Stack Overflow more than once, strikes me as reasonably self-explanatory even if you don’t know how it generalizes to arbitrary virtual targets:

    targets: [
      Target(
        name: "Tests”,
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ])
    ],

One small thing that might help with newcomer friendliness: allow (or require?) virtual targets to have a “group” arg instead of “name:”

    targets: [
      Target(
        group: "Tests”,
        dependencies: [
          .Package(url: "https://github.com/Quick/Quick", majorVersion: 0)
        ])
    ],

That does feel a bit hackish to me, though. I’d rather the package manifest have as few constructs and as few twists as possible.

Cheers, P

···

On Sep 6, 2016, at 4:27 PM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Sep 6, 2016, at 1:36 PM, Paul Cantrell <cantrell@pobox.com <mailto:cantrell@pobox.com>> wrote:

On Aug 29, 2016, at 11:10 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Aug 29, 2016, at 9:00 AM, Paul Cantrell <cantrell@pobox.com <mailto:cantrell@pobox.com>> wrote:

On Aug 29, 2016, at 12:07 AM, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote: