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 Go installation and benchmark tests for Apple Silicon #2698

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

paulmaunders
Copy link
Contributor

@paulmaunders paulmaunders commented Dec 24, 2024

Description

This PR addresses the Rosetta errors encountered when running Go benchmarks on Apple Silicon Macs. It modifies the benchmark Dockerfile to install the correct architecture-specific version of Go and adds tmp.benchmarks to .gitignore.

Changes

  • Added architecture detection in Dockerfile to install correct Go binary:
    • Uses arm64 version on Apple Silicon/aarch64
    • Uses amd64 version on x86_64
  • Added tmp.benchmarks to .gitignore to avoid committing benchmark results

Testing

  • Verified on Apple Silicon Mac:
    • Dockerfile now downloads correct arm64 Go binary
    • Go tests run without Rosetta errors
    • Benchmarks complete successfully

Problem

Previously on Apple Silicon Macs, Go tests would fail with:

rosetta error: Rosetta is only intended to run on Apple Silicon with a macOS host using Virtualization.framework with Rosetta mode enabled

This was because the wrong architecture (x86_64) Go binary was being installed.

@paulmaunders paulmaunders changed the title Fix macos go benchmarks Fix Go installation and benchmark tests for Apple Silicon Dec 24, 2024
@paul-gauthier
Copy link
Collaborator

Thanks for trying aider and filing this PR.

The benchmark is intended to be run inside the docker container. Are you experiencing this problem when running it that way?

@paulmaunders
Copy link
Contributor Author

Yes, I'm running the benchmark inside the docker container on my Macbook M3

@paul-gauthier paul-gauthier merged commit 98a0f1c into Aider-AI:main Dec 27, 2024
1 check passed
@paul-gauthier
Copy link
Collaborator

I see, thanks for the fix.

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