Closed Bug 1180498 Opened 5 years ago Closed 8 months ago

WebGL context can not be created on NetBSD: libGL.so.1 not found

Categories

(Core :: Canvas: WebGL, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox42 --- affected
firefox69 --- fixed

People

(Reporter: martin, Assigned: jgilbert)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 2 obsolete files)

On new NetBSD installations, the version of libGL.so is currently libGL.so.2.0, while libGL.so.1 is only found on some older installations.

Best fix is, like for OpenBSD, to use the unversioned "libGL.so" name (resolving via symlink to the latest version).

With the attached patch (tested on the release branch with firefox 39.0) WebGL works for me.
Attachment #8629652 - Flags: review?(dglastonbury)
Comment on attachment 8629652 [details] [diff] [review]
Extend the pre-exisiting #ifdef __OpenBSD__ for "libGL.so" instead of "libGL.so.1" to NetBSD

Review of attachment 8629652 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for your patch.

I'll accept a patch that explains why the NetBSD case is added with a reference to the original bug report.

::: gfx/gl/GLContextProviderGLX.cpp
@@ +82,4 @@
>          // see e.g. bug 608526: it is intrinsically interesting to know whether we have dynamically linked to libGL.so.1
>          // because at least the NVIDIA implementation requires an executable stack, which causes mprotect calls,
>          // which trigger glibc bug http://sourceware.org/bugzilla/show_bug.cgi?id=12225
> +#if defined(__OpenBSD__) || defined(__NetBSD__)

The comment doesn't match the cases.  How about reorganizing this way: 

#if defined(__OpenBSD__)
        // see e.g. bug 608526: it is intrinsically interesting to know whether we have dynamically linked to libGL.so.1
        // because at least the NVIDIA implementation requires an executable stack, which causes mprotect calls,
        // which trigger glibc bug http://sourceware.org/bugzilla/show_bug.cgi?id=12225
        libGLfilename = "libGL.so";
#elif defined(__NetBSD__)
        // Bug 1180498: On new NetBSD installations, the version of
        // libGL.so is currently libGL.so.2.0, while libGL.so.1 is
        // only found on some older installations. Best fix is to use
        // the unversioned "libGL.so" name (resolving via symlink to
        // the latest version).
        libGLfilename = "libGL.so";
#else
        libGLfilename = "libGL.so.1";
#endif
Attachment #8629652 - Flags: review?(dglastonbury) → review-
I may be confused, but I am pretty sure the OpenBSD case and NetBSD are there for basically identical reasons and the whole comment about bug 608526 does apply to Linux only (glibc not being used on neither NetBSD nor OpenBSD).

I'll happily provide a patch with the comment moved down into the #else part.

Maybe the comment is outdated anyway, now that all the glxtest dance (in the other part of the patch) is done?
Attached patch fix-webgl-for-netbsd-v2.patch (obsolete) — Splinter Review
Comments moved around to the relevant part of the #if
Attachment #8629652 - Attachment is obsolete: true
Attachment #8629820 - Flags: review?(dglastonbury)
Attachment #8629820 - Flags: review?(dglastonbury) → review+
Yeah, that comment made no sense for openbsd - the change to open libGL.so was from bug #687320

Minor nit - the patch has a typo, should be independent, not independend...
Keywords: checkin-needed
Oops, Landry, can you please manually fix the typo when landing?

Side note: assuming bug #687320 is Linux specific, would it make sense to completely invert the sense of the #ifdef and only force the .so.1 #ifdef __Linux__ ?
Such change would come with some unclear (at least to me) risc, and the issue is only noticable at runtime - so not a real suggestion, just some random thought.
(In reply to Landry Breuil (:gaston) from comment #4)
> Yeah, that comment made no sense for openbsd - the change to open libGL.so
> was from bug #687320
> 
> Minor nit - the patch has a typo, should be independent, not independend...

seems this need maybe some changes according to comment #5
Flags: needinfo?(landry)
Keywords: checkin-needed
Sorry, no time those days to do checkins or fix other's patches :(
Flags: needinfo?(landry)
Whiteboard: [gfx-noted]
Priority: -- → P3
Attached patch libgl.diffSplinter Review

Updated patch. Pinging likely reviewer.

Flags: needinfo?(jmuizelaar)
Attachment #9069543 - Flags: review+
Assignee: nobody → jgilbert
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bcc51aaa91e
Tolerate a different major libGL.so on NetBSD as well r=jgilbert

Thanks for providing this!

Flags: needinfo?(jmuizelaar)
Attachment #8629820 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.