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

Install LLVM 10 for OSX CI #8982

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

bcardiff
Copy link
Member

@bcardiff bcardiff commented Apr 1, 2020

LLVM 10 has been releases on homebrew.

The osx ci is failing since LLVM_CONFIG environment variable in test_darwin points to a non-aliased llvm-config path. Either that path or the formula version needs to change when a new llvm formula version is released in homebrew for now.

@bcardiff bcardiff added this to the 0.34.0 milestone Apr 1, 2020
Comment on lines +114 to +115
on_osx brew install llvm@10 gmp libevent pcre openssl pkg-config
on_osx brew link --force llvm@10
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be on par with the homebrew formula?

Suggested change
on_osx brew install llvm@10 gmp libevent pcre openssl pkg-config
on_osx brew link --force llvm@10
on_osx brew install llvm gmp libevent pcre openssl pkg-config
on_osx brew link --force llvm

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to use the alias to a fixed version and bump it manually when we are ready. Otherwise until we have support for the next llvm version we would need to add the @V as a fix.

Ideally the LLVM_CONFIG path should work on any alias picked in this line. But since we already support llvm@10, this was the shortest change.

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I note is that the definition of LLVM_CONFIG_FINDER in /Makefile hasn't been updated for a while. That seems .. related. How do these things interact?

@RX14
Copy link
Contributor

RX14 commented Apr 1, 2020

Would be nice if we fixed the CI scripts to set LLVM_CONFIG correctly regardless of if it's an old formula or no.

@bcardiff bcardiff merged commit e7e7786 into crystal-lang:master Apr 1, 2020
@bcardiff bcardiff deleted the osx-install-llvm-10 branch April 2, 2020 14:21
carlhoerberg pushed a commit to carlhoerberg/crystal that referenced this pull request Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants