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

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Conversation

rrmanukyan
Copy link
Contributor

@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
Contributor 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
Contributor 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
@ximinez ximinez added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Jan 7, 2025
@ximinez ximinez merged commit 040cd23 into XRPLF:develop Jan 7, 2025
19 checks passed
ximinez pushed a commit to ximinez/rippled that referenced this pull request Jan 13, 2025
q73zhao pushed a commit that referenced this pull request Feb 24, 2025
* python (3.13) and cmake (latest)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants