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

How about replacement jq with jaq? #24

Open
ynqa opened this issue Mar 23, 2024 · 5 comments · May be fixed by #39
Open

How about replacement jq with jaq? #24

ynqa opened this issue Mar 23, 2024 · 5 comments · May be fixed by #39
Labels
help wanted Extra attention is needed

Comments

@ynqa
Copy link
Owner

ynqa commented Mar 23, 2024

At the time of v0.1.0, the author deemed it most appropriate to use the original jq.

However, managing C-related stuff during build time had to be undertaken, and this was underestimated at the release of v0.1.0. After actually releasing and taking a look the installation error issues that were raised, it became apparent that continuing to use jq might not be the best decision.

Then, there is a project in Rust called jaq, which is a jq-clone. I hope to discuss whether or not to replace jq with jaq, and whether it is feasible to do so.

Here's a rough outline of the pros and cons:

jq (j9)

jaq

  • pros
    • pure Rust, there is no need to manage C dependencies
  • cons
    • jq-clone, not all filters may be supported
      • e.g. New filters in jq 1.7 01mf02/jaq#112
      • My concern is with this point. Ideally, it would be better if all features were available for use (Or maybe it's sufficient if only the basic filters are available...?)
@ynqa ynqa added the help wanted Extra attention is needed label Mar 23, 2024
@ynqa ynqa changed the title How about replacement jq binding (j9) with jaq? How about replacement jq with jaq? Mar 23, 2024
@dwvisser
Copy link

I say it’s okay to have jaq’s reduced set of available filters, if more people can build and use jnv. 😊

@moritzwilksch
Copy link

I agree with @dwvisser and would personally prefer an easy-to-build pure rust tool over packaging C deps. ripgrep took a similar approach where the rust regex crate they use lacks some features (eg look-ahead/behind is not supported) but in return doesn't depend on C.

@ardrigh
Copy link

ardrigh commented Mar 23, 2024

I would like to start using this tool if it was pure Rust. Even if there weren't a complete match of filters.

jq was unmaintained entirely for a number of years, which should be considered under Cons. They had to make a point release to fix a security issue in the update they made.

At least if you only have Rust dependencies it will be easier to manage if such a situation happens again.

@corneliusroemer
Copy link
Contributor

Can you maybe try out how it works with jaq, allowing choice of jaq and jaq, then deprecate jq if jaq is good enough.

@ynqa ynqa pinned this issue Apr 2, 2024
@ynqa ynqa unpinned this issue Apr 2, 2024
@pavelzw
Copy link

pavelzw commented Apr 7, 2024

I'm also having some issues compiling j9-sys

conda-forge/staged-recipes#25806 (comment)

checking host system type... 
  --- stderr
  configure.ac:11: installing 'config/ar-lib'
  configure.ac:8: installing 'config/compile'
  configure.ac:19: installing 'config/config.guess'
  configure.ac:19: installing 'config/config.sub'
  configure.ac:9: installing 'config/install-sh'
  configure.ac:9: installing 'config/missing'
  Makefile.am: installing 'config/depcomp'
  configure.ac: installing 'config/ylwrap'
  parallel-tests: installing 'config/test-driver'
  Invalid configuration `/home/conda/staged-recipes/build_artifacts/jnv_1712488275842/_build_env/bin/x86_64-conda-linux-gnu': more than four components
  configure: error: /bin/sh /home/conda/staged-recipes/build_artifacts/jnv_1712488275842/work/target/x86_64-unknown-linux-gnu/release/build/j9-sys-b50a7ed31c9b8fd9/out/jq_build/config/config.sub /home/conda/staged-recipes/build_artifacts/jnv_1712488275842/_build_env/bin/x86_64-conda-linux-gnu failed
  thread 'main' panicked at /home/conda/staged-recipes/build_artifacts/jnv_1712488275842/_build_env/.cargo/registry/src/index.crates.io-6f17d22bba15001f/autotools-0.2.6/src/lib.rs:781:5:

Switching to jaq would fix this 😅

@ynqa ynqa linked a pull request Apr 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@dwvisser @ardrigh @ynqa @corneliusroemer @pavelzw @moritzwilksch and others