Skip to content

Conversation

@MrMage
Copy link
Collaborator

@MrMage MrMage commented Jul 29, 2019

Disable static stdlib linking for our protoc plugins.

Motivation:

Seems like these cause some issues these days; let's see if our users can confirm that removing this flag helps.

Modifications:

Remove the -Xswiftc -static-stdlib compiler flags.

Result:

The plugins will build correctly with both Swift 5 and Swift 5.1.

By removing ` -Xswiftc -static-stdlib`. Seems like these cause some issues these days; let's see if our users can confirm that removing this flag helps.
@Krivoblotsky
Copy link
Contributor

Krivoblotsky commented Jul 29, 2019

Worked out for me 👍

$ make plugin
swift build --product protoc-gen-swift -c release
[4/4] Linking protoc-gen-swift
swift build --product protoc-gen-swiftgrpc -c release
[2/2] Linking protoc-gen-swiftgrpc
cp .build/release/protoc-gen-swift .
cp .build/release/protoc-gen-swiftgrpc .

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Complete nit: can you use the commit template?

@MrMage
Copy link
Collaborator Author

MrMage commented Jul 29, 2019

Complete nit: can you use the commit template?

Done. How about adding this as a GitHub PR template instead of a commit template?

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 29, 2019

Possibly; can we get the PR description as the commit message when we merge it in?

@MrMage
Copy link
Collaborator Author

MrMage commented Jul 29, 2019

Possibly; can we get the PR description as the commit message when we merge it in?

Good question. I think if there's a single commit, GitHub populates the PR description with the commit message by default. No idea if that changes when there is a PR template, however.

OTOH it seems like GitHub's "edit file" feature does not respect our current commit description template.

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 29, 2019

That's right. I believe it appends the commit message to the template.

I'm fine with having a PR template alongside the commit tempalte; it will be useful for new/infrequent contributors. I think we should enforce the commit message when the change is large or non-trivial though.

@saudeoncovr
Copy link

I'll take a look at using this branch today to confirm that this works in our docker container.

@saudeoncovr
Copy link

Yep, working for our docker container!

swift build --product protoc-gen-swift -c release
Fetching https://github.com/apple/swift-nio.git
Fetching https://github.com/apple/swift-log
Fetching https://github.com/apple/swift-nio-transport-services.git
Fetching https://github.com/apple/swift-protobuf.git
Fetching https://github.com/kylef/Spectre.git
Fetching https://github.com/kylef/Commander.git
Fetching https://github.com/apple/swift-nio-ssl.git
Fetching https://github.com/apple/swift-nio-http2.git
Completed resolution in 5.44s
Cloning https://github.com/apple/swift-nio-http2.git
Resolving https://github.com/apple/swift-nio-http2.git at 1.2.1
Cloning https://github.com/kylef/Spectre.git
Resolving https://github.com/kylef/Spectre.git at 0.9.0
Cloning https://github.com/apple/swift-log
Resolving https://github.com/apple/swift-log at 1.1.0
Cloning https://github.com/apple/swift-protobuf.git
Resolving https://github.com/apple/swift-protobuf.git at 1.5.0
Cloning https://github.com/apple/swift-nio-ssl.git
Resolving https://github.com/apple/swift-nio-ssl.git at 2.1.0
Cloning https://github.com/apple/swift-nio.git
Resolving https://github.com/apple/swift-nio.git at 2.2.0
Cloning https://github.com/apple/swift-nio-transport-services.git
Resolving https://github.com/apple/swift-nio-transport-services.git at 1.0.3
Cloning https://github.com/kylef/Commander.git
Resolving https://github.com/kylef/Commander.git at 0.8.0
[1/4] Compiling Swift Module 'SwiftProtobuf' (79 sources)
[2/4] Compiling Swift Module 'SwiftProtobufPluginLibrary' (18 sources)
[3/4] Compiling Swift Module 'protoc_gen_swift' (18 sources)
[4/4] Linking ./.build/x86_64-unknown-linux/release/protoc-gen-swift
swift build --product protoc-gen-swiftgrpc -c release
[1/2] Compiling Swift Module 'protoc_gen_swiftgrpc' (8 sources)
[2/2] Linking ./.build/x86_64-unknown-linux/release/protoc-gen-swiftgrpc
cp .build/release/protoc-gen-swift .
cp .build/release/protoc-gen-swiftgrpc .

@MrMage MrMage marked this pull request as ready for review July 29, 2019 15:05
@MrMage MrMage merged commit 2001ac7 into nio Jul 29, 2019
@MrMage MrMage deleted the MrMage-patch-1-1 branch July 29, 2019 15:06
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.

5 participants