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

added macos dependency installation #5233

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

rrmanukyan
Copy link

@rrmanukyan rrmanukyan commented Jan 3, 2025

Added missing dependency installation on generic MacOS runner

High Level Overview of Change

Installing Python and cmake before the build process

Type of Change

  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)

Test Plan

A test pipeline was run on the forked repo

Copy link

@shichengripple001 shichengripple001 left a comment

Choose a reason for hiding this comment

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

I think these can be moved to the github-runner provisioning pipeline, so it wont be run for every build

@rrmanukyan
Copy link
Author

I think these can be moved to the github-runner provisioning pipeline, so it wont be run for every build

When dependencies are installed, each step takes around a second and the packages are not being reinstalled every time.
example: https://github.com/ripple/rippledavid/actions/runs/12603322049/job/35128142629

The reason it was not put in runner provisioning script is to keep it generic and the build pipeline runner agnostic.

@shichengripple001
Copy link

we may need to specify the python version at least, once there is new python version, it might potentially cause some issue.
Also I remember brew install will try to upgrade versions for install packages, that might take some time.

@rrmanukyan
Copy link
Author

rrmanukyan commented Jan 6, 2025

we may need to specify the python version at least, once there is new python version, it might potentially cause some issue. Also I remember brew install will try to upgrade versions for install packages, that might take some time.

Added Python version and moved both python and cmake installation under condition which will prevent unexpected updates.

Tests:
First run on a fresh runner: https://github.com/rrmanukyan/rippled/actions/runs/12635228168/job/35204685814
Second run: https://github.com/rrmanukyan/rippled/actions/runs/12635752221/job/35206312378

@legleux legleux self-requested a review January 6, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants