Closed Bug 586867 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

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

People

(Reporter: khuey, Assigned: glandium)

References

Details

(Whiteboard: [qa-])

Attachments

(3 files, 3 obsolete files)

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) — 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) — 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)
Duplicate of this bug: 589886
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.
Duplicate of this bug: 590411
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.
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: 9 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: 9 years ago9 years ago
Resolution: --- → FIXED
fyi, this broke Fennec builds, as comment #4 said it would....
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.