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

prost: Pre-compile the tool #829

Merged
merged 1 commit into from
Nov 22, 2021
Merged

prost: Pre-compile the tool #829

merged 1 commit into from
Nov 22, 2021

Conversation

NickeZ
Copy link
Collaborator

@NickeZ NickeZ commented Nov 11, 2021

Compiling the tool at build time increases build time and complicates
things when we want to support "cargo test" in the rust libraries.

@@ -56,8 +58,7 @@ add_custom_command(
# Using prost-build the normal way as part of build.rs does not work due to a cargo bug:
# https://github.com/danburkert/prost/issues/344#issuecomment-650721245
Copy link
Collaborator

Choose a reason for hiding this comment

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

Repo moved, the comment is now here: tokio-rs/prost#344 (comment)

It seems the relevant bug has been fixed and stabilized in 1.51.

I think this means we can delete the prost-build altogether and use prost as it is documented, without using this custom tool as a workaround.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I made a new PR.

@NickeZ
Copy link
Collaborator Author

NickeZ commented Nov 16, 2021

Reopened. As I said in #831: #819 depends on either #829 or #831.

@benma
Copy link
Collaborator

benma commented Nov 19, 2021

Let's go with this one, seems to have many benefits over the other (faster build times, no explosion in the vendor folder, etc).

I build and pushed the Dockerfile with the changes of this PR as tag 22. If you rebase and point CI to :22 like here, CI here should pass:

https://github.com/digitalbitbox/bitbox02-firmware/pull/839/files

(or wait until that PR is merged and then rebase).

Copy link
Collaborator

@benma benma left a comment

Choose a reason for hiding this comment

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

tACK, but best to wait until https://github.com/digitalbitbox/bitbox02-firmware/pull/839/files is merged and then rebase, so the CI here can pass.

@NickeZ NickeZ force-pushed the pre-compile-prost-build branch from 0b88f3c to 5aa240e Compare November 20, 2021 23:04
@NickeZ
Copy link
Collaborator Author

NickeZ commented Nov 20, 2021

rebased

Compiling the tool at build time increases build time and complicates
things when we want to support "cargo test" in the rust libraries.
@NickeZ NickeZ force-pushed the pre-compile-prost-build branch from 5aa240e to 03fa841 Compare November 20, 2021 23:06
@benma benma merged commit d7e59db into master Nov 22, 2021
@benma benma deleted the pre-compile-prost-build branch November 22, 2021 08:38
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