Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Fix error during build: failed to run custom build command for sc-net… #12494

Merged
paritytech-processbot merged 1 commit into from
Oct 26, 2022
Merged

Fix error during build: failed to run custom build command for sc-net… #12494

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/network/bitswap/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const PROTOS: &[&str] = &["bitswap.v1.2.0.proto"];
const PROTOS: &[&str] = &["src/schema/bitswap.v1.2.0.proto"];
Copy link
Contributor

@altonen altonen Oct 14, 2022

Choose a reason for hiding this comment

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

I'm fine with this change because it seems to work either way and for some reason the example code prost-build also adds the include path to the source files. But it would be interesting to know why the code as it was before is not working for you.

Copy link
Member

@bkchr bkchr Oct 14, 2022

Choose a reason for hiding this comment

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

If we look some lines below, we give the path src/schema as includes. Looking at the docs, the files given in PROTOS should be found in the given includes. So, it should work.

So, I also want to know why it doesn't work for you and then we need to report this to upstream.

Copy link
Contributor Author

@anonymousGiga anonymousGiga Oct 20, 2022

Choose a reason for hiding this comment

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

I understand "protos - Paths to .proto files to compile." means that "the parameter protos is the path of the file, not the file name", so the protos here should be added with src/schema. Is my understanding correct?

Btw, I reinstall protobuf compiler 3.20.3 to solve this problem. If it is determined that no modification is needed, I will close the pr.

Copy link
Member

@bkchr bkchr Oct 20, 2022

Choose a reason for hiding this comment

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

https://docs.rs/prost-build/latest/prost_build/fn.compile_protos.html

If you read the docs there, we do it exactly as explained in the docs. Others have reported that maybe deleting the target dir fixed it for them. There is clearly some bug, but I don't think in our code. None of our devs also reported this issue.

Copy link
Contributor Author

@anonymousGiga anonymousGiga Oct 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I asked PROST about the issue, and maybe they can answer the question. tokio-rs/prost#734

Copy link
Member

@bkchr bkchr Oct 21, 2022

Choose a reason for hiding this comment

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

Ty @anonymousGiga ❤️

Copy link
Contributor Author

@anonymousGiga anonymousGiga Oct 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I got a response from this issue（ tokio-rs/prost#734 ） , may be we should use "src/schema/bitswap.v1.2.0.proto" instead of "bitswap.v1.2.0.proto" here. @bkchr

Copy link
Member

@bkchr bkchr Oct 25, 2022

Choose a reason for hiding this comment

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

@anonymousGiga could you then please update all usages of compile_protos?

Copy link
Contributor Author

@anonymousGiga anonymousGiga Oct 26, 2022

Choose a reason for hiding this comment

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

I checked that all the other places where compile_protos is used are correct, and they all use in the way "prefix the protos with the path". In addition, I also updated to the latest master branch. @bkchr


fn main() {
prost_build::compile_protos(PROTOS, &["src/schema"]).unwrap();
Expand Down