Closed Bug 939608 Opened 6 years ago Closed 6 years ago

Build xpconnect in unified mode

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: ehsan, Assigned: ehsan)

References

(Depends on 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(2 files)

No description provided.
Assignee: nobody → ehsan
Blocks: unified
Attachment #8333592 - Flags: review?(bobbyholley+bmo)
Comment on attachment 8333592 [details] [diff] [review]
Build xpconnect in unified mode

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

Brave new world. r=bholley I think, though it's not yet clear to me how this will impact my workflow.

::: js/xpconnect/loader/mozJSComponentLoader.h
@@ +145,5 @@
>      bool mInitialized;
>      bool mReuseLoaderGlobal;
>  };
> +
> +#endif

nit: /* mozJSComponentLoader_h */

::: js/xpconnect/wrappers/XrayWrapper.h
@@ +4,5 @@
>   * This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#ifndef XrayWrapper_h

For consistency with this directory, this should be __XrayWrapper_h__

@@ +192,5 @@
>  };
>  
>  }
>  
> +#endif

/* __XrayWrapper_h__ */

::: js/xpconnect/wrappers/moz.build
@@ +17,5 @@
>      'WaiveXrayWrapper.cpp',
>      'WrapperFactory.cpp',
> +]
> +
> +# XrayWrapper needs to be built separately becaue of template instantiations.

'because'
Attachment #8333592 - Flags: review?(bobbyholley+bmo) → review+
had to backout the changes in mozilla-inbound and mozilla-central because of earlier issues with PGO Build Bustages. (also backed out 3b1d81c4c118 (bug 938437) also for this reason)

During the backout test for that patch Glandium was running into a race condition and build failure because of this patch.

Glandium asked me in IRC to mention https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/Makefile.in#66
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Carsten Book [:Tomcat] from comment #5)
> had to backout the changes in mozilla-inbound and mozilla-central because of
> earlier issues with PGO Build Bustages. (also backed out 3b1d81c4c118 (bug
> 938437) also for this reason)

Do you have a log link for those issues?

> During the backout test for that patch Glandium was running into a race
> condition and build failure because of this patch.
> 
> Glandium asked me in IRC to mention
> https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/Makefile.
> in#66

Sigh, yes.
Flags: needinfo?(cbook)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6)
> (In reply to Carsten Book [:Tomcat] from comment #5)
> > had to backout the changes in mozilla-inbound and mozilla-central because of
> > earlier issues with PGO Build Bustages. (also backed out 3b1d81c4c118 (bug
> > 938437) also for this reason)
> 
> Do you have a log link for those issues?
> 
https://tbpl.mozilla.org/php/getParsedLog.php?id=30755726&tree=Mozilla-Inbound is one of the failed Windows PGO Builds
Flags: needinfo?(cbook)
Depends on: 939615
Landed the code changes for now: <https://hg.mozilla.org/integration/mozilla-inbound/rev/6be9a40c54b6>
Whiteboard: [leave open]
Carsten, was this PGO bustage intermittent by any chance?  I can't seem be able to reproduce it locally.
Flags: needinfo?(cbook)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #9)
> Carsten, was this PGO bustage intermittent by any chance?  I can't seem be
> able to reproduce it locally.

It was.
Flags: needinfo?(cbook)
(In reply to Carsten Book [:Tomcat] from comment #7)
> https://tbpl.mozilla.org/php/getParsedLog.php?id=30755726&tree=Mozilla-
> Inbound is one of the failed Windows PGO Builds

This is unrelated. The failure due to this bug is:
https://tbpl.mozilla.org/php/getParsedLog.php?id=30757717&tree=Mozilla-Inbound
Depends on: 940775
(In reply to Mike Hommey [:glandium] from comment #11)
> (In reply to Carsten Book [:Tomcat] from comment #7)
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=30755726&tree=Mozilla-
> > Inbound is one of the failed Windows PGO Builds
> 
> This is unrelated. The failure due to this bug is:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=30757717&tree=Mozilla-
> Inbound

OK you guys scared me!  ;-)

So this is only related to the race condition.  New patch coming up.
Attachment #8335341 - Flags: review?(mh+mozilla)
Comment on attachment 8335341 [details] [diff] [review]
Build xpconnect in unified mode; r=glandium

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

I have an idea that will make the original patch work.
Attachment #8335341 - Flags: review?(mh+mozilla) → review-
Can you be more specific?  There is very little value in making the original patch work...
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #16)
> Can you be more specific?  There is very little value in making the original
> patch work...

I want to remove all those manual dependencies between object files and generated headers.
(In reply to comment #17)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #16)
> > Can you be more specific?  There is very little value in making the original
> > patch work...
> 
> I want to remove all those manual dependencies between object files and
> generated headers.

That would be nice for sure, but if it's a lot of work I don't think that it needs to hold this bug.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #18)
> That would be nice for sure, but if it's a lot of work I don't think that it
> needs to hold this bug.

It's a review away.
Backed out because bug 941450 was backed out

https://hg.mozilla.org/integration/mozilla-inbound/rev/53f09386d367
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/488b55c9cf60
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.