Skip to content

Update instructions for building on macOS #1052

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

Merged
merged 8 commits into from
Feb 17, 2023
Merged

Update instructions for building on macOS #1052

merged 8 commits into from
Feb 17, 2023

Conversation

cesarcoatl
Copy link

  1. Update the command for building Python 3.7 through 3.9 by exporting PKG_CONFIG_PATH so pkg-config can find tcl and tk
  2. Instruct users on installing openssl with brew for building Python 3.10

Fixes: #1049

Signed-off-by: César Román thecesrom@gmail.com

Signed-off-by: César Román <thecesrom@gmail.com>
@ghost
Copy link

ghost commented Feb 10, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

getting-started/setup-building.rst:420
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Standard reminder: You can directly apply all the suggestions you want in one go with Files changed -> Add to batch -> Commit

Some suggestions and questions to clarify the advice and the structure

@CAM-Gerlach
Copy link
Member

(Standard reminder: You can directly apply all the suggestions you want in one go with Files changed -> Add to batch -> Commit)

César Román and others added 2 commits February 10, 2023 16:29
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

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

Thanks for making the PR! If the 3.10+ PKG_CONFIG_PATH line is restored as I've commented in line, the Homebrew instructions seem to build as expected for 3.7 through current main/3.12 on a current macOS 13 system.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

This looks good from my side, except that as mentioned previously, the immediately following introductory text (the last make step and the first line for MacPorts after it) should be updated to make sense with the changes here and ensure it is clearer for readers which instructions are specific to which distributor and Python version.

I can't make them as suggestions, but I can either just push a commit directly to your branch, or you can apply this diff:

``git diff`` output
diff --git a/getting-started/setup-building.rst b/getting-started/setup-building.rst
index b285a74..15e094a 100644
--- a/getting-started/setup-building.rst
+++ b/getting-started/setup-building.rst
@@ -419,7 +419,7 @@ For example, with **Homebrew**, install the dependencies::

     $ brew install pkg-config openssl@1.1 xz gdbm tcl-tk

-Then, for Python 3.10 and newer::
+Then, for Python 3.10 and newer, run ``configure``::

     $ CFLAGS="-I$(brew --prefix gdbm)/include -I$(brew --prefix xz)/include" \
       LDFLAGS="-L$(brew --prefix gdbm)/lib -I$(brew --prefix xz)/lib" \
@@ -438,11 +438,11 @@ Or, for Python 3.7 through 3.9::
               --with-tcltk-libs="$(pkg-config --libs tcl tk)" \
               --with-tcltk-includes="$(pkg-config --cflags tcl tk)"

-and ``make``::
+And finally, run ``make``::

     $ make -s -j2

-or **MacPorts**::
+Alternatively, with **MacPorts**::

     $ sudo port install pkgconfig openssl xz gdbm

Or am this patch:

``git format-patch`` output
From 8ae83afc0cbf616c1937408c8e5767ecd4625cf1 Mon Sep 17 00:00:00 2001
From: "C.A.M. Gerlach" <CAM.Gerlach@Gerlach.CAM>
Date: Wed, 15 Feb 2023 20:01:14 -0600
Subject: [PATCH] Update following text to match revised macOS building
 language

---
 getting-started/setup-building.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/getting-started/setup-building.rst b/getting-started/setup-building.rst
index b285a74..15e094a 100644
--- a/getting-started/setup-building.rst
+++ b/getting-started/setup-building.rst
@@ -419,7 +419,7 @@ For example, with **Homebrew**, install the dependencies::

     $ brew install pkg-config openssl@1.1 xz gdbm tcl-tk

-Then, for Python 3.10 and newer::
+Then, for Python 3.10 and newer, run ``configure``::

     $ CFLAGS="-I$(brew --prefix gdbm)/include -I$(brew --prefix xz)/include" \
       LDFLAGS="-L$(brew --prefix gdbm)/lib -I$(brew --prefix xz)/lib" \
@@ -438,11 +438,11 @@ Or, for Python 3.7 through 3.9::
               --with-tcltk-libs="$(pkg-config --libs tcl tk)" \
               --with-tcltk-includes="$(pkg-config --cflags tcl tk)"

-and ``make``::
+And finally, run ``make``::

     $ make -s -j2

-or **MacPorts**::
+Alternatively, with **MacPorts**::

     $ sudo port install pkgconfig openssl xz gdbm

--
2.37.1.windows.1

@cesarcoatl
Copy link
Author

I'll work on that

@cesarcoatl
Copy link
Author

@CAM-Gerlach patch has been applied

@pradyunsg pradyunsg merged commit 80cd894 into python:main Feb 17, 2023
@pradyunsg
Copy link
Member

pradyunsg commented Feb 17, 2023

Thanks @thecesrom!

PS: Merged this since OP has addressed all outstanding comments, the PR has an approval from the relevant expert, and changes look good to me!

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Feb 17, 2023

Thanks for handling this CAM and Ned (and Pradyun for merging)!

@cesarcoatl cesarcoatl deleted the docs/setup-building-macOS branch March 10, 2023 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Envvars not set in subshell in macOS Python 3.7-3.9 configure invocation
7 participants