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
Disable llvm on macos #4916
Disable llvm on macos #4916
Conversation
I accidentally re-enabled it when adding the flaky check.
Codecov Report
@@ Coverage Diff @@
## main #4916 +/- ##
==========================================
+ Coverage 89.51% 89.53% +0.01%
==========================================
Files 226 226
Lines 50678 50677 -1
==========================================
+ Hits 45366 45375 +9
+ Misses 5312 5302 -10
Continue to review full report at Codecov.
|
./configure --prefix=$HOME/$PG_INSTALL_DIR --with-llvm LLVM_CONFIG=${{ matrix.llvm_config }} --with-openssl --without-readline --without-zlib --without-libxml ${{ matrix.pg_extra_args }} | ||
if [[ "${{ runner.os }}" == "Linux" ]]; then | ||
./configure --prefix=$HOME/$PG_INSTALL_DIR --with-llvm LLVM_CONFIG=${{ matrix.llvm_config }} --with-openssl --without-readline --without-zlib --without-libxml ${{ matrix.pg_extra_args }} | ||
else |
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.
Maybe check for MacOS explicitly?
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.
Yep, I'll fix it here: #4897
For now just returted the old version to get the CI back fast.
if [[ "${{ runner.os }}" == "Linux" ]]; then | ||
./configure --prefix=$HOME/$PG_INSTALL_DIR --with-llvm LLVM_CONFIG=${{ matrix.llvm_config }} --with-openssl --without-readline --without-zlib --without-libxml ${{ matrix.pg_extra_args }} | ||
else | ||
# the current github macos image has a buggy llvm installation so we build without llvm on mac | ||
./configure --prefix=$HOME/$PG_INSTALL_DIR --with-openssl --without-readline --without-zlib --without-libxml ${{ matrix.pg_extra_args }} | ||
fi |
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.
Cant you put --without-llvm
in pg_extra_args in matrix builder instead
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.
Yep, I'll fix it here: #4897
For now just returted the old version to get the CI back fast.
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 didn't notice it because of the build caching, going to fix this problem as well.
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.
then we dont need matrix.llvm_config anymore if we move this all into matrix builder into pg_extra_args, keeps the workflow cleaner
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.
Yep, that's what I've done there.
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.
this PR has no matrix builder changes
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.
There in #4897 I mean.
Let's merge this PR first, which is a revert of the earlier incorrect changes. Our CI is actually broken now, Bharaty reported this failure.
I accidentally re-enabled it when adding the flaky check.