Closed Bug 827303 Opened 7 years ago Closed 7 years ago

Firefox core and crypto services components assume services/sync is built

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21
Tracking Status
firefox20 --- fixed
firefox21 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

Attached patch The fix (obsolete) — Splinter Review
Since bug 824330 landed, we've been seeing xpcshell-tests failures on Thunderbird builds.

The problem is that Thunderbird doesn't enable sync, but services/common and services/cypto both rely on items that are in services/sync.

The simple solution to this is to re-arrange the build items so that we take the required parts out of the services/sync directory. There's also one preference, but that can easily be defaulted.

Example test failures:

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/modules/tests/xpcshell/test_sqlite.js | test failed (with xpcshell return code: 3), see following log:
>>>>>>>
resource://gre/modules/Sqlite.jsm:17: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.import]
<<<<<<<
Attachment #698651 - Flags: review?(gps)
Perhaps we should remove services/crypto from the default dir set.

What part of services/common relies on services/sync? Just the resource:// registration?
(In reply to Gregory Szorc [:gps] from comment #1)
> What part of services/common relies on services/sync? Just the resource://
> registration?

That's probably. I'm inclined to think each should have its own.
Comment on attachment 698651 [details] [diff] [review]
The fix

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

This looks good to me.

Please nominate for Aurora uplift since trees are broken on 20.

As an aside, Splinter really doesn't handle copies that well :/
Attachment #698651 - Flags: review?(gps) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a38692ea87ae
Flags: in-testsuite+
Target Milestone: --- → mozilla21
Comment on attachment 698651 [details] [diff] [review]
The fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 824330
User impact if declined: None
Testing completed (on m-c, etc.): Landed in mozilla-inbound
Risk to taking this patch (and alternatives if risky): Extremely low - build-config only
String or UUID changes made by this patch: None

Patch needed to fix issues causing permanent orange for xpcshell-tests on apps that don't currently build sync, e.g. Thunderbird.
Attachment #698651 - Flags: approval-mozilla-aurora?
Comment on attachment 698651 [details] [diff] [review]
The fix

Bah, I suspect that'll be packaging.
Attachment #698651 - Flags: approval-mozilla-aurora?
(In reply to Ed Morley [:edmorley UTC+0] from comment #6)
> Backed out for xpcshell failures:

Also mochitest-browser-chrome timeouts:
https://tbpl.mozilla.org/php/getParsedLog.php?id=18652000&tree=Mozilla-Inbound
Attached patch The fix v2Splinter Review
Fixes the manifest issues. Note that the non-FF manifests also try to include WeaveCrypto.[js,manifest] but that's a separate bug because they've moved, and bug 610307 failed to update them.

I've pushed this to try to make sure I've fixed the bustage:

https://tbpl.mozilla.org/?tree=Try&rev=3241be89648a
Attachment #698651 - Attachment is obsolete: true
Try run for 3241be89648a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3241be89648a
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-3241be89648a
Comment on attachment 700236 [details] [diff] [review]
The fix v2

With the added manifests to the package-manifest.in files
Attachment #700236 - Flags: review?(gps)
Comment on attachment 700236 [details] [diff] [review]
The fix v2

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

Cleared to land assuming Try passes. If you do land, please also land on services-central (if it applies cleanly) because I'm currently touching some related files and want to deal with the merge sooner. If it doesn't apply cleanly, just comment and I'll do it.
Attachment #700236 - Flags: review?(gps) → review+
Status: NEW → ASSIGNED
Also pushed to services-central as requested:

https://hg.mozilla.org/services/services-central/rev/208226541afc
Thanks, Mark.
Whiteboard: [fixed in services][qa-]
https://hg.mozilla.org/mozilla-central/rev/6b8af725f322
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][qa-] → [qa-]
Comment on attachment 700236 [details] [diff] [review]
The fix v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 824330
User impact if declined: None
Testing completed (on m-c, etc.): Landed in mozilla-inbound
Risk to taking this patch (and alternatives if risky): Extremely low - build-config only
String or UUID changes made by this patch: None

Patch needed to fix issues causing permanent orange for xpcshell-tests on apps that don't currently build sync, e.g. Thunderbird.
Attachment #700236 - Flags: approval-mozilla-aurora?
Attachment #700236 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.