Skip to content

Conversation

@gballet
Copy link
Collaborator

@gballet gballet commented Feb 2, 2025

This is the list of changes that are necessary for running phant with 0.14.0

At this stage, it doesn't cross-compile to riscv, which is the next step.

Signed-off-by: Guillaume Ballet <[email protected]>
@gballet gballet force-pushed the upgrade-to-0.14.0 branch from 109c7dd to ce38f82 Compare May 10, 2025 08:43
gballet added 5 commits May 10, 2025 18:08
Signed-off-by: Guillaume Ballet <[email protected]>
Signed-off-by: Guillaume Ballet <[email protected]>
Signed-off-by: Guillaume Ballet <[email protected]>
@gballet gballet requested review from Copilot and jsign and removed request for Copilot May 10, 2025 16:09
@gballet gballet marked this pull request as ready for review May 10, 2025 16:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the repository to Zig v0.14.0, updating API usage, dependency references, and build configurations to align with the new version.

  • Updated API calls in src/main.zig to reflect changes in the httpz package.
  • Adjusted type information usage in src/blockchain/vm.zig for Zig’s updated type introspection.
  • Modified build configurations in build.zig.zon and build.zig as well as version references in README.md and CI workflow files.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main.zig Updated httpz server and router API calls with added error handling.
src/blockchain/vm.zig Replaced "Optional.child" with "optional.child" per Zig 0.14.0 syntax.
build.zig.zon Refactored dependency definitions and project metadata updates.
build.zig Switched from static library assembly to CMake builds and fixed linking redundancy in tests.
README.md Updated the Zig version reference to 0.14.0.
.github/workflows/ci.yml Updated Zig version for CI jobs to align with 0.14.0.

gballet and others added 2 commits May 10, 2025 18:11
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Guillaume Ballet <[email protected]>
jsign
jsign previously approved these changes May 27, 2025
Comment on lines -79 to -98
const ethash = b.addStaticLibrary(.{
.name = "ethash",
.optimize = optimize,
.target = target,
});
const cflags = [_][]const u8{
"-Wall", "-O3", "-fvisibility=hidden",
"-fvisibility-inlines-hidden", "-Wpedantic", "-Werror",
"-Wextra", "-Wshadow", "-Wconversion",
"-Wsign-conversion", "-Wno-unknown-pragmas", "-fno-stack-protector",
"-Wimplicit-fallthrough", "-Wmissing-declarations", "-Wno-attributes",
"-Wextra-semi", "-fno-exceptions", "-fno-rtti",
"-Wno-deprecated", // this one is used to remove a warning about char_trait deprecation
"-Wno-strict-prototypes", // this one is used by glue.c to avoid a warning that does not disappear when the prototype is added.
};
ethash.addCSourceFiles(.{ .root = b.path(""), .files = &[_][]const u8{"ethash/lib/keccak/keccak.c"}, .flags = &cflags });
ethash.addIncludePath(b.path("ethash/include"));
ethash.linkLibC();
ethash.linkLibCpp();
b.installArtifact(ethash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Uhm, I don't see any further import changes. Was this an unused module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So for context, with 0.14.0 the hacks I did to build evmone directly from zig stopped working. It looks like the new C++ lib that is provided doesn't really work with their C++ code. That's why I fell back to building the lib. (to be continued in the comment about cmake)

Comment on lines +79 to +80
const evmone_cmake_config_step = b.addSystemCommand(&.{ "cmake", "-S", "evmone", "-B", "zig-out/evmone_build" });
const evmone_cmake_build_step = b.addSystemCommand(&.{ "cmake", "--build", "zig-out/evmone_build" });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this means phant user needs now to have cmake installed and maybe further dependencies?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For now yes. I am working on a subsequent PR to gradually replace this with zevem. I could go back to your initial approach of including some pre-compiled lib, but I am building phant on three different archs and it's simpler for me to build evmone with the right arch, rather than maintaining 3 binary blobs.

There's an extra issue with this PR, which the next one is supposed to address: the riscv32 build doesn't work. That's why it will be the first target for zevem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and for the record, I don't like cmake, I think it's the wrong way to do a build system. That's why I was very happy to be able to build everything with zig.

jsign
jsign previously approved these changes May 27, 2025
@gballet
Copy link
Collaborator Author

gballet commented May 28, 2025

One of the issues is fixed by my follow-up PR so I'm not going to worry about this one. The other one, however, is a good point that it doesn't support cross-compilation.

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