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

bpo-44035: Check autoconf files thoroughly (GH-29935) #29935

Merged
merged 2 commits into from
Dec 6, 2021

Conversation

tiran
Copy link
Member

@tiran tiran commented Dec 6, 2021

Check that users don't push changes with outdated or patched autoconf.
The presence of runstatedir option and aclocal 1.16.3 are good markers.

Use my container image to regenerate autoconf files. "Check for changes"
will fail later when any file is regenerated.

Use ccache in check_generated_files to speed up testing.

https://bugs.python.org/issue44035

@tiran tiran added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Dec 6, 2021
@tiran tiran changed the title Check autoconf files thoroughly bpo-44035: Check autoconf files thoroughly Dec 6, 2021
Check that users don't push changes with outdated or patched autoconf.
The presence of runstatedir option and aclocal 1.16.3 are good markers.

Use my container image to regenerate autoconf files. "Check for changes"
will fail later when any file is regenerated.

Use ccache in check_generated_files to speed up testing.

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran marked this pull request as ready for review December 6, 2021 08:45
@arhadthedev
Copy link
Member

Can we make one step further and use GitHub API to automatically add a file-wide review with a suggestion code block if after a checkout of a tracking branch and the regeneration into its working dir, git diff returns something?

@tiran
Copy link
Member Author

tiran commented Dec 6, 2021

Can we make one step further and use GitHub API to automatically add a file-wide review with a suggestion code block if after a checkout of a tracking branch and the regeneration into its working dir, git diff returns something?

That's an interesting idea and certainly helpful for new contributors. Could you please open a new ticket for your proposal?

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor

Why not just add a regen-autoconf Make target, add that to make regen-all, and let the Check for changes step work it's magic?

Something like this:

diff --git a/Makefile.pre.in b/Makefile.pre.in
index 8e6e553554..5afe48a26f 100644
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -1156,6 +1156,10 @@ Python/frozen_modules/getpath.h: $(FREEZE_MODULE) Modules/getpath.py
 
 Tools/scripts/freeze_modules.py: $(FREEZE_MODULE)
 
+.PHONY: regen-autoconf
+regen-autoconf: configure.ac configure aclocal.m4
+       @docker run --rm -v $(shell pwd):/src quay.io/tiran/cpython_autoconf:269
+
 .PHONY: regen-frozen
 regen-frozen: Tools/scripts/freeze_modules.py $(FROZEN_FILES_IN)
        $(PYTHON_FOR_REGEN) $(srcdir)/Tools/scripts/freeze_modules.py
@@ -1176,7 +1180,7 @@ regen-limited-abi: all
 
 regen-all: regen-opcode regen-opcode-targets regen-typeslots \
        regen-token regen-ast regen-keyword regen-frozen clinic \
-       regen-pegen-metaparser regen-pegen regen-test-frozenmain
+       regen-pegen-metaparser regen-pegen regen-test-frozenmain regen-autoconf
        @echo
        @echo "Note: make regen-stdlib-module-names and autoconf should be run manually"
 

@tiran
Copy link
Member Author

tiran commented Dec 6, 2021

Because it's not that simple. Platforms with mandatory access control and LSMs need extra options. For example SELinux enforcing systems require :Z. There are also distros that don't ship docker. My distro of choice does not have a docker command unless I install the optional podman-docker shim package.

@erlend-aasland
Copy link
Contributor

Because it's not that simple. Platforms with mandatory access control and LSMs need extra options. For example SELinux enforcing systems require :Z. There are also distros that don't ship docker. My distro of choice does not have a docker command unless I install the optional podman-docker shim package.

Ah, true. I was only thinking of the CI, but of course the Makefile has a wider audience.

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

@tiran tiran changed the title bpo-44035: Check autoconf files thoroughly bpo-44035: Check autoconf files thoroughly (GH-29935) Dec 6, 2021
@tiran tiran merged commit 98fac8b into python:main Dec 6, 2021
@tiran tiran deleted the check_autoconf branch December 6, 2021 12:18
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 98fac8bc183c113903403793ffe411358ee40d8a 3.9

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 6, 2021
@bedevere-bot
Copy link

GH-29937 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 6, 2021
Check that users don't push changes with outdated or patched autoconf.
The presence of runstatedir option and aclocal 1.16.3 are good markers.

Use my container image to regenerate autoconf files. "Check for changes"
will fail later when any file is regenerated.

Use ccache in check_generated_files to speed up testing.
(cherry picked from commit 98fac8b)

Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit to tiran/cpython that referenced this pull request Dec 6, 2021
Check that users don't push changes with outdated or patched autoconf.
The presence of runstatedir option and aclocal 1.16.3 are good markers.

Use my container image to regenerate autoconf files. "Check for changes"
will fail later when any file is regenerated.

Use ccache in check_generated_files to speed up testing..
(cherry picked from commit 98fac8b)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Dec 6, 2021
@bedevere-bot
Copy link

GH-29938 is a backport of this pull request to the 3.9 branch.

tiran added a commit that referenced this pull request Dec 6, 2021
Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit that referenced this pull request Dec 6, 2021
Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot
Copy link

bedevere-bot commented Dec 6, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 FreeBSD Non-Debug 3.9 has failed when building commit f147248.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/187/builds/310) and take a look at the build logs.
  4. Check if the failure is related to this commit (f147248) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/187/builds/310

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 14, done.        
remote: Counting objects:   7% (1/13)        
remote: Counting objects:  15% (2/13)        
remote: Counting objects:  23% (3/13)        
remote: Counting objects:  30% (4/13)        
remote: Counting objects:  38% (5/13)        
remote: Counting objects:  46% (6/13)        
remote: Counting objects:  53% (7/13)        
remote: Counting objects:  61% (8/13)        
remote: Counting objects:  69% (9/13)        
remote: Counting objects:  76% (10/13)        
remote: Counting objects:  84% (11/13)        
remote: Counting objects:  92% (12/13)        
remote: Counting objects: 100% (13/13)        
remote: Counting objects: 100% (13/13), done.        
remote: Compressing objects:   7% (1/13)        
remote: Compressing objects:  15% (2/13)        
remote: Compressing objects:  23% (3/13)        
remote: Compressing objects:  30% (4/13)        
remote: Compressing objects:  38% (5/13)        
remote: Compressing objects:  46% (6/13)        
remote: Compressing objects:  53% (7/13)        
remote: Compressing objects:  61% (8/13)        
remote: Compressing objects:  69% (9/13)        
remote: Compressing objects:  76% (10/13)        
remote: Compressing objects:  84% (11/13)        
remote: Compressing objects:  92% (12/13)        
remote: Compressing objects: 100% (13/13)        
remote: Compressing objects: 100% (13/13), done.        
remote: Total 14 (delta 0), reused 8 (delta 0), pack-reused 1        
From https://github.com/python/cpython
 * branch                  3.9        -> FETCH_HEAD
Note: switching to 'f147248795bf22be05bbc891053d60ef6a2f035d'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at f147248795 [3.9] [bpo-44035](https://bugs.python.org/issue44035): Check autoconf files thoroughly (GH-29935) (GH-29938)
Switched to and reset branch '3.9'

In file included from /usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:43:
In file included from /usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/socketmodule.h:116:
In file included from /usr/include/bluetooth.h:54:
/usr/include/netgraph/bluetooth/include/ng_btsocket.h:244:2: warning: "Make sure new member of socket address initialized" [-W#warnings]
#warning "Make sure new member of socket address initialized"
 ^
/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:3116:27: error: implicit declaration of function 'SSLv3_method' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        ctx = SSL_CTX_new(SSLv3_method());
                          ^
/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:3116:27: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'const SSL_METHOD *' (aka 'const struct ssl_method_st *') [-Wint-conversion]
        ctx = SSL_CTX_new(SSLv3_method());
                          ^~~~~~~~~~~~~~
/usr/include/openssl/ssl.h:1503:47: note: passing argument to parameter 'meth' here
__owur SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth);
                                              ^
2 warnings and 1 error generated.

In file included from /usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:43:
In file included from /usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/socketmodule.h:116:
In file included from /usr/include/bluetooth.h:54:
/usr/include/netgraph/bluetooth/include/ng_btsocket.h:244:2: warning: "Make sure new member of socket address initialized" [-W#warnings]
#warning "Make sure new member of socket address initialized"
 ^
/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:3116:27: error: implicit declaration of function 'SSLv3_method' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        ctx = SSL_CTX_new(SSLv3_method());
                          ^
/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Modules/_ssl.c:3116:27: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'const SSL_METHOD *' (aka 'const struct ssl_method_st *') [-Wint-conversion]
        ctx = SSL_CTX_new(SSLv3_method());
                          ^~~~~~~~~~~~~~
/usr/include/openssl/ssl.h:1503:47: note: passing argument to parameter 'meth' here
__owur SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth);
                                              ^
2 warnings and 1 error generated.
test_turtle skipped -- No module named '_tkinter'
test_epoll skipped -- test works only on Linux 2.6
test_smtpnet skipped -- No module named '_ssl'
test_zipfile64 skipped -- test requires loads of disk-space bytes and a long time to run
test_dbm_gnu skipped -- No module named '_gdbm'
test_flock (__main__.FNTLEINTRTest) ... ok
test_lockf (__main__.FNTLEINTRTest) ... ok
test_read (__main__.OSEINTRTest) ... ok
test_wait (__main__.OSEINTRTest) ... ok
test_wait3 (__main__.OSEINTRTest) ... ok
test_wait4 (__main__.OSEINTRTest) ... ok
test_waitpid (__main__.OSEINTRTest) ... ok
test_write (__main__.OSEINTRTest) ... ok
test_devpoll (__main__.SelectEINTRTest) ... skipped 'need select.devpoll'
test_epoll (__main__.SelectEINTRTest) ... skipped 'need select.epoll'
test_kqueue (__main__.SelectEINTRTest) ... ok
test_poll (__main__.SelectEINTRTest) ... ok
test_select (__main__.SelectEINTRTest) ... ok
test_sigtimedwait (__main__.SignalEINTRTest) ... ok
test_sigwaitinfo (__main__.SignalEINTRTest) ... ok
test_accept (__main__.SocketEINTRTest) ... ok
test_open (__main__.SocketEINTRTest) ... ok
test_os_open (__main__.SocketEINTRTest) ... ok
test_recv (__main__.SocketEINTRTest) ... ok
test_recvmsg (__main__.SocketEINTRTest) ... ok
test_send (__main__.SocketEINTRTest) ... ok
test_sendall (__main__.SocketEINTRTest) ... ok
test_sendmsg (__main__.SocketEINTRTest) ... ok
test_sleep (__main__.TimeEINTRTest) ... ok

----------------------------------------------------------------------
Ran 24 tests in 11.384s

OK (skipped=2)
test_spwd skipped -- No module named 'spwd'
test_msilib skipped -- No module named '_msi'
test_tcl skipped -- No module named '_tkinter'
test_winconsoleio skipped -- test only relevant on win32
test_startfile skipped -- object <module 'os' from '/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/Lib/os.py'> has no attribute 'startfile'
test_ttk_textonly skipped -- No module named '_tkinter'
test_idle skipped -- No module named '_tkinter'
test_devpoll skipped -- test works only on Solaris OS family
test_ttk_guionly skipped -- No module named '_tkinter'
test_ioctl skipped -- Unable to open /dev/tty
test_winsound skipped -- No module named 'winsound'
test_tix skipped -- No module named '_tkinter'
test_ssl skipped -- No module named '_ssl'
test_tk skipped -- No module named '_tkinter'

Cannot open file '/usr/home/buildbot/python/3.9.koobs-freebsd-9e36.nondebug/build/test-results.xml' for upload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants