Add import paths to SPM Plugin #1373
Conversation
| // We include the target directory as the default | ||
| importPaths = [target.directory] |
There was a problem hiding this comment.
I think we probably should always include the target.directory even when importPaths are specified.
There was a problem hiding this comment.
sounds fair to me
Co-authored-by: Franz Busch <privat@franz-busch.de>
|
@FranzBusch by the way theres a matching PR in grpc/grpc-swift#1568 for the same thing and I made the same change there. |
* add import paths to config * add comment * Update Plugins/SwiftProtobufPlugin/plugin.swift Co-authored-by: Franz Busch <privat@franz-busch.de> * always include target.directory in import paths --------- Co-authored-by: Franz Busch <privat@franz-busch.de>
* add import paths to config * add comment * Update Plugins/SwiftProtobufPlugin/plugin.swift Co-authored-by: Franz Busch <privat@franz-busch.de> * always include target.directory in import paths --------- Co-authored-by: Franz Busch <privat@franz-busch.de>
|
@samisuteria I am just coming back to this since I wanted to try this out myself and actually noticed that this behaves differently than I assumed during review. (I will get some tests in place here in another PR). |
|
@FranzBusch I actually could not get it to work the way I expected it to. I wanted to reference a proto file that was outside the Sources directory and when running |
|
@samisuteria Thanks for replying. The plugin really requires all files to be in one of the targets directory since SPM needs to be able to include them in the build. Symlinking is definitely a valid approach another one could have been to add a target with a custom Is there a reason you cannot move the files to a targets source folder? Also do I understand it correctly that you are not using the new option you added after all? |
|
@FranzBusch We have a monorepo with multiple iOS/macOS apps and Swift packages so copying files to the target source would mean copying them to many target sources and updating them would be a manual process. We use a git submodule for all proto files and soft link that folder into the targets that need it. Correct, I am not using the new option I added. I have not had the time to come back and fix it to the behavior I was expecting. |
# Motivation In apple#1373 (comment), we added a new option to the SPM plugin config file that allows passing custom import paths (aka search paths) to protoc. Recently, I wanted to try this out but found out that this was not working as expected. 1. The option is configured "globally" across invocations. That doesn't really compose with the way we have structured the other settings 2. The option expects absolute paths right now which makes it almost impossible to use. I propose to remove this option again from the main branch so we are not shipping it with the 2.0 release. In the PR, I asked the adopter if he is actually using that and confirmed that it isn't working like he expected it to work and wasn't using it. # Modification Remove the `importPaths` option again.
|
Thanks for explaining! I think your use-case is a valid one and IMO submodule and soft linking is a good approach for this. Other ways that I have seen in the past to solve this in mono repos:
I also just opened a PR to remove the option you added here again on |
# Motivation In #1373 (comment), we added a new option to the SPM plugin config file that allows passing custom import paths (aka search paths) to protoc. Recently, I wanted to try this out but found out that this was not working as expected. 1. The option is configured "globally" across invocations. That doesn't really compose with the way we have structured the other settings 2. The option expects absolute paths right now which makes it almost impossible to use. I propose to remove this option again from the main branch so we are not shipping it with the 2.0 release. In the PR, I asked the adopter if he is actually using that and confirmed that it isn't working like he expected it to work and wasn't using it. # Modification Remove the `importPaths` option again.
Added an optional parameter to the config file so users can configure import paths for protoc.
Useful in a mono repo where the proto file is not in the target directory and requires imports that are defined relative to the proto.