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

Bug: sv-parser doesn't handle sources which are symlink. #86

Open
Graian opened this issue Jul 21, 2023 · 5 comments
Open

Bug: sv-parser doesn't handle sources which are symlink. #86

Graian opened this issue Jul 21, 2023 · 5 comments

Comments

@Graian
Copy link

Graian commented Jul 21, 2023

While building rust source with the rules_rust(rust_library or rust_binary) build rule in the bazel build system, I encountered a bug. Bazel creates working directories using symlinks to files, but sv-parser checks if it's a file and ignores it if it's a symlink. so, requires an update to allow sv-parser to handle symlinks.

This line cause this issue.

@Graian
Copy link
Author

Graian commented Jul 22, 2023

When I modified the above-mentioned line of code to the following, I found that it fixed the problem. I don't know what side-effects this fix has on the overall behaviour of sv-parser.

if entry.file_type().is_file() -> if entry.path().is_file()

@DaveMcEwan
Copy link

@Graian That modification looks sensible to me. There shouldn't be any unwanted side-effects.
Perhaps you could open a PR so that it's easy to merge?

I'd like to understand more about the issue because it likely affects the related project svlint (here and here).

  1. What are you using Bazel for? Instead of Cargo? Apologies if this is a silly question!
  2. How can I (or others) reproduce your issue?
  3. Is Bazel compatibility important enough to add CI checks?

@Graian
Copy link
Author

Graian commented Jul 25, 2023

@DaveMcEwan , Thanks for your comment. I have no permission to PR to dalance/sv-parser repository. and, here are my answers for your questions.

  1. What are you using Bazel for? Instead of Cargo? Apologies if this is a silly question!
  • In my development environment, I'm using bazel as my build system, so I'm trying to build an existing Cargo project on bazel. The basic external dependency is on Cargo.toml, and bazel leverages Cargo.toml and the Cargo.lock file.
  1. How can I (or others) reproduce your issue?
  1. Is Bazel compatibility important enough to add CI checks?
  • As far as I know, I haven't had any major problems with it other than the symlink issue. However, It would be nice to see it added to CI checks.

@Graian
Copy link
Author

Graian commented Jul 27, 2023

@DaveMcEwan , I've forked this repository and create a merge request!

@DaveMcEwan
Copy link

@Graian Would you mind closing this issue as it looks like your fix is 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

2 participants