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

Support build.rs git-lfs fetching for proto_descriptor #3038

Closed
conorsch opened this issue Sep 15, 2023 · 2 comments
Closed

Support build.rs git-lfs fetching for proto_descriptor #3038

conorsch opened this issue Sep 15, 2023 · 2 comments
Assignees

Comments

@conorsch
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When trying to build a Rust project that uses the Penumbra crates as dependencies, cargo build fails with an error on the penumbra-proto crate, because the proto_descriptor.bin file isn't found.

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A workaround is to set a path dependency in the Cargo.toml file, rather than e.g. a git dependency. Provided you have the git-lfs assets checked out locally, that'll work.

Additional context
We've encountered this problem before, e.g. in Galileo penumbra-zone/galileo#29 , and our "fix" was to use path dependencies as a workaround. However there's also the dope #2201 which resolves the lfs problem specifically for the proof params.

Let's port that build.rs logic to the proto crate so that external projects can use git dependencies.

Unsure what the implications of the build.rs strategy are for future hosting on crates.io; I suggest we figure that out later.

@hdevalence
Copy link
Member

In the meantime, we can disable LFS for the proto descriptor. (We're already checking in hex-formatted proto descriptors into the repo in the form of the generated Go code).

hdevalence added a commit that referenced this issue Sep 17, 2023
hdevalence added a commit that referenced this issue Sep 17, 2023
hdevalence added a commit that referenced this issue Sep 17, 2023
@conorsch conorsch self-assigned this Sep 22, 2023
@conorsch conorsch moved this to Next (Steal from here) in Testnets Sep 22, 2023
@hdevalence
Copy link
Member

Closing with #3044 as "good enough" for now.

@github-project-automation github-project-automation bot moved this from Next (Steal from here) to Testnet 61: Dione in Testnets Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Testnet 61: Dione
Development

No branches or pull requests

2 participants