chore: add Nimble commit customization and remove support for older Nim versions #71

romanzac · 2023-12-14T10:17:04Z

I would like to propose two changes to Nim build script we use for Nim-libp2p.

It has a been a while skipIntegrityCheck is available in koch.nim in default commit. I propose to remove "Custom buildchain for older versions" section from the code.

Second change is to add Nimble commit version to have control over which Nimble is being built together with Nim Lang. This change has the following benefits:

Possible newest Nimble even for Nim v1.x.
Have Nimble lock functionality for Nim v1.x.
Keep tooling versions same across different Nim versions. Perhaps only except Nim Devel.
Have issues fixed faster for newest Nimble version, rather than the older one - focus & capacity benefits.

- removed "Custom buildchain for older versions" - added Nimble commit preset

etan-status · 2024-01-15T15:26:13Z

When skipIntegrityCheck is available, Nimble is locked to NimbleStableCommit (Nim/koch.nim). If override support makes sense, maybe more appropriate to integrate it as an option to koch?

romanzac · 2024-01-16T03:12:41Z

When skipIntegrityCheck is available, Nimble is locked to NimbleStableCommit (Nim/koch.nim). If override support makes sense, maybe more appropriate to integrate it as an option to koch?

Hello, thanks for the reply. Before we talk about skipIntegrityCheck, may I ask a question? Would you see any problem in using Nim v1.6+ version together with Nimble v0.14.2? I haven't personally found any technical problems, although I am new to Nim lang, so I may not have a full picture yet.

etan-status

PR title is a bit generic; this adds the nimble commit customization and removes support for older nim versions

etan-status · 2024-06-06T08:17:25Z

scripts/build_nim.sh

@@ -71,6 +72,7 @@ nim_needs_rebuilding() {
 		# support old Git versions, like the one from Ubuntu-18.04
 		git restore . 2>/dev/null || git reset --hard
 		if ! git checkout -q ${NIM_COMMIT} 2>/dev/null; then
+		  echo "Downloading Nim sources..."


There is a mix of tabs and spaces throughout this file.

Thanks for feedback. FIxed at 323c494

etan-status · 2024-06-06T08:20:32Z

scripts/build_nim.sh

@@ -90,6 +92,23 @@ nim_needs_rebuilding() {
 		# NIM_COMMIT is empty, so assume the commit we need is already checked out
 		NIM_COMMIT_HASH="$(git rev-list -n 1 HEAD)"
 	fi
+
+	if [[ ! -d "$NIMBLE_DIR" ]] && [[ -n "$NIMBLE_COMMIT" ]]; then


When changing NIMBLE_COMMIT to a new value, it appears to me that NIMBLE_DIR will stick to the stale version that's already in it from a prior run.

etan-status · 2024-06-06T08:21:24Z

scripts/build_nim.sh

-			# We just want nimble
-			./koch nimble -d:release --skipUserCfg --skipParentCfg --warnings:off --hints:off
-		fi
-	else


Personally fine with removing this, but it affects building older Nim version. @arnetheduck @tersec ?

As far as I'm concerned, nothing before Nim 1.6 exists anymore.

does koch ignore unknown flags? if it does that, it's fine - otherwise, we can reduce this section to pass the flag or not depending on whether it's supported or not - seems like a cheap price to pay for the flexibility.

etan-status · 2024-06-06T08:23:25Z

scripts/build_nim.sh

@@ -18,7 +18,8 @@ set -e

 # After this Nim commit, use csources v2
 : ${CSOURCES_V2_START_COMMIT:=f7c203fb6c89b5cef83c4f326aeb23ef8c4a2c40}
-: ${NIMBLE_COMMIT:=d13f3b8ce288b4dc8c34c219a4e050aaeaf43fc9} # 0.13.1
+# Set Nimble commit in environment to have non default one.
+# e.g. NIMBLE_COMMIT=3575fd54a890d910ace56678aa74b4237d604175 # 0.14.2


Last time I checked, was advised that the Nimble version is tied to the Nim version (it hardcodes a specific commit to be used). So, not sure how functional overriding is. If there is a use case, it looks quite clean.

I am aware there is a relationship between Nim and Nimble versions.

I don't think however it is something which could create much of problems. At least when I tested this with nim-libp2p vacp2p/nim-libp2p#1004 I haven't had any problem. Right after that I opened this PR to contribute back.

PR 1004 was rejected based on this generic advice without any further explanation.

Nimble v0.14.2 is out more than 1 year. Entire Nim ecosystem might be behind similar competitors (Go, Rust) about several years.

Nimbus is the end2end application for Ethereum ecosystem. Single producer - IFT delivers to many consumers - validators. This is completely different with Waku and Codex, where IFT is a major producer of middleware expecting adoption by integrators. Nim ecosystem quality, dev experience, quality of libs directly affects adoption of Waku and Codex.

Nimbus has about 6% market share among Ethereum validators. While I do admire the achievement, I am far from thrilled about this number.

There is a substantial chance Waku and Codex potential will be capped by Nim ecosystem mediocrity. Especially when the scarcity mindset is taken for Nim development by a major Nim ecosystem contributor. Mediocre (at best) outcome is almost guaranteed.

Having said all the negative, there is an another approach main Nim contributor might take which can substantially improve odds for Waku and Codex (and potentially for Nimbus too) to make it big:

accountability: there must be someone working full time on compiler and tooling. Someone who is praised, blamed, hired or fired because of quality of their work. Tinkeners or hobbyist part time approach won't work. Sorry for those part timers out there. You can't be the steering force.

separation of responsibilities: Waku team builds Waku and complains or praises to Nim eco team. No compiler builds and similar repetitive slow downs.

clearly defined feedback loop for Nim eco dev

Bad dev experience scales exponentially into silence of those who tried and moved hands out. Good dev experience scales also exponentially, but into growth and positive impact!

Have a chat or meeting with me, feel free to dispute or challenge in productive manner.

Generically, could Waku and Codex use Nim 2.0 at this point (using refc initially probably, since some libraries such as nim-metrics don't work well in ORC yet)? 2.0.4 might be better quality than 1.6.x and 2.0.6, hopefully soon, should be fairly solid.

It reports

$ bin/nimble --version nimble v0.14.2 compiled at 2024-06-16 18:41:44 git hash: f8bd7b5fa6ea7a583b411b5959b06e6b5eb23667

fwiw, it's broadly an aim to start disconnecting nim from nimble and have the latter install the former - disconnecting their versions here is a good first step and indeed, it makes sense to move faster on nimble now that we're investing in its development.

something that needs doing for moving the needle here is to test this bump with a number of major nimble projects such as libp2p, chronos and maybe a few others from nimbus-eth2/vendor - changing the nimble here should also result in their ci:s starting to use nimble this way - there's a bit of work to do here, though it's fairly straightforward - it includes bumping some of those nimbles to newer versions - what we're looking for is to ideally use a similar version across projects.

In an ideal world, we'd actually have a github action that can download nimble - this nimble would be used to install nim (instead of having the action install nim and its included, out-of-date version.

cc @jmgomez

Generically, could Waku and Codex use Nim 2.0 at this point (using refc initially probably, since some libraries such as nim-metrics don't work well in ORC yet)? 2.0.4 might be better quality than 1.6.x and 2.0.6, hopefully soon, should be fairly solid.

It reports

$ bin/nimble --version nimble v0.14.2 compiled at 2024-06-16 18:41:44 git hash: f8bd7b5fa6ea7a583b411b5959b06e6b5eb23667

Indeed a valid option to move to Nim 2.0. Thanks for mentioning it.

I already know while playing on local linux box, latest Nimble won't let me install Nim 1.6.

report this as an issue in the nimble repo and we'll get it fixed - being able to install 1.6+ will be needed for the foreseeable future

Sorry I had no time on Friday. Nominal case for all tests run finished with 0 failed tests:

[Summary] 150 tests run (0.77s): 148 OK, 0 FAILED, 2 SKIPPED

https://github.com/status-im/nim-stew/actions/runs/9651769206/job/26620400645

I'll move forward with other combinations tomorrow. Potentially also opening issue for 1.6 installation.

nimbus-eth2 migrated to Nim 2.0.6: status-im/nimbus-eth2#6386

Waku migrated to Nim 2.0.8 (and nimbus-eth1 and nimbus-eth2 have as well, but at this point it's just a minor version change for them): waku-org/nwaku#2885

Good news Waku have migrated! We need to sustain this speed, quality and focus on Nim eco related development to create momentum. This would eventually draw attention of very capable non CC contributors. And we will literally multiply fruits of our effort by many folds.

arnetheduck · 2024-10-30T09:51:01Z

I think nimble is at a point now where this can work - nimble 0.16.2 supports nim 1.6 as well as newer nim versions meaning that we can go ahead with this change.

In the meantime, the way n-b-s builds nim has changed slightly - @romanzac can you update the PR? It's probably reasonable that the parts about "remove support for older nim versions" is removed from this PR so that it only includes the nimble changes.

romanzac added 2 commits December 14, 2023 11:07

chore: Nim build script update

12d4fe9

- removed "Custom buildchain for older versions" - added Nimble commit preset

fix: leave Nimble commit preset to the user

b9130d3

romanzac requested review from arnetheduck and Menduist December 14, 2023 12:38

romanzac marked this pull request as ready for review December 14, 2023 15:40

etan-status reviewed Jun 6, 2024

View reviewed changes

romanzac changed the title ~~chore: Nim build script update~~ chore: add Nimble commit customization and remove support for older Nim versions Jun 8, 2024

fix: have tabs for indent

323c494

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add Nimble commit customization and remove support for older Nim versions #71

chore: add Nimble commit customization and remove support for older Nim versions #71

romanzac commented Dec 14, 2023 •

edited

Loading

etan-status commented Jan 15, 2024

romanzac commented Jan 16, 2024

etan-status left a comment

etan-status Jun 6, 2024

romanzac Jun 8, 2024

etan-status Jun 6, 2024

etan-status Jun 6, 2024

tersec Jun 6, 2024

arnetheduck Jun 16, 2024

etan-status Jun 6, 2024

romanzac Jun 16, 2024

tersec Jun 16, 2024

arnetheduck Jun 16, 2024

romanzac Jun 17, 2024

arnetheduck Jun 20, 2024

romanzac Jun 24, 2024

tersec Jun 24, 2024

tersec Jul 11, 2024

romanzac Jul 11, 2024

arnetheduck commented Oct 30, 2024

chore: add Nimble commit customization and remove support for older Nim versions #71

Are you sure you want to change the base?

chore: add Nimble commit customization and remove support for older Nim versions #71

Conversation

romanzac commented Dec 14, 2023 • edited Loading

etan-status commented Jan 15, 2024

romanzac commented Jan 16, 2024

etan-status left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnetheduck commented Oct 30, 2024

romanzac commented Dec 14, 2023 •

edited

Loading