2054355 - build failure on OpenBSD (FreeBSD,Dragonfly ?) since bug 1908744 (missing def for pthread_setname_np)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Landry Breuil (:gaston)]

 4:41.64 E /home/landry/src/m-c/gfx/angle/checkout/src/common/system_utils_linux.cpp:66:5: error: use of undeclared identifier 'pthread_setname_np'            
 4:41.64 E    66 |     pthread_setname_np(pthread_self(), name);                                  
 4:41.64 E       |     ^~~~~~~~~~~~~~~~~~                     
 4:41.64 E 1 error generated.      

on OpenBSD/FreeBSD/Dragonfly that's pthread_set_name_np, see for other uses:

• https://searchfox.org/firefox-main/source/js/src/threading/posix/PosixThread.cpp#110
• https://searchfox.org/firefox-main/source/ipc/glue/ProcessUtils_bsd.cpp#16

it also needs # include <pthread_np.h> ofc

Comment 1

[Landry Breuil (:gaston)]

Erich, can i push an #ifdef quick fix to phabricator as-is so that my buildbot comes green again, or the process of touching angle codebase needs more insane hoops with external patches applied by a magical update script somewhere ?

Comment 2

[Jamie Nicol [:jnicol]]

Touching the angle codebase indeed needs more hoops with external patches applied by a magical update script.

Rather than patching the the contents of system_utils_linux.cpp, I wonder if we can get away with simply not compiling it. Does it build successfully if we comment out this line?

Comment 3

[Landry Breuil (:gaston)]

(In reply to Jamie Nicol [:jnicol] from comment #2)

Touching the angle codebase indeed needs more hoops with external patches applied by a magical update script.

Rather than patching the the contents of system_utils_linux.cpp, I wonder if we can get away with simply not compiling it. Does it build successfully if we comment out this line?

yes, it definitely builds fine for me (havent tested runtime/unit tests though) with those lines commented out in moz.build:

#if CONFIG["OS_ARCH"] not in ("Darwin", "WINNT"):
#    SOURCES += [
#        "../../checkout/src/common/system_utils_linux.cpp",
#    ]

:jnicol, are you pushing that (or the corresponding removal) to phabricator/m-c ?

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

:ErichDonGubler, since you are the author of the regressor, bug 1908744, could you take a look?

For more information, please visit BugBot documentation.

Comment 5

[Jamie Nicol [:jnicol]]

(In reply to Landry Breuil (:gaston) from comment #3)

:jnicol, are you pushing that (or the corresponding removal) to phabricator/m-c ?

Thanks for testing! I expect Erich will make the fix.

I suspect we want to change this line so that it does not pull in the _linux file on non-linux. Looking at the blame it seems we previously changed it to deliberately do so. But presumably upstream code has changed such that this is no longer required.

Now, in bug 2035541 we'll soon be rewriting the angle update script so that rather than adding this file manually on certain platforms, whether it is compiled will be determined programmatically from the upstream build scripts. The script we use to do so does generate a config for openbsd here. But the upstream build scripts do not include this file for openbsd, so we'll presumably have to patch that in.

Would you be able to test this branch for me and see whether it compiles successfully?

Comment 6

[Landry Breuil (:gaston)]

(In reply to Jamie Nicol [:jnicol] from comment #5)

(In reply to Landry Breuil (:gaston) from comment #3)

:jnicol, are you pushing that (or the corresponding removal) to phabricator/m-c ?

Thanks for testing! I expect Erich will make the fix.

I suspect we want to change this line so that it does not pull in the _linux file on non-linux. Looking at the blame it seems we previously changed it to deliberately do so. But presumably upstream code has changed such that this is no longer required.

Now, in bug 2035541 we'll soon be rewriting the angle update script so that rather than adding this file manually on certain platforms, whether it is compiled will be determined programmatically from the upstream build scripts. The script we use to do so does generate a config for openbsd here. But the upstream build scripts do not include this file for openbsd, so we'll presumably have to patch that in.

Would you be able to test this branch for me and see whether it compiles successfully?

that branch built fine for me on OpenBSD/amd64, took more time for git fetch/git checkout than the actual build.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1908744

Comment 8

[Landry Breuil (:gaston)]

per https://bugzilla.mozilla.org/show_bug.cgi?id=2054355#c5

Comment 9

[Jamie Nicol [:jnicol]]

There's probably no point in Erich fixing this until bug 2035541 lands, as the required fix is going to change again. I can take it after that. Hopefully you can patch on your end in the meantime.

Comment 10

[Landry Breuil (:gaston)]

well.. as long as bug 2035541 lands before the upcoming branch merge, because i wouldnt like the failure to percolate to 154 beta. I like my builders green and without local patches :)

Comment 11

[Landry Breuil (:gaston)]

my m-c builder is green again with bug 2035541, cf http://buildbot.rhaalovely.net/#/builders/3/builds/3585 - i guess we can close this one ?