Code generation build plugin #28

rnro · 2025-01-08T13:53:57Z

Overview

New build plugin to generate gRPC services and protobuf messages

The SwiftPM build plugin will locate protobuf files in the Sources directory (with the extension .proto) and attempt to invoke both the protoc-gen-swift and protoc-gen-grpc-swift protoc plugins on them to automatically generate Swift source. Behavior can be modified by specifying one or more configuration files (named grpc-swift-config.json).

Configuration

The configuration file is JSON which may contain the following entries.

/// The configuration of the plugin.
struct ConfigurationFile: Codable {
  enum Visibility: String, Codable {            /// The visibility of the generated files.
    case `internal`                             /// The generated files should have `internal` access level.
    case `public`                               /// The generated files should have `public` access level.
    case `package`                              /// The generated files should have `package` access level.
  }

  var visibility: Visibility?                   /// The visibility of the generated files.
  var server: Bool?                             /// Whether server code is generated.
  var client: Bool?                             /// Whether client code is generated.
  var message: Bool?                            /// Whether message code is generated.
  var protoPathModuleMappings: String?          /// Path to module map .asciipb file.
  var useAccessLevelOnImports: Bool?            /// Whether imports should have explicit access levels.

  /// 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]?

  var protocPath: String?                       /// The path to the `protoc` binary. If this is not set, SPM will try to find the tool itself.
}

For a given protobuf definition file the tool will search for configuration files in the same and all parent directories and will use the file lowest in the hierarchy.
Most configuration if not specified will use the protoc-plugin's own defaults.

Adoption

Users must add the plugin to any target which wishes to make use of it

  targets: [
    .executableTarget(
      name: "plugin-adopter",
      dependencies: [
        // ...
      ],
      plugins: [
        .plugin(name: "GRPCGeneratorPlugin", package: "grpc-swift-protobuf")
      ]
    )
  ]

This PR is split out of #26

Motivation: To make code generation more convenient for adopters. Modifications: * New build plugin to generate gRPC services and protobuf messages Result: * Users will be able to make use of the build plugin.

glbrntt · 2025-01-08T14:18:02Z

Package.swift

+    name: "GRPCGeneratorPlugin",
+    targets: ["GRPCGeneratorPlugin"]


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?

glbrntt · 2025-01-08T14:18:42Z

Package.swift

+    name: "GRPCGeneratorPlugin",
+    capability: .buildTool(),
+    dependencies: [
+      "protoc-gen-grpc-swift",


nit, for consistency:

Suggested change

"protoc-gen-grpc-swift",

.target(name: "protoc-gen-grpc-swift"),

Plugins/GRPCGeneratorPlugin/ConfigurationFile.swift

glbrntt · 2025-01-08T14:21:08Z

Plugins/GRPCGeneratorPlugin/ConfigurationFile.swift

+ */
+
+/// The configuration of the plugin.
+struct ConfigurationFile: Codable {


nit for consistency: gRPC uses "config" instead of "configuration".

"File" is also out of place here; we should nod to its use for generation here instead. "GeneratorConfig"? "PluginConfig"? "Config"?

So "File" is in the name because it corresponds specifically to the format of the configuration file on disk which is why it is Codable, there is an upcoming structure which knows about configuration which is passed as flags and the common configuration representation is already in this PR. I'm not sure how that fits into your existing naming schemes.

It being on disk is incidental though. The name should indicate what it's config for not where the config comes from.

If plugins were (much) more flexible you could imagine getting the config from e.g. a config service. Using the name ConfigFile for that type would be weird!

The original intent was to separate the on-disk format from the internal representation but I think combined with the abstraction we'll have for the more general config when combined with the command plugin that would lead to too much layering in this small application since none of it is public API.

glbrntt · 2025-01-08T14:21:28Z

Plugins/GRPCGeneratorPlugin/ConfigurationFile.swift

+  //  /// Whether reflection data is generated.
+  //  var reflectionData: Bool?


We can get rid of this as it's commented out

We can if you'd prefer - I didn't know how imminent the use of reflection data would be

glbrntt · 2025-01-08T15:04:35Z

Plugins/GRPCGeneratorPlugin/Plugin.swift

+  )
+
+  return Command.buildCommand(
+    displayName: "Generating protobuf Swift files for \(inputFile.relativePath)",


Suggested change

displayName: "Generating protobuf Swift files for \(inputFile.relativePath)",

displayName: "Generating Swift Protobuf files for \(inputFile.relativePath)",

glbrntt · 2025-01-08T15:05:53Z

Plugins/PluginsShared/PluginUtils.swift

+  // TODO: Don't currently support implementation only imports
+  //  // Add the implementation only imports flag if it was set
+  //  if let implementationOnlyImports = config.implementationOnlyImports {
+  //      protocArgs.append("--swift_opt=ImplementationOnlyImports=\(implementationOnlyImports)")
+  //  }


Let's remove this

glbrntt · 2025-01-08T15:09:21Z

Plugins/PluginsShared/PluginUtils.swift

+
+  if let importPaths = config.importPaths {
+    for path in importPaths {
+      protocArgs.append("-I")


-I is a synonym for --proto_path which we also set below, why do we have both and have two source of values for them? Also, the protoc-gen-swift args only have --proto_path it seems. Somethin' ain't right here!

The two sources of values is because one is user-specified and one is a default one which we add based on the path to the proto file we're currently processing. I've made them both use --proto_path and fixed up the missing source.

glbrntt · 2025-01-08T15:09:29Z

Plugins/PluginsShared/PluginUtils.swift

+  // TODO: Don't currently support reflection data
+  //  if let generateReflectionData = config.reflectionData {
+  //    protocArgs.append("--grpc-swift_opt=ReflectionData=\(generateReflectionData)")
+  //  }


Let's remove this

glbrntt · 2025-01-08T15:11:51Z

Plugins/PluginsShared/PluginUtils.swift

+///   - outputDirectory: The directory in which generated source files are created.
+///   - outputExtension: The file extension to be appended to generated files in-place of `.proto`.
+/// - Returns: The expected output file path.
+func deriveOutputFilePath(


IIRC the output file path is only needed by the build plugin which can only use the .fullPath file naming. If so we can simplify this, right?

glbrntt · 2025-01-14T15:16:57Z

Plugins/GRPCProtobufGenerator/BuildPluginConfig.swift

+  let defaultVisibility: GenerationConfig.Visibility = .internal
+  let defaultServer = true
+  let defaultClient = true
+  let defaultMessage = true
+  let defaultUseAccessLevelOnImports = false
+  let defaultImportPaths: [String] = []


We don't want to carry around the defaults on each instance, we should just have a static instance:

static let defaults = Self(...)

glbrntt · 2025-01-14T15:18:12Z

Plugins/GRPCProtobufGenerator/BuildPluginConfig.swift

+}
+
+extension GenerationConfig {
+  init(configurationFile: BuildPluginConfig, configurationFilePath: URL, outputPath: URL) {


nit: s/configuration/config

glbrntt · 2025-01-14T15:18:55Z

Plugins/GRPCProtobufGenerator/BuildPluginConfig.swift

+    // hard-code full-path to avoid collisions since this goes into a temporary directory anyway
+    self.fileNaming = .fullPath
+    self.useAccessLevelOnImports = configurationFile.useAccessLevelOnImports
+    self.importPaths = []


We set this below

Plugins/GRPCProtobufGenerator/BuildPluginConfig.swift

Plugins/GRPCProtobufGenerator/Plugin.swift

glbrntt · 2025-01-14T16:19:07Z

Plugins/PluginsShared/GenerationConfig.swift

+  /// Whether client code is generated.
+  var client: Bool
+  /// Whether message code is generated.
+  var message: Bool


nit: wondering whether this should be messages? WDYT?

I have changed all of the generation options to be plural and restructured the config to have a nested structure.

{ "generate": { "clients": true, "servers": true, "messages": true, }, "generatedSource": { "accessLevelOnImports": false, "accessLevel": "internal", } "protoc": { "executablePath": "/opt/homebrew/bin/protoc" "importPaths": [ "../directory_1", ], }, }

Plugins/PluginsShared/PluginError.swift

glbrntt · 2025-01-14T16:22:55Z

Plugins/PluginsShared/PluginUtils.swift

+/// - Returns: The constructed arguments to be passed to `protoc` when invoking the `proto-gen-swift` `protoc` plugin.
+func constructProtocGenSwiftArguments(
+  config: GenerationConfig,
+  using fileNaming: GenerationConfig.FileNaming?,


using in the context of constructing args feels way outta place. Let's just use fileNaming here

Plugins/PluginsShared/PluginUtils.swift

Co-authored-by: George Barnett <[email protected]>

glbrntt

This is looking really good. Just a couple of minor things.

Plugins/GRPCProtobufGenerator/Plugin.swift

glbrntt · 2025-01-21T09:40:13Z

Plugins/GRPCProtobufGenerator/Plugin.swift

+    displayName: "Generating gRPC Swift files for \(inputFile.relativePath)",
+    executable: protocPath,
+    arguments: arguments,
+    inputFiles: [inputFile, protocGenGRPCSwiftPath],


... still no config file here!

glbrntt · 2025-01-21T09:43:20Z

Plugins/PluginsShared/PluginError.swift

+ * limitations under the License.
+ */
+
+import Foundation


This import is unused

glbrntt · 2025-01-21T09:45:21Z

Plugins/PluginsShared/PluginUtils.swift

+func constructProtocGenSwiftArguments(
+  config: GenerationConfig,
+  fileNaming: GenerationConfig.FileNaming?,
+  inputFiles: [URL],


I'm probably misremembering but I thought we landed on one input file per invocation? I don't remember the reasoning so chances are I am misremembering.

We did but this code will be shared with the command plugin which doesn't have to work in that pattern.

rnro · 2025-01-21T10:52:05Z

#28 (comment)
I have definitely coded this once and piped it through - weird.

Co-authored-by: George Barnett <[email protected]>

glbrntt

Great work, thanks a lot @rnro!

Code generation build plugin

a5010cb

Motivation: To make code generation more convenient for adopters. Modifications: * New build plugin to generate gRPC services and protobuf messages Result: * Users will be able to make use of the build plugin.

rnro added the 🆕 semver/minor Adds new public API. label Jan 8, 2025

rnro requested review from glbrntt January 8, 2025 13:56

glbrntt requested changes Jan 8, 2025

View reviewed changes

rnro force-pushed the build_plugin branch from 84627d2 to 5deb609 Compare January 14, 2025 14:50

review comments

af7cfe0

rnro force-pushed the build_plugin branch from 5deb609 to af7cfe0 Compare January 14, 2025 15:03

glbrntt requested changes Jan 14, 2025

View reviewed changes

rnro and others added 4 commits January 14, 2025 16:28

add config file as a registered input file

256b3e9

Update Plugins/GRPCProtobufGenerator/Plugin.swift

1749e56

Co-authored-by: George Barnett <[email protected]>

Update Plugins/GRPCProtobufGenerator/Plugin.swift

87197c6

Co-authored-by: George Barnett <[email protected]>

review comments, rethink config & imports

585e856

rnro force-pushed the build_plugin branch from 13ddcf3 to 585e856 Compare January 17, 2025 15:53

rnro added 2 commits January 21, 2025 07:53

comments

b41a9eb

Merge branch 'main' into build_plugin

fe732a8

rnro requested a review from glbrntt January 21, 2025 07:55

glbrntt requested changes Jan 21, 2025

View reviewed changes

rnro and others added 2 commits January 21, 2025 12:06

Update Plugins/GRPCProtobufGenerator/Plugin.swift

86f8893

Co-authored-by: George Barnett <[email protected]>

review comments

15fbb1a

rnro force-pushed the build_plugin branch from 98cdaad to 15fbb1a Compare January 21, 2025 12:08

rnro requested a review from glbrntt January 21, 2025 12:08

glbrntt approved these changes Jan 21, 2025

View reviewed changes

glbrntt merged commit b036851 into grpc:main Jan 21, 2025
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code generation build plugin #28

Code generation build plugin #28

rnro commented Jan 8, 2025

glbrntt Jan 8, 2025

glbrntt Jan 8, 2025

glbrntt Jan 8, 2025

rnro Jan 8, 2025

glbrntt Jan 8, 2025

rnro Jan 21, 2025

glbrntt Jan 8, 2025

rnro Jan 8, 2025

glbrntt Jan 8, 2025

glbrntt Jan 8, 2025

glbrntt Jan 8, 2025

rnro Jan 14, 2025

glbrntt Jan 8, 2025

glbrntt Jan 8, 2025

glbrntt Jan 14, 2025

glbrntt Jan 14, 2025

glbrntt Jan 14, 2025

glbrntt Jan 14, 2025

rnro Jan 21, 2025

glbrntt Jan 14, 2025

glbrntt left a comment

glbrntt Jan 21, 2025

glbrntt Jan 21, 2025

glbrntt Jan 21, 2025

rnro Jan 21, 2025

rnro commented Jan 21, 2025

glbrntt left a comment

		name: "GRPCGeneratorPlugin",
		targets: ["GRPCGeneratorPlugin"]

	"protoc-gen-grpc-swift",
	.target(name: "protoc-gen-grpc-swift"),

		// /// Whether reflection data is generated.
		// var reflectionData: Bool?

	displayName: "Generating protobuf Swift files for \(inputFile.relativePath)",
	displayName: "Generating Swift Protobuf files for \(inputFile.relativePath)",

Code generation build plugin #28

Code generation build plugin #28

Conversation

rnro commented Jan 8, 2025

Overview

Configuration

Adoption

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glbrntt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnro commented Jan 21, 2025

glbrntt left a comment

Choose a reason for hiding this comment