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

buf lint trying to lint file that doesn't exist #10

Closed
sowelipililimute opened this issue Dec 30, 2021 · 6 comments
Closed

buf lint trying to lint file that doesn't exist #10

sowelipililimute opened this issue Dec 30, 2021 · 6 comments

Comments

@sowelipililimute
Copy link

example run: https://github.com/harmony-development/protocol/runs/4670337736?check_suite_focus=true

with a configuration like:

  lint-protos-staging:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: bufbuild/[email protected]
      - uses: bufbuild/buf-lint-action@v1
        with:
          input: 'staging'

and a file structure like

 .
├──  CMakeLists.txt
├──  FAQ.md
├──  GUIDELINES.md
├──  LICENSE
├──  README.md
├──  shell.nix
├──  stable
│  ├──  auth
│  │  └──  v1
│  │     └──  auth.proto
│  ├──  batch
│  │  └──  v1
│  │     └──  batch.proto
│  ├──  buf.yaml -> ../.buf.yaml
│  ├──  chat
│  │  └──  v1
│  │     ├──  channels.proto
│  │     ├──  chat.proto
│  │     ├──  guilds.proto
│  │     ├──  messages.proto
│  │     ├──  Permissions.md
│  │     ├──  permissions.proto
│  │     └──  stream.proto
│  ├──  CMakeLists.txt
│  ├──  emote
│  │  └──  v1
│  │     ├──  emote.proto
│  │     ├──  stream.proto
│  │     └──  types.proto
│  ├──  harmonytypes
│  │  └──  v1
│  │     └──  types.proto
│  ├──  mediaproxy
│  │  └──  v1
│  │     └──  mediaproxy.proto
│  ├──  name-resolution
│  │  ├──  example
│  │  │  ├──  go.mod
│  │  │  └──  main.go
│  │  └──  name-resolution.md
│  ├──  profile
│  │  └──  v1
│  │     ├──  appdata.proto
│  │     ├──  profile.proto
│  │     ├──  stream.proto
│  │     └──  types.proto
│  ├──  rest
│  │  └──  rest.md
│  └──  sync
│     └──  v1
│        └──  sync.proto
└──  staging
   ├──  buf.yaml -> ../.buf.yaml
   ├──  CMakeLists.txt
   └──  voice
      └──  v1
         └──  voice.proto

buf attempts to lint a file from stable, but fails because it's not in the staging directory

@doriable
Copy link
Member

doriable commented Dec 30, 2021

Hi, I took a look at this by cloning the source repo and running buf lint staging locally against the head of main. It appears that the file in staging has a dependency on the file in stable, but since you are only targeting the staging directory as your input, it is unable to build because the dependency is not found. There are a few options that you can try:

  1. Running lint against the entire repository but use the ignore field in the lint configuration for your module to exclude the stable directory (docs available here: https://docs.buf.build/lint/configuration#ignore). Note that in this case, your module file will be the one you have defined here https://github.com/harmony-development/protocol/blob/main/.buf.yaml and the symlinks would be removed from the subdirectories.
  2. Separating staging and stable into two different modules. However, they would need individually defined buf.yaml configurations (instead of the symlinked configs), and a dependency would be declared on the other. You can then use a workspace file to join these dependencies. Documentation for workspaces and the configuration files are available here:

Hope this helps and let me know if you have any other questions!

@sowelipililimute
Copy link
Author

ah, i see 👍

consider this a bug report for an unhelpful error message then, since "file not found" without further context makes it sound like buf is trying to lint something that doesn't exist instead of an import not being found

@lrewega
Copy link

lrewega commented Dec 30, 2021

"file not found" without further context

Agreed. Would something like voice/v1/voice.proto:3: import harmonytypes/v1/types.proto not found provide sufficient context?

@sowelipililimute
Copy link
Author

yes, that would be a big improvement from the status quo

@doriable
Copy link
Member

doriable commented Jan 4, 2022

I've added it to an issue we already have in buf to improve the error message. Thanks for filing this!

@emcfarlane
Copy link
Contributor

Closing as the issue above has been merged.

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

No branches or pull requests

4 participants