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] Subdirectory for grammar rust was not found #495

Closed
0323pin opened this issue Nov 29, 2022 · 9 comments
Closed

[BUG] Subdirectory for grammar rust was not found #495

0323pin opened this issue Nov 29, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@0323pin
Copy link
Contributor

0323pin commented Nov 29, 2022

Describe the bug

Building diffsitter yields the following error:

error: failed to run custom build command for `diffsitter v0.7.2 (/usr/pkgsrc/wip/diffsitter/work/diffsitter-0.7.2)`

Caused by:
  process didn't exit successfully: `/usr/pkgsrc/wip/diffsitter/work/diffsitter-0.7.2/target/release/build/diffsitter-b2fc85f3d27f4d0e/build-script-build` (exit status: 1)
  --- stderr
  Error: Subdirectory for grammar rust was not found
warning: build failed, waiting for other jobs to finish...
*** Error code 101

Stop.

To Reproduce
Steps to reproduce the behavior:

  1. Build diffsitter on NetBSD

Expected behavior
Complete build.

Log output/screenshots
See above

Platform:
OS: NetBSD-9.99.107, Rust-1.65.0

Additional context
Build in offline-mode.

@0323pin 0323pin added the bug Something isn't working label Nov 29, 2022
@afnanenayet
Copy link
Owner

afnanenayet commented Nov 30, 2022

The missing directory is a sub module, were the submodules initialized?

Alternatively if you don't want the submodules and would rather install the different grammars as shared libraries you can compile without the static-grammar-libraries feature (also worth noting that this is the approach Nix went with)

@0323pin
Copy link
Contributor Author

0323pin commented Nov 30, 2022

@afnanenayet

were the submodules initialized?

No, they were not. I was trying to create a native pkgsrc package for NetBSD and not attempting just a local install.

Alternatively if you don't want the submodules and would rather install the different grammars as shared libraries ...

I'll consider this as an alternative, thanks. I thought about compiling it with no default features before posting the issue but, then it wouldn't be a default build.

I need to check if we have a policy regarding the use of git submodule update --recursive --init as a pre-compile stage.

Thanks!
Feel free to close this issue if you want to.

@afnanenayet
Copy link
Owner

I think there is a strong argument from a package managers perspective to handle the dependencies on its own, I have the submodules mostly as a convenience for myself

@0323pin
Copy link
Contributor Author

0323pin commented Nov 30, 2022

@afnanenayet Yeah, I know.
Building with --no-default-features works fine,

===> Building for diffsitter-0.7.2
   Compiling proc-macro2 v1.0.47
   Compiling quote v1.0.21
   Compiling unicode-ident v1.0.5
   Compiling syn v1.0.104
   Compiling libc v0.2.137
   Compiling version_check v0.9.4
   Compiling thiserror v1.0.37
   Compiling autocfg v1.1.0
   Compiling cfg-if v1.0.0
   Compiling memchr v2.5.0
   Compiling jobserver v0.1.25
   Compiling cc v1.0.77
   Compiling crossbeam-utils v0.8.12
   Compiling memoffset v0.6.5
   Compiling crossbeam-epoch v0.9.11
   Compiling serde_derive v1.0.148
   Compiling rand_core v0.6.4
   Compiling siphasher v0.3.10
   Compiling rand v0.8.5
   Compiling phf_shared v0.11.1
   Compiling aho-corasick v0.7.19
   Compiling errno v0.2.8
   Compiling proc-macro-error-attr v1.0.4
   Compiling io-lifetimes v1.0.2
   Compiling serde v1.0.148
   Compiling anyhow v1.0.66
   Compiling io-lifetimes v0.7.5
   Compiling scopeguard v1.1.0
   Compiling regex-syntax v0.6.27
   Compiling bitflags v1.3.2
   Compiling phf_generator v0.11.1
   Compiling proc-macro-error v1.0.4
   Compiling rustix v0.35.13
   Compiling rustix v0.36.3
   Compiling rayon-core v1.10.1
   Compiling ucd-trie v0.1.5
   Compiling heck v0.4.0
   Compiling regex v1.6.0
   Compiling termcolor v1.1.3
   Compiling log v0.4.17
   Compiling thiserror-impl v1.0.37
   Compiling pest v2.3.1
   Compiling phf_macros v0.11.1
   Compiling crossbeam-deque v0.8.2
   Compiling crossbeam-channel v0.5.6
   Compiling num_cpus v1.14.0
   Compiling unicase v2.6.0
   Compiling once_cell v1.16.0
   Compiling rustversion v1.0.9
   Compiling pest_meta v2.3.1
   Compiling backtrace v0.3.66
   Compiling adler v1.0.2
   Compiling quick-error v1.2.3
   Compiling unicode-width v0.1.10
   Compiling either v1.8.0
   Compiling gimli v0.26.2
   Compiling os_str_bytes v6.3.1
   Compiling clap_lex v0.3.0
   Compiling rayon v1.6.0
   Compiling humantime v1.3.0
   Compiling miniz_oxide v0.5.4
   Compiling addr2line v0.17.0
   Compiling clap_derive v4.0.21
   Compiling is-terminal v0.4.0
   Compiling terminal_size v0.2.2
   Compiling pest_generator v2.3.1
   Compiling phf v0.11.1
   Compiling tree-sitter v0.20.9
   Compiling getrandom v0.2.7
   Compiling object v0.29.0
   Compiling atty v0.2.14
   Compiling dirs-sys v0.3.7
   Compiling rustc-demangle v0.1.21
   Compiling serde_json v1.0.89
   Compiling strsim v0.10.0
   Compiling cargo-emit v0.2.1
   Compiling diffsitter v0.7.2 (/usr/pkgsrc/wip/diffsitter/work/diffsitter-0.7.2)
warning: unused import: `anyhow::Result`
  --> build.rs:22:5
   |
22 | use anyhow::Result;
   |     ^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unused import: `std::fmt::Write`
  --> build.rs:23:5
   |
23 | use std::fmt::Write;
   |     ^^^^^^^^^^^^^^^

   Compiling clap v4.0.27
warning: `diffsitter` (build script) generated 2 warnings
   Compiling dirs v4.0.0
   Compiling strum_macros v0.24.3
   Compiling env_logger v0.7.1
   Compiling uuid v0.8.2
   Compiling pest_derive v2.3.1
   Compiling logging_timer_proc_macros v1.1.0
   Compiling toml v0.5.9
   Compiling os_type v2.4.0
   Compiling terminal_size v0.1.17
   Compiling itoa v1.0.4
   Compiling lazy_static v1.4.0
   Compiling ryu v1.0.11
   Compiling console v0.15.2
   Compiling human-panic v1.0.3
   Compiling logging_timer v1.1.0
   Compiling json5 v0.4.1
   Compiling strum v0.24.1
   Compiling clap_complete v4.0.6
   Compiling pretty_env_logger v0.4.0
   Compiling xdg v2.4.1
   Compiling libloading v0.7.4
   Compiling unicode-segmentation v1.10.0
    Finished release [optimized] target(s) in 3m 16s

...and, as I can see we already have tree-sitter in our repositories.
So, let's close this now. I'll make the package depend on tree-sitter and things should be fine :)

Edit: Package now merged into our repositories, https://mail-index.netbsd.org/pkgsrc-changes/2022/11/30/msg264430.html

@0323pin 0323pin closed this as completed Nov 30, 2022
@afnanenayet
Copy link
Owner

To be clear this requires different packages for the tree sitter grammars, not the library itself. Packages such as this: https://github.com/tree-sitter/tree-sitter-python provide a dynamic shared library such as libtreesitter-rust.so which is what diffsitter looks for when it's dynamically loading libraries.

Also the feature to load grammars dynamically isn't enabled automatically, it will have to be enabled with thedynamic-grammar-libs feature, so I think running with no default features might just load no grammars.

I'll add some docs on building/packaging diffsitter to make this clearer, I don't think I did a sufficient job of explaining this with my earlier comments, sorry

@afnanenayet afnanenayet reopened this Dec 1, 2022
@0323pin
Copy link
Contributor Author

0323pin commented Dec 1, 2022

@afnanenayet Thanks!
I see the issue, our package provides only the library which won't help, even with the dynamic-grammar-libs feature enable.

Guess I have two options now. Either package those or, build diffsitter with the static-grammar-libraries.

One question, does diffsitter expect to find these grammar libs in a specific path?

Edit: just checked the Alpine and Arch packages. They pull only these, tree-sitter-cpp, tree-sitter-python and tree-sitter-rust. Are these enough?

@afnanenayet
Copy link
Owner

afnanenayet commented Dec 2, 2022

@afnanenayet Thanks!

I see the issue, our package provides only the library which won't help, even with the dynamic-grammar-libs feature enable.

Guess I have two options now. Either package those or, build diffsitter with the static-grammar-libraries.

One question, does diffsitter expect to find these grammar libs in a specific path?

No. You can optionally provide paths or names of libraries when using the config to load extra library paths (mostly put there in case someone wanted to manually point to a library placed in a non-standard path).

The standard behavior is that someone will run diffsitter on files with a certain language and diffsitter will try to load libtree-sitter-{lang}.so with dlopen. Standard rules apply for search paths.

Edit: just checked the Alpine and Arch packages. They pull only these, tree-sitter-cpp, tree-sitter-python and tree-sitter-rust. Are these enough?

I did not realize that. It's enough if you only care about doing diffs on C++, Python and Rust files. The language-specific libraries are just implementations of tree sitter grammars, and diffsitter can operate on any tree-sitter grammar, so the program itself doesn't actually "need" all of the grammars.

@0323pin
Copy link
Contributor Author

0323pin commented Dec 3, 2022

@afnanenayet Package fixed now, http://mail-index.netbsd.org/pkgsrc-changes/2022/12/03/msg264617.html

I'll look into packaging the grammars in the next days.
Feel free to close this issue now.

@afnanenayet
Copy link
Owner

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants