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

feat: add static build config for MUSL #409

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: add static build config for MUSL #409

wants to merge 1 commit into from

Conversation

nrdxp
Copy link
Contributor

@nrdxp nrdxp commented Jan 21, 2025

Description

Currently the node does not build totally statically due to the rocksdb crate. Newer versions of the crate do contain flags to statically compile, though, if we can find a way to update the transitive dependency cleanly.

Everything else is compiled in statically, including the MUSL C standard library

Checklist

  • Commit sequence broadly makes sense and commits have useful messages.
  • New tests are added if needed and existing tests are updated.
  • Relevant logging and metrics added
  • CI passes. See note on CI.
  • Any changes are noted in the changelog.md for affected crate
  • Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG Partner Chains developers to do this
for you.

@LGLO
Copy link
Contributor

LGLO commented Jan 21, 2025

error: failed to run custom build command for `tikv-jemalloc-sys v0.5.4+5.3.0-patched`

Caused by:
  process didn't exit successfully: `/home/lglo/iohk/partner-chains/target/debug/build/tikv-jemalloc-sys-347c18ab3246c066/build-script-build` (exit status: 101)
  ...
src/malloc_io.c:107:9: error: incompatible pointer to integer conversion returning 'char *' from a function with result type 'int' [-Wint-conversion]
    107 |         return strerror_r(err, buf, buflen);
        |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 warning and 1 error generated.
  1 warning generated.
  make: *** [Makefile:479: src/malloc_io.sym.o] Error 

@skylar-simoncelli
Copy link
Contributor

Nice!

I guess we need to sit on the draft PR until we upgrade the rocksdb crate to support the static link compilation

If we get this working can then integrate the changes in the CI and CD workflows

@nrdxp
Copy link
Contributor Author

nrdxp commented Jan 23, 2025

Yeah the rocksdb dep is transitive, which makes a bit more difficult to upgrade. However, if we really need this we could potentially still clean this up a touch and merge. Having only one dependency to keep track of in dynamic environments still reduces complexity and attack surface considerably

@LGLO
Copy link
Contributor

LGLO commented Jan 23, 2025

if we really need this

  • Nick sometimes needs partner-chains-node image to give to people so they can play with it
  • we use the same (or similar) image for our tests

Providing a way to build partner-chains-node with statically linked dependencies has some value, but it really is not our business as long as the "demo/test" image is providing compatible dependencies.

If it turns out increase local or CI build times, I would rather think twice about it.

@nrdxp
Copy link
Contributor Author

nrdxp commented Jan 29, 2025

MUSL is lighter weight so it shouldn't really affect build times (at least it hasn't seemed to on my build server in testing).

I think the main benefit of static deps besides the improved simplicity of distribution is security, there is no way to inject a malicious dependency at runtime, which is a nice property. Still, since the current version of rocksdb leaves us hanging a bit, perhaps we can just leave off until the next update of the SDK

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.

3 participants