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

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

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]
<<<<<<<

Comment 1

[Gregory Szorc [: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?

Comment 2

[Mike Hommey [:glandium]]

(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 3

[Gregory Szorc [:gps]]

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 :/

Comment 4

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a38692ea87ae

Comment 5

[Mark Banner (:standard8)]

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.

Comment 6

[Ed Morley [:emorley]]

Backed out for xpcshell failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=a38692ea87ae

https://hg.mozilla.org/integration/mozilla-inbound/rev/c05ccd690319

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 698651 [details] [diff] [review]
The fix

Bah, I suspect that'll be packaging.

Comment 8

[Ed Morley [:emorley]]

(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

Comment 9

[Mark Banner (:standard8)]

Attachment: The fix v2

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

Comment 10

[Mozilla RelEng Bot]

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/[e-mail]-3241be89648a

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 700236 [details] [diff] [review]
The fix v2

With the added manifests to the package-manifest.in files

Comment 12

[Gregory Szorc [: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.

Comment 13

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b8af725f322

Comment 14

[Mark Banner (:standard8)]

Also pushed to services-central as requested:

https://hg.mozilla.org/services/services-central/rev/208226541afc

Comment 15

[Richard Newman [:rnewman]]

Thanks, Mark.

Comment 16

[Ed Morley [:emorley]]

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

Comment 17

[Ed Morley [:emorley]]

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

Comment 18

[Ed Morley [:emorley]]

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

Comment 19

[Mark Banner (:standard8)]

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.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c7eca8cb696