Closed
Bug 745065
Opened 13 years ago
Closed 13 years ago
Remove apps sync engine
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: anant, Assigned: anant)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
When the new AITC client (bug 744257) lands, the apps sync engine in the tree may removed as it duplicates functionality provided by AITC.
Comment 1•13 years ago
|
||
Is there any need to wait?
Assignee | ||
Comment 2•13 years ago
|
||
No reason to wait, in fact Fabrice also suggests we remove it now so we can check in the changes we need to DOMApplicationRegistry.
I'll attach a patch shortly.
Assignee | ||
Comment 3•13 years ago
|
||
This patch removes the app sync engine, which will be replaced by the new AITC client.
Comment 4•13 years ago
|
||
There are a bunch of removed-files.in files scattered in the tree. Do we need to update those as well? (I'm really not sure what they are used for.)
Comment 5•13 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #4)
> There are a bunch of removed-files.in files scattered in the tree. Do we
> need to update those as well? (I'm really not sure what they are used for.)
# Removed-files.in is processed at build time to create a list of files that
# should be removed during an application update.
So yes, files that were in a release at some point but should be removed in an update because they're no longer shipped, must be added to removed-files.in.
Assignee | ||
Comment 6•13 years ago
|
||
Ah, you're right; they are used to remove files on application updates, so we'll have to update that to make sure we don't leave apps.js lying around in modules/services-sync/engines/ on a Firefox update. I'll attach a patch to update removed-files.in.
Assignee | ||
Comment 7•13 years ago
|
||
Looking at removed-files.in, all of the sync engine files are already listed (line 1047). The blame for that line leads to bug 556644, which seems to indicate that since we switched to Omnijar it is unnecessary to add lines to removed-files.in.
Are there platforms where we still don't use Omnijar? If so we'd need to add an entry, but only if apps.js is also present on those platforms.
Assignee | ||
Comment 8•13 years ago
|
||
Add apps.js to removed-files.in. I've put it in two places (inside an #ifdef MOZ_OMNIJAR and outside) to be safe and remove the file from all platforms, whether or not we had Omnijar support on them.
Attachment #614681 -
Attachment is obsolete: true
Attachment #614681 -
Flags: review?(rnewman)
Attachment #614686 -
Flags: review?(rnewman)
Comment 9•13 years ago
|
||
Comment on attachment 614686 [details] [diff] [review]
Remove App Sync Engine - v1.1
Review of attachment 614686 [details] [diff] [review]:
-----------------------------------------------------------------
This looks fine. Please make sure that TPS and xpcshell tests pass.
You perhaps also want to revert some of the changes to webapps.jsm:
https://bug706545.bugzilla.mozilla.org/attachment.cgi?id=579890
Attachment #614686 -
Flags: review?(rnewman) → review+
Assignee | ||
Comment 10•13 years ago
|
||
Thanks! The changes to Webapps.jsm will be done in bug 745069. Pushing to try to check tests.
Whiteboard: [autoland-try]
Updated•13 years ago
|
Whiteboard: [autoland-try] → [autoland-in-queue]
Comment 11•13 years ago
|
||
Autoland Patchset:
Patches: 614686
Branch: mozilla-central => try
Destination: http://hg.mozilla.org/try/pushloghtml?changeset=b5099b5b4a39
Try run started, revision b5099b5b4a39. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=b5099b5b4a39
Comment 12•13 years ago
|
||
Try run for b5099b5b4a39 is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=b5099b5b4a39
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/autolanduser@mozilla.com-b5099b5b4a39
Updated•13 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Comment 13•13 years ago
|
||
Pushed: https://hg.mozilla.org/services/services-central/rev/e24ad9cee741
Forgot the r= in the commit message! Clearly, it's been a while since I did a hg push...
Comment 14•13 years ago
|
||
We add this whiteboard for s-c pushes to help track the tree.
Whiteboard: [fixed in services]
Assignee | ||
Comment 15•13 years ago
|
||
https://tbpl.mozilla.org/?tree=Services-Central&rev=e24ad9cee741 shows a few oranges for this check-in. What is the general procedure when this occurs and how do we investigate which failures are real and which are intermittent?
Comment 16•13 years ago
|
||
(In reply to Anant Narayanan [:anant] from comment #15)
> https://tbpl.mozilla.org/?tree=Services-Central&rev=e24ad9cee741 shows a few
> oranges for this check-in. What is the general procedure when this occurs
> and how do we investigate which failures are real and which are intermittent?
You star them! Take a look at tbpl now, as I do so.
Comment 17•13 years ago
|
||
In general, if we break something like this it'll be in xpcshell-tests (the "X" in tbpl), and it'll usually be on all platforms. You're just seeing intermittent orange here.
Assignee | ||
Comment 18•13 years ago
|
||
Nice, thanks!
TPS tests are also run for check-ins to services-central, right? Where can I find out whether they succeeded?
Comment 19•13 years ago
|
||
The last TPS report I received was on the 10th, so it doesn't appear to have run for your push.
Will ping jgriffin.
You can run them yourself:
https://developer.mozilla.org/en/TPS
or you can ask jgriffin to add you to the mailing list if you want to keep an eye on them over time.
Assignee | ||
Comment 20•13 years ago
|
||
We're holding off on removing the sync engine as the decision to use AITC exclusively on all Gecko platforms is being revisited. In the meantime, the plan is to have both the classic sync engine for apps as well as AITC, behind prefs so only one is active at any given time.
Once clear consensus has been reached, one of these will likely go away.
https://hg.mozilla.org/services/services-central/rev/c019b4bbe9ff
Whiteboard: [fixed in services]
Comment 21•13 years ago
|
||
(In reply to Anant Narayanan [:anant] from comment #20)
> We're holding off on removing the sync engine as the decision to use AITC
> exclusively on all Gecko platforms is being revisited.
Does that mean that Bug 705910 needs to be reopened?
Assignee | ||
Comment 22•13 years ago
|
||
The code in bug 705910 did not end up landing in mozilla-central, while the code from bug 706545 did. As for a re-review of the latter, it may or may not be required, but I suggest that we wait for consensus on which app sync solution is the right one going forward before spending any time on it.
Comment 23•13 years ago
|
||
services train in QA today. gps called out this bug, but it's actually a no action item, correct?
Whiteboard: [qa-]
Comment 24•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e24ad9cee741
https://hg.mozilla.org/mozilla-central/rev/c019b4bbe9ff
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Target Milestone: --- → mozilla15
Updated•6 years ago
|
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.
Description
•