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

CI: Fix macOS pipeline failure #1255

Merged

Conversation

RickyC0626
Copy link
Contributor

@RickyC0626 RickyC0626 commented Apr 5, 2021

Changelog / Overview

Update ci.yml to update and upgrade brew if necessary before installing dependencies, and remove the unsupported dvisvgm.86_64-darwin package.

Motivation

CI macOS pipeline failure due to installation of a basictex package that no longer exists.

Exploring this avenue has also revealed that dvisvgm.86_64-darwin is no longer supported, as part of TeX Live 2021 release to support both ARM and Intel architectures. Instead, dvisvgm.universal-darwin will be used for MacTex, supporting macOS 10.14+. The legacy dvisvgm.x86_64-darwinlegacy binary folder will support macOS 10.6+, but it is only available with Unix install-tl.

Explanation for Changes

- name: Install system dependencies (MacOS)
        if: runner.os == 'macOS'
        run: |
          brew update && brew upgrade && brew cleanup
          brew install openssl readline ffmpeg pyenv pyenv-virtualenv
          brew install --cask basictex
          eval "$(/usr/libexec/path_helper -s)"
          sudo tlmgr update --self
          brew install pkg-config libffi pango glib
          sudo tlmgr install --reinstall standalone preview doublestroke relsize fundus-calligra wasysym physics dvisvgm rsfs wasy cm-super
          echo "/Library/TeX/texbin" >> $GITHUB_PATH
          echo "$HOME/.poetry/bin" >> $GITHUB_PATH

Testing Status

Further Comments

Checklist

  • I have read the Contributing Guidelines
  • I have written a descriptive PR title (see top of PR template for examples)
  • I have written a changelog entry for the PR or deem it unnecessary
  • My new functions/classes either have a docstring or are private
  • My new functions/classes have tests added and (optional) examples in the docs
  • My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

  • The PR title is descriptive enough
  • The PR is labeled correctly
  • The changelog entry is completed if necessary
  • Newly added functions/classes either have a docstring or are private
  • Newly added functions/classes have tests added and (optional) examples in the docs
  • Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

@RickyC0626 RickyC0626 changed the title CI: Possible fix for macOS pipeline failure CI: Fix macOS pipeline failure Apr 5, 2021
@behackl behackl added the infrastructure Anything related to our infrastructure label Apr 5, 2021
@behackl behackl added this to the Release v0.6.0 milestone Apr 5, 2021
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for investigating and fixing the pipeline!

@markromanmiller
Copy link
Collaborator

This is super helpful. Thanks for the hard work and speedy fix.

I'm not familiar with brew; is the update / upgrade / cleanup necessary given we're running this in a CI env, or was it more a temporary fix?

@RickyC0626
Copy link
Contributor Author

This is super helpful. Thanks for the hard work and speedy fix.

I'm not familiar with brew; is the update / upgrade / cleanup necessary given we're running this in a CI env, or was it more a temporary fix?

It was more of a temporary fix to solve the basictex issue, but it led to finding out about the legacy package. I think we can keep it as redundancy in case a similar issue occurs in the future.

@markromanmiller
Copy link
Collaborator

How much time does it add to the build? If we compare it to #1200 's build, it looks like it adds 4 minutes across the board. That is a lot of time for a "might be helpful" change.

@RickyC0626
Copy link
Contributor Author

How much time does it add to the build? If we compare it to #1200 's build, it looks like it adds 4 minutes across the board. That is a lot of time for a "might be helpful" change.

I do plan on working on caching as described in #824, for dependency-heavy packages such as ffmpeg to make the installation process faster.

@markromanmiller
Copy link
Collaborator

That will be helpful. For the time being, though - and assuming it still builds - can you take out that update line? Otherwise it LGTM.

@jsonvillanueva
Copy link
Member

Looks like that line was necessary

@RickyC0626
Copy link
Contributor Author

How much time does it add to the build? If we compare it to #1200 's build, it looks like it adds 4 minutes across the board. That is a lot of time for a "might be helpful" change.

Sped up the time by a few minutes by only upgrading the brew packages we need.

@jsonvillanueva jsonvillanueva merged commit ace2bda into ManimCommunity:master Apr 5, 2021
@markromanmiller
Copy link
Collaborator

Amazing, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Anything related to our infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants