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

configure: ensure that we have jq, fail if it fails #7662

Merged

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Sep 13, 2024

Added as a requirement, but it's better to check than crash at runtime.

I also ensured that the build fails if JQ fails.

Fixes: #7657

Otherwise we can get other crashes it seems?

Fixes: ElementsProject#7657
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell added this to the v24.08.1 milestone Sep 13, 2024
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to have this check anyway, but looks like that it does not solve the problem? #7657 (comment)

ACK 91a2d03

@ShahanaFarooqui
Copy link
Collaborator

ShahanaFarooqui commented Sep 13, 2024

Good to have this check anyway, but looks like that it does not solve the problem? #7657 (comment)

Their file contents are incorrect, as detailed here: #7657 (comment).

You can reproduce the corrupted file by uninstalling jq and running make plugins/sql-schema_gen.h. Reinstalling jq restores the correct data.

@vincenzopalazzo
Copy link
Collaborator

You can reproduce the corrupted file by uninstalling jq and running make plugins/sql-schema_gen.h. Reinstalling jq restores the correct data.

Anyway this PR is not checking for the jq version, IMHO this is not a fix for the bug, probably we want to check also the version?

@rustyrussell rustyrussell changed the title configure: ensure that we have jq. configure: ensure that we have jq, fail if it fails Sep 16, 2024
Nested within the loop, jq would fail silently.

This can happen if jq is too old.

Fixes: ElementsProject#7657
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@vincenzopalazzo
Copy link
Collaborator

make[1]: Leaving directory '/home/runner/work/lightning/lightning/external/lowdown'
msggen cln-rpc/src/notifications.rs
error: failed to run custom build command for `cln-grpc v0.1.9 (/home/runner/work/lightning/lightning/cln-grpc)`

Caused by:
  process didn't exit successfully: `/home/runner/work/lightning/lightning/target/release/build/cln-grpc-8ca867bfe16fd099/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=proto/node.proto
  cargo:rerun-if-changed=proto

  --- stderr
  thread 'main' panicked at cln-grpc/build.rs:7:10:
  called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "protoc failed: node.proto:1228:1: Reached end of input in message definition (missing '}').\n" }

This is strange, is not a rust error but looks like this is a new error: protoc failed: node.proto:1228:1: Reached end of input in message definition (missing '}

@vincenzopalazzo vincenzopalazzo enabled auto-merge (rebase) September 17, 2024 14:33
@vincenzopalazzo vincenzopalazzo merged commit 4578748 into ElementsProject:master Sep 17, 2024
39 of 43 checks passed
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 this pull request may close these issues.

SQL Plugin crash after update to 24.08
3 participants