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

Update README for Halide 12 release. #6034

Merged
merged 1 commit into from
May 19, 2021
Merged

Update README for Halide 12 release. #6034

merged 1 commit into from
May 19, 2021

Conversation

alexreinking
Copy link
Member

Fixes #5961
Fixes #5154
Fixes #4216
Fixes #5633 (by not mentioning it)

@alexreinking alexreinking added the skip_buildbots Synonym for buildbot_test_nothing label May 19, 2021
@@ -156,16 +185,16 @@ Follow the above instructions to build LLVM or acquire a suitable binary
release. Then change directory to the Halide repository and run:

```
% cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_DIR=$LLVM_ROOT/lib/cmake/llvm -S . -B build
% cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_DIR=$LLVM_ROOT/lib/cmake/llvm -S . -B build
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this also require acquiring ninja, would would need to be added to the instructions? I'm not sure it's a good default.

Copy link
Member Author

@alexreinking alexreinking May 19, 2021

Choose a reason for hiding this comment

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

@abadams - I made this change for two reasons, let me know what you think:

  1. We only use Ninja with CMake on the buildbots, so if there are issues with the Makefile backend, we won't notice them.
  2. The user experience with Ninja is much nicer (parallel by default, cleaner output, etc.)

Also, it's perfectly optional and one can avoid ninja by simply dropping -G Ninja. I wrote a note on that below.

Copy link
Member

Choose a reason for hiding this comment

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

I think that people copy-paste build steps without reading or understanding them, and that this one isn't going to work without first telling them to acquire ninja. Maybe the text below could more explicitly say that if you get the error , try again without -G Ninja

@abadams
Copy link
Member

abadams commented May 19, 2021

LGTM except for a minor concern about making ninja the default.

@alexreinking alexreinking merged commit b5a34c3 into master May 19, 2021
@alexreinking alexreinking deleted the readme-v-12 branch May 19, 2021 20:47
@alexreinking alexreinking added this to the v12.0.0 milestone May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip_buildbots Synonym for buildbot_test_nothing
Projects
None yet
3 participants