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

Update dependency: deps/k_release #534

Merged
merged 36 commits into from
Mar 24, 2024
Merged

Conversation

rv-jenkins
Copy link
Contributor

@rv-jenkins rv-jenkins commented Mar 1, 2024

Summary by CodeRabbit

  • Chores
    • Updated a key dependency to enhance performance and stability.

This comment was marked as spam.

coderabbitai[bot]

This comment was marked as outdated.

geo2a
geo2a previously approved these changes Mar 18, 2024
@jberthold
Copy link
Member

The switch to using a system-provided secp256k1 library causes some headache in this upgrade....

@jberthold jberthold force-pushed the _update-deps/runtimeverification/k branch from 931529c to 699cb55 Compare March 22, 2024 04:56
@@ -14,6 +14,8 @@ cabal test llvm-integration

# The runDirectoryTest.sh script expects the following env vars to be set
export PLUGIN_DIR=$(nix build github:runtimeverification/blockchain-k-plugin/$PLUGIN_VERSION --no-link --json | jq -r '.[].outputs | to_entries[].value')
export SECP256K1_DIR=$(nix build nixpkgs#secp256k1 --no-link --json | jq -r '.[].outputs | to_entries[].value')
Copy link
Member

@jberthold jberthold Mar 22, 2024

Choose a reason for hiding this comment

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

This seems to be necessary now... root cause is that we don't actually get a linkable plugin library from including the blockchain-k-plugin, we rather get the sources and have to build things ourselves.
Also see the change to the plugin build and derivation for context.

"name": "Lbl'Hash'if'UndsHash'then'UndsHash'else'UndsHash'fi'Unds'K-EQUAL-SYNTAX'Unds'Sort'Unds'Bool'Unds'Sort'Unds'Sort",
"name": "Lblite",
Copy link
Member

Choose a reason for hiding this comment

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

This, of course, is a very important name change...

Comment on lines +88 to +94

if [ -z "${server_port}" ]; then
echo "Unable to identify the server, aborting"
exit 4
else
echo "Server listening on port ${server_port}"
fi
Copy link
Member

Choose a reason for hiding this comment

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

Previously we would happily continue with an empty server_port variable (garbling the rpc client command).

@jberthold jberthold dismissed geo2a’s stale review March 22, 2024 05:17

Needs plugin update, currently pointing to a temporary branch

@jberthold
Copy link
Member

jberthold commented Mar 22, 2024

Blocked on runtimeverification/blockchain-k-plugin#172

  • deps/blockchain-k-plugin must refer to master with this change merged

@jberthold jberthold self-assigned this Mar 22, 2024
@rv-jenkins rv-jenkins merged commit 99a39cb into main Mar 24, 2024
5 checks passed
@rv-jenkins rv-jenkins deleted the _update-deps/runtimeverification/k branch March 24, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants