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

Use pure-rust protoc #255

Merged
merged 2 commits into from
Jun 20, 2018
Merged

Use pure-rust protoc #255

merged 2 commits into from
Jun 20, 2018

Conversation

eira-fransham
Copy link
Contributor

@eira-fransham eira-fransham commented Jun 18, 2018

Closes #183

This was basically a couple hours of grep + sed pipelines and running cargo test until it compiled

@ghost ghost assigned eira-fransham Jun 18, 2018
@ghost ghost added the in progress label Jun 18, 2018
@tomaka
Copy link
Member

tomaka commented Jun 18, 2018

Shouldn't we generate the Rust files in the directory pointed by the OUT env var (idiomatic cargo)?
https://doc.rust-lang.org/cargo/reference/build-scripts.html#case-study-code-generation

@jamesray1
Copy link
Contributor

Unfortunately (like #259), this is also failing on Windows for me.

I can confirm that the diff locally between this PR and master is the same. Here is the cargo build output.

@eira-fransham
Copy link
Contributor Author

eira-fransham commented Jun 20, 2018

@jamesray1

process didn't exit successfully: `C:\Users\James\jrl\target\debug\build\ring-4327ef7c900cd242\build-script-build` (exit code: 101)

This is ring failing and has nothing to do with protoc.

@tomaka

I mentioned this on Riot but for posterity: rust-lang/rfcs#752 blocks us from doing this properly. There's a plugin implementing a workaround but A) it requires nightly and B) it requires an old nightly (it doesn't work anymore)

@tomaka
Copy link
Member

tomaka commented Jun 20, 2018

I suggest to not close #183 (or reopen it after merging) so that we fix that correctly eventually.

@ghost ghost assigned tomaka Jun 20, 2018
@tomaka tomaka merged commit 217fae4 into libp2p:master Jun 20, 2018
@ghost ghost removed the in progress label Jun 20, 2018
tomaka added a commit to tomaka/libp2p-rs that referenced this pull request Jun 21, 2018
tomaka added a commit that referenced this pull request Jun 21, 2018
* Revert "Remove old protoc scripts and artifacts (#262)"

This reverts commit 32ef50b.

* Revert "Use pure-rust protoc (#255)"

This reverts commit 217fae4.
@gnunicorn gnunicorn mentioned this pull request Jul 30, 2018
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.

3 participants