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

Jump to bottom

bpo-36508: python-config don't export LINKFORSHARED #12661

Merged
merged 2 commits into from Apr 9, 2019
Merged

bpo-36508: python-config don't export LINKFORSHARED #12661

merged 2 commits into from Apr 9, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 2, 2019 •
edited by bedevere-bot

python-config --ldflags no longer includes flags of the LINKFORSHARED
variable. The LINKFORSHARED variable must only be used to build
executables.

https://bugs.python.org/issue36508

@vstinner
bpo-36508: python-config don't export LINKFORSHARED
59ff554 
python-config --ldflags no longer includes flags of the LINKFORSHARED
variable. The LINKFORSHARED variable must only be used to build
executables.
@vstinner vstinner requested review from pitrou, ned-deily and doko42 Apr 2, 2019
@vstinner vstinner mentioned this pull request Apr 5, 2019
Merged
@vstinner
Copy link
Member Author

vstinner commented Apr 5, 2019

According to Azure Pipelines PR, macOS build failed with:

/bin/sh: -c: line 1: syntax error: unexpected end of file

Maybe my Makefile.pre.in change is wrong?


 	@ # Substitution happens here, as the completely-expanded BINDIR
 	@ # is not available in configure
 	sed -e "s,@EXENAME@,$(BINDIR)/python$(LDVERSION)$(EXE)," < $(srcdir)/Misc/python-config.in >python-config.py
-	@ # Replace makefile compat. variable references with shell script compat. ones; $(VAR) -> ${VAR}
-	LC_ALL=C sed -e 's,\$$(\([A-Za-z0-9_]*\)),\$$\{\1\},g' < Misc/python-config.sh >python-config
 	@ # On Darwin, always use the python version of the script, the shell
 	@ # version doesn't use the compiler customizations that are provided
 	@ # in python (_osx_support.py).
 	@if test `uname -s` = Darwin; then \
 		cp python-config.py python-config; \
+	else \
+		# Replace makefile compat. variable references with shell script compat. ones; $(VAR) -> ${VAR} \
+		LC_ALL=C sed -e 's,\$$(\([A-Za-z0-9_]*\)),\$$\{\1\},g' < Misc/python-config.sh >python-config; \
 	fi

@vstinner
Copy link
Member Author

vstinner commented Apr 5, 2019

Maybe my Makefile.pre.in change is wrong?

It was just an unrelated refactoring change, I reverted it.

@vstinner
Copy link
Member Author

vstinner commented Apr 5, 2019

I tested manually, the change works as expected.

Without the change (master branch), --ldflags contains -Xlinker -export-dynamic:

$ git clean -fdx; ./configure --with-pydebug -C && make

$ ./python python-config.py --ldflags
-L/usr/local/lib/python3.8/config-3.8dm-x86_64-linux-gnu -lpython3.8dm -lcrypt -lpthread -ldl -lutil -lm -lm -Xlinker -export-dynamic

$ bash python-config --ldflags
-L/home/vstinner/prog/python/lib/python3.8/config-3.8dm-x86_64-linux-gnu -L/home/vstinner/prog/python/lib -lpython3.8dm -lcrypt -lpthread -ldl  -lutil -lm -lm  -Xlinker -export-dynamic

With the change, --ldflags doesn't contain -Xlinker -export-dynamic:

$ git clean -fdx; ./configure --with-pydebug -C && make

$ ./python python-config.py --ldflags
-L/usr/local/lib/python3.8/config-3.8dm-x86_64-linux-gnu -lpython3.8dm -lcrypt -lpthread -ldl -lutil -lm -lm

$ bash python-config --ldflags
-L/home/vstinner/prog/python/lib/python3.8/config-3.8dm-x86_64-linux-gnu -L/home/vstinner/prog/python/lib -lpython3.8dm -lcrypt -lpthread -ldl  -lutil -lm -lm

I'm not sure why python-config (shell) and python-config.py (Python) return different output for --ldflags (this behavior can be seen without my change, my change isn't related to that).

Note: (with or without the change) configure and Python don't add any LDFLAGS on my Fedora 29:

$ ./python -m sysconfig|grep LDFLAGS
	CONFIGURE_LDFLAGS = ""
	CONFIGURE_LDFLAGS_NODIST = ""
	LDFLAGS = ""
	LDFLAGS_NODIST = ""
	OPENSSL_LDFLAGS = ""
	PY_CORE_LDFLAGS = ""
	PY_LDFLAGS = ""
	PY_LDFLAGS_NODIST = ""

python3 from Fedora 29 contains way more LDFLAGS:

$ python3 -m sysconfig|grep LDFLAGS
	CONFIGURE_LDFLAGS = "-Wl,-z,relro   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g"
	CONFIGURE_LDFLAGS_NODIST = "-flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none -g"
	CONFIG_ARGS = "'--build=x86_64-redhat-linux-gnu' '--host=x86_64-redhat-linux-gnu' '--program-prefix=' '--disable-dependency-tracking' '--prefix=/usr' '--exec-prefix=/usr' '--bindir=/usr/bin' '--sbindir=/usr/sbin' '--sysconfdir=/etc' '--datadir=/usr/share' '--includedir=/usr/include' '--libdir=/usr/lib64' '--libexecdir=/usr/libexec' '--localstatedir=/var' '--sharedstatedir=/var/lib' '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--enable-ipv6' '--enable-shared' '--with-computed-gotos=yes' '--with-dbmliborder=gdbm:ndbm:bdb' '--with-system-expat' '--with-system-ffi' '--enable-loadable-sqlite-extensions' '--with-dtrace' '--with-lto' '--with-ssl-default-suites=openssl' '--with-valgrind' '--without-ensurepip' '--enable-optimizations' 'build_alias=x86_64-redhat-linux-gnu' 'host_alias=x86_64-redhat-linux-gnu' 'CFLAGS=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv ' 'LDFLAGS=-Wl,-z,relro   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g ' 'CPPFLAGS=' 'PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'"
	LDFLAGS = "-Wl,-z,relro   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g"
	OPENSSL_LDFLAGS = ""
	PY_CORE_LDFLAGS = "-Wl,-z,relro   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g -flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none -g"
	PY_LDFLAGS = "-Wl,-z,relro   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g"
	PY_LDFLAGS_NODIST = "-flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none -g"

@vstinner vstinner merged commit e65f01f into python:master Apr 9, 2019
@miss-islington
Copy link
Contributor

miss-islington commented Apr 9, 2019

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.7.
🐍🍒⛏🤖

@vstinner vstinner deleted the linkforshared branch Apr 9, 2019
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 9, 2019
@vstinner @miss-islington
bpo-36508: python-config don't export LINKFORSHARED (pythonGH-12661)
93b0a54 
python-config --ldflags no longer includes flags of the LINKFORSHARED
variable. The LINKFORSHARED variable must only be used to build
executables.
(cherry picked from commit e65f01f)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@bedevere-bot
Copy link

bedevere-bot commented Apr 9, 2019

GH-12748 is a backport of this pull request to the 3.7 branch.

@miss-islington
Copy link
Contributor

miss-islington commented Apr 9, 2019

Sorry, @vstinner, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e65f01f78d7bda3013fc5be485afa87ff56511d9 2.7

vstinner added a commit that referenced this pull request Apr 16, 2019
@miss-islington @vstinner
bpo-36508: python-config don't export LINKFORSHARED (GH-12661) (GH-12748
cd46b09 
)

python-config --ldflags no longer includes flags of the LINKFORSHARED
variable. The LINKFORSHARED variable must only be used to build
executables.
(cherry picked from commit e65f01f)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@thp thp mentioned this pull request Jun 4, 2019
Closed
@rgommers rgommers mentioned this pull request Sep 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pitrou pitrou

@ned-deily ned-deily

@doko42 doko42

Assignees

@vstinner vstinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vstinner @miss-islington @bedevere-bot @the-knights-who-say-ni