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

SVM: Move protobuf into separate conformance crate #1946

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

buffalojoec
Copy link

@buffalojoec buffalojoec commented Jul 1, 2024

Problem

The use of prost and protobuf fixtures in the SVM crate's test suite depends on a
build script using prost-build. Since Cargo doesn't currently offer a smooth way to
add test-only build scripts, this build script gets run whenever someone imports
solana-svm.

The prost-build library requires the host operating system to have the protobuf
compiler installed in order to successfully execute the build script to build the protobuf
bindings.

Rather than impose this resource requirement on downstream users in their applications,
since we only use the protobuf bindings to test the SVM, we can instead extract them
into a separate crate for testing only.

Summary of Changes

Extract the protobuf files, the prost-build build script, and the proto module out
of solana-svm tests and into a new crate solana-svm-conformance.

Note the new crate is non-publish.

LucasSte
LucasSte previously approved these changes Jul 2, 2024
@buffalojoec
Copy link
Author

Sorry @LucasSte I had to rebase to resolve conflicts. My code is unchanged.

@buffalojoec buffalojoec merged commit 1d9d6fd into anza-xyz:master Jul 2, 2024
51 checks passed
samkim-crypto pushed a commit to samkim-crypto/agave that referenced this pull request Jul 31, 2024
* init svm-conformance crate

* svm-conformance: add protos

* svm: use svm-conformance for tests

* svm: drop protos
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.

2 participants