ci: skipping test for Windows Clang failure #5062

henryiii · 2024-03-20T16:48:23Z

Description

Skipping a test for a Clang failure. Also hiding a warning in Clang that we hide in GCC.

Last successful log: 2024-02-20T21:40:01.6656694Z https://github.com/pybind/pybind11/actions/runs/7980348964/job/21789806071

First failing log: 2024-02-27T17:04:02.7790106Z https://github.com/pybind/pybind11/actions/runs/8067833464/job/22041600407

The log files are permanently archived here: https://github.com/rwgk/issues/tree/master/pybind11/pr_5062

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

.github/workflows/ci.yml

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii · 2024-03-20T20:02:59Z

pybind11_tests.compiler_info.startswith('Homebrew Clang') is this ever True? To me it looks like it can be msvc version or __version__, and nothing else. Edit: maybe the problem is only that MSVC comes first, and Clang loves to pretend it's MSVC on windows (and GCC on Unix).

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

rwgk · 2024-03-20T21:42:29Z

tests/pybind11_tests.cpp

-#if defined(_MSC_FULL_VER)
-    m.attr("compiler_info") = "MSVC " PYBIND11_TOSTRING(_MSC_FULL_VER);
-#elif defined(__VERSION__)
+#if defined(__VERSION__)


I think it's unfortunate either way around.

I didn't realize before that Windows Clang has this problem. It's also not great as is.

Ideally I think we'd have something like this:

#if defined(_MSC_FULL_VER) # if defined(__VERSION__) m.attr("compiler_info") = __VERSION__ " MSVC " PYBIND11_TOSTRING(_MSC_FULL_VER); # else m.attr("compiler_info") = "MSVC " PYBIND11_TOSTRING(_MSC_FULL_VER); # endif #elif defined(__VERSION__) m.attr("compiler_info") = __VERSION__;

__version__ is a string that describes the compiler (name and version), and it's not provided by MSVC. So if __version__ is set, use that, otherwise provide the fallback. I don't see a reason to add a case that's not possible (from what I understand). And if it was, then MSVC would be provided twice.

I checked a variety of MSVC builds, they all print correctly.

rwgk · 2024-03-20T21:43:51Z

tests/test_exceptions.py

@@ -248,6 +248,11 @@ def pycatch(exctype, f, *args):  # noqa: ARG001
    assert str(excinfo.value) == "this is a helper-defined translated exception"


+# TODO: Investigate this crash
+@pytest.mark.skipif(


In similar situations before I created a bug, tracking information from

the last GHA log that still worked

the first GHA log that was broken

I can do that a little later, then we can reference that bug here.

Logs are deleted after a time; I guess you mean copy that info here then link here?

I guess you mean copy that info here then link here?

Yes

(trying to find an example now; writing from a train with choppy internet connectivity ...)

Here is an example: #4889

I'm pointing to github.com/rwgk, where I checked in copies of the logs.

I can do the same for this PR. We can simple use a comment here for the equivalent of the 4889 PR description.

Okay, will it be ready to merge once you do that?

I see this in the log:

C++ Info: Clang 18.1.0rc C++17 __pybind11_internals_v5_msvc_mscver1939_debug__ PYBIND11_SIMPLE_GIL_MANAGEMENT=False

Would I had in mind is to produce (made up):

C++ Info: Clang 18.1.0rc MSVC 193933522 C++17 __pybind11_internals_v5_msvc_mscver1939_debug__ PYBIND11_SIMPLE_GIL_MANAGEMENT=False

This could well help in the future easily mining logs programmatically (grep).

But if you're strongly opposed for some reason, what you have seems acceptable.

I'd make this specific to the Clang version, e.g. "Clang 18".

and for the reason: "See pybind/pybind11#5062 for background."

Good to merge with that.

I believe Clang defines the MSVC macros for compatibility, just like it defined the GCC macros for compatibility on Unix. The value they are defined to isn't really relevant, you just need to know the clang version.

It's happening on at least 16 and 18, so I don't think it's clang version specific. I think it's just installing clang from chocolaty, not sure what versions are available, but 16 is pre-installed on the runner and our setup action is updating it to 18.

The value they are defined to isn't really relevant

They are informative. Could make the difference between being confused about what path is taken and being completely clear, when debugging.

Seeing “MSVC” when MSVC isn’t being used is more likely to be confusing IMO. And why are we printing out the MSVC compat values and not the GCC ones? Information that is misleading and redundant may be worse than no information at all.

The purpose of the compiler info is to tell the user what compiler is being used and to help with test skips. Adding MSVC doesn’t tell the use what compiler is being used and will confuse any tests that actually need to be skipped on MSVC (for example).

rwgk · 2024-03-20T23:38:26Z

I will make this comment more complete as I get a chance (asap).

Last successful log: 2024-02-20T21:40:01.6656694Z https://github.com/pybind/pybind11/actions/runs/7980348964/job/21789806071

First failing log: 2024-02-27T17:04:02.7790106Z https://github.com/pybind/pybind11/actions/runs/8067833464/job/22041600407

tests/test_exceptions.py

rwgk · 2024-03-21T08:06:28Z

tests/test_exceptions.py

+# TODO: Investigate this crash, see pybind/pybind11#5062 for background
+@pytest.mark.skipif(
+    sys.platform.startswith("win32") and "Clang" in pybind11_tests.compiler_info,
+    reason="Started segfaulting February 2024",


The error was actually:

Windows fatal exception: stack overflow

ci: trying things for Windows Clang failure

41b2295

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii added the python dev Working on development versions of Python label Mar 20, 2024

WIP: try using older clang

161718b

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii removed the python dev Working on development versions of Python label Mar 20, 2024

tests: skip broken test

3d81045

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

rwgk reviewed Mar 20, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

tests: try to skip test in tests

8e4348a

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

fix(tests): Prefer __version__ over MSVC

6daeb9c

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii marked this pull request as ready for review March 20, 2024 20:39

chore: avoid warning on Clang

209083c

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii changed the title ~~ci: trying things for Windows Clang failure~~ ci: skipping test for Windows Clang failure Mar 20, 2024

rwgk reviewed Mar 20, 2024

View reviewed changes

henryiii commented Mar 21, 2024

View reviewed changes

tests/test_exceptions.py Outdated Show resolved Hide resolved

Update tests/test_exceptions.py

a65c308

rwgk reviewed Mar 21, 2024

View reviewed changes

tests/test_exceptions.py Outdated Show resolved Hide resolved

rwgk approved these changes Mar 21, 2024

View reviewed changes

Update tests/test_exceptions.py

4ac43f6

henryiii merged commit ec73bda into master Mar 21, 2024
84 checks passed

henryiii deleted the henryiii/ci/winfail branch March 21, 2024 06:27

rwgk pushed a commit to rwgk/issues that referenced this pull request Mar 21, 2024

CI log files referenced under pybind/pybind11#5062

8920e61

rwgk reviewed Mar 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: skipping test for Windows Clang failure #5062

ci: skipping test for Windows Clang failure #5062

henryiii commented Mar 20, 2024 •

edited by rwgk

Loading

henryiii commented Mar 20, 2024 •

edited

Loading

rwgk Mar 20, 2024

henryiii Mar 20, 2024 •

edited

Loading

henryiii Mar 20, 2024

rwgk Mar 20, 2024

henryiii Mar 20, 2024

rwgk Mar 20, 2024

rwgk Mar 20, 2024

henryiii Mar 20, 2024

rwgk Mar 20, 2024

rwgk Mar 20, 2024

henryiii Mar 20, 2024

rwgk Mar 20, 2024

henryiii Mar 21, 2024

rwgk commented Mar 20, 2024

rwgk Mar 21, 2024

ci: skipping test for Windows Clang failure #5062

ci: skipping test for Windows Clang failure #5062

Conversation

henryiii commented Mar 20, 2024 • edited by rwgk Loading

Description

henryiii commented Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

henryiii Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwgk commented Mar 20, 2024

Choose a reason for hiding this comment

henryiii commented Mar 20, 2024 •

edited by rwgk

Loading

henryiii commented Mar 20, 2024 •

edited

Loading

henryiii Mar 20, 2024 •

edited

Loading