Skip to content

Conversation

@Lutzifer
Copy link
Contributor

@Lutzifer Lutzifer commented Jul 1, 2020

This implements the automatic Podspec Generation part for plugins as discussed in #802

As there was some discussion about Automating Releases (see #830 ) via Github Actions, I left the uploading part of the plugins to Github manual for now.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 1, 2020

CLA Check
The committers are authorized under a signed CLA.

Copy link
Contributor

@Jake-Prickett Jake-Prickett left a comment

Choose a reason for hiding this comment

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

Script changes look good! Would it be worth it to add any information about this in the Cocoapods Section of the README?

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.

Good start @Lutzifer -- I've left some comments and questions.

if self.module_name:
podspec += indent + "s.source = { :git => \"https://github.com/grpc/grpc-swift.git\", :tag => s.version }\n\n"
else:
podspec += indent + "s.source = { :http => \"https://github.com/grpc/grpc-swift/releases/download/#{s.version}/protoc-swift-plugins-#{s.version}.zip\"}\n\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to be more specific with the naming here: protoc-grpc-swift-plugins-VERSION-macOS-x86_64.zip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added -macOS-x86_64.zip for now, but it is not yet dynamic

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still missing grpc from the name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one also hasn't been resolved yet.

@glbrntt glbrntt added cocoapods Relates to CocoaPods nio 🔨 semver/patch No public API change. labels Jul 1, 2020
@Lutzifer
Copy link
Contributor Author

Lutzifer commented Jul 1, 2020

Caveat: In this configuration, the Licence File must be included in the zip file along with the two plugins.

s.homepage = 'https://www.grpc.io'
s.authors = { 'The gRPC contributors' => 'grpc-packages@google.com' }

s.source = { :http => "https://github.com/grpc/grpc-swift/releases/download/#{s.version}/protoc-swift-plugins-#{s.version}-macOS-x86_64.zip"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I think I've changed my position on this slightly: we need to wary of the fact that Macs on Apple Silicon are a thing. Either we need separate pods: or we should structure the zip appropriately and bundle them up together. What do you think @MrMage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think including binaries for both architectures in the zip either in folders or as fat binaries if possible would be better than having separate pods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change the name of the zip again, then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Unresolving since I'd like to hear @MrMage's opinion here)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Full-fledged Mac app bundles can be distributed as a "fat" binary containing both architectures. Would that be a possibility here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When looking at the project generated by "make project" I would dare to say that this should be possible. Have not tried it in xcode12 though.

Copy link
Collaborator

@glbrntt glbrntt Jul 3, 2020

Choose a reason for hiding this comment

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

Not sure if that would work or not. I don't think the details are important at the moment though; the relevant question is do we distribute them in a single zip? It seems like we have consensus that we should.

@Lutzifer Lutzifer requested a review from glbrntt July 2, 2020 12:37
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.

I'm pretty much happy with this. I'd just like to hear @MrMage's opinion before approving.

if self.module_name:
podspec += indent + "s.source = { :git => \"https://github.com/grpc/grpc-swift.git\", :tag => s.version }\n\n"
else:
podspec += indent + "s.source = { :http => \"https://github.com/grpc/grpc-swift/releases/download/#{s.version}/protoc-swift-plugins-#{s.version}.zip\"}\n\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still missing grpc from the name.

@@ -0,0 +1,15 @@
Pod::Spec.new do |s|
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has been useful to see in the review but I don't think we should publish it to master yet (since the zip doesn't exist and no pod has been pushed).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one hasn't been resolved yet.

Co-authored-by: George Barnett <gbrntt@gmail.com>
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 3, 2020
Motivation:

grpc#869 adds a plugin containing the
protoc binaries for CocoaPods. It relies on having a zip of the binaries
in the tagged release.

Modifications:

- Add a script to generate a zip of the protoc binaries.

Result:

We can generate a bundle of plugins which may be pulled in by a
CocoaPod.
@glbrntt glbrntt added semver/none No version bump required. and removed 🔨 semver/patch No public API change. labels Jul 3, 2020
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.

This is ready to go aside from a couple of comments which haven't been addressed yet.

@@ -0,0 +1,15 @@
Pod::Spec.new do |s|
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one hasn't been resolved yet.

if self.module_name:
podspec += indent + "s.source = { :git => \"https://github.com/grpc/grpc-swift.git\", :tag => s.version }\n\n"
else:
podspec += indent + "s.source = { :http => \"https://github.com/grpc/grpc-swift/releases/download/#{s.version}/protoc-swift-plugins-#{s.version}.zip\"}\n\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one also hasn't been resolved yet.

@Lutzifer Lutzifer requested a review from glbrntt July 3, 2020 12:06
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.

Great stuff, thanks @Lutzifer!

@glbrntt glbrntt merged commit 419b441 into grpc:master Jul 3, 2020
glbrntt added a commit that referenced this pull request Jul 3, 2020
Motivation:

#869 adds a plugin containing the
protoc binaries for CocoaPods. It relies on having a zip of the binaries
in the tagged release.

Modifications:

- Add a script to generate a zip of the protoc binaries.

Result:

We can generate a bundle of plugins which may be pulled in by a
CocoaPod.
IceRocky pushed a commit to IceRocky/grpc-swift that referenced this pull request May 28, 2024
Motivation:

grpc/grpc-swift#869 adds a plugin containing the
protoc binaries for CocoaPods. It relies on having a zip of the binaries
in the tagged release.

Modifications:

- Add a script to generate a zip of the protoc binaries.

Result:

We can generate a bundle of plugins which may be pulled in by a
CocoaPod.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cocoapods Relates to CocoaPods semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants