Skip to content

bpo-27640: Add --disable-test-modules configure option #23886

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

Merged
vstinner merged 14 commits into from
Dec 30, 2020
Merged

bpo-27640: Add --disable-test-modules configure option #23886

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9cc3b30
Add --enable-test-modules option for configure
pxinwr Dec 22, 2020
9a2fcad
fix no result issues when checking
pxinwr Dec 24, 2020
3031f42
Update setup.py
pxinwr Dec 28, 2020
c4f193f
rename news file
pxinwr Dec 28, 2020
96bd20c
add info in whatsnew
pxinwr Dec 28, 2020
c655075
elaborate the --enable-test-modules in comments
pxinwr Dec 28, 2020
cd21bd6
sort the directories listed in LIBSUBDIRS and TESTSUBDIRS
pxinwr Dec 28, 2020
f48d197
regenerate configure
pxinwr Dec 28, 2020
2d5f0e3
update news
pxinwr Dec 28, 2020
17c08dd
Improve Doc/whatsnew/3.10.rst
pxinwr Dec 30, 2020
cdd5fa6
credit Xavier de Gaye and Thomas Petazzoni in Doc/whatsnew/3.10.rst
pxinwr Dec 30, 2020
23daf7a
improve help string for the added option
pxinwr Dec 30, 2020
90e4114
change the help and message for the new option
pxinwr Dec 30, 2020
9e5dada
credit Xavier and Thomas in news
pxinwr Dec 30, 2020
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
58 changes: 37 additions & 21 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,8 +1365,29 @@ maninstall: altmaninstall

# Install the library
XMLLIBSUBDIRS= xml xml/dom xml/etree xml/parsers xml/sax
LIBSUBDIRS= tkinter tkinter/test tkinter/test/test_tkinter \
tkinter/test/test_ttk site-packages test \
LIBSUBDIRS= tkinter \

@vstinner vstinner Dec 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since you rewrite LIBSUBDIRS and TESTSUBDIRS, can you please sort these lists? You may even put one item per line, but you can keep groups of subdirectories of the same directory (like "venv venv/scripts venv/scripts/common venv/scripts/posix").

@pxinwr pxinwr Dec 30, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

site-packages \
importlib \
importlib/metadata \
asyncio \
collections concurrent concurrent/futures encodings \
email email/mime \
ensurepip ensurepip/_bundled \
html json http dbm xmlrpc \
sqlite3 \
logging csv wsgiref urllib \
lib2to3 lib2to3/fixes lib2to3/pgen2 \
ctypes ctypes/macholib \
idlelib idlelib/Icons \
distutils distutils/command $(XMLLIBSUBDIRS) \
turtledemo \
multiprocessing multiprocessing/dummy \
unittest \
venv venv/scripts venv/scripts/common venv/scripts/posix \
curses pydoc_data \
zoneinfo
TESTSUBDIRS= tkinter/test tkinter/test/test_tkinter \
tkinter/test/test_ttk test \
test/audiodata \
test/capath test/data \
test/cjkencodings test/decimaltestdata \
Expand All @@ -1384,8 +1405,6 @@ LIBSUBDIRS= tkinter tkinter/test tkinter/test/test_tkinter \
test/test_import/data/package \
test/test_import/data/package2 \
test/test_import/data/unwritable \
importlib \
importlib/metadata \
test/test_importlib \
test/test_importlib/builtin \
test/test_importlib/data \
Expand Down Expand Up @@ -1426,28 +1445,25 @@ LIBSUBDIRS= tkinter tkinter/test tkinter/test/test_tkinter \
test/test_importlib/zipdata02 \
test/test_zoneinfo test/test_zoneinfo/data \
test/ziptestdata \
asyncio \
test/test_asyncio \
collections concurrent concurrent/futures encodings \
email email/mime test/test_email test/test_email/data \
ensurepip ensurepip/_bundled \
html json test/test_json http dbm xmlrpc \
sqlite3 sqlite3/test \
logging csv wsgiref urllib \
lib2to3 lib2to3/fixes lib2to3/pgen2 lib2to3/tests \
test/test_email test/test_email/data \
test/test_json \
sqlite3/test \
lib2to3/tests \
lib2to3/tests/data lib2to3/tests/data/fixers \
lib2to3/tests/data/fixers/myfixes \
ctypes ctypes/test ctypes/macholib \
idlelib idlelib/Icons idlelib/idle_test \
distutils distutils/command distutils/tests $(XMLLIBSUBDIRS) \
ctypes/test \
idlelib/idle_test \
distutils/tests \
test/test_peg_generator \
test/test_tools test/test_warnings test/test_warnings/data \
turtledemo \
multiprocessing multiprocessing/dummy \
unittest unittest/test unittest/test/testmock \
venv venv/scripts venv/scripts/common venv/scripts/posix \
curses pydoc_data \
zoneinfo
unittest/test unittest/test/testmock

TEST_MODULES=@TEST_MODULES@
ifeq ($(TEST_MODULES),yes)
LIBSUBDIRS += $(TESTSUBDIRS)
endif

libinstall: build_all $(srcdir)/Modules/xxmodule.c
@for i in $(SCRIPTDIR) $(LIBDEST); \
do \
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Build/2020-12-22-17-57-04.bpo-31904.j3a8r0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add --enable-test-modules option for configure.

@vstinner vstinner Dec 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please document it also in https://docs.python.org/dev/whatsnew/3.10.html#build-changes (Doc/whatsnew/3.10.rst).

@pxinwr pxinwr Dec 28, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

20 changes: 20 additions & 0 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ ac_includes_default="\
#endif"

ac_subst_vars='LTLIBOBJS
TEST_MODULES
OPENSSL_LDFLAGS
OPENSSL_LIBS
OPENSSL_INCLUDES
Expand Down Expand Up @@ -852,6 +853,7 @@ with_openssl
with_ssl_default_suites
with_builtin_hashlib_hashes
with_experimental_isolated_subinterpreters
enable_test_modules
'
ac_precious_vars='build_alias
host_alias
Expand Down Expand Up @@ -1503,6 +1505,7 @@ Optional Features:
--enable-big-digits[=15|30]
use big digits (30 or 15 bits) for Python longs
(default is system-dependent)]
--enable-test-modules enable test modules (default is yes)

Optional Packages:
--with-PACKAGE[=ARG] use PACKAGE [ARG=yes]
Expand Down Expand Up @@ -17723,6 +17726,23 @@ $as_echo "no" >&6; }
fi


# check whether to disable test modules
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for --enable-test-modules" >&5
$as_echo_n "checking for --enable-test-modules... " >&6; }
# Check whether --enable-test-modules was given.
if test "${enable_test_modules+set}" = set; then :
enableval=$enable_test_modules;
fi

if test "$enable_test_modules" = no; then
TEST_MODULES=no
else
TEST_MODULES=yes
fi

{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $TEST_MODULES" >&5
$as_echo "$TEST_MODULES" >&6; }


# generate output files
ac_config_files="$ac_config_files Makefile.pre Misc/python.pc Misc/python-embed.pc Misc/python-config.sh"
Expand Down
12 changes: 12 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5833,6 +5833,18 @@ else
fi],
[AC_MSG_RESULT(no)])

# check whether to disable test modules
AC_MSG_CHECKING(for --enable-test-modules)

@vstinner vstinner Dec 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the default is to enable tests, I would prefer an option to disable tests. https://bugs.python.org/issue27640 proposed to add --disable-test-suite to configure.

Can you please elaborate the effect of the option? I understand that if it's used, it disables the compilation of test extension modules, and prevent to install tests in "make install". Also elaborate it in the NEWS and What's New entries.

@pxinwr pxinwr Dec 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See below copied from configure help. When we define an option enable-test-modules, the counterpart disable-test-modules will be also defined automatically. That is, disable-test-modules is already defined. The existing option --enable-ipv6 is an example that is default as yes.

Optional Features:
  --disable-option-checking  ignore unrecognized --enable/--with options
  --disable-FEATURE       do not include FEATURE (same as --enable-FEATURE=no)
  --enable-FEATURE[=ARG]  include FEATURE [ARG=yes]
  --enable-universalsdk[=SDKDIR]
                          create a universal binary build. SDKDIR specifies
                          which macOS SDK should be used to perform the build,
                          see Mac/README.rst. (default is no)
  --enable-framework[=INSTALLDIR]
                          create a Python.framework rather than a traditional
                          Unix install. optional INSTALLDIR specifies the
                          installation path. see Mac/README.rst (default is
                          no)
  --enable-shared         enable building a shared Python library (default is
                          no)
  --enable-profiling      enable C-level code profiling with gprof (default is
                          no)
  --enable-optimizations  enable expensive, stable optimizations (PGO, etc.)
                          (default is no)
  --enable-loadable-sqlite-extensions
                          support loadable extensions in _sqlite module, see
                          Doc/library/sqlite3.rst (default is no)
  --enable-ipv6           enable ipv6 (with ipv4) support, see
                          Doc/library/socket.rst (default is yes if supported)
  --enable-big-digits[=15|30]
                          use big digits (30 or 15 bits) for Python longs
                          (default is system-dependent)]

@vstinner vstinner Dec 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"When we define an option enable-test-modules, the counterpart disable-test-modules will be also defined automatically."

Ah ok. But you should document that the change adds the --disable-test-modules option.

@vstinner vstinner Dec 30, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You forgot run "autoconf": configure it outdated, it contains the old documentation.

@pxinwr pxinwr Dec 30, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"Ah ok. But you should document that the change adds the --disable-test-modules option."
Done.
"You forgot run "autoconf": configure it outdated, it contains the old documentation."
Actually I did run it.

AC_ARG_ENABLE(test-modules,
AS_HELP_STRING([--enable-test-modules], [enable test modules (default is yes)]))
Comment thread
pxinwr marked this conversation as resolved.
Resolved
if test "$enable_test_modules" = no; then
TEST_MODULES=no
else
TEST_MODULES=yes
fi
AC_SUBST(TEST_MODULES)
AC_MSG_RESULT($TEST_MODULES)


# generate output files
AC_CONFIG_FILES(Makefile.pre Misc/python.pc Misc/python-embed.pc Misc/python-config.sh)
Expand Down
3 changes: 2 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@


# Compile extensions used to test Python?
TEST_EXTENSIONS = True
ENABLE_TEST_MODULES = (sysconfig.get_config_var('TEST_MODULES') == 'yes')
TEST_EXTENSIONS = (True if ENABLE_TEST_MODULES else False)
Comment thread
pxinwr marked this conversation as resolved.
Resolved

# This global variable is used to hold the list of modules to be disabled.
DISABLED_MODULE_LIST = []
Expand Down