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

Document helpful tips for dev on Apple-silicon/ARM #215

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

tarilabs
Copy link
Member

Resolves #214

Description

bootstrap documentation as discussed.
Follows-up also on:

How Has This Been Tested?

n/a

Merge criteria:

  • The commits and have meaningful messages; the author will squash them after approval or will ask to merge with squash.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@tarilabs tarilabs requested review from isinyaaa and a team November 29, 2023 13:29
@spolti
Copy link
Member

spolti commented Nov 29, 2023

A small change that I do when something fails on apple, is to force podman to use amd64 in the build commands.
podman build --arch=amd64.

works pretty nice.

We could have in the Makefile a if to append args, e.g. when using podman && arm, appends args.

@tarilabs
Copy link
Member Author

Ciao @spolti !! 👋

I've tried Podman, but for some reasons it didn't work well on Mac (at least I've installed all necessary Mac-helpers) specifically when using TestContainers for Go it required code-changes (beyond config changes) unfortunately.
It's a bit sad because I really preferred to use Podman if possible.

I'm not against using it, but at least when developing for this project, so far the best experience was with Colima.

Always happy to hear if anyone can report difference experiences!

@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (da35fd3) 74.52% compared to head (a127c05) 74.52%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #215   +/-   ##
=======================================
  Coverage   74.52%   74.52%           
=======================================
  Files          17       17           
  Lines        2296     2296           
  Branches       73       73           
=======================================
  Hits         1711     1711           
  Misses        420      420           
  Partials      165      165           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@spolti
Copy link
Member

spolti commented Nov 30, 2023

Thanks for the explanation @tarilabs, how did you install podman?
I didn't install any helper, it works ok (also exposes the podman registry for docker so docker understand that it does have a registry).

All I did was install podman and podman-desktop.

@tarilabs
Copy link
Member Author

tarilabs commented Dec 1, 2023

Thanks for the explanation @tarilabs, how did you install podman?

Homebrew, as instructed here: https://podman.io/docs/installation#macos:~:text=to%20install%20Podman%3A-,brew%20install%20podman,-Next%2C%20create%20and

The thing is I would like to be sure to leverage Rosetta more than just plain qemu, especially for the DevContainer (per this doc shows). But maybe I misunderstood qemu on Mac...

DevContainer part aside; additionally, I was not able for Testcontainers for Go, to reckon Podman was installed, hence I needed to modify lines such as this one:

https://github.com/opendatahub-io/model-registry/blob/ee6455355f140e3843b56bd17e4185a4e7551b7a/internal/testutils/test_container_utils.go#L18

to specify explicitly podman to be used.
It's not an impossible task, but I needed to make sure each time not-to-commit that change, or to reperform the change locally each time I checkout a PR 😞

Always happy to hear if anyone get different experience (I'm on Apple-silicon Mac OSX)

@spolti
Copy link
Member

spolti commented Dec 1, 2023

For that line in specific, maybe not hardcode it?
Instead, use a env, and default to that value if the given env is not set :)

@tarilabs
Copy link
Member Author

tarilabs commented Dec 1, 2023

Instead, use a env, and default to that value if the given env is not set :)

that is clearly an option, but somehow according to Testcontainers that should not even be needed; unfortunately I didn't have bandwidth to look further into this, given using alternative docker-engine managers just worked

@tarilabs tarilabs force-pushed the tarilabs-20231129-214 branch 3 times, most recently from 2a7d469 to f37bba8 Compare December 5, 2023 07:31
...
```

and now you can substitute `gmake` every time the make command is mentioned in guides (or perform the path management per the caveat).
Copy link
Member

Choose a reason for hiding this comment

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

isn't better to use just make as stated in the comment? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean to adopt the caveat instructions per homebrew install log? :)
that is optional, so I want to keep it same options for our readers (personally I'm just using gmake)

CONTRIBUTING.md Outdated Show resolved Hide resolved
@tarilabs tarilabs merged commit 8ee2d82 into opendatahub-io:main Dec 11, 2023
7 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.

Document helpful tips when developing on Apple-silicon/ARM
4 participants