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

Update tezos/sys build to download headers #544

Closed
wants to merge 36 commits into from

Conversation

tizoc
Copy link
Contributor

@tizoc tizoc commented May 4, 2021

Still a draft, right now requires the environment vaiable OCAML_WHERE_PATH=$(pwd)/tezos/sys/lib_tezos/artifacts/include to be set when running cargo commands so that the OCaml headers can be found, waiting for cargo 1.52 to be release which includes a way to set environment variables that get passed down to the build process:

rust-lang/cargo#9175

@bkontur btw, I am thinking that it may be better to remove the libtezos download/copy logic from the build script and handle it on a separate standalone script, doing so will simplify things.

Bruno Deferrari and others added 30 commits April 20, 2021 10:51
@tizoc tizoc force-pushed the protocol-runner-storage branch from cf289bf to 2de786c Compare May 4, 2021 19:24
@bkontur
Copy link
Contributor

bkontur commented May 5, 2021

Still a draft, right now requires the environment vaiable OCAML_WHERE_PATH=$(pwd)/tezos/sys/lib_tezos/artifacts/include to be set when running cargo commands so that the OCaml headers can be found, waiting for cargo 1.52 to be release which includes a way to set environment variables that get passed down to the build process:

rust-lang/cargo#9175

@bkontur btw, I am thinking that it may be better to remove the libtezos download/copy logic from the build script and handle it on a separate standalone script, doing so will simplify things.

@tizoc
could you, please, write more about "separate standalone scipt" solution?
Do you mean it like manual step before running cargo?
We need to think about to have possibility to build/run it, just with one command cargo run / cargo test because of CI, if anybody wants to add it to theirs CI and so on

@tizoc
Copy link
Contributor Author

tizoc commented May 5, 2021

@bkontur I am still thinking about it, but the basic idea is to move the current build script to it's own cargo subcommand (cargo prepare-tezos or something like that, I have to investigate how that works -- I just want to avoid having to rewrite it in bash, I like how the Rust one works).

So it would be the same code, but will have to be called manually once before the actual build/test/run commands. Then the build script will just verify that the hashes match and fail if they don't.

There are two reasons for this:

  • It all happening as part of the build is kind of a PITA because sometimes it inadvertently overrides things (not an issue most of the time, but kind of annoying when you are working on changes to libtezos)
  • More importantly, this script now needs to download some files (the headers) that are needed by it's dependencies, so by the time the build script runs, it is already too late (sometimes it doesn't even run because the dependency fails to build because the headers are still not there).

@tizoc tizoc mentioned this pull request May 6, 2021
22 tasks
@tizoc tizoc force-pushed the protocol-runner-storage branch from d6fcb1c to b57daa2 Compare May 7, 2021 11:18
@tizoc
Copy link
Contributor Author

tizoc commented May 14, 2021

Replaced by #551 for now, closing.

@tizoc tizoc closed this May 14, 2021
@tizoc tizoc deleted the tezos-sys-new-build branch July 19, 2021 01:52
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