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

[WIP] gh-93744: Remove configure --with-cxx-main option #94063

Closed
wants to merge 1 commit into from
Closed

[WIP] gh-93744: Remove configure --with-cxx-main option #94063

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

Remove "configure --with-cxx-main" option: it didn't work for many
years. Remove the following configure and Makefile variables: CXX,
LDCXXSHARED, MAINCC.

Remove "configure --with-cxx-main" option: it didn't work for many
years.  Remove the following configure and Makefile variables: CXX,
LDCXXSHARED, MAINCC.

* MAINCC was added by issue gh-42471:
  commit 0f48d98.
  Previously, --with-cxx-main was named --with-cxx.
* LDCXXSHARED was added by issue gh-42093:
  commit 0000295.
* CXX variable was used by MAINCC and LDCXXSHARED variables.
@vstinner
Copy link
Member Author

The main change of this PR is that sysconfig.get_config_var('CXX') becomes None instead of: 'g++' (for example on Linux).

I'm not sure about removing the CXX variable. It seems to be used by a few files:

Lib/_osx_support.py:18: 'BLDSHARED', 'LDSHARED', 'CC', 'CXX',
Lib/_osx_support.py:23: _COMPILER_CONFIG_VARS = ('BLDSHARED', 'LDSHARED', 'CC', 'CXX')
Lib/_osx_support.py:254: cv_split[0] = cc if cv != 'CXX' else cc + '++'

Lib/distutils/sysconfig.py:219: get_config_vars('CC', 'CXX', 'CFLAGS',
Lib/distutils/sysconfig.py:231: if 'CXX' in os.environ:
Lib/distutils/sysconfig.py:232: cxx = os.environ['CXX']

Modules/makesetup:243: *.cc)  obj=`basename $src .cc`.o; cc='$(CXX)';;
Modules/makesetup:244: *.c++) obj=`basename $src .c++`.o; cc='$(CXX)';;
Modules/makesetup:245: *.C)   obj=`basename $src .C`.o; cc='$(CXX)';;
Modules/makesetup:246: *.cxx) obj=`basename $src .cxx`.o; cc='$(CXX)';;
Modules/makesetup:247: *.cpp) obj=`basename $src .cpp`.o; cc='$(CXX)';;

Can distutils, setuptools and pip expect the CXX environment variable to be set? Or does sysconfig have to define a CXX configuration variable?

sysconfig.customize_compiler() uses CXX environment variable if it's set, or use the CXX sysconfig configuration variable.

@tiran
Copy link
Member

tiran commented Jun 21, 2022

The Makefile values are included in sysconfig data file and are available through sysconfig. Are you sure that no 3rd party project relies on the settings?

@@ -807,62 +804,6 @@ rm -f conftest.c conftest.out
# _POSIX_SOURCE, _POSIX_1_SOURCE, and more
AC_USE_SYSTEM_EXTENSIONS

AC_SUBST(CXX)
Copy link
Member

@tiran tiran Jun 21, 2022

Choose a reason for hiding this comment

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

makesetup and final Makefile rely on CXX variable.

@vstinner
Copy link
Member Author

It seems like removing CXX is dangerous: it seems to be used, so it would be safer... to just keep it, just in case of doubt :-) MAINCC and LDCXXSHARED don't seem to be used outside Python itself.

Are you sure that no 3rd party project relies on the settings?

I ran a code search in the pypa GitHub organization:

makesetup and final Makefile rely on CXX variable.

Yep, as I wrote.

makesetup:

			*.cc)  obj=`basename $src .cc`.o; cc='$(CXX)';;
			*.c++) obj=`basename $src .c++`.o; cc='$(CXX)';;
			*.C)   obj=`basename $src .C`.o; cc='$(CXX)';;
			*.cxx) obj=`basename $src .cxx`.o; cc='$(CXX)';;
			*.cpp) obj=`basename $src .cpp`.o; cc='$(CXX)';;

Python Git repository has no file with extensions: .cc, .c++, .C, .cxx.

There are a few files with .cpp extensions:

Lib/test/_testcppext.cpp
PC/pyshellext.cpp
PC/python_uwp.cpp
Tools/msi/bundle/bootstrap/PythonBootstrapperApplication.cpp
Tools/msi/bundle/bootstrap/pch.cpp
Tools/msi/bundle/bootstrap/pythonba.cpp
  • Lib/test/_testcppext.cpp is built with setuptools in test_cppext
  • PC/pyshellext.cpp and PC/python_uwp.cpp: I don't think that the Windows build system (VS) uses makesetup.
  • Tools/msi/bundle/: I don't expect it to use makesetup.

I only see "CXX" at one line in the final Makefile: "CXX='$(CXX)'", does it copy the environment variable or the Makefile variable?

buildbottest: all
		-@if which pybuildbot.identify >/dev/null 2>&1; then \
			pybuildbot.identify "CC='$(CC)'" "CXX='$(CXX)'"; \
		fi
		$(TESTRUNNER) -j 1 -u all -W --slowest --fail-env-changed --timeout=$(TESTTIMEOUT) $(TESTOPTS)

@erlend-aasland
Copy link
Contributor

I only see "CXX" at one line in the final Makefile: "CXX='$(CXX)'", does it copy the environment variable or the Makefile variable?

It copies the Makefile variable CXX to an environment variable.

BTW, what did you conclude here? The cxx stuff stays?

@vstinner
Copy link
Member Author

vstinner commented Aug 4, 2022

BTW, what did you conclude here?

I'm not brave. I abandon this PR and I wrote PR #95651 instead which only removes the MAINCC variables but keeps unused CXX and LDCXXSHARED variables.

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.

None yet

4 participants