GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-126554: correct detection of `gcc` for `TestNullDlsym.test_null_dlsym` by petermarko · Pull Request #129872 · python/cpython

Conversation

@petermarko

@petermarko petermarko commented Feb 8, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

In case gcc is not available, it will throw exception and test fails. So catch the exception to skip the test correctly.

======================================================================
ERROR: test_null_dlsym (test.test_ctypes.test_dlerror.TestNullDlsym.test_null_dlsym)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.12/test/test_ctypes/test_dlerror.py", line 61, in test_null_dlsym
    retcode = subprocess.call(["gcc", "--version"],
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 391, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 1028, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.12/subprocess.py", line 1963, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'gcc'
  • Issue: ctypes: We do not correctly handle NULL dlsym() return values #126554

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ghost

ghost commented Feb 8, 2025 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Feb 8, 2025
@bedevere-app

bedevere-app Bot commented Feb 8, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added the awaiting review label Feb 8, 2025
@picnixz

picnixz commented Feb 9, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Wait, how come the returncode is not catching this kind of issue. I remember we changed the subprocess call but I don't remember why we left it as is.

  1. Can you also use the issue number we used for adding this test?
  2. Can you narrow down the exception being caught instead of a bare except? or find a way to avoid a try-except and use another feature of subprocess if possible?

cc @encukou (I don't remember why we didn't catch this)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@petermarko

petermarko commented Feb 9, 2025

Copy link
Copy Markdown
Contributor Author

The trace is actually from 3.12.9, but the subprocess call throws exception also in 3.13 and 3.14-rc although I didn't try to run the testsuite in those.
The test was introduced based on gh-126554 and backported to 3.12 in gh-127764.
I can also just target 3.12 branch if it's not a problem for newer branches.

I'll rewrite this to catch just FileNotFoundError

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz changed the title ctypes: correct gcc check in test gh-126554: correct detection of gcc for TestNullDlsym.test_null_dlsym Feb 9, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Feb 9, 2025
ctypes: We do not correctly handle NULL dlsym() return values #126554
Closed
@picnixz picnixz added the skip news label Feb 9, 2025
@petermarko
petermarko force-pushed the fix/test_null_dlsym-main branch from 88e485a to 4835b2b Compare February 9, 2025 11:07
@petermarko

petermarko commented Feb 9, 2025

Copy link
Copy Markdown
Contributor Author

Beautified by adding FileNotFoundError exception type

Meanwhile I retested also 3.13.2, so I guess it's valid for all releases.

======================================================================
ERROR: test_null_dlsym (test.test_ctypes.test_dlerror.TestNullDlsym.test_null_dlsym)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.13/test/test_ctypes/test_dlerror.py", line 61, in test_null_dlsym
    retcode = subprocess.call(["gcc", "--version"],
                              stdout=subprocess.DEVNULL,
                              stderr=subprocess.DEVNULL)
  File "/usr/lib/python3.13/subprocess.py", line 397, in call
    with Popen(*popenargs, **kwargs) as p:
         ~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/subprocess.py", line 1038, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        pass_fds, cwd, env,
                        ^^^^^^^^^^^^^^^^^^^
    ...<5 lines>...
                        gid, gids, uid, umask,
                        ^^^^^^^^^^^^^^^^^^^^^^
                        start_new_session, process_group)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/subprocess.py", line 1974, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'gcc'
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Feb 9, 2025
@picnixz

picnixz commented Feb 9, 2025

Copy link
Copy Markdown
Member

Ok, actually there is a way to make it work without a try-except: We need to pass the shell=True parameter. However it appears that the following happens:

>>> subprocess.call(["gcc", "--version"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
0
>>> subprocess.call(["gcc", "--version"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, shell=True)
1

So, we need to be a bit more careful with the actual return code! So, instead I either indeed suggest using a FileNotFoundError guard. Note that using which could work but this utility is not present everywhere:

subprocess.call(["which", "abc"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) # == 1
subprocess.call(["which", "gcc"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) # == 0

So, let's use the guard.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz
picnixz approved these changes Feb 9, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Feb 9, 2025
picnixz
picnixz reviewed Feb 9, 2025
View reviewed changes
Comment thread Lib/test/test_ctypes/test_dlerror.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@petermarko
ctypes: correct gcc check in test
2777a08
In case gcc is not available, it will throw exception and test fails.
So catch the exception to skip the test correctly.

======================================================================
ERROR: test_null_dlsym (test.test_ctypes.test_dlerror.TestNullDlsym.test_null_dlsym)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.12/test/test_ctypes/test_dlerror.py", line 61, in test_null_dlsym
    retcode = subprocess.call(["gcc", "--version"],
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 391, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 1028, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.12/subprocess.py", line 1963, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'gcc'

Signed-off-by: Peter Marko <[email protected]>
@petermarko
petermarko force-pushed the fix/test_null_dlsym-main branch from 4835b2b to 2777a08 Compare February 9, 2025 12:02
encukou
encukou reviewed Feb 10, 2025
View reviewed changes
Comment thread Lib/test/test_ctypes/test_dlerror.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Feb 10, 2025

Copy link
Copy Markdown
Member

There is shutil.which, but, catching the error is more appropriate here.

Perhaps it's time to add a test helper to find gcc/clang, and skip test if it's not available. (But that would be for a new PR.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
Use OSError
cbec27b
@encukou
encukou merged commit 978211c into python:main Feb 10, 2025
@miss-islington-app

miss-islington-app Bot commented Feb 10, 2025

Copy link
Copy Markdown

Thanks @petermarko for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Feb 10, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 10, 2025
@petermarko @miss-islington
pythongh-126554: correct detection of gcc for `TestNullDlsym.test_n…
b9f8346
…ull_dlsym` (pythonGH-129872)

In case gcc is not available, the test will fail with FileNotFoundError.
So catch the exception to skip the test correctly.
(cherry picked from commit 978211c)

Co-authored-by: Peter Marko <[email protected]>
Signed-off-by: Peter Marko <[email protected]>
@bedevere-app

bedevere-app Bot commented Feb 10, 2025

Copy link
Copy Markdown

GH-129944 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Feb 10, 2025
@encukou

encukou commented Feb 10, 2025

Copy link
Copy Markdown
Member

Thank you for the report & fix, @petermarko!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 10, 2025
@petermarko @miss-islington
pythongh-126554: correct detection of gcc for `TestNullDlsym.test_n…
c3994fa
…ull_dlsym` (pythonGH-129872)

In case gcc is not available, the test will fail with FileNotFoundError.
So catch the exception to skip the test correctly.
(cherry picked from commit 978211c)

Co-authored-by: Peter Marko <[email protected]>
Signed-off-by: Peter Marko <[email protected]>
@bedevere-app

bedevere-app Bot commented Feb 10, 2025

Copy link
Copy Markdown

GH-129945 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Feb 10, 2025
picnixz pushed a commit that referenced this pull request Feb 10, 2025
@miss-islington @petermarko
[3.12] gh-126554: correct detection of gcc for `TestNullDlsym.test_…
3d2ca74
…null_dlsym` (GH-129872) (#129945)

gh-126554: correct detection of `gcc` for `TestNullDlsym.test_null_dlsym` (GH-129872)

In case gcc is not available, the test will fail with FileNotFoundError.
So catch the exception to skip the test correctly.
(cherry picked from commit 978211c)

Signed-off-by: Peter Marko <[email protected]>
Co-authored-by: Peter Marko <[email protected]>
picnixz pushed a commit that referenced this pull request Feb 10, 2025
@miss-islington @petermarko
[3.13] gh-126554: correct detection of gcc for `TestNullDlsym.test_…
cef406b
…null_dlsym` (GH-129872) (#129944)

gh-126554: correct detection of `gcc` for `TestNullDlsym.test_null_dlsym` (GH-129872)

In case gcc is not available, the test will fail with FileNotFoundError.
So catch the exception to skip the test correctly.
(cherry picked from commit 978211c)

Signed-off-by: Peter Marko <[email protected]>
Co-authored-by: Peter Marko <[email protected]>
@bedevere-bot

bedevere-bot commented Feb 10, 2025

Copy link
Copy Markdown

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

Hi! The buildbot AMD64 Debian root 3.13 has failed when building commit cef406b.

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/#/builders/1441/builds/897) and take a look at the build logs.
  4. Check if the failure is related to this commit (cef406b) 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/#/builders/1441/builds/897

Failed tests:

  • test_os

Failed subtests:

  • test_timerfd_ns_select - test.test_os.TimerfdTests.test_timerfd_ns_select

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

==

Click to see traceback logs
Traceback (most recent call last):
  File "/root/buildarea/3.13.angelico-debian-amd64/build/Lib/test/test_os.py", line 4450, in test_timerfd_ns_select
    self.assertEqual(self.read_count_signaled(fd), 1)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 3 != 1


Traceback (most recent call last):
  File "/root/buildarea/3.13.angelico-debian-amd64/build/Lib/test/test_os.py", line 4450, in test_timerfd_ns_select
    self.assertEqual(self.read_count_signaled(fd), 1)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 4 != 1
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz mentioned this pull request Jun 14, 2025
v3.13.1 test test_ctypes/test_dlerror.py fails instead of skipping if gcc not in path #127626
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments
@picnixz picnixz picnixz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@petermarko @picnixz @encukou @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.