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

Support for executableTarget in the manifest, to allow use of @main in package targets #3045

Merged
merged 5 commits into from
Dec 9, 2020

Conversation

abertelrud
Copy link
Contributor

@abertelrud abertelrud commented Nov 12, 2020

Implementation of support for executableTarget in the manifest. This allows use of @main by having targets be explicitly declared as executable rather than by looking for particular file names.

One preliminary change compared with the evolution proposal is that, in order to make the upgrading from tools version 5.3 smoother, regular targets containing main.swift etc are still supported but cause a warning that the target declaration should be changed to executableTarget if the tools version is newer than 5.3. This is a bit more clear than silently treating the target as a library once the tools version is upgraded.

Motivation:

See https://forums.swift.org/t/se-0294-declaring-executable-targets-in-package-manifests/42404

Changes:

Implements the evolution proposal (including the warning mentioned above) but temporarily adds a guard on the environment variable SWIFTPM_ENABLE_EXECUTABLE_TARGETS until the proposal has been accepted.

@abertelrud abertelrud marked this pull request as draft November 12, 2020 23:44
@abertelrud abertelrud changed the title Draft: Support for executableTarget in the manifest, in service of supporting @main Draft: Support for executableTarget in the manifest, as a step toward supporting @main Nov 12, 2020
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@tomerd
Copy link
Contributor

tomerd commented Nov 13, 2020

this looks very nice 👍

@abertelrud
Copy link
Contributor Author

We should also consider whether swift package init --type=executable should generate one of these automatically. That's a bit more complicated, because at least for now, we certainly don't want to create packages that the most recently released SwiftPM can't build. So updating that will likely be a later PR.

@abertelrud
Copy link
Contributor Author

abertelrud commented Nov 21, 2020

Updated to main and resolved conflicts.

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud changed the title Draft: Support for executableTarget in the manifest, as a step toward supporting @main Support for executableTarget in the manifest, to allow use of @main in package targets Dec 7, 2020
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

…will allow executable package targets to use `@main`, since the targets will no longer be required to have a source file named `main.swift`.
…till recognize `.target()`s with main sources as executable targets in `.vNext`, but emit a warning asking to change to `.executableTarget()`. This should make it much clearer instead of just silently interpreting the target as a library.
…TABLE_TARGETS so we can land this and test it out even before the proposal is adopted. In practice this just gates the warning about use of `target()` for executables in post-5.3 manifests, since it's not possible to gate the availability of the new API for .vNext clients on an environment variable, and not really important to do so (it remains unavailable for 5.3 and earlier).
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud marked this pull request as ready for review December 8, 2020 23:40
@abertelrud
Copy link
Contributor Author

The evolution proposal runs until Dec 13, but I added support for a temporary environment variable SWIFTPM_ENABLE_EXECUTABLE_TARGETS so we could in theory land this and start testing it for real. There is no easy way to gate the availability of the API in the package manifest on the env var (it can be done but that's a lot more change that then has to be unwound again), so in practice this just enables the warning about implicitly discovered executable targets. The API changes are anyway gated on using the Next tools version for the package, so won't have any effect on existing packages.

@abertelrud
Copy link
Contributor Author

What remains is a Swift test fixture that uses @main and doesn't have main.swift, and a C test fixture that doesn't have main.c.

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

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

<3

@abertelrud abertelrud merged commit e92def3 into swiftlang:main Dec 9, 2020
@abertelrud abertelrud deleted the wip-executable-targets branch December 9, 2020 21:11
federicobucchi pushed a commit to federicobucchi/swift-package-manager that referenced this pull request Jan 6, 2021
…n` in package targets (swiftlang#3045)

* Correct the naming of the PackageDescription5_3LoadingTests test suite.

* Implement support for `executableTarget` in PackageDescription.  This will allow executable package targets to use `@main`, since the targets will no longer be required to have a source file named `main.swift`.

* In response to evolution proposal feedback, change the semantics to still recognize `.target()`s with main sources as executable targets in `.vNext`, but emit a warning asking to change to `.executableTarget()`.  This should make it much clearer instead of just silently interpreting the target as a library.

* Gated support for executable products on env var SWIFTPM_ENABLE_EXECUTABLE_TARGETS so we can land this and test it out even before the proposal is adopted.  In practice this just gates the warning about use of `target()` for executables in post-5.3 manifests, since it's not possible to gate the availability of the new API for .vNext clients on an environment variable, and not really important to do so (it remains unavailable for 5.3 and earlier).

* Added test fixture with use of `@main` and a unit test that tests that it builds.
@nashysolutions
Copy link

nashysolutions commented Apr 11, 2021

Does this address an issue I raised over at the following Forum regarding unnecessary dependencies being pulled?

Edit: Just read through the source and I can see that this probably doesn't fix this issue. Looks good though!

As you can see, I am wondering around GitHub like a nomad, trying to find out more. You guys know if this issue has been raised already?

@SDGGiesbrecht
Copy link
Contributor

I posted an answer back on the forums.

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

4 participants