Skip to content
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

Provide environment variable configuration option #1279

Merged

Conversation

FranzBusch
Copy link
Member

Motivation

We are currently providing a config option to provide a path to the protoc binary. This works great if your protoc is always the same; however, for a lot of packages you want to build them on various systems and the path to protoc is always different. We really can't solve this with any config file.

Modification

This PR adds the option to set the PROTOC_PATH env variable and picks it up in the plugin to find the protoc binary.

Result

We can now dynamically pass the protoc path to the plugin.

@FranzBusch
Copy link
Member Author

@thomasvl @tbkka Would love to get your opinion on this one. While the config option that I provided in the JSON file is good to have and solves most problems. Sometimes, that didn't really work if you needed your package to be developed cross platform. Furthermore, non-leaf packages couldn't use it at all.
I have been trying a few different things for the last days and was almost going to give up but then I came across an idea of passing an env variable to the plugin. The great thing is this works across command line and Xcode if you start Xcode in the right way.

The real fix is still in SPM however I think it is worth adding this env variable escape hatch.

@tbkka
Copy link
Contributor

tbkka commented Aug 25, 2022

I'm fine with it.

@thomasvl
Copy link
Collaborator

Looking at the rendered docs, two of the subsections (###) are getting detected. Can you try an extra blank line before them so we can fix those in the same PR? You can preview it before we merge it at https://github.com/FranzBusch/swift-protobuf/blob/fb-env-variable-for-protoc/Sources/protoc-gen-swift/Docs.docc/spm-plugin.md.

@FranzBusch FranzBusch force-pushed the fb-env-variable-for-protoc branch 2 times, most recently from b6ace7f to 62da224 Compare August 26, 2022 13:43
# Motivation
We are currently providing a config option to provide a path to the `protoc` binary. This works great if your `protoc` is always the same; however, for a lot of packages you want to build them on various systems and the path to `protoc` is always different. We really can't solve this with any config file.

# Modification
This PR adds the option to set the `PROTOC_PATH` env variable and picks it up in the plugin to find the protoc binary.

# Result
We can now dynamically pass the protoc path to the plugin.
@thomasvl thomasvl merged commit c3d181c into apple:1_x_release_branch Aug 26, 2022
@thomasvl
Copy link
Collaborator

I guess we need to pull this to main also.

@thomasvl
Copy link
Collaborator

#1283 is a cherry-pick of it over to main.

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.

3 participants