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.

Sign up for GitHub

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

Jump to bottom

[macOS] Update Clang/LLVM test to use absolute path #5819

Merged
merged 1 commit into from
Jun 28, 2022
Merged

[macOS] Update Clang/LLVM test to use absolute path #5819

merged 1 commit into from
Jun 28, 2022

Conversation

al-cheb
Copy link
Contributor

@al-cheb al-cheb commented Jun 27, 2022 •
edited
Loading

Description

After Homebrew/homebrew-core@34db459 commit was merged there is no anymore the /usr/local/opt/llvm symlink.

Before:

lrwxr-xr-x  1 runner  admin  23 Jun 22 16:15 /usr/local/opt/llvm -> ../Cellar/llvm/13.0.1_1
lrwxr-xr-x  1 runner  admin  23 Jun 22 16:15 /usr/local/opt/llvm@13 -> ../Cellar/llvm/13.0.1_1

After:

lrwxr-xr-x  1 runner  admin  24 Jun 27 02:43 /usr/local/opt/llvm@13 -> ../Cellar/llvm@13/13.0.1

vsphere-clone: llvm@13 is keg-only, which means it was not symlinked into /usr/local,
vsphere-clone: because this is an alternate version of another formula.

Related issue:

https://github.com/actions/virtual-environments-internal/issues/3917

Check list

  • Related issue / work item is attached
  • Tests are written (if applicable)
  • Documentation is updated (if applicable)
  • Changes are tested and related VM images are successfully generated

@al-cheb
Copy link
Contributor Author

al-cheb commented Jun 27, 2022

cc @MikeMcQuaid, @danielnachun, Could you please review?

@al-cheb
Copy link
Contributor Author

al-cheb commented Jun 27, 2022

If I try to link the formular I get:

Error: Could not symlink lib/libomp.dylib
Target /usr/local/lib/libomp.dylib
is a symlink belonging to libomp. You can unlink it:
  brew unlink libomp

To force the link and overwrite all conflicting files:
  brew link --overwrite llvm@13

To list all files that would be deleted:
  brew link --overwrite --dry-run llvm@13
Linking /usr/local/Cellar/llvm@13/13.0.1...

@danielnachun
Copy link

/usr/local/opt/llvm is always supposed to exist regardless of whether llvm is linked or not. What was supposed to happen was that /usr/local/opt/llvm is should point to LLVM 14 now instead of LLVM 13. How was llvm being installed here? brew install llvm should be enough for that directory to exist - no linking is needed.

@al-cheb
Copy link
Contributor Author

al-cheb commented Jun 27, 2022

/usr/local/opt/llvm is always supposed to exist regardless of whether llvm is linked or not. What was supposed to happen was that /usr/local/opt/llvm is should point to LLVM 14 now instead of LLVM 13. How was llvm being installed here? brew install llvm should be enough for that directory to exist - no linking is needed.

@danielnachun, currently, we install LLVM@13.

@danielnachun
Copy link

If you're explicitly installing llvm@13, then it will only be found through /usr/local/opt/llvm@13. /usr/local/opt/llvm will only exist if you've installed llvm, and it will point to LLVM 14. So if anything was previously hardcoded to /usr/local/opt/llvm, it will have to be changed to /usr/local/opt/llvm@13 or LLVM 14 will have to be used instead.

@miketimofeev miketimofeev merged commit b495a03 into actions:main Jun 28, 2022
MikeMcQuaid
MikeMcQuaid reviewed Jun 28, 2022
View reviewed changes
Copy link
Contributor

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Makes sense to me 👍🏻

@al-cheb al-cheb deleted the macos-fix-llvm-test branch June 28, 2022 10:07
@al-cheb al-cheb mentioned this pull request Jun 30, 2022
Closed
10 tasks
@michaelsbradleyjr michaelsbradleyjr mentioned this pull request Jul 4, 2022
Merged
michaelsbradleyjr added a commit to codex-storage/nim-codex that referenced this pull request Jul 6, 2022
@michaelsbradleyjr
ci: update GitHub Actions CI workflow to use msys2/setup-msys2@v2
4d059fa 
Main goal is to update the nim-codex CI workflow to use the
[`msys2/setup-msys2@v2`][setmsys2] GitHub Action, as used by the
[nimbus-eth2 workflow][ne2w] per changes made to it several months ago. The
`msys2/setup-msys2@v2` action has been used by other Status-org projects prior
to this commit, e.g. nim-leopard, nim-datastore, nim-status.

A fix is included for the failing macOS builds, related to
[actions/runner-images#5819][ave5819]. See [L151][L151].

All builds [succeed][succeed] (with build arch-targets verified as far as
possible via `ldd`, `otool`, `ntldd`), including `linux-i386` and
`windows-i386`, though the `i386` builds are presently disabled (commented
out). The `i386` builds can be enabled simply by uncommenting:

 ```
 # - os: linux
 #   cpu: i386
 ...
 # - os: windows
 #   cpu: i386
 ```

The resulting `.github/workflows/ci.yml` is a "remix" of the current workflows
for nimbus-eth2 and nim-codex (i.e. prior to this commit) along with techniques
learned from developing workflows for other Status-org repos. Some comments and
code-reorg help to clarify/explain what's done in the
`Derive environment variables` step.

`-d:limitStackUsage` has been adopted for `linux-amd64` builds from
[nimbus-eth2's workflow][ne2wL155] and [related code][ne2config] has been
copied into `config.nims`

`-d:limitStackUsage` can easily be dropped if it's not desirable for Codex.

Build targets use `-latest` for `runs-on`, i.e. `macos-latest`,
`ubuntu-latest`, `windows-latest`.

Through a combination of local testing and iterative pushes to GitHub, the
workflow's embedded Bash scripts have been revised to include only the
necessary steps for all builds to succeed, including `linux-i386` and
`windows-i386`.

The GitHub Actions workflow `.github/workflows/codecov.yml` has been removed,
while coverage data generation/upload steps have been added to
`.github/workflows/ci.yml` as the final steps conditional on
`if: runner.os == 'Linux' && matrix.target.cpu == 'amd64' && matrix.nim_branch == matrix.cov_branch`.

A redundant `--passC:'-m32 -mno-adx'` is used for `linux-i386` builds; the
redundant flags do not affect the build, but can be helpful when eyeballing
GitHub Actions builds with increased compile-time verbosity.

Some variable expansions used in `github/workflows/ci.yml` could result in
compilation failures if related paths include whitespace. It's not a problem
for this commit but could be a problem for a user copy-pasting from the
workflow; solving that problem is left as an exercise for the reader.

[setmsys2]: https://github.com/msys2/setup-msys2#readme
[ne2w]: https://github.com/status-im/nimbus-eth2/blob/stable/.github/workflows/ci.yml
[ave5819]: actions/runner-images#5819
[L151]: https://github.com/status-im/nim-codex/blob/ci/msys2/.github/workflows/ci.yml#L151
[succeed]: https://github.com/status-im/nim-codex/actions/runs/2606854455
[ne2wL155]: https://github.com/status-im/nimbus-eth2/blob/stable/.github/workflows/ci.yml#L155-L159
[ne2config]: https://github.com/status-im/nimbus-eth2/blob/stable/config.nims#L43-L49
michaelsbradleyjr added a commit to codex-storage/nim-codex that referenced this pull request Jul 7, 2022
@michaelsbradleyjr
ci: update GitHub Actions CI workflow to use msys2/setup-msys2@v2
2b31955 
Main goal is to update the nim-codex CI workflow to use the
[`msys2/setup-msys2@v2`][setmsys2] GitHub Action, as used by the
[nimbus-eth2 workflow][ne2w] per changes made to it several months ago. The
`msys2/setup-msys2@v2` action has been used by other Status-org projects prior
to this commit, e.g. nim-leopard, nim-datastore, nim-status.

A fix is included for the failing macOS builds, related to
[actions/runner-images#5819][ave5819]. See [L151][L151].

All builds [succeed][succeed] (with build arch-targets verified as far as
possible via `ldd`, `otool`, `ntldd`), including `linux-i386` and
`windows-i386`, though the `i386` builds are presently disabled (commented
out). The `i386` builds can be enabled simply by uncommenting:

 ```
 # - os: linux
 #   cpu: i386
 ...
 # - os: windows
 #   cpu: i386
 ```

The resulting `.github/workflows/ci.yml` is a "remix" of the current workflows
for nimbus-eth2 and nim-codex (i.e. prior to this commit) along with techniques
learned from developing workflows for other Status-org repos. Some comments and
code-reorg help to clarify/explain what's done in the
`Derive environment variables` step.

`-d:limitStackUsage` has been adopted for `linux-amd64` builds from
[nimbus-eth2's workflow][ne2wL155] and [related code][ne2config] has been
copied into `config.nims`

`-d:limitStackUsage` can easily be dropped if it's not desirable for Codex.

Build targets use `-latest` for `runs-on`, i.e. `macos-latest`,
`ubuntu-latest`, `windows-latest`.

Through a combination of local testing and iterative pushes to GitHub, the
workflow's embedded Bash scripts have been revised to include only the
necessary steps for all builds to succeed, including `linux-i386` and
`windows-i386`.

The GitHub Actions workflow `.github/workflows/codecov.yml` has been removed,
while coverage data generation/upload steps have been added to
`.github/workflows/ci.yml` as the final steps conditional on
`if: runner.os == 'Linux' && matrix.target.cpu == 'amd64' && matrix.nim_branch == matrix.cov_branch`.

A redundant `--passC:'-m32 -mno-adx'` is used for `linux-i386` builds; the
redundant flags do not affect the build, but can be helpful when eyeballing
GitHub Actions builds with increased compile-time verbosity.

Some variable expansions used in `github/workflows/ci.yml` could result in
compilation failures if related paths include whitespace. It's not a problem
for this commit but could be a problem for a user copy-pasting from the
workflow; solving that problem is left as an exercise for the reader.

[setmsys2]: https://github.com/msys2/setup-msys2#readme
[ne2w]: https://github.com/status-im/nimbus-eth2/blob/stable/.github/workflows/ci.yml
[ave5819]: actions/runner-images#5819
[L151]: https://github.com/status-im/nim-codex/blob/ci/msys2/.github/workflows/ci.yml#L151
[succeed]: https://github.com/status-im/nim-codex/actions/runs/2606854455
[ne2wL155]: https://github.com/status-im/nimbus-eth2/blob/stable/.github/workflows/ci.yml#L155-L159
[ne2config]: https://github.com/status-im/nimbus-eth2/blob/stable/config.nims#L43-L49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid left review comments

@miketimofeev miketimofeev miketimofeev approved these changes

@ddobranic ddobranic ddobranic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@al-cheb @danielnachun @MikeMcQuaid @miketimofeev @ddobranic