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

gh-115737: Correct the install name for non-framework macOS libPython builds. #115750

Merged
merged 2 commits into from
Feb 21, 2024
Merged

gh-115737: Correct the install name for non-framework macOS libPython builds. #115750

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
662c204
Correct the install name for non-framework macOS libPython builds.
freakboy3742 Feb 21, 2024
b1a161f
Revert the DYLIBINSTALLNAMEPREFIX rename in favor of cleaning up the …
freakboy3742 Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Correct the install name for non-framework macOS libPython builds.
  • Loading branch information
@freakboy3742
freakboy3742 committed Feb 21, 2024
commit 662c204f36bbc871f14166d2965c81eb8fb34684
6 changes: 3 additions & 3 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ PYTHONFRAMEWORK= @PYTHONFRAMEWORK@
PYTHONFRAMEWORKDIR= @PYTHONFRAMEWORKDIR@
PYTHONFRAMEWORKPREFIX= @PYTHONFRAMEWORKPREFIX@
PYTHONFRAMEWORKINSTALLDIR= @PYTHONFRAMEWORKINSTALLDIR@
PYTHONFRAMEWORKINSTALLNAMEPREFIX= @PYTHONFRAMEWORKINSTALLNAMEPREFIX@
DYLIBINSTALLNAMEPREFIX= @DYLIBINSTALLNAMEPREFIX@
RESSRCDIR= @RESSRCDIR@
# Deployment target selected during configure, to be checked
# by distutils. The export statement is needed to ensure that the
Expand Down Expand Up @@ -869,7 +869,7 @@ libpython3.so: libpython$(LDVERSION).so
$(BLDSHARED) $(NO_AS_NEEDED) -o $@ -Wl,-h$@ $^

libpython$(LDVERSION).dylib: $(LIBRARY_OBJS)
$(CC) -dynamiclib $(PY_CORE_LDFLAGS) -undefined dynamic_lookup -Wl,-install_name,$(PYTHONFRAMEWORKINSTALLNAMEPREFIX)/lib/libpython$(LDVERSION).dylib -Wl,-compatibility_version,$(VERSION) -Wl,-current_version,$(VERSION) -o $@ $(LIBRARY_OBJS) $(DTRACE_OBJS) $(SHLIBS) $(LIBC) $(LIBM); \
$(CC) -dynamiclib $(PY_CORE_LDFLAGS) -undefined dynamic_lookup -Wl,-install_name,$(DYLIBINSTALLNAMEPREFIX)/lib/libpython$(LDVERSION).dylib -Wl,-compatibility_version,$(VERSION) -Wl,-current_version,$(VERSION) -o $@ $(LIBRARY_OBJS) $(DTRACE_OBJS) $(SHLIBS) $(LIBC) $(LIBM); \
Copy link
Member

Choose a reason for hiding this comment

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

Since this target is only used for macOS --enable-shared builds (and not for framework or non-shared builds), I think the only change we really need is to revert the original change here, though still removing the obsolete -Wl,-single_module. Then there is no need to make any of the other rename changes. So:

$(CC) -dynamiclib $(PY_CORE_LDFLAGS) -undefined dynamic_lookup -Wl,-install_name,$(prefix)/lib/libpython$(LDVERSION).dylib -Wl,-compatibility_version,$(VERSION) -Wl,-current_version,$(VERSION) -o $@ $(LIBRARY_OBJS) $(DTRACE_OBJS) $(SHLIBS) $(LIBC) $(LIBM); \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point - I'm not sure why I changed this one in the original patch. I can only assume I got a bit search-and-replace happy looking for install_name.



libpython$(VERSION).sl: $(LIBRARY_OBJS)
Expand Down Expand Up @@ -900,7 +900,7 @@ $(PYTHONFRAMEWORKDIR)/Versions/$(VERSION)/$(PYTHONFRAMEWORK): \
$(INSTALL) -d -m $(DIRMODE) $(PYTHONFRAMEWORKDIR)/Versions/$(VERSION)
$(CC) -o $(LDLIBRARY) $(PY_CORE_LDFLAGS) -dynamiclib \
-all_load $(LIBRARY) \
-install_name $(DESTDIR)$(PYTHONFRAMEWORKINSTALLNAMEPREFIX)/$(PYTHONFRAMEWORK) \
-install_name $(DESTDIR)$(DYLIBINSTALLNAMEPREFIX)/$(PYTHONFRAMEWORK) \
-compatibility_version $(VERSION) \
-current_version $(VERSION) \
-framework CoreFoundation $(LIBS);
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2024-02-21-11-58-30.gh-issue-115737.dpNl2T.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The install name for libPython is now correctly set for non-framework macOS
builds.
8 changes: 4 additions & 4 deletions configure
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ AC_ARG_ENABLE([framework],
PYTHONFRAMEWORKDIR=no-framework
PYTHONFRAMEWORKPREFIX=
PYTHONFRAMEWORKINSTALLDIR=
PYTHONFRAMEWORKINSTALLNAMEPREFIX=
DYLIBINSTALLNAMEPREFIX=${prefix}
RESSRCDIR=
FRAMEWORKINSTALLFIRST=
FRAMEWORKINSTALLLAST=
Expand Down Expand Up @@ -564,7 +564,7 @@ AC_ARG_ENABLE([framework],
esac

prefix=$PYTHONFRAMEWORKINSTALLDIR/Versions/$VERSION
PYTHONFRAMEWORKINSTALLNAMEPREFIX=${prefix}
DYLIBINSTALLNAMEPREFIX=${prefix}
RESSRCDIR=Mac/Resources/framework

# Add files for Mac specific code to the list of output
Expand All @@ -584,7 +584,7 @@ AC_ARG_ENABLE([framework],
PYTHONFRAMEWORKDIR=no-framework
PYTHONFRAMEWORKPREFIX=
PYTHONFRAMEWORKINSTALLDIR=
PYTHONFRAMEWORKINSTALLNAMEPREFIX=
DYLIBINSTALLNAMEPREFIX=${prefix}
RESSRCDIR=
FRAMEWORKINSTALLFIRST=
FRAMEWORKINSTALLLAST=
Expand All @@ -605,7 +605,7 @@ AC_SUBST([PYTHONFRAMEWORKIDENTIFIER])
AC_SUBST([PYTHONFRAMEWORKDIR])
AC_SUBST([PYTHONFRAMEWORKPREFIX])
AC_SUBST([PYTHONFRAMEWORKINSTALLDIR])
AC_SUBST([PYTHONFRAMEWORKINSTALLNAMEPREFIX])
AC_SUBST([DYLIBINSTALLNAMEPREFIX])
AC_SUBST([RESSRCDIR])
AC_SUBST([FRAMEWORKINSTALLFIRST])
AC_SUBST([FRAMEWORKINSTALLLAST])
Expand Down
Loading