gh-85454: Remove distutils.ccompiler from Tools/c-analyzer #95171

corona10 · 2022-07-23T14:01:50Z

Issue: Deprecate and remove distutils #85454

corona10 · 2022-07-23T14:04:29Z

Tools/c-analyzer/c_parser/preprocessor/__init__.py

@@ -168,9 +170,17 @@ def handling_errors(ignore_exc=None, *, log_err=None):
 }


+def _get_default_compiler():


Minimalized ported version from

cpython/Lib/distutils/ccompiler.py

Lines 937 to 956 in f4c0348

def get_default_compiler(osname=None, platform=None):

"""Determine the default compiler to use for the given platform.

osname should be one of the standard Python OS names (i.e. the

ones returned by os.name) and platform the common value

returned by sys.platform for the platform in question.

The default values are os.name and sys.platform in case the

parameters are not given.

"""

if osname is None:

osname = os.name

if platform is None:

platform = sys.platform

for pattern, compiler in _default_compilers:

if re.match(pattern, platform) is not None or \

re.match(pattern, osname) is not None:

return compiler

# Default to Unix compiler

return 'unix'

It may be worth having a short comment indicating this was derived from the distutils code.

kumaraditya303 · 2022-07-23T17:05:07Z

Tools/c-analyzer/c_parser/preprocessor/__init__.py

@@ -168,9 +170,17 @@ def handling_errors(ignore_exc=None, *, log_err=None):
 }


+def _get_default_compiler():
+    if re.match('cygwin.*', sys.platform) is not None:


You can just remove cygwin as it is unsupported.

Are you sure?

We have a bunch of Cygwin-related codes (__CYGWIN__) in CPython code base, Did we decide to remove them?
Even if PEP 11 defined tier-based platforms, I don't interpret them as removing all unsupported platform codes.

Cygwin is certainly still alive to some extent, so I'd say let's not worry about that in the PR.

If you remove cygwin branch, it will fallback to unix which is better than special casing it but it's minor and you can ignore this too.

If you remove cygwin branch, it will fallback to unix which is better than special casing it but it's minor and you can ignore this too.

It's care about Windows/Cygwin case, see: https://docs.python.org/3/library/sys.html.
It can be gone to the wrong fallback logic.

ericsnowcurrently

LGTM

pythongh-85454: Remove distutils.ccompiler from Tools/c-analyzer

d1f8cfa

bedevere-bot added the awaiting core review label Jul 23, 2022

corona10 added the skip news label Jul 23, 2022

corona10 requested a review from ericsnowcurrently July 23, 2022 14:02

corona10 commented Jul 23, 2022

View reviewed changes

kumaraditya303 reviewed Jul 23, 2022

View reviewed changes

ericsnowcurrently approved these changes Jul 25, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Jul 25, 2022

tiran merged commit a15ae19 into python:main Jul 25, 2022

bedevere-bot removed the awaiting merge label Jul 25, 2022

corona10 deleted the gh-85454-c-analyzer branch July 25, 2022 23:38

ericsnowcurrently mentioned this pull request Aug 3, 2022

warnings.filters was modified by test_check_c_globals #95349

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-85454: Remove distutils.ccompiler from Tools/c-analyzer #95171

gh-85454: Remove distutils.ccompiler from Tools/c-analyzer #95171

corona10 commented Jul 23, 2022 •

edited by bedevere-bot

Loading

corona10 Jul 23, 2022 •

edited

Loading

ericsnowcurrently Jul 25, 2022

kumaraditya303 Jul 23, 2022

corona10 Jul 23, 2022

corona10 Jul 24, 2022 •

edited

Loading

ericsnowcurrently Jul 25, 2022

kumaraditya303 Jul 25, 2022

corona10 Jul 26, 2022 •

edited

Loading

ericsnowcurrently left a comment

		@@ -168,9 +170,17 @@ def handling_errors(ignore_exc=None, *, log_err=None):
		}


		def _get_default_compiler():

	def get_default_compiler(osname=None, platform=None):
	"""Determine the default compiler to use for the given platform.

	osname should be one of the standard Python OS names (i.e. the
	ones returned by os.name) and platform the common value
	returned by sys.platform for the platform in question.

	The default values are os.name and sys.platform in case the
	parameters are not given.
	"""
	if osname is None:
	osname = os.name
	if platform is None:
	platform = sys.platform
	for pattern, compiler in _default_compilers:
	if re.match(pattern, platform) is not None or \
	re.match(pattern, osname) is not None:
	return compiler
	# Default to Unix compiler
	return 'unix'

gh-85454: Remove distutils.ccompiler from Tools/c-analyzer #95171

gh-85454: Remove distutils.ccompiler from Tools/c-analyzer #95171

Conversation

corona10 commented Jul 23, 2022 • edited by bedevere-bot Loading

corona10 Jul 23, 2022 • edited Loading

Choose a reason for hiding this comment

ericsnowcurrently Jul 25, 2022

Choose a reason for hiding this comment

kumaraditya303 Jul 23, 2022

Choose a reason for hiding this comment

corona10 Jul 23, 2022

Choose a reason for hiding this comment

corona10 Jul 24, 2022 • edited Loading

Choose a reason for hiding this comment

ericsnowcurrently Jul 25, 2022

Choose a reason for hiding this comment

kumaraditya303 Jul 25, 2022

Choose a reason for hiding this comment

corona10 Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

ericsnowcurrently left a comment

Choose a reason for hiding this comment

corona10 commented Jul 23, 2022 •

edited by bedevere-bot

Loading

corona10 Jul 23, 2022 •

edited

Loading

corona10 Jul 24, 2022 •

edited

Loading

corona10 Jul 26, 2022 •

edited

Loading