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

Don't concatenate paths in --proto_path #109

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

mortbopet
Copy link
Contributor

protoc's --proto_path argument is expected to be provided multiple times if multiple directories are to be considered. Concatenating with : is invalid on some systems (Windows):

> protoc --help
  -IPATH, --proto_path=PATH   Specify the directory in which to search for
                              imports.  May be specified multiple times;
                              directories will be searched in order.  If not
                              given, the current working directory is used.
                              If not found in any of the these directories,
                              the --descriptor_set_in descriptors will be
                              checked for required proto file.

`protoc`'s `--proto_path` argument is expected to be provided multiple times if multiple directories are to be considered. Concatenating with `:` is invalid on some systems (Windows):

```sh
> protoc --help
  -IPATH, --proto_path=PATH   Specify the directory in which to search for
                              imports.  May be specified multiple times;
                              directories will be searched in order.  If not
                              given, the current working directory is used.
                              If not found in any of the these directories,
                              the --descriptor_set_in descriptors will be
                              checked for required proto file.
```
@CLAassistant
Copy link

CLAassistant commented Sep 3, 2024

CLA assistant check
All committers have signed the CLA.

mortbopet pushed a commit to mortbopet/substrait-cpp that referenced this pull request Sep 3, 2024
This + PRs substrait-io#109 and substrait-io#108 should be sufficient for a minimum-viable Windows (MSVC) build of this project.
Copy link
Member

@EpsilonPrime EpsilonPrime left a comment

Choose a reason for hiding this comment

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

Thanks!

mortbopet pushed a commit to mortbopet/substrait-cpp that referenced this pull request Sep 4, 2024
This + PRs substrait-io#109 and substrait-io#108 should be sufficient for a minimum-viable Windows (MSVC) build of this project.
@mortbopet
Copy link
Contributor Author

@EpsilonPrime thank you for the review; looks like CI is back in the green now.

EpsilonPrime pushed a commit that referenced this pull request Sep 4, 2024
This + PRs #109 and
#108 should be
sufficient for a minimum-viable Windows (MSVC) build of this project.
@EpsilonPrime EpsilonPrime merged commit 9737507 into substrait-io:main Sep 4, 2024
3 checks passed
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