waku-org / nwaku

Waku node and protocol.
Other
201 stars 53 forks source link

chore: run update target along with build target #1876

Open chaitanyaprem opened 1 year ago

chaitanyaprem commented 1 year ago

Background

When i update local repo and build wakunode, sometimes i am facing random errors which are caused due to vendor modules getting updated. Current workaround is to do make update and then build again. This may be frustrating for someone not used to this codebase, as they may start debugging as to if it is some local env issue etc.

Details

It would be great if the make update can be run as dependent task for all the build tasks in Makefile itself.

Acceptance criteria

vpavlin commented 1 year ago

Will this result in you building locally something else than what is actually in master (with regards to vendor/ content)?

chaitanyaprem commented 1 year ago

Looks that way, as all i did was a git pull origin to update my local master and tried to build make wakunode2 yesterday. This gave me the below error which only got resolved after i had to do make update. It was difficult from such an error to figure it out by myself that i had to do make update. Probably because i am new to nim. :)

Users/prem/Code/nwaku/waku/v2/node/waku_node.nim(113, 38) Error: type mismatch: got <typedesc[AutonatService], autonatClient: AutonatClient, rng: ref HmacDrbgContext, scheduleInterval: Opt[timer.Duration], askNewConnectedPeers: bool, numPeersToAsk: int literal(3), maxQueueSize: int literal(3), minConfidence: float64>
but expected one of:
proc new(T: typedesc[AutonatService]; autonatClient: AutonatClient;
         rng: ref HmacDrbgContext;
         scheduleInterval: Option[Duration] = none(Duration);
         askNewConnectedPeers = true; numPeersToAsk: int = 5;
         maxQueueSize: int = 10; minConfidence: float = 0.3;
         dialTimeout = 30.seconds; enableAddressMapper = true): T:type
  first type mismatch at position: 4
  required type for scheduleInterval: Option[timer.Duration]
  but expression 'scheduleInterval =
  Opt[T](oResultPrivate: true, vResultPrivate: seconds(120))' is of type: Opt[timer.Duration]
proc new(t: typedesc): auto
  first type mismatch at position: 2
  unknown named parameter: autonatClient
37 other mismatching symbols have been suppressed; compile with --showAllMismatches:on to see them

expression: new(AutonatService, autonatClient = new(AutonatClient), rng = rng, scheduleInterval =
  Opt[T](oResultPrivate: true, vResultPrivate: seconds(120)),
    askNewConnectedPeers = false, numPeersToAsk = 3, maxQueueSize = 3,
    minConfidence = 0.7)
stack trace: (most recent call last)
/Users/prem/Code/nwaku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(429, 18)
/Users/prem/Code/nwaku/waku.nimble(63, 15) wakunode2Task
/Users/prem/Code/nwaku/waku.nimble(35, 8) buildBinary
/Users/prem/Code/nwaku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(273, 7) exec
/Users/prem/Code/nwaku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(273, 7) Error: unhandled exception: FAILED: nim c --out:build/wakunode2  --verbosity:0 --hints:off -d:chronicles_log_level=TRACE -d:git_version="v0.19.0-rc.0-13-g2fc488" -d:release apps/wakunode2/wakunode2.nim [OSError]
Ivansete-status commented 1 year ago

I personally don't see any harm in doing make update automatically, but I think @jm-clius has a different opinion about that.

If we don't make it automatic, maybe we could enhance the documentation to make clearer the need of make update.

jm-clius commented 1 year ago

Mmm, make update takes a very long time to complete, so will block builds for significant periods of time. It's also only necessary when you've missed a submodule change and your local submodules have not been synced for the git ref you're pointing to. Such submodule changes should be relatively rare though (although they do happen from time to time).

chaitanyaprem commented 1 year ago

Mmm, make update takes a very long time to complete, so will block builds for significant periods of time. It's also only necessary when you've missed a submodule change and your local submodules have not been synced for the git ref you're pointing to. Such submodule changes should be relatively rare though (although they do happen from time to time).

We wouldn't want delay in every build because we have to run make update. Was going through git submodule's documentation and found some tips they have provided while working with submodules. Probably we can document this in our Readme the below option where git pull automatically pulls sub-module changes as well. Hoping this doesn't affect any other dev workflows

If you want to automate this process, you can add the --recurse-submodules flag to the git pull command (since Git 2.14). This will make Git run git submodule update right after the pull, putting the submodules in the correct state. Moreover, if you want to make Git always pull with --recurse-submodules, you can set the configuration option submodule.recurse to true (this works for git pull since Git 2.15). This option will make Git use the --recurse-submodules flag for all commands that support it (except clone).