436055 - Weave (for Fennec)

Status: VERIFIED INVALID

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

Description

[Christian Sejersen]

Comment 1

[Justin Dolske [:Dolske]]

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.

Comment 2

[Justin Dolske [:Dolske]]

Unassigning for now, I think any Fennec work is blocked on the outcome of the Weave redesign stuff labs is doing.

Comment 3

[Daniel Brooks [:db48x]]

Attachment: 436055-1.diff

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.

Comment 4

[Daniel Brooks [:db48x]]

Comment on attachment 349411 [details] [diff] [review]
436055-1.diff

tonikitoo volunteered you :)

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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?

Comment 6

[Daniel Brooks [:db48x]]

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

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

why wouldn't we build weave like a normal extension (like we do for firefox?)

Comment 9

[Daniel Brooks [:db48x]]

Attachment: 436055-2.diff

Blassey: could you clarify your question? What would you prefer to do?

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

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.

Comment 12

[Daniel Brooks [:db48x]]

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

Comment 13

[Daniel Brooks [:db48x]]

Attachment: 436055-3.diff

Comment 14

[Stuart Parmenter]

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.

Comment 15

[Daniel Brooks [:db48x]]

Fair enough. The patch will keep until it's needed.

Comment 16

[Daniel Brooks [:db48x]]

Attachment: 436055-4.diff

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.

Comment 17

[Stuart Parmenter]

why do we want to build/copy weave as part of fennec?

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

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

Comment 19

[Daniel Brooks [:db48x]]

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.

Comment 20

[Stuart Parmenter]

just going to close this bug out -- weave is being worked on in a separate place

Comment 21

[Joel Maher ( :jmaher ) (UTC -8)]

verified status of this bug.  We have weave for Fennec via labs, and it is a WIP but making great progress.

Comment 22

[Biju]

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.