1428678 - gfx/gl/MozFramebuffer.h : No such file or directory

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Edmund Wong (:ewong)]

In file included from /builds/slave/c-cen-t-lnx-ntly/build/objdir/dom/canvas/Unified_cpp_dom_canvas1.cpp:137:0:
/builds/slave/c-cen-t-lnx-ntly/build/mozilla/dom/canvas/WebGLContext.cpp:17:35: fatal error: gfx/gl/MozFramebuffer.h: No such file or directory
 #include "gfx/gl/MozFramebuffer.h"
                                   ^
compilation terminated.
make[4]: *** [Unified_cpp_dom_canvas1.o] Error 1
make[4]: *** Waiting for unfinished jobs....

Comment 1

[Edmund Wong (:ewong)]

Attachment: feedback patch

this patch is mostly for feedback and clarification.

Comment 2

[Edmund Wong (:ewong)]

Comment on attachment 8940572 [details] [diff] [review]
feedback patch

As the author of https://hg.mozilla.org/mozilla-central/rev/e2b263515feb,
Jeff, what do you think about this patch?  

I realize tomprince is fixing this bug in bug 1428608; and i had filed this
bug before knowing about that one.  I'm just wondering why MozFramebuffer.h
didn't need Exporting while others do? 

any clarifications appreciated.

Comment 3

[Edmund Wong (:ewong)]

Well, fwiw... the try worked.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73022cfd40977526ea1eddbec46bbf359a0ec0ca

Comment 4

[Kelsey Gilbert [:jgilbert]]

We can take it if I can't get what I want to work working today.

Comment 5

[Kelsey Gilbert [:jgilbert]]

This is probably the same issue as bug 1428608.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Edmund Wong (:ewong) from comment #2)
> Comment on attachment 8940572 [details] [diff] [review]
> feedback patch
> 
> As the author of https://hg.mozilla.org/mozilla-central/rev/e2b263515feb,
> Jeff, what do you think about this patch?  
> 
> I realize tomprince is fixing this bug in bug 1428608; and i had filed this
> bug before knowing about that one.  I'm just wondering why MozFramebuffer.h
> didn't need Exporting while others do? 
> 
> any clarifications appreciated.

If this will be fixed in 1428608, let's close this in favor of that one.
This is a trial of a more explicit approach to include paths, as opposed to the sometimes-problematic virtual pseudo-namespace includes.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8940572 [details] [diff] [review]
feedback patch

This would be how you would revert this to the old way, which we should avoid if we can.

Comment 8

[Edmund Wong (:ewong)]

(In reply to Jeff Gilbert [:jgilbert] from comment #7)
> Comment on attachment 8940572 [details] [diff] [review]
> feedback patch
> 
> This would be how you would revert this to the old way, which we should
> avoid if we can.

Thanks for the feedback Jeff.  Will wait for the other bug to be fixed then.
The patch was purely educational for me.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8940572 [details] [diff] [review]
feedback patch

Jeff, would there be a possibility to land this three line patch whilst people are discussing the correct solution in bug 1428608? It could be backed out later.

Our builds and Dailies are broken since Saturday and bug 1428608 isn't progressing at the desirable speed since somehow all the time-strapped build luminaries got involved.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8940572 [details] [diff] [review]
feedback patch

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

::: gfx/gl/moz.build
@@ +46,5 @@
>      'GLTextureImage.h',
>      'GLTypes.h',
>      'GLUploadHelpers.h',
>      'HeapCopyOfStackArray.h',
> +    'MozFramebuffer.h',

Try landing without this line first.

Comment 11

[Edmund Wong (:ewong)]

(In reply to Jeff Gilbert [:jgilbert] from comment #10)
> Comment on attachment 8940572 [details] [diff] [review]
> feedback patch
> 
> Review of attachment 8940572 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/gl/moz.build
> @@ +46,5 @@
> >      'GLTextureImage.h',
> >      'GLTypes.h',
> >      'GLUploadHelpers.h',
> >      'HeapCopyOfStackArray.h',
> > +    'MozFramebuffer.h',
> 
> Try landing without this line first.

Just did a trypush:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e4cc6dc033227098509817ec000aac73d136d42

Comment 12

[Jorg K (CEST = GMT+2)]

You did a TaskCluster try which doesn't fail, with or without the patch. For a Buildbot try, add --buildbot to the try string.

Personally, a local build could prove the point or a try build on *one* platform since the platforms won't differ with respect to include file visibility.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: bug_1428678.diff (v2)

Carrying forward Jeff's r+ after removing the hunk he wanted removed.

Comment 14

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #12)
> You did a TaskCluster try which doesn't fail, with or without the patch. For
> a Buildbot try, add --buildbot to the try string.
> 

I don't understand.  "Doesn't fail?"  

> Personally, a local build could prove the point or a try build on *one*
> platform since the platforms won't differ with respect to include file
> visibility.

Ok.  Will try that.

Comment 15

[Jorg K (CEST = GMT+2)]

Thanks Jeff, I understand that this is what you wanted, so I'll get this landed. I've changed the component to get the sheriff's attention.

Comment 16

[Jorg K (CEST = GMT+2)]

(In reply to Edmund Wong (:ewong) from comment #14)
> (In reply to Jorg K (GMT+1) from comment #12)
> > You did a TaskCluster try which doesn't fail, with or without the patch. For
> > a Buildbot try, add --buildbot to the try string.
> I don't understand.  "Doesn't fail?"  
TaskCluster builds for Thunderbird were never affected, only Buildbot builds. See bug 1428608. Thanks for the patch, I compiled locally and it compiled with the reduced version. So given the pressing nature of the problem, I'm going to ask the sheriffs to land this directly in M-C unless I hear any objection in the next 45 minutes.

Comment 17

[Edmund Wong (:ewong)]

(In reply to Jorg K (GMT+1) from comment #16)
> (In reply to Edmund Wong (:ewong) from comment #14)
> > (In reply to Jorg K (GMT+1) from comment #12)
> > > You did a TaskCluster try which doesn't fail, with or without the patch. For
> > > a Buildbot try, add --buildbot to the try string.
> > I don't understand.  "Doesn't fail?"  
> TaskCluster builds for Thunderbird were never affected, only Buildbot
> builds. See bug 1428608. Thanks for the patch, I compiled locally and it
> compiled with the reduced version. So given the pressing nature of the
> problem, I'm going to ask the sheriffs to land this directly in M-C unless I
> hear any objection in the next 45 minutes.

Oh right.  Sorry.  I forgot the issue was with comm-central.  Should've done a try patch with
the try-comm-central.  My bad.

Comment 18

[Jorg K (CEST = GMT+2)]

Well, my bad, you did a M-C try, so we have proof that this doesn't break FF. Thanks! So let's get this landed.

Comment 19

[Jorg K (CEST = GMT+2)]

I'm talking to the sheriffs on #developers.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/6f5fac320fcb
Include MozFramebuffer.h instead of gfx/gl/MozFramebuffer.h to work around Thunderbird builtbot problem. r=jgilbert a=thunderbird-bustagefix

Comment 21

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/6f5fac320fcb