Closed Bug 436055 Opened 12 years ago Closed 11 years ago

Weave (for Fennec)

Categories

(Firefox for Android Graveyard :: General, enhancement, P1)

Other
Maemo
enhancement

Tracking

(Not tracked)

VERIFIED INVALID
fennec1.0a2

People

(Reporter: christian.bugzilla, Assigned: db48x)

Details

Attachments

(1 file, 3 obsolete files)

 
Assignee: nobody → jsullivan
Flags: blocking-fennec1.0+
Priority: -- → P2
Priority: P2 → P1
OS: Mac OS X → Linux (embedded)
Hardware: PC → Other
Status: NEW → ASSIGNED
Assignee: jsullivan → dolske
Status: ASSIGNED → NEW
Target Milestone: Fennec M5 → Fennec M6
Need to talk with Jay/Chris/thunder some more about what this entails, exactly.

I have successfully built and used Weave [on a straight Minefield (for N810)], but there's significant work to do still:
  * Scope features/requirements/usecases/etc
  * Preferences UI for Fennec?
  * Performance issues (CPU+memory) with Weave sync algorithm

M6 is probably at risk for this. Can probably get something crudely bolted on, but too many unknowns for a firmer schedule.
Summary: Weave → Weave (for Fennec)
Target Milestone: Fennec M6 → Fennec A1
Target Milestone: Fennec A1 → Fennec A2
Unassigning for now, I think any Fennec work is blocked on the outcome of the Weave redesign stuff labs is doing.
Assignee: dolske → nobody
Attached patch 436055-1.diff (obsolete) — Splinter Review
This patch builds Weave if the source can be found in mozilla/mobile/weave. At present, this builds the Firefox version of the Weave plugin, as there needs to be some restructuring before it can build the Fennec version.
Assignee: nobody → db48x
Status: NEW → ASSIGNED
Attachment #349411 - Flags: review?(blassey)
Comment on attachment 349411 [details] [diff] [review]
436055-1.diff

tonikitoo volunteered you :)
Comment on attachment 349411 [details] [diff] [review]
436055-1.diff

>diff -r 858dc712ea4b -r 4cbf9717388e Makefile.in
>--- a/Makefile.in	Fri Nov 14 16:57:27 2008 -0500
>+++ b/Makefile.in	Fri Nov 21 07:04:25 2008 -0600
>@@ -44,4 +44,10 @@
> 
> DIRS       = chrome locales components app
> 
>+libs::
>+	if [ -d $(srcdir)/weave ]; then                                            \
>+	  $(MAKE) -C $(srcdir)/weave xpi sdkdir=$(DIST) &&                         \
>+	  cp $(srcdir)/weave/weave-`uname -s`.xpi $(DIST)/bin/extensions;          \
>+	fi
>+
> include $(topsrcdir)/config/rules.mk


have you tried this using a xulrunner sdk? I think you should be using LIBXUL_DIST instead of DIST. Also, why aren't we using "--enable-extensions=weave" in our mozconfig for this?
(In reply to comment #5)
> have you tried this using a xulrunner sdk? I think you should be using
> LIBXUL_DIST instead of DIST. Also, why aren't we using

That would probably be more correct, though I wasn't sure what the best way to get to it would be. Is that what LIBXUL_DIST is?

> "--enable-extensions=weave" in our mozconfig for this?

Because the weave build isn't anything like the idiomatic mozilla build. It doesn't have the same targets, etc. I didn't really want to modify the weave makefiles much since most people will be building it standalone.
(In reply to comment #6)
> (In reply to comment #5)
> > have you tried this using a xulrunner sdk? I think you should be using
> > LIBXUL_DIST instead of DIST. Also, why aren't we using
> 
> That would probably be more correct, though I wasn't sure what the best way to
> get to it would be. Is that what LIBXUL_DIST is?

Yeah, LIBXUL_DIST points to the xulrunner SDK if you're using one. If not, it points to the normal DIST folder.
why wouldn't we build weave like a normal extension (like we do for firefox?)
Attached patch 436055-2.diff (obsolete) — Splinter Review
Blassey: could you clarify your question? What would you prefer to do?
Attachment #349411 - Attachment is obsolete: true
Attachment #349545 - Flags: review?(blassey)
Attachment #349411 - Flags: review?(blassey)
(In reply to comment #9)
> Created an attachment (id=349545) [details]
> 436055-2.diff

Thin only issue I have with the current patch is that it lands in our topmost Makefile.in file. I think we'd be better off creating an "extensions" folder with a Makefile.in that handles this.
weave shouldn't be built as part of fennec.  Instead it should be built as a separate extension, or standalone as you referred to it.  This bug should really be about modifying weave to build on arm and hook into fennec's UI rather than modifying fennec to build weave.
(In reply to comment #10)
> (In reply to comment #9)
> > Created an attachment (id=349545) [details] [details]
> > 436055-2.diff
> 
> Thin only issue I have with the current patch is that it lands in our topmost
> Makefile.in file. I think we'd be better off creating an "extensions" folder
> with a Makefile.in that handles this.

That's fair enough, I'll go ahead and do it.

(In reply to comment #11)
> weave shouldn't be built as part of fennec.  Instead it should be built as a
> separate extension, or standalone as you referred to it.  This bug should
> really be about modifying weave to build on arm and hook into fennec's UI
> rather than modifying fennec to build weave.

If Weave is to be included in Fennec releases, shouldn't we make it as simple as possible to build both together? This eliminates a step.

Also, I don't intend that this patch would fix this bug, only that it's the Fennec half of the equation.
Attached patch 436055-3.diff (obsolete) — Splinter Review
Attachment #349545 - Attachment is obsolete: true
Attachment #349633 - Flags: review?(blassey)
Attachment #349545 - Flags: review?(blassey)
I agree with Brad that we shouldn't land this right now changing the Fennec build system, lets get Weave as an extension working first.
Fair enough. The patch will keep until it's needed.
No longer blocks: 454642
Attached patch 436055-4.diffSplinter Review
Weave works on arm now, and can even connect to the weave server and sync bookmarks (sorta), so let's go ahead and get this checked in.
Attachment #349633 - Attachment is obsolete: true
Attachment #353330 - Flags: review?(bugmail)
Attachment #349633 - Flags: review?(bugmail)
why do we want to build/copy weave as part of fennec?
Attachment #353330 - Flags: review?(bugmail) → review-
Comment on attachment 353330 [details] [diff] [review]
436055-4.diff

as of now, weave is an extension and shouldn't be part of the fennec build
If Weave is to be included with Fennec by default then this seems like the easiest way to get both built at once and put into the same package.
tracking-fennec: --- → 1.0+
just going to close this bug out -- weave is being worked on in a separate place
Status: ASSIGNED → RESOLVED
tracking-fennec: 1.0+ → ---
Closed: 11 years ago
Resolution: --- → INVALID
verified status of this bug.  We have weave for Fennec via labs, and it is a WIP but making great progress.
Status: RESOLVED → VERIFIED
sorry for bug spam.
Many of the bugs which are marked invalid, I see comments telling it occurred in one version or other. But later it was fixed due to 1) by backingout the patch which made regression or 2) by fixing some other bug.
So if we can identify the bug/patch/reason then we should state that and mark those as FIXED. If not mark as WORKSFORME in that case.
You need to log in before you can comment on or make changes to this bug.