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 macos-x86_64: Upgrade to the macos-13 image #3387

Merged
merged 31 commits into from
Oct 21, 2024

Conversation

kazarmy
Copy link
Member

@kazarmy kazarmy commented Oct 20, 2024

Your checklist for this pull request

Detailed description

The GitHub macos-12 image is in the process of being removed so this pr replaces it with the macos-13 image.

Test plan (required)

The replacement is appropriate.

Closing issues

...

@kazarmy
Copy link
Member Author

kazarmy commented Oct 20, 2024

Redness is unrelated

Copy link
Member

@karliss karliss left a comment

Choose a reason for hiding this comment

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

In my opinion macos-13 would be better.

Both for running the build on oldest reasonably available mac, and to avoid the "large" GHA instances.

@kazarmy kazarmy changed the title ci macos-x86_64: Upgrade to the macos-14-large image ci macos-x86_64: Upgrade to the macos-13 image Oct 20, 2024
@kazarmy
Copy link
Member Author

kazarmy commented Oct 20, 2024

In my opinion macos-13 would be better.

Both for running the build on oldest reasonably available mac, and to avoid the "large" GHA instances.

I agree 78a9998

@karliss
Copy link
Member

karliss commented Oct 20, 2024

As for the hdiutil failures I guess it's worth trying the second workaround described here.

actions/runner-images#7522 (comment)

@kazarmy
Copy link
Member Author

kazarmy commented Oct 20, 2024

Ok 29931e7

@kazarmy kazarmy marked this pull request as draft October 20, 2024 08:52
@karliss
Copy link
Member

karliss commented Oct 20, 2024

That's separate problem, but the M1 macos failure can be fixed by adding

                -DPython3_FIND_STRATEGY="LOCATION" \
                -DPython_FIND_STRATEGY="LOCATION" \

@kazarmy
Copy link
Member Author

kazarmy commented Oct 20, 2024

That's separate problem, but the M1 macos failure can be fixed by adding

                -DPython3_FIND_STRATEGY="LOCATION" \
                -DPython_FIND_STRATEGY="LOCATION" \

There's no guarantee that the first Python version found won't be 3.13.

Btw, the macos-14-arm64 runner image swung from 20241007.259 (Python 3.12) to 20241014.301 (Python 3.13) and back to 20241007.259 (Python 3.12) in the course of a few hours.

@kazarmy kazarmy marked this pull request as ready for review October 20, 2024 15:07
@karliss
Copy link
Member

karliss commented Oct 20, 2024

There's no guarantee that the first Python version found won't be 3.13.

I would hope that having explicit "Python_ROOT_DIR" hint has higher priority in the search order compared to whatever default search folders are.

Copy link
Member

@karliss karliss left a comment

Choose a reason for hiding this comment

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

<

.github/workflows/ci.yml Show resolved Hide resolved
@wargio
Copy link
Member

wargio commented Oct 21, 2024

WTF. why libquickjs| Compiler for C supports arguments /experimental:c11atomics: NO is not supported? is required for qjs

@wargio
Copy link
Member

wargio commented Oct 21, 2024

@kazarmy i merged my fix on jsdec repo (with also a test for msvc 2019)

# https://github.com/actions/runner-images/issues/7522#issuecomment-1556766641
echo killing XProtectBehaviorService; sudo pkill -9 XProtect >/dev/null || true;
echo waiting for XProtectBehaviorService kill; while pgrep XProtect; do sleep 3; done;
brew install kadwanev/brew/retry
Copy link
Member

Choose a reason for hiding this comment

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

maybe move this eariler since you are installing a dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok 0d5b7db

echo killing XProtectBehaviorService; sudo pkill -9 XProtect >/dev/null || true;
echo waiting for XProtectBehaviorService kill; while pgrep XProtect; do sleep 3; done;
brew install kadwanev/brew/retry
retry 'rm -rf ../jsdec ../libswift ../rz_libyara ../rz-silhouette; make package'
Copy link
Member

Choose a reason for hiding this comment

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

why this is done on some plugins and not all the plugins? is this related to some redness where they do not compile? if yes, add a comment explaining why this is here.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's due to (https://github.com/rizinorg/cutter/actions/runs/11425970615/job/31788307472#step:7:9706):

rimraf-reason

the other plugins were rimrafed because the build is so slow and it would have taken too long to check the plugins one-by-one

Copy link
Member Author

Choose a reason for hiding this comment

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

if yes, add a comment explaining why this is here.

ok 97b93a5

@kazarmy kazarmy marked this pull request as draft October 21, 2024 11:23
@kazarmy kazarmy marked this pull request as ready for review October 21, 2024 11:35
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@kazarmy kazarmy merged commit 22094c2 into rizinorg:dev Oct 21, 2024
14 checks passed
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