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

Fix for build failure due to change in TypeId hash size (64 to 128 bits) in Rust 1.72.0. #194

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

timmyjose
Copy link
Contributor

@timmyjose timmyjose commented Aug 29, 2023

Changes (due to Rust 1.72 having changed TypeId from 64 to 128 bits):

  • updated v8 to 0.75.1 (latest) for the gateway crate.

Notes: (full notes on Linear):

v0.62.2:

const _: () = {
  assert!(size_of::<TypeId>() == size_of::<u64>());
  assert!(align_of::<TypeId>() == align_of::<u64>());
};

latest (v0.75.1):

const _: () = {
  assert!(
    size_of::<TypeId>() == size_of::<u64>()
      || size_of::<TypeId>() == size_of::<u128>()
  );
  assert!(
    align_of::<TypeId>() == align_of::<u64>()
      || align_of::<TypeId>() == align_of::<u128>()
  );
};

  - updated `v8` to `0.75.1` (latest) for the `gateway` crate.
@linear
Copy link

linear bot commented Aug 29, 2023

ENG-1017 `cargo build` on `Polybase-rust` fails with rustc 1.72.0 due to changes in `TypeId`.

The v8 crate dependency (for the gateway crate) uses v0-62.2 which has the following check in isolate.rs:

assert!(size_of::<TypeId>() == size_of::<u64>());

which fails in Rust 1.72 due to TypeId having been changed to 128bits:

Release notes: https://github.com/rust-lang/rust/releases/tag/1.72.0

Relevant issue: rust-lang/compiler-team#608

This is causing a failure in CI as well (the other jobs passed due to caching, I presume):

 #27 58.37 error[E0080]: evaluation of constant value failed
#27 58.37     --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/v8-0.62.2/src/isolate.rs:1715:3
#27 58.37      |
#27 58.37 1715 |   assert!(size_of::<TypeId>() == size_of::<u64>());
#27 58.37      |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: size_of::<TypeId>() == size_of::<u64>()', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/v8-0.62.2/src/isolate.rs:1715:3
#27 58.37      |
#27 58.37      = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
#27 58.37 
#27 58.42 For more information about this error, try `rustc --explain E0080`.
#27 58.43 error: could not compile `v8` (lib) due to previous error
#27 58.43 warning: build failed, waiting for other jobs to finish...
#27 81.40 thread 'main' panicked at 'Exited with status code: 101', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-chef-0.1.62/src/recipe.rs:204:27
#27 81.40 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
#27 ERROR: process "/bin/sh -c cargo chef cook --recipe-path /recipe.json $(if [ \"$RELEASE\" = \"1\" ]; then echo \"--release\"; fi)" did not complete successfully: exit code: 101
------
 > [builder  2/13] RUN --mount=type=cache,target=/usr/local/cargo/registry     cargo chef cook --recipe-path /recipe.json $(if [ "1" = "1" ]; then echo "--release"; fi):
#27 58.37 1715 |   assert!(size_of::<TypeId>() == size_of::<u64>());
#27 58.37      |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: size_of::<TypeId>() == size_of::<u64>()', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/v8-0.62.2/src/isolate.rs:1715:3
#27 58.37      |
#27 58.37      = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
#27 58.37 
#27 58.42 For more information about this error, try `rustc --explain E0080`.
#27 58.43 error: could not compile `v8` (lib) due to previous error
#27 58.43 warning: build failed, waiting for other jobs to finish...
#27 81.40 thread 'main' panicked at 'Exited with status code: 101', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-chef-0.1.62/src/recipe.rs:204:27
#27 81.40 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@timmyjose
Copy link
Contributor Author

Sanity checks done:

  • Ran the e2e tests locally
  • Ran some manual operations locally (create collections and collection records).

@timmyjose timmyjose changed the title Changes (due to Rust 1.72 having changed TypeId from 64 to 128 bits): Fix for build failure due to change in TypeId hash size (64 to 128 bits) in Rust 1.72.0. Aug 29, 2023
Copy link
Collaborator

@mateuszmlc mateuszmlc left a comment

Choose a reason for hiding this comment

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

We should also change the Rust version in the Dockerfile either to 1.72 or back to just 1 (and let docker pick latest minor) https://github.com/polybase/polybase-rust/blob/main/docker/Dockerfile#L2

@timmyjose
Copy link
Contributor Author

We should also change the Rust version in the Dockerfile either to 1.72 or back to just 1 (and let docker pick latest minor) https://github.com/polybase/polybase-rust/blob/main/docker/Dockerfile#L2

Good catch! I think changing that to "1" makes sense - let Docker pick up the latest minor version.

@timmyjose timmyjose merged commit 8ede7cc into main Aug 29, 2023
6 checks passed
@calummoore
Copy link
Contributor

Nice work team!

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