nvc: use llvm as runtime dependency #54741
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jnozsc
force-pushed
the
fix-nvc
branch
2 times, most recently
from
8949dc4 to
54a89fb
Compare
bayandin
requested changes
May 18, 2020
| stable do | ||
| url "https://github.com/nickg/nvc/releases/download/r1.4.0/nvc-1.4.0.tar.gz" | ||
| sha256 "1a874bde284408c137a93b22f8f12b5b8c3368cefe30f3a5458ccdeffa0c6ad6" | ||
| # Upstream issue, only fix on master branch |
There was a problem hiding this comment.
Could you please provide a short summary of what this patch is fixing
Comment on lines
+32
to
+37
| # llvm 8+ is not supported https://github.com/nickg/nvc/commit/c3d1ae5700cfba6070293ad1bb5a6c198c631195 | ||
| # and llvm 7 has issue on stable https://github.com/nickg/nvc/commit/dfd5606a182e00d5f7a9e28902234b374c7b2863 | ||
| depends_on "llvm@6" |
There was a problem hiding this comment.
Can we add a patch from nickg/nvc@dfd5606 and use llvm@7?
There was a problem hiding this comment.
the patch does not seem work https://github.com/Homebrew/homebrew-core/runs/685654208. that's why I had to use llvm@6. lmk any thought
5 tasks
|
@bayandin will you be ok if i just rollback to |
Yep, that's fine. Sorry, it looks I've missed the last one comment. |
|
no worries, thanks for confirm that |
|
@bayandin ready for review |
|
Well done @jnozsc, thanks 🎉 |
bayandin
approved these changes
May 27, 2020
|
🤖 A scheduled task has triggered a merge. |
mitchblank
added a commit
to mitchblank/homebrew-core
that referenced
this pull request
Feb 18, 2021
The restriction on building against llvm@6 (later updated to llvm@7) was introduced by @jnozsc (Homebrew#54741) However the issue that was referenced was fixed before 1.5.0 was released... nickg/nvc@f48c651 ...so when homebrew updated to 1.5.0 this restriction should probably be dropped. According to the project's README it is tested with llvm {7,8,9} so at a minimum we should probably bump its dependency to llvm@9. However, this wouldn't be enough to fix the Big Sur bottling issue. Nothing in the project's build instructions suggests that llvm versions newer than 9.x are to be avoided these days so just switching to our default llvm (currently 11.1) instead. Seems to build and test fine.
BrewTestBot
pushed a commit
that referenced
this pull request
Feb 21, 2021
The restriction on building against llvm@6 (later updated to llvm@7) was introduced by @jnozsc (#54741) However the issue that was referenced was fixed before 1.5.0 was released... nickg/nvc@f48c651 ...so when homebrew updated to 1.5.0 this restriction should probably be dropped. According to the project's README it is tested with llvm {7,8,9} so at a minimum we should probably bump its dependency to llvm@9. However, this wouldn't be enough to fix the Big Sur bottling issue. Nothing in the project's build instructions suggests that llvm versions newer than 9.x are to be avoided these days so just switching to our default llvm (currently 11.1) instead. Seems to build and test fine. Closes #71444. Signed-off-by: Carlo Cabrera <[email protected]> Signed-off-by: BrewTestBot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install <formula>)?from #54700 (comment)