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

Protobuf imports not resolved outside of the current buffer's directory #1453

Closed
mcos opened this issue Apr 17, 2018 · 5 comments · Fixed by #1590
Closed

Protobuf imports not resolved outside of the current buffer's directory #1453

mcos opened this issue Apr 17, 2018 · 5 comments · Fixed by #1590

Comments

@mcos
Copy link

mcos commented Apr 17, 2018

I'm using flycheck as a syntax checker for protobuf files. Within an example protobuf file at path proto/path/to/foo.proto, I have the following import line:

import 'proto/other_path/to/bar.proto';

This import line gives the following error message:

Suspicious state from syntax checker protobuf-protoc: Flycheck checker protobuf-protoc returned non-zero exit code 1, but its output contained no errors: proto/other_path/to/bar.proto: File not found.
flycheck_foo.proto: Import "proto/other_path/to/bar.proto" was not found or had errors.

Try installing a more recent version of protobuf-protoc, and please open a bug report if the issue persists in the latest release.  Thanks!

I suspect that because the import file isn't present in the buffer's directory, the path isn't being included in the proto_path argument here:

(eval (concat "--proto_path=" (file-name-directory (buffer-file-name))))

Is it possible to configure flycheck to add extra --proto_path arguments to the compiler to help resolve this error?

Answers

  • What you did, and what you expected to happen instead

I imported a protobuf file from outside the current buffer's directory and expected no syntax errors.

  • Whether and how you were able to [reproduce the issue in emacs -Q][emacsQ]

Yes, the issue also arose in the same manner

  • Your Flycheck setup from M-x flycheck-verify-setup
Syntax checkers for buffer foo.proto in protobuf-mode:

  protobuf-protoc
    - may enable: yes
    - predicate:  t
    - executable: Found at /usr/local/bin/protoc

Flycheck Mode is enabled.  Use C-u C-c ! x to enable disabled
checkers.

--------------------

Flycheck version: 32snapshot (package: 20180116.147)
Emacs version:    25.2.1
System:           x86_64-apple-darwin13.4.0
Window system:    ns
  • Your operating system

MacOS 10.12.6

  • Your Emacs version from M-x emacs-version

GNU Emacs 25.2.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version 10.9.5 (Build 13F1911)) of 2017-04-21

  • Your Flycheck version from M-x flycheck-version

Flycheck version: 32snapshot (package: 20180116.147)

@cpitclaudel
Copy link
Member

Thanks, great report. Could you show what a valid command line invocation would look like?

There's a secondary issue here, which is that our patterns are incomplete. You shouldn't get a bad error like this (instead, you should get an error on line one complaining about the failed import).

@mcos
Copy link
Author

mcos commented Apr 18, 2018

Let me first link to the documentation around the --proto_path argument here: https://developers.google.com/protocol-buffers/docs/proto#importing-definitions

The protocol compiler searches for imported files in a set of directories specified on the protocol compiler command line using the -I/--proto_path flag. If no flag was given, it looks in the directory in which the compiler was invoked. In general you should set the --proto_path flag to the root of your project and use fully qualified names for all imports.

So based on this, from the above example, if I'm compiling proto/path/to/foo.proto, which imports proto/other_path/to/bar.proto. The conventional approach would be to invoke the protoc compiler in the parent of the proto/ directory, as follows:

protoc --proto_path=proto/ proto/path/to/foo.proto

Hope this helps. Happy to provide more info if needed.

@drielsma
Copy link

Our project has a pretty complex set of protobuf models that requires compilation with multiple -I arguments. This would be a huge win for us!

@cpitclaudel
Copy link
Member

@drielsma Thanks for the feedback. I'm OK with adding support for a list of proto_path arguments. I also think the current one (passing the current directory) can be removed, right? Since the compiler runs from that directory.

Looping in @marsam (hi!) since he wrote the original checker.

@drielsma, would you like to write a patch? The new argument would work just like flycheck-rust-library-path, which should be a good source of inspiration.

@fishtreesugar
Copy link

fishtreesugar commented Jun 28, 2019

It seems use uber/prototool's lint command could solve this issue

POC Impl

(flycheck-define-checker protobuf-prototool
  :command ("prototool" "lint"
            (eval (expand-file-name (buffer-file-name)))
            )
  :error-patterns
  ((error line-start (file-name) ":" line ":" column
          ":" (message) line-end))
  :modes protobuf-mode
  :predicate (lambda () (buffer-file-name)))

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 a pull request may close this issue.

4 participants