-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
Re-organize the LLVM build instructions #6059
Re-organize the LLVM build instructions #6059
Conversation
BUILDING_FROM_SOURCE.md
Outdated
@@ -104,25 +104,33 @@ For any OS, you can use [`zigup`](https://github.com/marler8997/zigup) to manage | |||
If you prefer a package manager, you can try the following: | |||
|
|||
- For MacOS, you can install with `brew install [email protected]` | |||
- For, Ubuntu, you can use Snap, you can install with `snap install zig --classic --beta` | |||
- For systems with Snap, you can install with `snap install zig --classic --beta` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think referencing Ubuntu by name here is probably helpful to people running Ubuntu.
How about this?
- For systems with Snap, you can install with `snap install zig --classic --beta` | |
- For systems with Snap (such as Ubuntu), you can install with `snap install zig --classic --beta` |
BUILDING_FROM_SOURCE.md
Outdated
You may also need to manually specify a prefix env var like so: | ||
|
||
```sh | ||
export LLVM_SYS_160_PREFIX=/usr/local/opt/llvm@16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, shouldn't this also use brew --prefix
? 🤔 e.g.
export LLVM_SYS_160_PREFIX=/usr/local/opt/llvm@16 | |
export LLVM_SYS_160_PREFIX=$(brew --prefix llvm@16) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of suggestions/questions, but looking good so far! Thanks @stefanv! 😃
- `zig` needs `lib` directory - `llvm` needs development headers - Keep installation instructions together - Group by platform - No longer split installation and debugging instructions
37db031
to
6883c76
Compare
Thanks for the review @rtfeldman; I've updated the PR accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks so much @stefanv! 🎉
zig
needslib
directoryllvm
needs development headers