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

make: Add macos M1 support #4988

Merged
merged 3 commits into from
Jul 11, 2022
Merged

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Dec 22, 2021

The M1 Macs support both x86_64 and arm64 architectures, which forced
homebrew to use a different path for its storage (/opt/homebrew/
instead of /usr/local). If we don't adjust the path we'd mix x86_64
and arm64 libraries which can lead to weird compiler and linker
errors.

This patch just introduces CPATH and LIBRARY_PATH as suggested by
the homebrew team, and detects the current architecture automatically.

Changelog-Added: macos: Added m1 architecture support for macos

@cdecker cdecker self-assigned this Dec 22, 2021
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

LGTM, but have no possibility to test.

ack b76c377

@cdecker
Copy link
Member Author

cdecker commented Dec 24, 2021

I'll likely be adding a few more fixes to this PR before undrafting it, but it should be ready for the next release.

@cdecker cdecker added this to the v0.11 milestone Dec 24, 2021
@cdecker cdecker force-pushed the arm64-macos branch 2 times, most recently from cef0fcc to 1fcd284 Compare January 10, 2022 12:07
@valentinewallace
Copy link
Contributor

FYI, this is what ended up working for me to build on the M1:

  1. Checkout v0.10.2
  2. Cherry-pick commits from make: Add macos M1 support #4988
    1. HEAD at the time: 0b26ae370d319555769a60f407e71706d22b9713
  3. python3.9 -m venv venv
  4. source venv/bin/activate
  5. export CPATH=/opt/homebrew/include
  6. export LIBRARY_PATH=/opt/homebrew/lib
  7. export CFLAGS=-Wno-error=implicit-function-declaration
  8. pip3 install -r requirements.txt
  9. PG_CONFIG='' ./configure
  10. make -j8 # This will fail
  11. pip3 install MarkupSafe==2.0.1
  12. make -j8 # This will fail again
  13. pip3 install mistune==0.8.4

cc @dongcarl

@vincenzopalazzo
Copy link
Contributor

@valentinewallace have you tried the pip3 install -r requirements.lock may fix your issue

@dongcarl
Copy link
Contributor

dongcarl commented Mar 15, 2022

@vincenzopalazzo

Right, that solves all the specific pip install problems. There are a few extra things needed in addition to @valentinewallace's instructions on a fresh M1 mac (before running pip3 install -r requirements.lock)

  1. You need to brew install postgres since otherwise setup.py for psycopg2-binary will compain that:
  Error: pg_config executable not found.

  pg_config is required to build psycopg2 from source.  Please add the directory
  containing pg_config to the $PATH or specify the full executable path with the
  option:

      python setup.py build_ext --pg-config /path/to/pg_config build ...

  or with the pg_config option in 'setup.cfg'.

  If you prefer to avoid building psycopg2 from source, please install the PyPI
  'psycopg2-binary' package instead.

  For further information please check the 'doc/src/install.rst' file (also at
  <https://www.psycopg.org/docs/install.html>).
  1. You need to brew install openssl and set export LDFLAGS="-L/opt/homebrew/opt/openssl@3/lib" since otherwise setup.py for psycopg2-binary will compain that: ld: library not found for -lssl. Note that export LIBRARY_PATH=/opt/homebrew/lib is not sufficient here because openssl is not symlinked into /opt/homebrew/lib. See brew info openssl for more details

Overall, I think a good solution might be to have ./configure generate contrib/pyln-testing/requirements.txt from a contrib/pyln-testing/requirements.txt.in, so that installing postgres isn't a hard requirement for devving on C-Lightning.

@cdecker
Copy link
Member Author

cdecker commented May 9, 2022

Ok, while it didn't address the failing tests yet, this should be merged so we can at least compile for M1s.

@cdecker cdecker marked this pull request as ready for review May 9, 2022 09:26
@cdecker cdecker added this to the v0.12 milestone May 9, 2022
@ddustin
Copy link
Collaborator

ddustin commented May 10, 2022

LGTM but I don't have an M1 to test on either.

ACK 5c3e07e

Christian Decker added 3 commits July 11, 2022 17:45
The M1 Macs support both x86_64 and arm64 architectures, which forced
homebrew to use a different path for its storage (`/opt/homebrew/`
instead of `/usr/local`). If we don't adjust the path we'd mix x86_64
and arm64 libraries which can lead to weird compiler and linker
errors.

This patch just introduces `CPATH` and `LIBRARY_PATH` as suggested by
the homebrew team, and detects the current architecture automatically.

Changelog-Added: macos: Added m1 architecture support for macos
@cdecker
Copy link
Member Author

cdecker commented Jul 11, 2022

It's been a while and this has been working ok on my M1, so let's merge it.

@niftynei niftynei merged commit 36ab0e5 into ElementsProject:master Jul 11, 2022
@jb55
Copy link
Collaborator

jb55 commented Jul 11, 2022

@cdecker I am getting

make distclean
./configure
...
*** We need a database, but neither sqlite3 nor postgres found

on my linux machine

and I bisected it to:

e3f53e0

@justinmoon
Copy link
Contributor

This worked on my m1 a month or so ago, btw.

@cdecker
Copy link
Member Author

cdecker commented Jul 12, 2022

@cdecker I am getting

make distclean ./configure ... *** We need a database, but neither sqlite3 nor postgres found

on my linux machine

and I bisected it to:

e3f53e0

What linker flags are being passed to the compiler? You can make compilation verbose via make V=1 iirc. Seems the linker path and/or include paths may be wrongly identified.

@jb55
Copy link
Collaborator

jb55 commented Jul 12, 2022 via email

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.

8 participants