Skip to content

Add support for XcodeProjectPlugin #1621

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

Merged
merged 4 commits into from
Jul 4, 2023
Merged

Conversation

denil-ct
Copy link
Contributor

Carrying over the changes from apple/swift-protobuf#1408

Can anyone point me on how to test these changes for grpc as I'm not very familiar with grpc.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 23, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Collaborator

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. I just realised that we should update the documentation in both repos though. We should add two things:

  1. That now the config doesn't need to be at the root of the target anymore since we just search through the sources of a target
  2. We should state that how it works for an Xcode project.

@denil-ct
Copy link
Contributor Author

denil-ct commented Jun 26, 2023

That now the config doesn't need to be at the root of the target anymore since we just search through the sources of a target

One thing to note here is that the target directory is now calculated by deleting the last component of the config. So whatever folder the config resides in would become the target directory, and the file paths mentioned in the config should be relative to that.

@FranzBusch
Copy link
Collaborator

One thing to note here is that the target directory is now calculated by deleting the last component of the config. So whatever folder the config resides in would become the target directory, and the file paths mentioned in the config should be relative to that.

That's actually a very important call out! Thanks for stating this. We should clarify this in the Docc docs for the plugins. This is particularly important for the Xcode plugin since there the files can be all over the place.

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 3, 2023

@denil-ct could you bring over the changes from protobuf if you get a chance? We'd like to do a release soon and it would be nice to include this change 🙂

@denil-ct
Copy link
Contributor Author

denil-ct commented Jul 3, 2023

Hey @glbrntt , got a bit busy. Will make the changes by tomorrow.

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 3, 2023

No worries, thank you!

@denil-ct
Copy link
Contributor Author

denil-ct commented Jul 4, 2023

Hey @glbrntt , I'm facing some troubles verifying my changes.

I have this proto file

syntax = "proto3";

service NoteService {
    rpc Get (NoteRequestId) returns (Note) {}
}

message Note {
    string id = 1;
    string title = 2;
    string content = 3;
}

message NoteRequestId {
    string id = 1;
}

But after the Note.grpc.swift file gets generated, message types like NoteRequestId and Note are not in scope, which results in a compilation error when compiling Note.grpc.swift.

The plugin tries to execute this command

/opt/homebrew/bin/protoc --plugin=protoc-gen-grpc-swift=/${BUILD_DIR}/${CONFIGURATION}/protoc-gen-grpc-swift" --grpc-swift_out=/Users/denilct/Library/Developer/Xcode/DerivedData/ABCApp-gkihnhqrytzfbsbcqflpkdwowxum/SourcePackages/plugins/ABCApp.output/ABCApp/GRPCSwiftPlugin -I /Users/denilct/Documents/ABCApp_2/ABCApp_2 --grpc-swift_opt=Visibility=Public --grpc-swift_opt=Server=false --grpc-swift_opt=Client=true ProtoFiles/Notes/Note.proto

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 4, 2023

Are you running the protobuf plugin too? The grpc-swift plugin will only generate the service code, it won't generate the Note or NoteRequestId.

@denil-ct
Copy link
Contributor Author

denil-ct commented Jul 4, 2023

Ah, I see. Adding the protobuf plugin fixed it. Thanks.

@denil-ct
Copy link
Contributor Author

denil-ct commented Jul 4, 2023

You can review the changes. @glbrntt

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 looks good to me but could you run ./scripts/format.sh to run the formatter so that CI check passes?

@glbrntt glbrntt added 🔨 semver/patch No public API change. 🆕 semver/minor Adds new public API. and removed 🔨 semver/patch No public API change. labels Jul 4, 2023
@denil-ct
Copy link
Contributor Author

denil-ct commented Jul 4, 2023

Have added the changes done by swift-format.

@glbrntt glbrntt requested a review from FranzBusch July 4, 2023 12:41
@glbrntt glbrntt merged commit 473f466 into grpc:main Jul 4, 2023
@glbrntt
Copy link
Collaborator

glbrntt commented Jul 4, 2023

Thanks @denil-ct 🙏

@denil-ct denil-ct deleted the xcode-project-plugin branch July 4, 2023 12:56
WendellXY pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
pinlin168 pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants