Skip to content

fix: add support for macos in cert chain script - #143

Open
Jesse Wright (jeswr) wants to merge 2 commits into
microsoft:mainfrom
jeswr:fix/macOS-support
Open

fix: add support for macos in cert chain script#143
Jesse Wright (jeswr) wants to merge 2 commits into
microsoft:mainfrom
jeswr:fix/macOS-support

Conversation

@jeswr

Copy link
Copy Markdown

This gets the setup scripts working on a (my) macOS machine.

I have NOT tested this change on Linux and Windows - these changes should be tested on those environments before merging this PR.

Copilot AI left a comment

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.

Pull Request Overview

This PR aims to enable macOS compatibility for the certificate chain generation script.

  • Updated directory path computations to work across Linux, Windows, and macOS
  • Changed how scriptsdir and outdir are determined using realpath
Comments suppressed due to low confidence (1)

circuit_setup/scripts/gen_x509_cert_chain.sh:17

  • The updated scriptsdir is set to an absolute path despite the comment stating that relative paths are to be used. Please verify if the intention is to use absolute paths on macOS and update the comment accordingly, or adjust the command to compute a relative path.
scriptsdir=$(realpath "$current_dir")

@christianpaquin

Copy link
Copy Markdown
Contributor

Thanks Jesse Wright (@jeswr). Does the rest of the build works on macOS? Specifically the ./build_all.sh? What about the tests in /creds: cargo test --release?

@jeswr

Copy link
Copy Markdown
Author

I haven't tested those; and am going to be AFK for the next 3 days.

Can test when I return

@moven0831

Copy link
Copy Markdown

I ran into the same issue on my macbook, and thanks to Jesse Wright (@jeswr) for the elegant solution here.

I also ran ./build_all.sh, which went smoothly for all of the circuit setup. But for ./setup-sample.sh, the getrandom crate failed to compile:

# Enable wasm build for getrandom
[target.'cfg(target_arch = "wasm32")'.dependencies]
getrandom_03 = { package = "getrandom", version = "0.3", features = ["wasm_js"] }
getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] }

Also, to pass cargo test --release, the halo2curve's "asm" feature needs to be disabled since it only supports x86_64 targets:

# Enable 'asm' feature when NOT a wasm build
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
halo2curves = { version = "0.8.0", features = ["asm"] }
"

@christianpaquin

Copy link
Copy Markdown
Contributor

I also ran ./build_all.sh, which went smoothly for all of the circuit setup. But for ./setup-sample.sh, the getrandom crate failed to compile:

# Enable wasm build for getrandom
[target.'cfg(target_arch = "wasm32")'.dependencies]
getrandom_03 = { package = "getrandom", version = "0.3", features = ["wasm_js"] }
getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] }

Any recommended fix for the getrandom build failure?

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.

4 participants