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

prefer system installed protoc #60

Closed
someone1 opened this issue Jan 5, 2021 · 8 comments · Fixed by #64
Closed

prefer system installed protoc #60

someone1 opened this issue Jan 5, 2021 · 8 comments · Fixed by #64
Labels
enhancement New feature or request

Comments

@someone1
Copy link

someone1 commented Jan 5, 2021

We have a monorepo and use protoc for more than our web client which is why it's installed and configured in our development container systemwide. As such, the protoc installed into our node_modules/.bin folder from @protobuf-ts/plugin is getting priority in our build scripts and is missing a bunch of includes we rely on. Is there a way to disable installing this dependency and/or have a config option to prefer the system-wide installed version?

our "build" scripts are scripts defined in our package.json

@timostamm
Copy link
Owner

Thanks for the report. There is no built-in option to support this.

@protobuf-ts/protoc is already looking at the package.json. If pkg.config.protocVersion is set, it tries to install protoc in this version.

It should be possible to add a second key pkg.config.useProtocPath. If set, @protobuf-ts/protoc could execute protoc from this path, and not install it's own version at all.

Would this solve the issue?

@someone1
Copy link
Author

someone1 commented Jan 6, 2021

Hmm - I think that'd be fine, I was otherwise thinking to auto-detect if protoc is already found in the $PATH (sans the node_modules/.bin directory) and use that before resorting to installing it.

timostamm added a commit that referenced this issue Jan 6, 2021
Searches protoc in $PATH (unless package.json config.protocVersion is set).

If not found in $PATH, installs latest version.

Needs doc update.
@timostamm
Copy link
Owner

Agreed, preferring $PATH is better.

Users could opt out of $PATH and ensure installation by setting pkg.config.protocVersion explicitly.

I think this commit should do it. Do you have a way to test this?

@timostamm timostamm added the enhancement New feature or request label Jan 6, 2021
timostamm added a commit that referenced this issue Jan 6, 2021
timostamm added a commit that referenced this issue Jan 6, 2021
@someone1
Copy link
Author

someone1 commented Jan 6, 2021

just ran it, seems to have done the trick!

Thank you!

@timostamm
Copy link
Owner

👍

Released in v2.0.0-alpha.11

@gganley
Copy link

gganley commented Mar 1, 2021

Hello,

Is it possible to backport this fix to v1.x?

@timostamm
Copy link
Owner

Could you try updating @protobuf-ts/protoc ?

npm i @protobuf-ts/protoc@next

This updates the "protoc" package only and keeps the 1.x version for everything else.

I am pretty sure that the new version of the "protoc" package should work just fine, but it would help a lot if you could confirm.

If this does not cause any problems, I can backport and publish as 1.x.

@gganley
Copy link

gganley commented Mar 2, 2021

Yes that did work! Thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants