Fix DescriptionPackage resolves targets not exported via products

[dmhts]

Issue

The DescriptionPackage in createPackage mode may currently export internal targets that are not intended to be exported via products. This occurs because PackageBuilder implicitly generates additional products in ResolvedPackage.products for executable and test targets (in addition to the actually exported products). These generated products may depend on internal targets that must not be built by Scipio.

Solution

Unlike ResolvedPackage, the Manifest object accurately reflects all the original manifest properties and is not affected by the issue/feature described above. I suggest using the products from Manifest instead of the ones from ResolvedPackage.

Example

Swift-DocC Plugin exports two products: "Swift-DocC" and "Swift-DocC Preview", but Scipio in the current implementation resolves (for the reason described above) 4 products:

po package.products
▿ 4 elements
  ▿ 0 : <Product: Swift-DocC Preview>
  ▿ 1 : <Product: Swift-DocC>
  ▿ 2 : <Product: SwiftDocCPluginPackageTests>
  ▿ 3 : <Product: snippet-extract>

The last two products SwiftDocCPluginPackageTests and snippet-extract are test and executable targets respectively that were implicitly generated by SwiftPM. In addition, these generated products export some internal targets that should be ignored and not pre-built by Scipio.

Detailed description

products for ResolvedPackage are constructed in PackageBuilder.constructProducts. During the product construction process the builder treats test and executable targets in a special way, including them in the resulting product arrays, namely:

• An implicit product for test targets with the PackageTests prefix is appended to the resulting products .
• An implicit product is generated for executable targets.

Questions

• @giginet Would it be fine to open PRs for similar issues right away in the future, or would it be better to discuss them in Issues first?

[giginet]

Thank you for this contribution!
Never mind making a new issue about this.

This PR seems to be almost good, but I found some minor points. Could you resolve them?

I'll check this patch for our product and merge this after them.

[giginet]

Could you just add a trailing comma here to follow the SwiftLint violation.

Suggested change

	"TestingPackage"
	"TestingPackage",

[dmhts]

Of course, it's fixed now. I forgot to run the linter locally.

[giginet]

The existing test might aim to test whether it can extract binary targets correctly so it's better to remain the existing tests.

Could you make a new test case for this change and rename the existing tests to testBuildProductsForBinaryPackage or something?

[dmhts]

Sure, my reasoning was that BinaryPackage was already involved in the integration tests, unlike TestingPackage, so I felt safer replacing the former. Of course, keeping the previous test in place makes sense for extra peace of mind. Just fixed that.

[dmhts]

Let me know if you'd like to change the test name. I thought a package couldn't really be binary, unlike its targets (or wrapping them build products).

[dmhts]

By the way, I also found that this PR fixes the issue of violating Hashable's requirements in debug builds. My guess is that it was somehow related to these implicitly generated products. I'll close the issue for now and keep an eye on it.

[giginet]

Thank you for everything!

I'll check this PR for our product next week and I'll merge this and publish a new release

[giginet]

Released as 0.20.3 🎉
https://github.com/giginet/Scipio/releases/tag/0.20.3