Closed Bug 586867 Opened 14 years ago Closed 14 years ago

Weave should be built from browser/ and should work in a firefox on xulrunner setup

Categories

(Firefox :: Sync, defect)

Product:
Firefox
Component:
Sync
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: khuey, Assigned: glandium)

References

Details

(Whiteboard: [qa-])

Attachments

(3 files, 3 obsolete files)

Part to have resource://services-sync work with firefox-on-xulrunner with services-sync on the application side
(deleted), patch
philikon
: review+
Details | Diff | Splinter Review
Define sync services as app tiers, not platform
(deleted), patch
khuey
: review+
Details | Diff | Splinter Review
mobile-browser patch
(deleted), patch
Details | Diff | Splinter Review
All of Weave lives in services/ and browser/. We should move Weave out of tier-toolkit and remove the configure bits.
Attached patch POC (obsolete) (deleted) — — Splinter Review
Probably should move browser/browservars.mk to browser/config/config.mk Thoughts appreciated.
Attachment #465545 - Flags: review?(ted.mielczarek)
Attached patch POC (obsolete) (deleted) — — Splinter Review
Add remove helps
Attachment #465545 - Attachment is obsolete: true
Attachment #465547 - Flags: review?(ted.mielczarek)
Attachment #465545 - Flags: review?(ted.mielczarek)
I don't really understand the purpose of this patch. Can you elaborate? Having to sprinkle that include browservars.mk all around seems pretty crappy.
This will need a similar patch for the mobile-browser repo.
The basic idea is that things that only affect the browser app shouldn't be configure variables.
Why not simply remove all these ifdefs ? (also, you could use tier_app_dirs in browser/build.mk instead of DIRS in browser/Makefile.in)
Note that if services-sync is not made part of xulrunner, then resource://services-sync/some-file urls don't work on firefox-on-top-of-xulrunner setups.
Comment on attachment 468445 [details] [diff] [review] Part to have resource://services-sync work with firefox-on-xulrunner with services-sync on the application side PhiliKON, can you review this? I think we should just WONTFIX this bug because sync is on for good now, but I'm leaving it open to deal with this patch.
Attachment #468445 - Flags: review?(philipp)
Comment on attachment 468445 [details] [diff] [review] Part to have resource://services-sync work with firefox-on-xulrunner with services-sync on the application side It doesn't seem right to me to do this without changing the build setup over from toolkit to browser as well.
Attached patch Define sync services as app tiers, not platform (obsolete) (deleted) — — Splinter Review
This is the other patch I apply on debian to have sync build and work properly in FF-on-xulrunner setup.
Attachment #478775 - Flags: review?(philipp)
Comment on attachment 478775 [details] [diff] [review] Define sync services as app tiers, not platform Note that fennec needs the same thing.
Attachment #478775 - Flags: review?(philipp) → review+
Comment on attachment 468445 [details] [diff] [review] Part to have resource://services-sync work with firefox-on-xulrunner with services-sync on the application side Note that this needs to go to fx-sync.
Attachment #468445 - Flags: review?(philipp) → review+
Requesting to land this on m-c because Linux distros using firefox-on-xulrunner setups need both patches to build and work properly (at least SuSE and Debian, not sure for others).
blocking2.0: --- → ?
Changing the summary to reflect what this bug became.
Summary: Enabling/disabling Weave should not require a clobber/reconfigure → Weave should be built from browser/ and should work in a firefox on xulrunner setup
So part of that landed in m-c now but what about the build changes?
blocking2.0: ? → betaN+
Assignee: nobody → mh+mozilla
m-c part landed http://hg.mozilla.org/mozilla-central/rev/111e055a49dc
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Backed-out http://hg.mozilla.org/mozilla-central/rev/f64feb757627
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
So, as we discussed on IRC, the problem with the existing patch is that anything built after browser/app doesn't get packaged on Mac. The fix is to move services/sync before browser and add some big scary comments to the next person to wander down this road.
Take 2.
Attachment #478775 - Attachment is obsolete: true
Attachment #483448 - Flags: review?(khuey)
Comment on attachment 483448 [details] [diff] [review] Define sync services as app tiers, not platform I'd like the comment to say why ("they won't get packaged properly on mac"), but other than that, it's good.
Attachment #483448 - Flags: review?(khuey) → review+
http://hg.mozilla.org/mozilla-central/rev/36aa9c19420e
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
fyi, this broke Fennec builds, as comment #4 said it would....
Attached patch mobile-browser patch (deleted) — — Splinter Review
Attachment #484112 - Flags: review?(mark.finkle)
Comment on attachment 484112 [details] [diff] [review] mobile-browser patch Nevermind, I just found bug 605100.
Attachment #484112 - Flags: review?(mark.finkle)
Whiteboard: [qa-]
Component: Firefox Sync: Build → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: