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(rln-relay): fetch release from zerokit ci, or build #1603

Merged
merged 2 commits into from
Mar 21, 2023

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Mar 13, 2023

This PR introduces a script, build_rln.sh that extracts the host triple, checks if a zerokit release asset exists for it, and if not, builds zerokit from scratch.

Zerokit supports 6 target architectures at the moment, and it is possible that some users will need to build it manually when compiling nwaku with experimental features enabled. I propose that we continue to use zerokit as a submodule for this reason.

build_rln.sh Outdated
@@ -0,0 +1,31 @@
#!/usr/bin/env bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jm-clius, thoughts on where this script should go?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm. Good question. Since there are no obvious alternatives, I think the root folder is the best option for now, since this is where other build and environment scripts are.

build_rln.sh Outdated
host_triplet=$(rustup show | grep "Default host: " | cut -d' ' -f3)

# Download the prebuilt rln library if it is available
if wget https://github.com/vacp2p/zerokit/releases/download/nightly/$host_triplet-rln.tar.gz
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
if wget https://github.com/vacp2p/zerokit/releases/download/nightly/$host_triplet-rln.tar.gz
if wget https://github.com/vacp2p/zerokit/releases/download/v0.2/$host_triplet-rln.tar.gz

note: we will use a static version some time in the future, keeping nightly in for now will allow us to detect breaking changes faster

@rymnc rymnc marked this pull request as ready for review March 15, 2023 08:42
@rymnc rymnc requested review from jm-clius and LNSD March 15, 2023 11:01
@rymnc rymnc mentioned this pull request Mar 15, 2023
4 tasks
@rymnc rymnc added this to the Release 0.17.0 milestone Mar 15, 2023
@rymnc rymnc self-assigned this Mar 15, 2023
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Right, I'm still not sure how we should prevent cluttering in build resources and scripts, but I agree that this is better than building everytime.
One worry about using nightly is that it may cause CI builds to become indeterminate?

@rymnc
Copy link
Contributor Author

rymnc commented Mar 17, 2023

Yes you're right about experimental ci becoming indeterminate, however, using nightly is just temporary, till the next release of zerokit is created (which includes the build artifacts for different targets). it should be soon!

Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@rymnc
Copy link
Contributor Author

rymnc commented Mar 20, 2023

@jakubgs, should I add wget to the shell.nix file?

Copy link
Contributor

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

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

Maybe we should have a scripts folder for build_rln.sh?
Leaving stuff at repo root unless absolutely necessary should be avoided.

And yes, we can add wget to shell.nix, though honestly I'd prefer curl, as it's more extendable and has more option, like the combination of --silent --fail-with-body.

build_rln.sh Outdated Show resolved Hide resolved
build_rln.sh Outdated Show resolved Hide resolved
@rymnc rymnc merged commit 179be68 into master Mar 21, 2023
@rymnc rymnc deleted the use-rln-staticlib branch March 21, 2023 07:37
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.

None yet

4 participants