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

rust: make cargo build slightly more debuggable #227

Merged
merged 3 commits into from
Feb 15, 2024

Conversation

woodruffw
Copy link
Member

Handing this PR off to @tnytown, who will switch us back to AOT generation, once again completing the cycle of pain.

Signed-off-by: William Woodruff <[email protected]>
@woodruffw woodruffw added the rust Rust bindings label Feb 13, 2024
@woodruffw
Copy link
Member Author

To summarize:

  • We were previously trying to release the prost-based crate with JIT generation via build.rs.
  • This was causing cargo publish to fail, since cargo publish uses a different CARGO_MANIFEST_DIR than cargo build, meaning that the relative paths to the protos directory wasn't correct. This is seemingly completely undocumented.
  • In a way, it's "good" that this was failing: if we had published this crate it would have failed even harder at cargo install time, since the JIT generation assumes the presence of the protos (including protoc's WKTs and the Google API types).

To fix all of that, we would have had to check all of the .proto files into the crate as well, meaning that we get very little benefit out of JIT generation. So we're going to switch back to AOT generation and hopefully simplify everything here drastically, at the cost of slightly more checked-in code.

@tnytown tnytown force-pushed the ww/fix-rust-publish branch from 91da056 to 7eecf40 Compare February 15, 2024 20:58
@woodruffw woodruffw marked this pull request as ready for review February 15, 2024 21:19
@woodruffw
Copy link
Member Author

LGTM, thanks @tnytown!

@@ -0,0 +1,11 @@
[package]
name = "sigstore-protobuf-specs-codegen"
version = "0.0.1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(whoops comment didnt post) Does this version need to be in sync with the protobuf-specs version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay for us to bump this version separately, similar to what we're doing with the derive crate -- at any rate, this shouldn't ever be published :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is purely an internal, helper-only crate (it performs the codegen for the actual crate, which will remain in sync with the protobuf-specs version).

@woodruffw woodruffw merged commit 08b04be into sigstore:main Feb 15, 2024
7 checks passed
@woodruffw woodruffw deleted the ww/fix-rust-publish branch February 15, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Rust bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants