Skip to content

Rewrite build.rs#512

Merged
mohanson merged 4 commits intonervosnetwork:developfrom
mohanson-fork:ci
Apr 16, 2026
Merged

Rewrite build.rs#512
mohanson merged 4 commits intonervosnetwork:developfrom
mohanson-fork:ci

Conversation

@mohanson
Copy link
Copy Markdown
Collaborator

@mohanson mohanson commented Apr 13, 2026

  1. Build using clang instead of gcc on Windows.
  2. The code in build.rs has been significantly simplified.
  3. The test worked fine on SP1.

Ref #511

@mohanson
Copy link
Copy Markdown
Collaborator Author

mohanson commented Apr 13, 2026

@eval-exec Do you want this change to be included in the release version?

cc @chenyukang

Comment thread build.rs Outdated
println!("cargo:rustc-cfg=has_asm")
"riscv64" => {
cc::Build::new()
.compiler(clang_finder::find())
Copy link
Copy Markdown
Contributor

@eval-exec eval-exec Apr 13, 2026

Choose a reason for hiding this comment

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

Not sure do we really need to hardcode clang and ignore CC env var here?
cc-rs allow cargo usage like: CC=clang cargo build .....

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, perhaps we don't need to specify compiler.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

However, if we do this, we will always need to set the CC environment variable when compiling on Windows, see https://github.com/nervosnetwork/ckb-vm/actions/runs/24328961143/job/71030179557?pr=512

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Or should we do it the old way: not specifying a compiler on Unix, but specifying a compiler on Windows?

Copy link
Copy Markdown
Contributor

@eval-exec eval-exec Apr 13, 2026

Choose a reason for hiding this comment

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

How about create .cargo/Cargo.toml, config CC=clang in [env]?
https://doc.rust-lang.org/cargo/reference/config.html#env

Or should we do it the old way: not specifying a compiler on Unix, but specifying a compiler on Windows?

Both are fine.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not a good idea. On Windows, clang is simply called clang, but on Unix, clang is usually named clang-19, clang-20, clang-21, and so on. This is why I initially introduced the clang-finder package.

Copy link
Copy Markdown
Contributor

@eval-exec eval-exec Apr 13, 2026

Choose a reason for hiding this comment

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

cc-rs support: <var>_<target> - for example:

  • CC_x86_64_pc_windows_msvc
  • CC_x86_64_unknown_linux_gnu

https://docs.rs/cc/latest/cc/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does ckb-vm have min/max version requirements for clang?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There are no restrictions on clang versions.

@mohanson mohanson requested a review from eval-exec April 13, 2026 07:13
@mohanson mohanson merged commit 01fbe38 into nervosnetwork:develop Apr 16, 2026
8 checks passed
@mohanson mohanson deleted the ci branch April 16, 2026 05:46
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