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

Figure out better way to include mopro-core without custom build error #83

Closed
oskarth opened this issue Feb 27, 2024 · 1 comment · Fixed by #190
Closed

Figure out better way to include mopro-core without custom build error #83

oskarth opened this issue Feb 27, 2024 · 1 comment · Fixed by #190
Labels

Comments

@oskarth
Copy link
Collaborator

oskarth commented Feb 27, 2024

Problem

Currently if we include mopro-core as an external dependency and try to run cargo build we get:

   Compiling mopro-core v0.1.0 (https://github.com/oskarth/mopro.git#67382964)
The following warnings were emitted during compilation:

warning: mopro-core@0.1.0: BUILD_CONFIG_PATH not set. Using default configuration.

error: failed to run custom build command for `mopro-core v0.1.0 (https://github.com/oskarth/mopro.git#67382964)`

Caused by:
  process didn't exit successfully: `/Users/user/repos/github.com/oskarth/mopro/mopro-cli-example/target/debug/build/mopro-core-5e7bbc90bb897fab/build-script-build` (exit status: 101)
  --- stdout
  cargo:warning=BUILD_CONFIG_PATH not set. Using default configuration.

  --- stderr
  thread 'main' panicked at /Users/user/.cargo/git/checkouts/mopro-471ecc433f4d4aff/6738296/mopro-core/build.rs:96:5:
  Make sure the zkey file exists. Did you forget to run a trusted setup? Adjust prepare.sh if necessary.
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

This is less than ideal because it breaks a common default workflow.

Notes

Using a config.toml file gets rid of this, but it shouldn't necessarily be a requirement for building whole project.

The default fallback is relative to mopro-core example circuits. Not clear how to deal with this in external dependency.

Can we have some other kind of fallback? Or get rid of that build complexity altogether somehow?

Strictly not necessary as we can just say "don't use cargo build", but would be nice to address.

Acceptance criteria

Make default cargo build work without additional work from user when including mopro as a dependency.

@oskarth oskarth added the DevEx label Feb 27, 2024
@oskarth
Copy link
Collaborator Author

oskarth commented Feb 27, 2024

One thing we could do is to be less aggressive in asserts here https://github.com/oskarth/mopro/blob/main/mopro-core/build.rs#L95-L105

And instead bubble up to run time, or compile time of user application. So if you try to run init on circuit that doesn't have arkzkey etc you get a panic with instruction to prepare this material. That way default cargo build would work.

(Also this is mopro semantics, so perhaps it is mopro build that should fail, not cargo build)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant