Skip to content

chore: depr vendor and leverage nimble #3682

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.

Sign up for GitHub

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

Jump to bottom
Open
darshankabariya wants to merge 2 commits into
base: master
Choose a base branch
from
Open

chore: depr vendor and leverage nimble #3682

darshankabariya wants to merge 2 commits into from
+1,175 −329

Conversation

@darshankabariya
Copy link
Contributor

@darshankabariya darshankabariya commented Jan 5, 2026 •
edited
Loading

  • Removed all submodules except the native ones still required (nim-ffi, zerokit, waku-rlnv2-contract, nph).
  • Day-to-day commands (make wakunode2, make test, etc.) remain unchanged.
  • we no longer need nimbus-build-system.
  • Update CI according.
  • latest version of all submodule
  • Plan to replace the remaining four vendor libraries with nimble packages and delete ./vendor entirely.

closes #3672

arnetheduck
arnetheduck reviewed Jan 5, 2026
View reviewed changes
.github/workflows/ci.yml Outdated
uses: actions/checkout@v4

- name: Install Nim
uses: jiro4989/setup-nim-action@v2
Copy link
Contributor

@arnetheduck arnetheduck Jan 5, 2026

Choose a reason for hiding this comment

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

instead of installing nim, it makes more sense to install nimble and have nimble manage nim.

Nim itself also needs to be locked down for a reproducible build.

Copy link
Contributor

@arnetheduck arnetheduck Jan 5, 2026

Choose a reason for hiding this comment

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

See also github.com/nim-lang/setup-nimble-action

Copy link
Contributor

@arnetheduck arnetheduck Jan 5, 2026

Choose a reason for hiding this comment

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

also, for a reproducible build, you need to lock the nimble version, which setup-nimble-action allows you to do.

arnetheduck
arnetheduck reviewed Jan 5, 2026
View reviewed changes
waku.nimble Outdated
requires "nim >= 2.2.4"

# Pure Nim packages - pinned to exact commits via Nimble
requires "https://github.com/status-im/nim-chronicles#54f5b726025e8c7385e3a6529d3aa27454c6e6ff"
Copy link
Contributor

@arnetheduck arnetheduck Jan 5, 2026

Choose a reason for hiding this comment

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

using requires this way will effectively prevent third-parties from depending on waku, unless they use the exact same commit of everything.

nimble lock can be used to produce a lock file that allows an application to use a locked set of deps while at the same time allowing others that depend on waku to be more flexible in the versions they use. For example, an application might want to use a newer version of chronicles for logging, and it's unreasonable for waku to limit this.

Copy link
Contributor Author

@darshankabariya darshankabariya Jan 9, 2026

Choose a reason for hiding this comment

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

Thanks for the idea. I’m now using nimble.lock to get an exact replica of the packages for the waku build.

@darshankabariya darshankabariya force-pushed the deprect_vendor branch 5 times, most recently from 7d8fbd3 to 351cac6 Compare January 6, 2026 07:36
@NagyZoltanPeter
Copy link
Contributor

NagyZoltanPeter commented Jan 6, 2026

@darshankabariya FYI in this #3670 I bumped versions on nim-chronos and nim-libp2p which involves new deps: nim-lsquic and nim-jwt.
These changes comes with some code adjustments. I suggest later incorporate these changes into this PR.

@darshankabariya darshankabariya force-pushed the deprect_vendor branch 8 times, most recently from 685ba5c to 7d34b1e Compare January 8, 2026 11:31
Ivansete-status
Ivansete-status reviewed Jan 8, 2026
View reviewed changes
@darshankabariya
Copy link
Contributor Author

darshankabariya commented Jan 9, 2026

@darshankabariya FYI in this #3670 I bumped versions on nim-chronos and nim-libp2p which involves new deps: nim-lsquic and nim-jwt. These changes comes with some code adjustments. I suggest later incorporate these changes into this PR.

Sure, will do. I tried earlier, but it failed. I need to resolve the conflict carefully now.

@github-actions
Copy link

github-actions bot commented Jan 14, 2026 •
edited
Loading

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3682

Built from dc00236

@darshankabariya darshankabariya force-pushed the deprect_vendor branch 4 times, most recently from 4b1e00f to f13113c Compare January 20, 2026 06:24
@igor-sirotin
Copy link

igor-sirotin commented Jan 27, 2026

@darshankabariya can you please share, what's the state of this PR?
I believe it also closes #1324?

@darshankabariya
Copy link
Contributor Author

darshankabariya commented Jan 27, 2026

@darshankabariya can you please share, what's the state of this PR? I believe it also closes #1324?

Hi @igor-sirotin, this was Ivan’s idea to remove the git submodules and use Nimble instead.

Most of the work is done. I’ve moved all vendors to waku.nimble, except zerokit and waku-rln-contract, due to Nimble limitations. It’s working fine locally—builds and tests pass on macOS and Linux—but a few CI checks are still failing.

Right now it’s blocked because of a security compliance issue.

@igor-sirotin
Copy link

igor-sirotin commented Jan 27, 2026

Most of the work is done. I’ve moved all vendors to waku.nimble, except zerokit and waku-rln-contract, due to Nimble limitations. It’s working fine locally—builds and tests pass on macOS and Linux—but a few CI checks are still failing.

Ok cool, would be cool to get it merged. Let me know if there's anything I can help with.

@darshankabariya
Copy link
Contributor Author

darshankabariya commented Jan 27, 2026

Ok cool, would be cool to get it merged. Let me know if there's anything I can help with.

it's not ready merge. will update you soon.

@darshankabariya darshankabariya force-pushed the deprect_vendor branch 3 times, most recently from 51c304b to 564ecb4 Compare January 28, 2026 08:12
@NagyZoltanPeter
Copy link
Contributor

NagyZoltanPeter commented Jan 29, 2026

Most of the work is done. I’ve moved all vendors to waku.nimble, except zerokit and waku-rln-contract, due to Nimble limitations. It’s working fine locally—builds and tests pass on macOS and Linux—but a few CI checks are still failing.

Ok cool, would be cool to get it merged. Let me know if there's anything I can help with.
@darshankabariya , @igor-sirotin : As this comes with changes that can make other ongoing PRs hard to merge, I propose to adjust before merging this. Would be nice to have this in a close clean state. WDYT?
cc: @Ivansete-status , @fcecin

@fcecin
Copy link
Contributor

fcecin commented Jan 29, 2026

@darshankabariya , @igor-sirotin : As this comes with changes that can make other ongoing PRs hard to merge, I propose to adjust before merging this. Would be nice to have this in a close clean state. WDYT?
cc: @Ivansete-status , @fcecin

Probably a good idea to not change the build system before we merge the send API and health API PRs.

@darshankabariya
Copy link
Contributor Author

darshankabariya commented Jan 30, 2026

propose to adjust before merging this. Would be nice to have this in a close clean state. WDYT?
cc: @Ivansete-status , @fcecin

Hi @fcecin - there’s no hurry to merge this branch. pls let me know when you’re planning to merge the send and Health API PRs. @NagyZoltanPeter

@NagyZoltanPeter
Copy link
Contributor

NagyZoltanPeter commented Jan 30, 2026

propose to adjust before merging this. Would be nice to have this in a close clean state. WDYT?
cc: @Ivansete-status , @fcecin

Hi @fcecin - there’s no hurry to merge this branch. pls let me know when you’re planning to merge the send and Health API PRs. @NagyZoltanPeter

Send-api is now merged! 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arnetheduck arnetheduck arnetheduck left review comments

@Ivansete-status Ivansete-status Ivansete-status left review comments

@NagyZoltanPeter NagyZoltanPeter Awaiting requested review from NagyZoltanPeter

@fcecin fcecin Awaiting requested review from fcecin

At least 2 approving reviews are required to merge this pull request.

Assignees

@darshankabariya darshankabariya

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

chore: completely stop using vendor folder and only use Nimble

7 participants

@darshankabariya @NagyZoltanPeter @igor-sirotin @fcecin @arnetheduck @Ivansete-status