-
Notifications
You must be signed in to change notification settings - Fork 14
Code generation build plugin #28
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
Changes from 1 commit
a5010cb
af7cfe0
256b3e9
1749e56
87197c6
585e856
b41a9eb
fe732a8
86f8893
15fbb1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -26,6 +26,10 @@ let products: [Product] = [ | |||||
name: "protoc-gen-grpc-swift", | ||||||
targets: ["protoc-gen-grpc-swift"] | ||||||
), | ||||||
.plugin( | ||||||
name: "GRPCGeneratorPlugin", | ||||||
targets: ["GRPCGeneratorPlugin"] | ||||||
), | ||||||
] | ||||||
|
||||||
let dependencies: [Package.Dependency] = [ | ||||||
|
@@ -101,6 +105,16 @@ let targets: [Target] = [ | |||||
], | ||||||
swiftSettings: defaultSwiftSettings | ||||||
), | ||||||
|
||||||
// Code generator build plugin | ||||||
.plugin( | ||||||
name: "GRPCGeneratorPlugin", | ||||||
capability: .buildTool(), | ||||||
dependencies: [ | ||||||
"protoc-gen-grpc-swift", | ||||||
|
"protoc-gen-grpc-swift", | |
.target(name: "protoc-gen-grpc-swift"), |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,79 @@ | ||||||
/* | ||||||
* Copyright 2024, gRPC Authors All rights reserved. | ||||||
glbrntt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
* | ||||||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||||||
* you may not use this file except in compliance with the License. | ||||||
* You may obtain a copy of the License at | ||||||
* | ||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||
* | ||||||
* Unless required by applicable law or agreed to in writing, software | ||||||
* distributed under the License is distributed on an "AS IS" BASIS, | ||||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
* See the License for the specific language governing permissions and | ||||||
* limitations under the License. | ||||||
*/ | ||||||
|
||||||
/// The configuration of the plugin. | ||||||
struct ConfigurationFile: Codable { | ||||||
|
||||||
/// The visibility of the generated files. | ||||||
enum Visibility: String, Codable { | ||||||
/// The generated files should have `internal` access level. | ||||||
case `internal` | ||||||
/// The generated files should have `public` access level. | ||||||
case `public` | ||||||
/// The generated files should have `package` access level. | ||||||
case `package` | ||||||
} | ||||||
|
||||||
/// The visibility of the generated files. | ||||||
|
||||||
var visibility: Visibility? | ||||||
/// Whether server code is generated. | ||||||
var server: Bool? | ||||||
/// Whether client code is generated. | ||||||
var client: Bool? | ||||||
/// Whether message code is generated. | ||||||
var message: Bool? | ||||||
// /// Whether reflection data is generated. | ||||||
// var reflectionData: Bool? | ||||||
|
||||||
/// Path to module map .asciipb file. | ||||||
var protoPathModuleMappings: String? | ||||||
|
||||||
/// Whether imports should have explicit access levels. | ||||||
var useAccessLevelOnImports: Bool? | ||||||
|
||||||
/// Specify the directory in which to search for | ||||||
/// imports. May be specified multiple times; | ||||||
/// directories will be searched in order. | ||||||
/// The target source directory is always appended | ||||||
/// to the import paths. | ||||||
var importPaths: [String]? | ||||||
|
||||||
|
||||||
/// The path to the `protoc` binary. | ||||||
/// | ||||||
/// If this is not set, SPM will try to find the tool itself. | ||||||
|
/// If this is not set, SPM will try to find the tool itself. | |
/// If this is not set, Swift Package Manager will try to find the tool itself. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just have one Visibility
type which is shared?
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,270 @@ | ||||||
/* | ||||||
* Copyright 2024, gRPC Authors All rights reserved. | ||||||
|
* Copyright 2024, gRPC Authors All rights reserved. | |
* Copyright 2025, gRPC Authors All rights reserved. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be impossible, right?
I also find it a bit weird that findApplicableConfigFor
returns the config file URL. Surely we only care about the config and that's what it should return?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've restructured this now to return the config object.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the optionality of all the config is useful to be honest. I think, where possible, we should make config values required but allow them to be unspecified in the JSON (i.e. use decodeIfPresent
).
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDK if an error is appropriate here; the user literally can't do anything about it. I think we should precondition
that the file naming is full path here. If it isn't then we've made a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the config restructuring this doesn't exist anymore.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here re: error, if this isn't set then we've screwed up and the user can't do anything about it.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here re: errors
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displayName: "Generating protobuf Swift files for \(inputFile.relativePath)", | |
displayName: "Generating Swift Protobuf files for \(inputFile.relativePath)", |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use a clearer name here (I know it's the name used for v1) -- this config is specific to protobuf code generation, so maybe grpc-swift-proto-generator-config.json
?
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../PluginsShared |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having plugin in the name is weird. I think we also want to nod to it being the generator for protobuf, so can we call it
GRPCProtobufGenerator
?