Closed
Bug 1317674
Opened 8 years ago
Closed 8 years ago
Port |Bug 1316450 - Enforce that nothing new depends on the XPCOM glue| to TB etc.
Categories
(Thunderbird :: Build Config, defect)
Thunderbird
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 53.0
People
(Reporter: jorgk-bmo, Assigned: aleth)
References
Details
Attachments
(1 file, 11 obsolete files)
Bustage due to bug 1316450:
mozbuild.frontend.reader.SandboxValidationError:
==============================
ERROR PROCESSING MOZBUILD FILE
==============================
The error occurred while processing the following file or one of the files it includes:
c:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/calendar/base/backend/libical/build/moz.build
The error occurred when validating the result of the execution. The reported error is:
calbasecomps depends on the XPCOM glue. No new dependency on the XPCOM glue is allowed.
First seen Daily build here Tue Nov 15, 11:17:54
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=f88a5eda49535c043f7b61cb11582b2a7c920b9f
and then here Tue Nov 15, 12:02:48
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=e8c2d64742f4c6a33ae4463e0b6436dc37edabcb
Reporter | ||
Comment 1•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Component: General → Build Config
Flags: needinfo?(philipp)
Product: Thunderbird → Calendar
Assignee | ||
Comment 2•8 years ago
|
||
If it's just libical, would it make sense to bite the bullet now and switch to ical.js?
Assignee | ||
Comment 3•8 years ago
|
||
...it's not just libical. There's also (independently) one of the perennial split build system path issues for 'thunderbird' itself, for which:
obj.relativedir = mail/app
MOZ_BUILD_APP = ../mail/app
and so the check in emitter.py fails.
Product: Calendar → Thunderbird
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to aleth [:aleth] from comment #2)
> If it's just libical, would it make sense to bite the bullet now and switch
> to ical.js?
No. This has been discussing in the regular Tuesday meetings for a long time now. JS is apparently not ready. On top of some minor problems, like the notorious bug 1294668, there are apparently performance issues. MakeMyDay always talks about "rewriting the parser". I started using IcalJS in Daily until I got hit by some of the notorious problems, events not showing due to that, so I had to switch back to libical for my day-to-day operation.
Sticking libical into C-C core has also been discussed, but as far as I see no action was taken. Miraculously bug 1314955 didn't get landed on Gecko52 despite r+. That bug would have broken the binary add-on completely.
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #4)
> Sticking libical into C-C core has also been discussed, but as far as I see
> no action was taken. Miraculously bug 1314955 didn't get landed on Gecko52
> despite r+. That bug would have broken the binary add-on completely.
As you say, libical is probably going to be broken in a matter of days anyway, therefore some decision from Calendar on how to proceed is required now anyway.
Assignee | ||
Comment 6•8 years ago
|
||
This is a somewhat hacky check for whether it's a split build system build or not, but I can't think of any scenario where a m-c build would have such a relative path in MOZ_BUILD_APP, so it seems the easiest solution.
Attachment #8810821 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Reporter | ||
Comment 7•8 years ago
|
||
Aleth, is this meant to fix the build failure described in comment #0? I still see it in a local build. I will hack this now to get some daily builds going on try.
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(aleth)
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #7)
> Aleth, is this meant to fix the build failure described in comment #0? I
> still see it in a local build. I will hack this now to get some daily builds
> going on try.
No, the patch is for comment 3 - you can see that failure locally when you disable building calendar (comment 0).
Flags: needinfo?(aleth)
Assignee | ||
Comment 9•8 years ago
|
||
Here's a (temporary while libical is with us) patch to avoid the build bustage from comment 0.
Attachment #8810904 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 10•8 years ago
|
||
Comment 11•8 years ago
|
||
> The error occurred when validating the result of the execution. The reported error is:
> suite depends on the XPCOM glue. No new dependency on the XPCOM glue is allowed.
Thanks for trying to fix this. SeaMonkey still breaks with the patch. Does im build?
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Frank-Rainer Grahl from comment #11)
> > The error occurred when validating the result of the execution. The reported error is:
> > suite depends on the XPCOM glue. No new dependency on the XPCOM glue is allowed.
>
> Thanks for trying to fix this. SeaMonkey still breaks with the patch. Does
> im build?
Instantbird would be fine with this patch if it were not for purplexpcom.
Assignee | ||
Comment 13•8 years ago
|
||
This would be very helpful to temporarily remove the bustage from Instantbird (temporary because the way purplexpcom is built will have to change once binary components are no more).
Attachment #8810999 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 14•8 years ago
|
||
I took the liberty to roll these into one. (There was also a problem with a comma at the end of the line.) Apparently SeaMonkey still has a problem building.
Attachment #8810821 -
Attachment is obsolete: true
Attachment #8810904 -
Attachment is obsolete: true
Attachment #8810999 -
Attachment is obsolete: true
Attachment #8810821 -
Flags: review?(mh+mozilla)
Attachment #8810904 -
Flags: review?(mh+mozilla)
Attachment #8810999 -
Flags: review?(mh+mozilla)
Attachment #8811042 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 15•8 years ago
|
||
Sorry for rolling these into one without Aleth's agreement. If this patch can't be approved, then please consider the predecessor in attachment 8810821 [details] [diff] [review].
Updated•8 years ago
|
Attachment #8811042 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 16•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cd867e3258be1a56c712f6dfa9f8b8532f7a8ff
Bug 1317674 - Use the correct path to the binary for comm builds in the XPCOM glue check + exceptions for Calendar and Instantbird. r=glandium
I had to back this out for make check failures like https://treeherder.mozilla.org/logviewer.html#?job_id=39269184&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fa59ec4e1addd2f5e02748cc487e51c3fc41d5b
Flags: needinfo?(aleth)
Reporter | ||
Comment 18•8 years ago
|
||
This shouldn't be too hard to fix. Looks like this is coming from
+ moz_build_app = substs.get('MOZ_BUILD_APP')
+ if moz_build_app.startswith('../'):
and moz_build_app comes out null or undefined. Sorry, not a python expert at all.
In JS we would just go
moz_build_app = substs.get('MOZ_BUILD_APP') || ""
;-)
Funny thought that before it didn't complain about:
- (substs.get('MOZ_APP_NAME'), '%s/app' % substs.get('MOZ_BUILD_APP')),
if substs.get('MOZ_BUILD_APP') was indeed undefined.
Reporter | ||
Comment 19•8 years ago
|
||
How about:
if moz_build_app is not None and moz_build_app.startswith('../'):
?
Trying this here:
https://hg.mozilla.org/try-comm-central/rev/68daceeb88f3762ed336f0f01b1e6c748c0ac2a9#l1.44
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=955d0f2291eea74e97c90da9b13d751354c873b3
Assignee | ||
Comment 20•8 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #19)
> How about:
> if moz_build_app is not None and moz_build_app.startswith('../'):
> ?
I don't think that's the right fix. If moz_build_app is None adding the exceptions makes no sense in the first place. I'll try to understand what's going on here later.
Reporter | ||
Comment 21•8 years ago
|
||
Agreed, but that's a pre-existing problem that we shouldn't try to fix here. As the backout shows, it even occurs in an M-C-only environment.
Reporter | ||
Comment 22•8 years ago
|
||
This basically worked:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=955d0f2291eea74e97c90da9b13d751354c873b3&selectedJob=25610
Somehow the build still came out in orange, but the "startswith" error is gone. Now we see:
State Changed: unlock buildroot
program finished with exit code 2
elapsedTime=562.579136
TinderboxPrint: check<br/>6294/<em class="testfail">1</em>
Unknown Error: command finished with exit code: 2
It would still be good to land this and follow up any remaining problems elsewhere.
Reporter | ||
Comment 23•8 years ago
|
||
Attachment #8811211 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 24•8 years ago
|
||
I have another try run here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=beec811dd591ca836bf356b97442f3fc687ae93c
This time the error is a bit more conclusive:
TEST-UNEXPECTED-FAIL | /builds/slave/tb-try-c-cen-m64-d-00000000000/build/mozilla/python/mozbuild/mozbuild/test/frontend/test_emitter.py | TestEmitterBasic.test_allowed_xpcom_glue, line 1193: Lists differ: [(u'purplexpcom', u'extensions... != []
Reporter | ||
Comment 25•8 years ago
|
||
OK, let's see whether we can get rid of the error mentioned in comment #24 like this.
New try run on C-C try here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=21dd9e8a812e50e4f4efccd63e20149030add55a
Attachment #8811211 -
Attachment is obsolete: true
Attachment #8811211 -
Flags: review?(mh+mozilla)
Attachment #8811324 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 26•8 years ago
|
||
This fixes the tests on try. (Sorry jorgk for the overlap with your work, which I saw too late.)
Attachment #8811347 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Attachment #8811042 -
Attachment is obsolete: true
Flags: needinfo?(aleth)
Reporter | ||
Comment 27•8 years ago
|
||
Comment on attachment 8811324 [details] [diff] [review]
Slight change to fix "startswith" error and another issue (v2).
Aleth'es looks better ;-)
Attachment #8811324 -
Attachment is obsolete: true
Attachment #8811324 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 28•8 years ago
|
||
Refreshed.
Attachment #8811347 -
Attachment is obsolete: true
Attachment #8811347 -
Flags: review?(mh+mozilla)
Attachment #8811524 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 29•8 years ago
|
||
With this patch, Thunderbird builds fine:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a6e9cea2087441d4d4e2d9357ceafeaa000a7b30
Please approve asap.
Comment 30•8 years ago
|
||
Comment on attachment 8811524 [details] [diff] [review]
Use the correct path to the binary for comm builds in the XPCOM glue check + exceptions for Calendar and Instantbird
Review of attachment 8811524 [details] [diff] [review]:
-----------------------------------------------------------------
::: python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ +1180,5 @@
>
> for path, names in allowed.iteritems():
> + if (path.startswith('mailnews/') or
> + path.startswith('calendar/') or
> + path.startswith('extensions/')):
path.starswith(('mailnews/', 'calendar/', 'extensions/'))
Please be more specific for extensions/ because that adds mozilla-central's extensions directory to the whitelist as a side effect, and that's not desirable.
Attachment #8811524 -
Flags: review?(mh+mozilla)
Reporter | ||
Comment 31•8 years ago
|
||
Issues addressed.
Attachment #8811524 -
Attachment is obsolete: true
Attachment #8811620 -
Flags: review?(mh+mozilla)
Comment 32•8 years ago
|
||
Comment on attachment 8811620 [details] [diff] [review]
Use the correct path to the binary for comm builds in the XPCOM glue check + exceptions for Calendar and Instantbird
Review of attachment 8811620 [details] [diff] [review]:
-----------------------------------------------------------------
Please run though try before landing ;)
::: python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ +1178,5 @@
> for name, path in ALLOWED_XPCOM_GLUE:
> allowed[path].append(name)
>
> for path, names in allowed.iteritems():
> + if path.starswith(('mailnews/', 'calendar/', 'extensions/purple/purplexpcom')):
heh, I typoed in my comment, that's supposed to be startswith, not starswith.
Attachment #8811620 -
Flags: review?(mh+mozilla) → review+
Reporter | ||
Comment 33•8 years ago
|
||
Thanks, which try, just a build on Linux to save resources?
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 34•8 years ago
|
||
Attachment #8811623 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Attachment #8811620 -
Attachment is obsolete: true
Reporter | ||
Comment 35•8 years ago
|
||
Typo fixed.
Attachment #8811623 -
Attachment is obsolete: true
Attachment #8811623 -
Flags: review?(mh+mozilla)
Attachment #8811624 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8811620 -
Attachment is obsolete: false
Reporter | ||
Updated•8 years ago
|
Attachment #8811620 -
Attachment is obsolete: true
Reporter | ||
Comment 36•8 years ago
|
||
Oops. Two people working on the same thing here.
I changed:
if path.startswith(('mailnews/', 'calendar/', 'extensions/purple/purplexpcom')):
And Aleth changed(attachment 8811620 [details] [diff] [review]):
if (path.startswith('mailnews/') or
path.startswith('calendar/') or
path.startswith('extensions/purple/')):
So who's doing the try push?
Flags: needinfo?(aleth)
Reporter | ||
Comment 37•8 years ago
|
||
Oh, mine got r+ so let's go with that.
Repeating the question for Mike: which try, just a build on Linux to save resources?
Assignee | ||
Comment 38•8 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #37)
> Oh, mine got r+ so let's go with that.
>
> Repeating the question for Mike: which try, just a build on Linux to save
> resources?
Push it to m-c try.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(aleth)
Reporter | ||
Comment 39•8 years ago
|
||
That doesn't answer the question. Which parameters? I suggested |try: -b o -p linux64 -u none -t none|.
Reporter | ||
Comment 40•8 years ago
|
||
Patch already doesn't apply any more.
Comment 41•8 years ago
|
||
Yes, that would be enough.
Reporter | ||
Comment 43•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Attachment #8811624 -
Attachment is obsolete: true
Assignee | ||
Comment 45•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5f032bd4cf861c3058f244140399f7fb2fea663
Bug 1317674 - Use the correct path to the binary for comm builds in the XPCOM glue check + exceptions for Calendar and Instantbird. r=glandium
Comment 46•8 years ago
|
||
Pushed by aleth@instantbird.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5f032bd4cf8
Use the correct path to the binary for comm builds in the XPCOM glue check + exceptions for Calendar and Instantbird. r=glandium
Keywords: checkin-needed
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(philipp)
Comment 47•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
You need to log in
before you can comment on or make changes to this bug.
Description
•