Introduce Rust/librustzcash into the zcash build system #2183

ebfull · 2017-03-15T02:11:31Z

Introduces Rust into the build system and brings a trivial xor operation into the code (near where we will likely be working).

ebfull · 2017-03-15T02:11:39Z

@zkbot try

zkbot · 2017-03-15T02:11:43Z

⌛ Trying commit e1e9daa with merge 9638725...

zkbot · 2017-03-15T02:22:39Z

💔 Test failed - zcash

ebfull · 2017-03-15T04:10:13Z

@zkbot try

zkbot · 2017-03-15T04:10:17Z

⌛ Trying commit 48409a1 with merge a9f6c45...

[WIP] Introduce Rust into the zcash build system This is currently just an experiment to see what kind of weird linking and building problems might arise.

daira · 2017-03-15T04:33:22Z

Can we call it librustzcash?

ebfull · 2017-03-15T04:35:01Z

I'm a little worried about the name we give it. There could be a ton of different things we'd want to call "zcash" or "rust-zcash" in the Rust namespace-land.

zkbot · 2017-03-15T04:59:46Z

☀️ Test successful - zcash

ebfull · 2017-03-17T17:47:27Z

I named it librustzcash and this PR is ready for review.

zkbot · 2017-03-17T17:48:27Z

⌛ Trying commit 6a0c7ce with merge a609d36...

zkbot · 2017-03-17T18:38:52Z

☀️ Test successful - zcash

bitcartel · 2017-03-17T20:10:18Z

We should disable this by default since there is no actual rust based functionality yet. Add an option to build.sh to enable it.

daira · 2017-03-18T01:05:58Z

I was thinking that if we're sure that we will be using Rust in future, then we should include this by default and add an option to disable it, since then we'll see any build/dependency problems that users run into.

ebfull · 2017-03-18T02:05:42Z

I think we are absolutely going to be integrating Rust code into Zcash this year. We need to ensure that there aren't any problems with deterministic builds, linking, etc. and get versions of Zcash shipping with Rust before we start implementing or using Rust code in Zcash.

As a result we absolutely should enable this by default. I would even argue against a --disable-rust flag unless users experience problems. This is because I don't want us to get in the habit of making Rust an optional dependency and keeping track of consistency between duplicated Rust and C++ functionality. Also, I don't forsee long before features are integrated into Zcash that are only written in Rust, so such a flag would be short lived anyway.

ebfull · 2017-03-18T02:06:53Z

Ideally, this PR could be accepted and placed into the 1.0.8 release.

daira · 2017-03-18T04:09:53Z

As release manager for 1.0.8, I'd accept this but only with a --disable-rust flag, because I want us to be able to tell people to use that if they do encounter problems. Later we'll remove the flag (at the point where we start depending on Rust for anything that isn't optional).

daira · 2017-03-18T04:15:04Z

Concept ACK (and the code looks good too). Needs --disable-rust to go into 1.0.8.

ebfull · 2017-03-18T04:19:48Z

Alright, will add that soon.

ebfull · 2017-03-18T13:50:23Z

Done.

@zkbot try

zkbot · 2017-03-18T13:50:27Z

⌛ Trying commit 802ea76 with merge 92b65dc...

Introduce Rust/librustzcash into the zcash build system Introduces Rust into the build system and brings a trivial xor operation into the code (near where we will likely be working).

zkbot · 2017-03-18T14:40:21Z

☀️ Test successful - zcash

daira

utACK.

str4d

utACK, with non-blocking comments.

str4d · 2017-03-21T01:37:19Z

zcutil/build.sh

@@ -73,9 +76,17 @@ then
    shift
 fi

+# If --disable-rust is the next argument, disable Rust code:


In #2120 I added a CONFIGURE_FLAGS env var for passing flags to ./configure; I think that's a more extensible way to proceed.

Hmm, in this case it might be necessary because of the NO_RUST argument to the depends system. So I'm non-blocking on this now, as it will be removed anyway once we make Rust a required dependency.

Can you push to this PR with what you're talking about? I'm on my laptop. 😊

Oh, you're saying it's not blocking. Never mind then.

str4d · 2017-03-21T01:41:22Z

configure.ac

@@ -88,6 +88,12 @@ AC_ARG_ENABLE([mining],
  [enable_mining=$enableval],
  [enable_mining=yes])

+AC_ARG_ENABLE([rust],
+  [AS_HELP_STRING([--enable-rust],


Should be --disable-rust here and in the help text (but leaving enable_rust as-is), as the default is to enable. Upstream has fixed this for e.g. the wallet, and I should have done the same for -mining when I added it. Non-blocking, as AC_ARG_ENABLE transparently defines --disable-foo as --enable-foo=no.

(In an engineering meeting we decided not to make --disable-rust the default, but to add it to the User Guide instead. #2191)

str4d · 2017-03-21T01:48:39Z

src/pow.cpp

@@ -96,6 +100,14 @@ bool CheckEquihashSolution(const CBlockHeader *pblock, const CChainParams& param
    // H(I||V||...
    crypto_generichash_blake2b_update(&state, (unsigned char*)&ss[0], ss.size());

+    #ifdef ENABLE_RUST


Non-blocking: indentation.

I prefer this indentation while simultaneously disliking precompiler macros like this.

str4d · 2017-03-21T01:52:58Z

configure.ac

+  RUST_LIBS="-lrustzcash"
+fi
+
+LIBZCASH_LIBS="-lsnark -lgmp -lgmpxx -lboost_system-mt -lcrypto -lsodium -fopenmp $RUST_LIBS"


Is there a reason why -lrustzcash is moved to the end? AFAICT it shouldn't make a difference, but when we revert this commit it might matter.

I put it at the end deliberately because it should help when we need to link against libsodium in Rust. Nice observation though!

Actually, I think the linker will want -lsodium to appear after -lrustzcash in that case. We can deal with it later.

In that case, simply reverting the PR should be fine, because it will shift -lrustzcash back before -lsodium.

ebfull · 2017-03-21T02:34:27Z

@zkbot r+

zkbot · 2017-03-21T02:34:28Z

📌 Commit 802ea76 has been approved by ebfull

zkbot · 2017-03-21T02:34:32Z

⌛ Testing commit 802ea76 with merge acb65cd...

zkbot · 2017-03-21T03:26:58Z

💔 Test failed - zcash

ebfull · 2017-03-21T03:28:29Z

This PR previously passed the test suite. What's this about?

ebfull · 2017-03-21T03:38:54Z

@zkbot retry

zkbot · 2017-03-21T03:38:59Z

⌛ Testing commit 802ea76 with merge 3b11e24...

Introduce Rust/librustzcash into the zcash build system Introduces Rust into the build system and brings a trivial xor operation into the code (near where we will likely be working).

daira · 2017-03-21T04:29:34Z

@ebfull wrote:

This PR previously passed the test suite. What's this about?

Race condition in the failing test? (rpc-tests/prioritisetransaction.py)

zkbot · 2017-03-21T04:30:31Z

☀️ Test successful - zcash

ebfull · 2017-03-21T04:31:47Z

Filed #2193

zkbot added a commit that referenced this pull request Mar 15, 2017

Auto merge of #2183 - ebfull:rust-introduction, r=<try>

a9f6c45

[WIP] Introduce Rust into the zcash build system This is currently just an experiment to see what kind of weird linking and building problems might arise.

Introduce librustzcash and Rust to depends system.

6a0c7ce

ebfull force-pushed the rust-introduction branch from 48409a1 to 6a0c7ce Compare March 17, 2017 17:47

ebfull changed the title ~~[WIP] Introduce Rust into the zcash build system~~ Introduce Rust/librustzcash into the zcash build system Mar 17, 2017

daira added this to the 1.0.8 milestone Mar 18, 2017

Allow Rust-language related assets to be disabled with --disable-rust.

802ea76

daira approved these changes Mar 18, 2017

View reviewed changes

daira mentioned this pull request Mar 18, 2017

The arguments to zcutil/build.sh should not need to be in a specific order #2190

Open

str4d approved these changes Mar 21, 2017

View reviewed changes

daira mentioned this pull request Mar 21, 2017

Modify User Guide to say build with --disable-rust unless user is adventurous #2191

Closed

zkbot merged commit 802ea76 into zcash:master Mar 21, 2017

zkbot mentioned this pull request Mar 21, 2017

Add AMQP 1.0 support via Apache Qpid Proton C++ API 0.17.0 #2189

Merged

ebfull mentioned this pull request Mar 21, 2017

Non-deterministic test failure of prioritisetransaction.py #2193

Closed

zkbot mentioned this pull request Mar 21, 2017

Experimental feature: remote proving service #2120

Closed

daira added the L-rust Involves Rust code. label Apr 1, 2017

daira mentioned this pull request Apr 12, 2019

RFC: Rust code integration bitcoin/bitcoin#15798

Closed

dagurval mentioned this pull request Apr 20, 2019

[build] Add Rust depends package BitcoinUnlimited/BitcoinUnlimited#1677

Merged

Introduce Rust/librustzcash into the zcash build system #2183

Introduce Rust/librustzcash into the zcash build system #2183

Conversation

ebfull commented Mar 15, 2017 • edited Loading

ebfull commented Mar 15, 2017

zkbot commented Mar 15, 2017

zkbot commented Mar 15, 2017

ebfull commented Mar 15, 2017

zkbot commented Mar 15, 2017

daira commented Mar 15, 2017

ebfull commented Mar 15, 2017

zkbot commented Mar 15, 2017

ebfull commented Mar 17, 2017

zkbot commented Mar 17, 2017

zkbot commented Mar 17, 2017

bitcartel commented Mar 17, 2017

daira commented Mar 18, 2017

ebfull commented Mar 18, 2017

ebfull commented Mar 18, 2017

daira commented Mar 18, 2017

daira commented Mar 18, 2017 • edited Loading

ebfull commented Mar 18, 2017

ebfull commented Mar 18, 2017

zkbot commented Mar 18, 2017

zkbot commented Mar 18, 2017

daira left a comment

Choose a reason for hiding this comment

str4d 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

ebfull commented Mar 21, 2017

zkbot commented Mar 21, 2017

zkbot commented Mar 21, 2017

zkbot commented Mar 21, 2017

ebfull commented Mar 21, 2017

ebfull commented Mar 21, 2017

zkbot commented Mar 21, 2017

daira commented Mar 21, 2017

zkbot commented Mar 21, 2017

ebfull commented Mar 21, 2017

ebfull commented Mar 15, 2017 •

edited

Loading

daira commented Mar 18, 2017 •

edited

Loading