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

Update MacOS runner to Ventura, add MacOS Sonoma (M1) runner #4393

Merged
merged 5 commits into from
Feb 29, 2024
Merged

Update MacOS runner to Ventura, add MacOS Sonoma (M1) runner #4393

merged 5 commits into from
Feb 29, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Feb 5, 2024 •
edited
Loading

Fixes #4390. Build P4C for MacOS Sonoma (M1) and Ventura.
https://github.blog/changelog/2024-01-30-github-actions-macos-14-sonoma-is-now-available/

TODO: Add support for the behavioral model and P4Testgen. I might do that in a separate PR.

@fruffy fruffy force-pushed the fruffy/m1_mac branch 2 times, most recently from b4925d1 to e8e3413 Compare February 6, 2024 17:16
@fruffy fruffy added the infrastructure Topics related to code style and build and test infrastructure. label Feb 20, 2024
@fruffy fruffy force-pushed the fruffy/m1_mac branch 6 times, most recently from d90750f to 390d1ef Compare February 27, 2024 02:13
@fruffy fruffy mentioned this pull request Feb 27, 2024
Merged
@fruffy fruffy force-pushed the fruffy/m1_mac branch 4 times, most recently from 425db07 to 11b7e36 Compare February 27, 2024 16:49
@fruffy fruffy changed the title [WiP] Check M1 macOS support in CI Update MacOS runner to Ventura, add MacOS Sonoma (M1) runner Feb 27, 2024
@fruffy fruffy marked this pull request as ready for review February 27, 2024 20:34
@fruffy fruffy requested review from rst0git, asl and grg February 27, 2024 20:35
asl
asl reviewed Feb 27, 2024
View reviewed changes
tools/install_mac_deps.sh Outdated
# Install some custom requirements on OS X using brew
BOOST_LIB="boost@1.84"
brew install autoconf automake bdw-gc ccache cmake \
libtool openssl pkg-config coreutils bison grep \
Copy link
Contributor

Choose a reason for hiding this comment

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

What's about flex? I remember macos shipped some ancient version by default, I do not recall the present status (esp. on builders)

Copy link
Contributor

Choose a reason for hiding this comment

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

On Ventura laptop I'm having:

/usr/bin/flex --version
flex 2.6.4 Apple(flex-34)

This seems to be enough, ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, I have an older version on Sonoma (14.3.1):

flex 2.5.35 Apple(flex-32)

Copy link
Contributor

@asl asl Feb 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Probably depends on installed xcode / command line tools as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, should I add a custom version? I believe in any case flex should be new enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the default version is fine for recent versions of MacOS. I've built it locally on my system in the past with 2.5.35.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, default is fine now.

grg
grg approved these changes Feb 27, 2024
View reviewed changes
tools/install_mac_deps.sh Outdated
BOOST_LIB="boost@1.76"
# Set up Homebrew differently for arm64.
if [[ $(uname -m) == 'arm64' ]]; then
(echo; echo 'eval "$(/opt/homebrew/bin/brew shellenv)"') >> ~/.zprofile
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a clean environment each time CI runs? I assume it is, but asking in case we need to check for the eval line before adding it to .zprofile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you have a suggestion for a cleaner command to set up brew? I do not have a Mac so I am just pasting what people suggest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ideally, one should be able to run this command locally, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems reasonable to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Used ChatGPT to "enhance" the script with some guards, please take another look.

@asl
Copy link
Contributor

asl commented Feb 27, 2024 •
edited
Loading

Just side one: in order to test the action before commit the proper way is:

  • Ensure the workflow yaml does exist in main branch (it is)
  • Add workflow_dispatch event
  • Now you can trigger it manually from the branch and see if it works.

asl
asl reviewed Feb 27, 2024
View reviewed changes
tools/install_mac_deps.sh Outdated Show resolved Hide resolved
@fruffy
Copy link
Collaborator Author

fruffy commented Feb 27, 2024

Just side one: in order to test the action before commit the proper way is:

* Ensure the workflow yaml does exist in `main` branch (it is)

* Add `workflow_dispatch` event

* Now you can trigger it manually from the branch and see if it works.

Interesting, never tried this. Although you still need to have your changes uploaded, no? This is a workflow to test without creating a PR or a fork?

@asl
Copy link
Contributor

asl commented Feb 27, 2024

Interesting, never tried this. Although you still need to have your changes uploaded, no? This is a workflow to test without creating a PR or a fork?

Yes. exactly. Just push the branch and trigger from branch by hands. Might be better and not waste CI cycles for other actions

rst0git
rst0git approved these changes Feb 29, 2024
View reviewed changes
Copy link
Member

@rst0git rst0git left a comment

Choose a reason for hiding this comment

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

LGTM

@fruffy fruffy added this pull request to the merge queue Feb 29, 2024
Merged via the queue into main with commit 3b72025 Feb 29, 2024
17 checks passed
@fruffy fruffy deleted the fruffy/m1_mac branch February 29, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grg grg grg approved these changes

@rst0git rst0git rst0git approved these changes

@asl asl asl approved these changes

Assignees
No one assigned
Labels
infrastructure Topics related to code style and build and test infrastructure.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

P4C arm64/aarch64 support
4 participants
@fruffy @asl @grg @rst0git