Closed Bug 1428678 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: ewong, Assigned: ewong)

References

Details

Attachments

(1 file, 1 obsolete file)

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....
Attached patch feedback patch (obsolete) — Splinter Review
this patch is mostly for feedback and clarification.
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.
Attachment #8940572 - Flags: feedback?(jgilbert)
We can take it if I can't get what I want to work working today.
This is probably the same issue as bug 1428608.
See Also: → 1428608
(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.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1428608
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.
Attachment #8940572 - Flags: feedback?(jgilbert) → feedback+
(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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
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.
Attachment #8940572 - Flags: review?(jgilbert)
Status: REOPENED → NEW
Product: SeaMonkey → Thunderbird
Version: unspecified → Trunk
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.
Attachment #8940572 - Flags: review?(jgilbert) → review+
(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
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.
Carrying forward Jeff's r+ after removing the hunk he wanted removed.
Attachment #8940572 - Attachment is obsolete: true
Attachment #8940996 - Flags: review+
Assignee: nobody → 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.
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.
Component: Build Config → Canvas: WebGL
Keywords: checkin-needed
Product: Thunderbird → Core
(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.
(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.
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.
I'm talking to the sheriffs on #developers.
Pushed by ebalazs@mozilla.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6f5fac320fcb
Status: NEW → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.