Skip to content

Conversation

furkanonder
Copy link
Contributor

@furkanonder furkanonder commented Sep 14, 2024

@furkanonder furkanonder added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 14, 2024
@Zheaoli
Copy link
Contributor

Zheaoli commented Sep 15, 2024

I think we need to link this PR to the NetBSD community. We don't know whether this issue is caused by the NetBSD in a specific version or this is a wide exist bug.

I think we need to hold on this PR before we confirm the bug scope

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

If strsignal is not supported on this platform, then the configure check must have detected it. However, it appears that your C program works correctly (namely that strsignal is present), so HAVE_STRSIGNAL should be defined. This also means that the following somehow returns None.

    errno = 0;
    res = strsignal(signalnum);

    if (errno || res == NULL || strstr(res, "Unknown signal") != NULL)
        Py_RETURN_NONE;

Either errno is actually set differently or res is NULL. But this seems quite weird that res is NULL since the C program works. So errno is somewhat modified.

If HAVE_STRSIGNAL is not specified, then the switch we are using is wrong. We should also check that the constants being check are actually the correct ones. I don't have access to a NetBSD machine but something more delicate could be hidden.

@Zheaoli
Copy link
Contributor

Zheaoli commented Sep 15, 2024

If HAVE_STRSIGNAL is not specified, then the switch we are using is wrong. We should also check that the constants being check are actually the correct ones. I don't have access to a NetBSD machine but something more delicate could be hidden.

I'm working on the root cause. This is a little bit wired

@vstinner
Copy link
Member

@Zheaoli: There are more details in the issue: #124083 (comment) signal.strsignal() returns None for most signals.

@vstinner vstinner enabled auto-merge (squash) September 18, 2024 21:09
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner vstinner merged commit 36682c0 into python:main Sep 18, 2024
35 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2024
…H-124084)

Skip test_strsignal() on NetBSD due to TypeError.
(cherry picked from commit 36682c0)

Co-authored-by: Furkan Onder <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 18, 2024

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2024
…H-124084)

Skip test_strsignal() on NetBSD due to TypeError.
(cherry picked from commit 36682c0)

Co-authored-by: Furkan Onder <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 18, 2024
@bedevere-app
Copy link

bedevere-app bot commented Sep 18, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 18, 2024
@vstinner
Copy link
Member

Merged, thanks for the fix @furkanonder.

vstinner pushed a commit that referenced this pull request Sep 18, 2024
) (#124224)

gh-124083: Skip test_signal.test_strsignal() on NetBSD (GH-124084)

Skip test_strsignal() on NetBSD due to TypeError.
(cherry picked from commit 36682c0)

Co-authored-by: Furkan Onder <[email protected]>
@furkanonder
Copy link
Contributor Author

Merged, thanks for the fix @furkanonder.

You are welcome!

@picnixz
Copy link
Member

picnixz commented Sep 19, 2024

I'm still convinced that the fact it returns None is something that should be investigated. But I can live with this fix for now

@vstinner
Copy link
Member

I'm still convinced that the fact it returns None is something that should be investigated

strsignal() is basically not implemented on NetBSD: #124083 (comment)

What do you want to investigate?

@picnixz
Copy link
Member

picnixz commented Sep 19, 2024

Then why does it work with the C example on the issue? The function seems present and it returns a non-NULL result! (or am I misunderstanding something?)

@vstinner
Copy link
Member

Then why does it work with the C example on the issue?

If fails with errno 2.

@picnixz
Copy link
Member

picnixz commented Sep 19, 2024

Oh! I thought the result would be NULL in this case! My bad. Ok so nothing to investigate. Sorry for the noise :')

@vstinner
Copy link
Member

No need to be sorry, strsignal() on NetBSD has a strange behavior (return a string and report an error). IMO it's easier to skip the test, there is no need to spend too much time on it until NetBSD is fixed.

savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this pull request Sep 22, 2024
Yhg1s added a commit that referenced this pull request Sep 24, 2024
) (#124223)

gh-124083: Skip test_signal.test_strsignal() on NetBSD (GH-124084)

Skip test_strsignal() on NetBSD due to TypeError.
(cherry picked from commit 36682c0)

Co-authored-by: Furkan Onder <[email protected]>
Co-authored-by: T. Wouters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants