Skip to content

Implement SE-208 System Library Targets#1586

Merged
aciidgh merged 1 commit into
swiftlang:masterfrom
aciidgh:system-packages
May 30, 2018
Merged

Implement SE-208 System Library Targets#1586
aciidgh merged 1 commit into
swiftlang:masterfrom
aciidgh:system-packages

Conversation

@aciidgh

@aciidgh aciidgh commented May 29, 2018

Copy link
Copy Markdown
Contributor

rdar://problem/39418939 [SR-7434]: Implement SE-208 Package Manager System Library Targets

@aciidgh aciidgh requested a review from hartbit May 29, 2018 20:35
@aciidgh aciidgh force-pushed the system-packages branch 6 times, most recently from ec1d0e9 to 43af9f6 Compare May 29, 2018 22:08
<rdar://problem/39418939> [SR-7434]: Implement SE-208 Package Manager System Library Targets
@aciidgh aciidgh force-pushed the system-packages branch from 43af9f6 to a06eb21 Compare May 29, 2018 22:50
@aciidgh aciidgh changed the title [WIP] Implement SE-208 System Library Targets Implement SE-208 System Library Targets May 29, 2018
@aciidgh

aciidgh commented May 29, 2018

Copy link
Copy Markdown
Contributor Author

@swift-ci please smoke test

@aciidgh aciidgh merged commit 40f2b4b into swiftlang:master May 30, 2018
@aciidgh aciidgh deleted the system-packages branch May 30, 2018 00:01
let implicitSystemTargetDeps = packageBuilder.dependencies
.flatMap({ $0.targets })
.filter({ $0.target.type == .systemModule })
.filter({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we simplify this to ($0.target as? SystemLibraryTarget)?.isImplicit ?? false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't call one or the other a simplified version, heh.

return nil
}
return SystemLibraryTarget(
name: potentialModule.name,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: indentation

struct SystemPackageProductValidationDiagnostic: DiagnosticData {
static let id = DiagnosticID(
type: SystemPackageProductValidationDiagnostic.self,
name: "org.swift.diags.pkg-builder.sys-pkg-product-validatoin",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo: validation

let targets: [String]
}

struct SystemPackageProductValidationDiagnostic: DiagnosticData {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand this diagnostic. Can you explain?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

System library products shouldn't have a type (automatic/static/dynamic) and should contains only one target.

// Peform special validations if this product is exporting
// a system library target.
if targets.contains(where: { $0 is SystemLibraryTarget }) {
if type != .library(.automatic) || targets.count != 1 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor comment: I find the double-ifs hard to read (perhaps that's just me).

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.

2 participants