Automate project dependencies & General optimizations#134
Merged
itamarcps merged 27 commits intodevelopmentfrom Nov 14, 2024
Merged
Automate project dependencies & General optimizations#134itamarcps merged 27 commits intodevelopmentfrom
itamarcps merged 27 commits intodevelopmentfrom
Conversation
d7faac6 to
d16530a
Compare
Contributor
|
Wonderful!!! Would be cool to have a "rebuild-all.sh" in the project root, like: Works as an instant go-to for people who download software but don't read documentation. |
fcecin
approved these changes
Sep 19, 2024
Contributor
|
About a possible |
itamarcps
approved these changes
Nov 13, 2024
lambdart
approved these changes
Nov 13, 2024
Pedrozo
approved these changes
Nov 13, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
UPDATE (2024-11-09): this PR now includes several improvements (mostly performance/RAM-wise) made in a former separate
optimizebranch that was merged locally today. In short we have:#includelines were revised across the WHOLE project - that alone reduced RAM usage significantly during compilation (see image below - tested withcmake .. && cmake --build . -- -j8, the gap in the middle is the divisory betweensrcandtests)utils.hwas considerably debloated:SafeUint_tandSafeInt_taliases were migrated to their respectivecontract/variables/safeXint.hfiles (no need to includeutils.hanymore just to use those aliasesuint_t/int_taliases and some int conversion functions (uintXToBytes,intXToBytes,bytesToUintX,bytesToIntX)utils/intconv.hutils/evmcconv.hutils/strconv.hutils.h(those should be migrated too but I didn't get to them yet):stringToBytes,bytesToString,uintToBytes(the templated one), maybeappendBytesandbytesRequiredNOTE: this was made BEFORE #135 was opened, there might be conflicts since that branch deprecated
bytes::Viewand also made a bunch of other changes across the project. Whoever manages to get merged first, keep in mind there'll be a lot of fixing to be done after the fact.NOTE 2: right now, running benchmark tests with
-DBUILD_BENCHMARK=ONyields an AddressSanitizer error. This needs fixing, though it's an isolated problem - the rest of the tests are working just fine, it's just the[benchmark]tag that's borked.This PR introduces a script in
scripts/deps.shthat effectively decouples the project itself from its dependencies. So now we have a slightly more elaborate flow when setting up the environment, but we won't have to recompile the dependencies anymore for each clean build - just once in a fresh system, then off we go.Keep in mind the assumed "minimum supported distro version" is now Debian 13 (Trixie) or greater - if you're on an older version or a distro that doesn't use APT, you'll have to do some things manually. I've updated the README with more details. As usual, docs will be updated accordingly after merge.
"Gimme a TL;DR"
If you're using Docker: simply rebuild your container. Dockerfiles were updated to use the script directly, dependencies will be handled automatically
If you're doing it manually:
deps.sh --checkto know if your system has all the required dependencies installeddeps.sh --installor install them yourself by hand to either/usr(for repo packages) or/usr/local(for compiling from source)"Ok, now lift the hood and show me the engine!"
Right, so our dependencies are now divided into a few logical categories:
The script is hardcoded to check into both
/usrand/usr/localfor those dependencies, giving preference to the latter if there's anything installed there (e.g. we need Boost 1.83 but you're on Debian 12 which only has Boost 1.74, you can manually compile a newer version and install it to/usr/localso it'll be detected first).Any internal dependency you need that doesn't match the minimum in the repos will have to be installed this way (the script smartly skips this part if if doesn't find APT). External dependencies can still be installed through the script as those are fetched by
git.Likewise, the project's
CMakeLists.txtnow also checks the system for those dependencies, instead of pulling them viaExternalProject_Addlike we've been doing up until now, so the script has become the "single source of dependency truth". Any modifications to the project's dependencies must be reflected in the script from now on.Caveats to keep in mind
--cleanflag or something of the sortI have no idea if this will break SonarQube's CI/CD as I've updated its Dockerfile as well (and we can't test it unless it is merged) - I hope it won't, but if it does please let me knowThis was already fixed