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

Build usable diem-node binary in libra-framework project #310

Open
dboreham opened this issue Aug 15, 2024 · 16 comments
Open

Build usable diem-node binary in libra-framework project #310

dboreham opened this issue Aug 15, 2024 · 16 comments
Assignees

Comments

@dboreham
Copy link
Contributor

diem-node is a package dependency in the libra-framework project.
It (alone) can be built from within the project root directory with:

$ cargo build -p diem-node

The resulting binary is created here:

ls target/debug/diem-node
target/debug/diem-node

However this binary file is not executable:

$ ./target/debug/diem-node
thread 'main' panicked at /home/david/.cargo/git/checkouts/diem-7dee55d666bb1ba0/668b495/diem-move/diem-vm/src/natives.rs:57:5:

diem-node was compiled with feature flags that shouldn't be enabled.

This is caused by cargo's feature unification.
When you compile two crates with a shared dependency, if one enables a feature flag for the dependency, then it is also enabled for the other crate.

PLEASE RECOMPILE DIEM-NODE SEPARATELY using the following command:
    cargo build --package diem-node

Task is to resolve this, generating instead an executable...executable.

@dboreham dboreham self-assigned this Aug 15, 2024
@dboreham
Copy link
Contributor Author

Some discussion on this here: rust-lang/cargo#10489

@dboreham
Copy link
Contributor Author

Narrowing in on what it means by "feature flags that shouldn't be enabled":

stack backtrace:
   0: rust_begin_unwind
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panicking.rs:262:5
   3: diem_vm::natives::assert_no_test_natives::panic_cold_display
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panic.rs:99:13
   4: diem_vm::natives::assert_no_test_natives
             at /home/david/.cargo/git/checkouts/diem-7dee55d666bb1ba0/668b495/diem-move/diem-vm/src/natives.rs:57:5
   5: diem_node::main
             at /home/david/.cargo/git/checkouts/diem-7dee55d666bb1ba0/668b495/diem-node/src/main.rs:16:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@dboreham
Copy link
Contributor Author

dboreham commented Aug 15, 2024

@dboreham
Copy link
Contributor Author

Not sure, but I think this is where the features for that code are defined: https://github.com/0LNetworkCommunity/diem/blob/release/diem-move/diem-vm/Cargo.toml#L63

@dboreham
Copy link
Contributor Author

Error string is in-tree, which is why it doesn't appear in Google searches: https://github.com/0LNetworkCommunity/diem/blob/release/diem-node/src/utils.rs#L14

@dboreham
Copy link
Contributor Author

Having loaded the context into my LLM, I think the solution is to just specify the "correct" set of features in our Cargo.tomldirective that specifies the dependency on diem-node (here).

Challenge is to determine what that set is.

@dboreham
Copy link
Contributor Author

Actually that was off-track. Reading the code (which of course has zero comments) in more detail, I think the error message is a red herring. The code is actually checking that we're not trying to run a binary that has test versions of native functions. I assume this is a safety check to stop foot-gunning problems in production?

@dboreham
Copy link
Contributor Author

Feature that enables the functions it's looking for seems to be #[test_only]

@dboreham
Copy link
Contributor Author

@dboreham
Copy link
Contributor Author

More digging:

In the diem repo, cargo tree -e features reports this:

├── diem-node feature "default" (command-line)

whereas in the libra-framework repo it reports this:

├── diem-node feature "default" (*)

@dboreham
Copy link
Contributor Author

Building in libra-framework, the features passed to the compiler for the dependency diem-framework:

     Running `/home/david/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc --crate-name diem_framework --edition=2021 /home/david/.cargo/git/checkouts/diem-7dee55d666bb1ba0/668b495/diem-move/framework/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=169 --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C split-debuginfo=unpacked --cfg 'feature="default"' --cfg 'feature="testing"' --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("default", "fuzzing", "proptest", "proptest-derive", "testing"))' -C 

@dboreham
Copy link
Contributor Author

In diem repo:

     Running `/home/david/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc --crate-name diem_framework --edition=2021 diem-move/framework/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=169 --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 --cfg 'feature="default"' --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("default", "fuzzing", "proptest", "proptest-derive", "testing"))' -C metadata=532e9a0dea2c8718

@dboreham
Copy link
Contributor Author

This is present in the libra build: --cfg 'feature="testing"'
While it is not present in the diem build.
This seems to be the origin of the problem.

@dboreham
Copy link
Contributor Author

Tentatively have this diagnosed:

The problem package here is diem-framework. The runtime error occurs because diem-node has a footgun check in main that looks for various test-mode-only native move functions. It finds some and so bails. Those functions in turn are present because the diem-framework package was built with the testing function enabled.

That happens because depending packages have testing enabled:

│       ├── diem-vm feature "testing"
│       │   ├── diem-vm v0.1.0 (https://github.com/0LNetworkCommunity/diem.git?branch=release#668b4953) (*)
│       │   ├── diem-framework feature "testing"
│       │   │   ├── diem-framework v0.1.0 (https://github.com/0LNetworkCommunity/diem.git?branch=release#668b4953) (*)
│       │   │   └── diem-move-stdlib feature "testing"
│       │   │       └── diem-move-stdlib v0.1.1 (https://github.com/0LNetworkCommunity/diem.git?branch=release#668b4953) (*)

This in turn is cause by cargo being dumb. It can't understand how to build two different binaries in the same workspace with different feature sets. So you get features randomly and silently enabled for no good reason.

In our case the reason is that there is also another package (that we didn't ask to build) in the workspace called libra-smoke-tests that depends on a package called smoke-test that in turn depends on diem-framework.

It turns on the testing feature (not surprising since it's a test utility): https://github.com/0LNetworkCommunity/libra-framework/blob/main/smoke-tests/Cargo.toml#L29

Referenced here: https://github.com/0LNetworkCommunity/libra-framework/blob/main/smoke-tests/Cargo.toml#L20
and here: https://github.com/0LNetworkCommunity/diem/blob/668b4953b660fba9313da90f0e7c5f9479f3ad39/testsuite/smoke-test/Cargo.toml#L26

@dboreham
Copy link
Contributor Author

dboreham commented Aug 19, 2024

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

No branches or pull requests

1 participant