Last Comment Bug 571902 - Land sync and crypto components on trunk
: Land sync and crypto components on trunk
Status: RESOLVED FIXED
:
Product: Cloud Services
Classification: Client Software
Component: Firefox Sync: Backend (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal with 1 vote (vote)
: 1.4
Assigned To: Philipp von Weitershausen [:philikon]
:
Mentors:
http://hg.mozilla.org/services/fx-syn...
Depends on: 569355 570636 572436 572970 573015 573108 573217 573668 573679 573691 573740 573842 573870
Blocks: 571896 574380 576970 579175
  Show dependency treegraph
 
Reported: 2010-06-14 09:24 PDT by Mike Connor [:mconnor]
Modified: 2010-09-07 09:53 PDT (History)
16 users (show)
edilee: blocking‑fx‑sync1.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
hg convert filemap (503 bytes, text/plain)
2010-06-17 18:54 PDT, Philipp von Weitershausen [:philikon]
no flags Details
Hook Sync into the build system (18.49 KB, patch)
2010-06-17 20:13 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
Quick fix: make constants.js importable (1.14 KB, patch)
2010-06-17 20:48 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
Quick fix: provide resource://services-sync (1.39 KB, patch)
2010-06-17 20:50 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
hg convert filemap (v2) (511 bytes, text/plain)
2010-06-18 10:18 PDT, Philipp von Weitershausen [:philikon]
no flags Details
[LANDED] Map resource://services-sync to resource://gre/modules/... (1.38 KB, patch)
2010-06-18 13:28 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
hg convert filemap (v3) (537 bytes, text/plain)
2010-06-18 15:33 PDT, Philipp von Weitershausen [:philikon]
l10n: review-
Details
mozilla-central: Makefiles and build helpers for 'services' (19.22 KB, patch)
2010-06-18 15:36 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (1.64 KB, patch)
2010-06-18 15:37 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
fx-sync: handle absence of debug preference gracefully (770 bytes, patch)
2010-06-18 16:08 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
[LANDED] fx-sync: harmonize locale directory structure with m-c conventions (1.47 KB, patch)
2010-06-18 17:03 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
hg convert filemap (v4) (482 bytes, text/plain)
2010-06-18 17:29 PDT, Philipp von Weitershausen [:philikon]
no flags Details
mozilla-central: Makefiles and build helpers for 'services' (v2) (19.22 KB, patch)
2010-06-18 18:04 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (v2) (2.04 KB, patch)
2010-06-18 18:05 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (v3) (2.66 KB, patch)
2010-06-19 09:37 PDT, Philipp von Weitershausen [:philikon]
mconnor: review+
Details | Diff | Splinter Review
fx-sync super diff (614.06 KB, patch)
2010-06-19 10:34 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
hg convert filemap (v5) (2.99 KB, text/plain)
2010-06-21 02:37 PDT, Ed Lee :Mardak
no flags Details
f-sync super diff (651.96 KB, patch)
2010-06-21 02:44 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
fx-sync super diff (651.96 KB, patch)
2010-06-21 09:56 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
mozilla-central: Makefiles and build helpers for 'services' (v3) (13.46 KB, patch)
2010-06-21 17:06 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (v4) (6.57 KB, patch)
2010-06-21 17:09 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
hg convert filemap (v6) (2.95 KB, text/plain)
2010-06-22 01:09 PDT, Ed Lee :Mardak
no flags Details
mozilla-central: hook 'services' into the build system (v5) (6.88 KB, patch)
2010-06-22 01:11 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review
mozilla-central: Makefiles and build helpers for 'services' (v4) (13.38 KB, patch)
2010-06-22 01:12 PDT, Ed Lee :Mardak
ted: review+
Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (v6) (7.44 KB, patch)
2010-06-22 18:31 PDT, Ed Lee :Mardak
ted: review+
Details | Diff | Splinter Review
mozilla-central: hardcode constants (1.17 KB, patch)
2010-06-23 08:16 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
mozilla-central: hook 'services' into the build system (v7) (6.68 KB, patch)
2010-06-23 15:11 PDT, Ed Lee :Mardak
no flags Details | Diff | Splinter Review

Description Mike Connor [:mconnor] 2010-06-14 09:24:15 PDT
These should land as-is, with the appropriate build glue.

The intent is for this to land as a standalone component, available at build time.
Comment 1 Ed Lee :Mardak 2010-06-16 19:39:12 PDT
Sounds like we want services to be a new top-level directory in mozilla-central. Under that we'll have 2 directories for crypto and sync.

The Makefile under services should probably just have those two as PARALLEL_DIRS that are conditionally added ifdef SERVICES_SYNC ?

SERVICES_SYNC would probably get configure-d based on --enable-services-sync

The crypto directory would have public and src with the Makefile DIRS = public src

crypto/public has IWeaveCrypto.idl with XPIDLSRCS = to build and MODULE = crypto

crypto/src has WeaveCrypto.js with EXTRA_COMPONENTS = to have the component located correctly and perhaps MODULE = crypto_s or _src?

sync/src doesn't need to compile anything but needs to declare the EXTRA_JS_MODULES and EXTRA_COMPONENTS = Weave.js (for about: registration and perhaps resource: mapping https://developer.mozilla.org/en/Using_JavaScript_code_modules#Programmatically_adding_aliases)

If EXTRA_JS_MODULES doesn't allow subdirectory, we might need to add to config/rules.mk for a special EXTRA_SYNC_MODULES ?

sync/test would point to XPCSHELL_TESTS = unit with the actual tests living under sync/test/unit. I believe head_ files will automatically get loaded.
Comment 2 Ed Lee :Mardak 2010-06-16 19:42:24 PDT
Not sure what's the magic for getting the new services directory picked up.

For hg convert of fx-sync repository to mozilla-central, we might eventually have a tracemonkey-like repository for service-specific landings that later merge to mozilla-central. But to start, we'll need to hg merge mozilla-central and the hg converted fx-sync.
Comment 3 Ed Lee :Mardak 2010-06-17 13:40:45 PDT
Here's some the build changes from adding the ipc top-level directory and --disable-ipc flags:

http://hg.mozilla.org/mozilla-central/rev/fc6ed914e44e

I'm not sure if we want --disable-services-sync or --enable-services-sync
Comment 4 Philipp von Weitershausen [:philikon] 2010-06-17 13:55:34 PDT
I'm going with --enable-services-sync so far.
Comment 5 Philipp von Weitershausen [:philikon] 2010-06-17 18:54:08 PDT
Created attachment 452159 [details]
hg convert filemap

This is the filemap I use to get fx-sync into mozilla-central. Here's how to use it:

1. hg clone .../fx-sync
2. hg clone .../mozilla-central
3. hg convert --filemap attachment fx-sync sync-for-mc
4. cd mozilla-central
5. hg pull -f ../sync-for-mc
6. hg merge; hg commit

I'm guessing steps 3-6 are repeatable whenever fx-sync has updates. The 'hg pull' step in step 5 should recognize commits we already have merged.
Comment 6 Philipp von Weitershausen [:philikon] 2010-06-17 20:13:33 PDT
Created attachment 452165 [details] [diff] [review]
Hook Sync into the build system

This hooks Sync into the build system underneath the 'services' top-level directory. Apply this patch on a repository that has been merged with an hg converted fx-sync repo.
Comment 7 Philipp von Weitershausen [:philikon] 2010-06-17 20:48:29 PDT
Created attachment 452167 [details] [diff] [review]
Quick fix: make constants.js importable

Make constants.js importable. This is a quick fix to get things going on trunk for now, we should really be applying this to the fx-sync repo.
Comment 8 Philipp von Weitershausen [:philikon] 2010-06-17 20:50:13 PDT
Created attachment 452168 [details] [diff] [review]
Quick fix: provide resource://services-sync

Alias resource://gre/modules/services-sync to resource://services-sync. This too should probably land in fx-sync.
Comment 9 Philipp von Weitershausen [:philikon] 2010-06-17 21:32:56 PDT
Tests (make xpcshell-tests) fail with 

  OBJDIR/_tests/xpcshell/test_services_sync/unit/head_http_server.js:2: ReferenceError: Cu is not defined

That's because the trunk xpcshell test harness (testing/xpcshell/remoteexpcshelltests.py) looks for head_*.js and thus misses head.js which happens to be where Cu et.al. are defined. Renaming head.js to head_first.js or similar should fix the problem.
Comment 10 Philipp von Weitershausen [:philikon] 2010-06-17 21:57:45 PDT
After renaming head.js to head_first.js, tests still fail:

  uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: OBJDIR/_tests/xpcshell/test_services_sync/unit/head_first.js :: <TOP_LEVEL> :: line 9"  data: no]

The relevant line in head_first.js (head.js) is:

  let appinfo = Cc["@mozilla.org/xre/app-info;1"].
                getService(Ci.nsIXULAppInfo);
Comment 11 Philipp von Weitershausen [:philikon] 2010-06-18 09:33:55 PDT
Comment on attachment 452167 [details] [diff] [review]
Quick fix: make constants.js importable

With bug 572970 landed this quick fix is no longer needed.
Comment 12 Philipp von Weitershausen [:philikon] 2010-06-18 10:18:42 PDT
Created attachment 452286 [details]
hg convert filemap (v2)

Update hg convert filemap after bug 572970 was landed: no longer exclude prefs.js.in. Do exclude services/sync/tests/unit/Makefile, though.
Comment 13 Philipp von Weitershausen [:philikon] 2010-06-18 13:28:15 PDT
Created attachment 452326 [details] [diff] [review]
[LANDED] Map resource://services-sync to resource://gre/modules/...

This is identical to the previous patch except that
a) it's against fx-sync (where it should end up)
b) it doesn't do anything if resource://services-sync already exists (add-on case)
Comment 14 Ed Lee :Mardak 2010-06-18 14:12:00 PDT
http://hg.mozilla.org/services/fx-sync/rev/03a87dd91969
If resource://services-sync isn't defined yet, alias it to resource://gre/modules/services-sync.
Comment 15 Philipp von Weitershausen [:philikon] 2010-06-18 15:33:36 PDT
Created attachment 452366 [details]
hg convert filemap (v3)

Update hg convert filemap to include locales directory (in the convention as used by mozilla-central).
Comment 16 Philipp von Weitershausen [:philikon] 2010-06-18 15:36:14 PDT
Created attachment 452367 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services'

Splitting previous patch into two: this now provides the necessary build glue to make everything below the 'services' directory. New: support for 'locale' directory.
Comment 17 Philipp von Weitershausen [:philikon] 2010-06-18 15:37:24 PDT
Created attachment 452368 [details] [diff] [review]
mozilla-central: hook 'services' into the build system

Second part of the previously combined patch: hook the 'services' directory into the global mozilla-central build system.
Comment 18 Philipp von Weitershausen [:philikon] 2010-06-18 16:08:32 PDT
Created attachment 452374 [details] [diff] [review]
fx-sync: handle absence of debug preference gracefully

Handle absence of debug preference gracefully in WeaveCrypto.js.
Comment 19 Robert Kaiser 2010-06-18 16:12:45 PDT
(In reply to comment #16)
> Created an attachment (id=452367) [details]
> mozilla-central: Makefiles and build helpers for 'services'
> 
> Splitting previous patch into two: this now provides the necessary build glue
> to make everything below the 'services' directory. New: support for 'locale'
> directory.

This really should be locales/ and probably even live at the top of services/ - please check with Axel on that before landing in mozilla-central, so that the L10n infrastructure fits what we actually need to have things working well.
Comment 20 Philipp von Weitershausen [:philikon] 2010-06-18 16:18:38 PDT
(In reply to comment #19)
> This really should be locales/

Both singular and plural spellings seem to be used equally throughout mozilla-central :/. We were using 'locale' in the add-on so I stuck with it.
Comment 21 Axel Hecht [:Pike] 2010-06-18 16:19:49 PDT
Comment on attachment 452366 [details]
hg convert filemap (v3)

This won't work with the l10n build systems.

Whether this is services/locales/en-US/(sync) or services/sync/locales/en-US doesn't matter to how l10n is set up, fwiw.
Comment 22 Philipp von Weitershausen [:philikon] 2010-06-18 16:23:18 PDT
(In reply to comment #21)
> Whether this is services/locales/en-US/(sync) or services/sync/locales/en-US
> doesn't matter to how l10n is set up, fwiw.

So s/locale/locales/ would do the job?
Comment 23 Axel Hecht [:Pike] 2010-06-18 16:26:15 PDT
For the l10n build systems to have a chance to pick this up, the new module will need to make it into the l10n.ini files. From the bug so far, I'm not sure if this is supposed to be part of toolkit or firefox, the "what's supposed to be localized" pieces go into different files.

But s/locale/locales/ is a good start, yes.

Side note, I guess the configure options should be run by ted.
Comment 24 Philipp von Weitershausen [:philikon] 2010-06-18 16:32:41 PDT
(In reply to comment #23)
> For the l10n build systems to have a chance to pick this up, the new module
> will need to make it into the l10n.ini files. From the bug so far, I'm not sure
> if this is supposed to be part of toolkit or firefox, the "what's supposed to
> be localized" pieces go into different files.

It's really neither (which is why we're landing it a new top level dir), but it's more toolkit than browser, so we'll be hooking into toolkit/locales/l10n.ini. Thanks!
Comment 25 Robert Kaiser 2010-06-18 16:37:50 PDT
(In reply to comment #24)
> It's really neither (which is why we're landing it a new top level dir)

For this case, "toolkit" means "the platform", and toolkit's l10n.ini is being included by all Mozilla-based applications - I guess that's what we want for this part.
Comment 26 Mike Connor [:mconnor] 2010-06-18 16:38:58 PDT
Perhaps we should add our own l10n.ini ?  sync will likely not be built by all toolkit consumers, at least to start.
Comment 27 Axel Hecht [:Pike] 2010-06-18 16:45:43 PDT
It's really a question on whether comm-central apps will have services on. KaiRo says for SeaMonkey that'd be a yes, so a separate l10n.ini sounds like a good idea.
Comment 28 Philipp von Weitershausen [:philikon] 2010-06-18 17:03:32 PDT
Created attachment 452390 [details] [diff] [review]
[LANDED] fx-sync: harmonize locale directory structure with m-c conventions
Comment 29 Ed Lee :Mardak 2010-06-18 17:21:35 PDT
http://hg.mozilla.org/services/fx-sync/rev/fbba677a4ad9
Harmonize locale directory structure with mozilla-central conventions.
Comment 30 Philipp von Weitershausen [:philikon] 2010-06-18 17:29:00 PDT
Created attachment 452398 [details]
hg convert filemap (v4)

Updated hg convert filemap now that fx-sync matches the directory layout conventions of mozilla-central for locales (removed the rename).
Comment 31 Philipp von Weitershausen [:philikon] 2010-06-18 18:04:02 PDT
Created attachment 452404 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services' (v2)

* Fixed nit in licensing header.
* s/locale/locales
* Fix jar.mn to yield the same chrome URIs as the add-on
* Provide our own l10n.ini
Comment 32 Philipp von Weitershausen [:philikon] 2010-06-18 18:05:21 PDT
Created attachment 452405 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v2)

Hook services/sync/locales/l10n.ini into browser/locales/l10n.ini.
Comment 33 Ed Lee :Mardak 2010-06-18 23:52:21 PDT
Do we want the sync tags to land? Probably not so remove the "convert-repo update tags" changeset when landing on mozilla-central?
Comment 34 Ed Lee :Mardak 2010-06-19 00:25:35 PDT
Actually nevermind, I wasn't running hg convert correctly.

How much revision history do we want to keep? Do we want to keep at least the history from when modules and such lived in source/modules? Right now the history starts from it moved to services/sync.

I've pushed the convert + 3 patches (crypto, makefiles, buildsystem) to try just to make sure nothing blows up.
Comment 35 Axel Hecht [:Pike] 2010-06-19 03:38:40 PDT
We'll need some buildmaster magic for the releng build infra to pick up the new dirs, I filed bug 573217 on that.

As that's a tad laborious, we should have the full set of new top-level dirs worked out. Is 'services/sync' all that we'll need for sync?

For the landing order, it should be fine to have the releng patch land a good deal before the mozilla-central landing, having additional dirs in that logic shouldn't hurt.
Background: l10n.ini is used to run compare-locales locally, and the dashboard uses it for build logic, releng decided to drop that code, and thus needs a patch.

Mardak, do you want me to do a local testrun whether things work like we'd expect wrt to repacks and compare-locales etc? If so, could you create a patch for me to test? hg diff -r -r would probably do it on your tree that you pushed to try.
Comment 36 Philipp von Weitershausen [:philikon] 2010-06-19 04:23:24 PDT
(In reply to comment #34)
> How much revision history do we want to keep? Do we want to keep at least the
> history from when modules and such lived in source/modules? Right now the
> history starts from it moved to services/sync.

Hmm, I thought hg convert would follow renames back in history. If we want to include history from before the rename, we'll just need to add "include source" to the filemap.
Comment 37 Philipp von Weitershausen [:philikon] 2010-06-19 04:26:36 PDT
(In reply to comment #35)
> As that's a tad laborious, we should have the full set of new top-level dirs
> worked out. Is 'services/sync' all that we'll need for sync?

In terms of l10n, yes. The UI parts for sync are landing in browser (see bug 571897).
Comment 38 Axel Hecht [:Pike] 2010-06-19 04:30:10 PDT
Thanks, updated the releng bug.
Comment 39 Axel Hecht [:Pike] 2010-06-19 08:50:07 PDT
Comment on attachment 452405 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v2)

You'll also need to hook services/sync up with the repacks, that's in 
http://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#184, probably something like

        @$(MAKE) -C ../../services/sync/locales AB_CD=$* XPI_NAME=locale-$* BOTH_MANIFESTS=1
Comment 40 Philipp von Weitershausen [:philikon] 2010-06-19 09:37:02 PDT
Created attachment 452467 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v3)

Updated: hook services/sync into the repacks. Thanks, Axel!
Comment 41 Ed Lee :Mardak 2010-06-19 09:43:35 PDT
http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry

(In reply to comment #36)
> include history from before the rename, we'll just need to add "include source"
> to the filemap.
We also need to pick what they're renamed to. And we need to make sure the names don't overlap as it'll cause problems.. So something like source -> services/sync/source and modules -> services/sync/modules ? I guess we don't need to worry about the chrome bits yet.
Comment 42 Philipp von Weitershausen [:philikon] 2010-06-19 09:59:20 PDT
(In reply to comment #41)
> http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry

Looks pretty green to me :)

> We also need to pick what they're renamed to. And we need to make sure the
> names don't overlap as it'll cause problems.. So something like source ->
> services/sync/source and modules -> services/sync/modules ? I guess we don't
> need to worry about the chrome bits yet.

Yeah. Probably something like this then:

include source
rename source services/sync

(modules was a subdir of source, no?)
Comment 43 Ed Lee :Mardak 2010-06-19 10:11:25 PDT
Nod. You knew what I meant ;) And seems to work pretty well.. Better than fx-sync repository! It seems like the files were never moved :p

include modules
rename modules services/sync/modules
include source/modules
rename source/modules services/sync/modules
include services
...

It's as if thunder originally created the file at services/sync/modules/service.js ;)
Comment 44 Ed Lee :Mardak 2010-06-19 10:18:50 PDT
Oh and in terms of merging, we'll want to clean up branches and tags?

hg up 1.0.x && hg ci --close-branch -m 'Close fx-sync 1.0.x branch.'
hg up 1.2.x && hg ci --close-branch -m 'Close fx-sync 1.2.x branch.'
hg up 1.3.x && hg ci --close-branch -m 'Close fx-sync 1.3.x branch.'

and strip out all fx-sync related .hgtags (not sure why it got included with the convert, but we can exclude it explicitly perhaps? first time I ran it, it also did .hgignore.. ??)
Comment 45 Philipp von Weitershausen [:philikon] 2010-06-19 10:29:02 PDT
(In reply to comment #41)
> http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry

Perhaps we should also have try-server do builds with --enable-services-sync (if there's no way to have it call configure with extra arguments we could hardcode MOZ_SERVICES_SYNC=1 in configure.in).
Comment 46 Ed Lee :Mardak 2010-06-19 10:34:35 PDT
Created attachment 452471 [details] [diff] [review]
fx-sync super diff
Comment 47 Axel Hecht [:Pike] 2010-06-19 13:05:03 PDT
slightly off topic, I was wondering, do you guys want to change the provider of the chrome urls from weave to sync?
Comment 48 Philipp von Weitershausen [:philikon] 2010-06-19 13:44:26 PDT
(In reply to comment #47)
> slightly off topic, I was wondering, do you guys want to change the provider of
> the chrome urls from weave to sync?

Eventually yes, but not now :). Thanks for noticing, though.
Comment 49 Ed Lee :Mardak 2010-06-19 19:14:13 PDT
I pushed a build with MOZ_SERVICES_SYNC=1 I think..
http://hg.mozilla.org/try/rev/b4a53290abf0

Didn't seem to copy modules into services-sync:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/edward.lee@engineering.uiuc.edu-b4a53290abf0/tryserver-macosx/

And tests didn't seem to run.
Comment 50 Philipp von Weitershausen [:philikon] 2010-06-20 00:45:12 PDT
(In reply to comment #49)
> Didn't seem to copy modules into services-sync:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/edward.lee@engineering.uiuc.edu-b4a53290abf0/tryserver-macosx/

Hmm... If I build with --enable-services-sync, MinefieldDebug.app contains modules/services-sync. If I then do 'make package', the resulting .dmg will contain it, too.
Comment 51 Mike Connor [:mconnor] 2010-06-20 10:55:09 PDT
Ed, can you retry with a mozconfig-extra (tryserver build docs have this)
Comment 52 Ted Mielczarek [:ted.mielczarek] 2010-06-20 14:32:48 PDT
Comment on attachment 452404 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services' (v2)

>diff --git a/services/crypto/Makefile.in b/services/crypto/Makefile.in
>new file mode 100644
>--- /dev/null
>+++ b/services/crypto/Makefile.in
>+DIRS = idl src

FYI, we're no longer separating interfaces and code into subdirectories. This just adds extra invocations of make and slows down the build. Just put the contents of idl/Makefile.in and src/Makefile.in directly into this Makefile instead. (There's not a whole lot going on in those Makefiles anyway.)
Comment 53 Ted Mielczarek [:ted.mielczarek] 2010-06-20 14:35:01 PDT
Comment on attachment 452467 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v3)

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -6936,6 +6936,15 @@
> AC_SUBST(MOZ_THEME_FASTSTRIPE)
> 
> dnl ========================================================
>+dnl = Mozilla Sync
>+dnl ========================================================
>+MOZ_ARG_ENABLE_BOOL(services-sync,
>+[  --enable-services-sync  Enable Mozilla Sync components],
>+    MOZ_SERVICES_SYNC=1,
>+    MOZ_SERVICES_SYNC= )
>+AC_SUBST(MOZ_SERVICES_SYNC)

Is there a compelling reason to add this as a configure option? If you just want it to be controllable per-app, then you can simply have the AC_SUBST line, and have apps set it in confvars.sh. (More configure options means more things that people will put in their mozconfigs, and more funny build configurations that we wind up supporting.)
Comment 54 Ed Lee :Mardak 2010-06-21 02:37:44 PDT
Created attachment 452678 [details]
hg convert filemap (v5)

Added more include/rename entries to preserve various history across renames of files that we're interested in. Also renames files so it's as if they've always had the right names.

There's a bug with hg convert that I've patched locally that sometimes resulted in files disappearing if a renamed file is already renamed by convert:
http://mercurial.selenic.com/bts/issue2243
Comment 55 Ed Lee :Mardak 2010-06-21 02:44:01 PDT
Created attachment 452680 [details] [diff] [review]
f-sync super diff

I've been pushing these super diffs (as one changeset) to try because pushing the converted history seems to result in 504 gateway timeout or something.

Seems to build on maemo and fails on linux:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1277109725.1277112471.18474.gz
make[5]: *** No rule to make target `FormNotifier.js', needed by `libs'.  Stop.

The commands I've been using to create the repo:
hg clone mozilla-central sync-trunk
hg clone -b default weave weave-default
hg convert --filemap filemap weave-default sync-trunk
hg -R sync-trunk strip tip
hg -R sync-trunk merge
hg -R sync-trunk ci -m 'Merge fx-sync to mozilla-central.'

I clone weave to weave-default to only convert the default branch so the other branches like 1.3.x don't need to be closed before landing on mozilla-central. Pretend hg convert does the right thing with my mercurial patch. The strip tip is to remove the .hgtags auto-generated commit.
Comment 56 Ed Lee :Mardak 2010-06-21 09:56:21 PDT
Created attachment 452750 [details] [diff] [review]
fx-sync super diff

Not sure why FormNotifier.js disappeared from the previous super diff, but I'm guessing that's why the build failed. Trying again with full history..
Comment 57 Ed Lee :Mardak 2010-06-21 10:52:01 PDT
First build for the latest push has appeared for maemo.. unpacking it doesn't seem to have services-sync stuff:
https://build.mozilla.org/tryserver-builds/edward.lee@engineering.uiuc.edu-try-18337e111a7a/

But then again, maemo builds have never failed, so seems like we'll need to tweak some other makefiles for mobile-browser ?
Comment 58 Axel Hecht [:Pike] 2010-06-21 10:55:05 PDT
I suspect you're missing something in http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in
Comment 59 Ed Lee :Mardak 2010-06-21 11:54:42 PDT
(In reply to comment #58)
> I suspect you're missing something in
> http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in
Thanks. Seems like we need to explicitly add our 1 crypto and 2 sync components, but modules are getting added fine.

Also, seems like tests are failing:
  /builds/slave/tryserver-linux-opt-unittest-xpcshell/build/xpcshell/tests/test_services_sync/unit/head_http_server.js:2: ReferenceError: Cu is not defined

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1277145373.1277146201.2013.gz
Comment 60 Ed Lee :Mardak 2010-06-21 12:14:30 PDT
Locally adding the missing components to the os x build, I get this error while starting up:

Error: Ci.IWeaveCrypto is undefined
Source File: Minefield.app/Contents/MacOS/components/WeaveCrypto.js
Line: 471

Where are we supposed to put IWeaveCrypto.xpt? Doing the idl thing doesn't add it as an interface?
Comment 61 Robert Kaiser 2010-06-21 12:25:32 PDT
The xpt should 1) be in components and 2) should be listed in packages-manifest. In normal builds, it will be linked together with other xpts into browser.xpt due to step 2, but for manual testing, just doing step 1 in a normal build should work fine.
Comment 62 Philipp von Weitershausen [:philikon] 2010-06-21 14:23:56 PDT
(In reply to comment #52)
> FYI, we're no longer separating interfaces and code into subdirectories. This
> just adds extra invocations of make and slows down the build. Just put the
> contents of idl/Makefile.in and src/Makefile.in directly into this Makefile
> instead. (There's not a whole lot going on in those Makefiles anyway.)

That's good to know, will do!

(In reply to comment #53)
> Is there a compelling reason to add this as a configure option? If you just
> want it to be controllable per-app, then you can simply have the AC_SUBST line,
> and have apps set it in confvars.sh. (More configure options means more things
> that people will put in their mozconfigs, and more funny build configurations
> that we wind up supporting.)

Fair enough. I didn't have a particular reason for it other than that it was suggested by mconnor and/or Mardak I think ;). I'm fine with just having the AC_SUBST line.
Comment 63 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2010-06-21 14:30:31 PDT
I'm thinking we may want to have bug 573194 block this... It's not strictly necessary in that it's easy to fix on extension side before next merge or fix in m-c post-merge, but we might as well do it now.
Comment 64 Ed Lee :Mardak 2010-06-21 17:06:48 PDT
Created attachment 452900 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services' (v3)

Flatten crypto directory (idl/src -> .) and sync (src -> .) for idl and components bits. Also add a pref.js as remapped from bug 569355.

+PREF_JS_EXPORTS = $(srcdir)/services-sync.js
Comment 65 Ed Lee :Mardak 2010-06-21 17:09:10 PDT
Created attachment 452904 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v4)

Add components: .xpt and 3 .js to package-manifest.in plus .xpt for removed (for when it's packaged into browser.xpt)

--enable-services-sync is still there though..
Comment 66 Robert Kaiser 2010-06-21 17:57:17 PDT
(In reply to comment #65)
> Created an attachment (id=452904) [details]
> mozilla-central: hook 'services' into the build system (v4)

Note that it's usually best to request review from ted or some build system peer for those build system changes.
Comment 67 Ed Lee :Mardak 2010-06-21 21:14:32 PDT
(In reply to comment #10)
> After renaming head.js to head_first.js, tests still fail:
> 
>   uncaught exception: [Exception... "Component returned failure code:
> 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult:
> "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame ::
> OBJDIR/_tests/xpcshell/test_services_sync/unit/head_first.js :: <TOP_LEVEL> ::
> line 9"  data: no]
> 
> The relevant line in head_first.js (head.js) is:
> 
>   let appinfo = Cc["@mozilla.org/xre/app-info;1"].
>                 getService(Ci.nsIXULAppInfo);

Yeah, tryservers are giving that too:

  uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: /builds/slave/tryserver-linux-opt-unittest-xpcshell/build/xpcshell/tests/test_services_sync/unit/head_first.js :: <TOP_LEVEL> :: line 9"  data: no]

Not sure why it's failing for us as it seems like other xpcshell tests use that as well.. ?
Comment 68 Mark Finkle (:mfinkle) (use needinfo?) 2010-06-21 21:38:01 PDT
(In reply to comment #67)

> > The relevant line in head_first.js (head.js) is:
> > 
> >   let appinfo = Cc["@mozilla.org/xre/app-info;1"].
> >                 getService(Ci.nsIXULAppInfo);
> 
> Yeah, tryservers are giving that too:
> 
>   uncaught exception: [Exception... "Component returned failure code:
> 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult:
> "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame ::
> /builds/slave/tryserver-linux-opt-unittest-xpcshell/build/xpcshell/tests/test_services_sync/unit/head_first.js
> :: <TOP_LEVEL> :: line 9"  data: no]
> 
> Not sure why it's failing for us as it seems like other xpcshell tests use that
> as well.. ?

Are you guys making your own fake nsIXULAppInfo? Like this http://mxr.mozilla.org/mozilla-central/source/chrome/test/unit/test_bug399707.js#45
Comment 69 Ed Lee :Mardak 2010-06-22 01:07:25 PDT
Comment on attachment 452374 [details] [diff] [review]
fx-sync: handle absence of debug preference gracefully

We'll land with prefs so no need to handle missing default
Comment 70 Ed Lee :Mardak 2010-06-22 01:09:06 PDT
Created attachment 453008 [details]
hg convert filemap (v6)

Updated filemap for flatter src/components structure and adds history for tests, etc.
Comment 71 Ed Lee :Mardak 2010-06-22 01:11:29 PDT
Created attachment 453009 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v5)
Comment 72 Ed Lee :Mardak 2010-06-22 01:12:24 PDT
Created attachment 453010 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services' (v4)
Comment 73 Ed Lee :Mardak 2010-06-22 07:39:53 PDT
uncaught exception: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.import]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: /builds/slave/tryserver-linux-debug-unittest-xpcshell/build/xpcshell/tests/test_services_sync/unit/head_first.js :: <TOP_LEVEL> :: line 68"  data: no]

Cu.import("resource://services-sync/log4moz.js");

Weave.js component is being loaded:
*** registering Weave.js: [ Weave Service, about:sync-log, about:sync-log.1 ]

But it's not registering it's resource://gre/modules/services-sync mapping to resouce://services-sync until app-startup.

We'll need to register it earlier or register it as part of a head_
Comment 74 Ed Lee :Mardak 2010-06-22 18:31:43 PDT
Created attachment 453265 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v6)

Missed the pref file.
Comment 75 Ed Lee :Mardak 2010-06-22 21:37:16 PDT
Woo!

Your Try Server test (d9d65b8ef3f5) was successfully completed on mac on builder: OS X 10.5.2 tryserver debug test xpcshell.

Summary of test results:
TinderboxSummaryMessage: s: try-mac-slave12
xpcshell: 840/0

All tests pass! (And seems like running the build with the sync addon (latest) works correctly too!)
Comment 76 Philipp von Weitershausen [:philikon] 2010-06-23 08:16:27 PDT
Created attachment 453383 [details] [diff] [review]
mozilla-central: hardcode constants

On trunk we want to hardcode values that are normally filled in during the Sync add-on build process.
Comment 77 Ted Mielczarek [:ted.mielczarek] 2010-06-23 10:37:40 PDT
Comment on attachment 453010 [details] [diff] [review]
mozilla-central: Makefiles and build helpers for 'services' (v4)

>diff --git a/services/sync/locales/Makefile.in b/services/sync/locales/Makefile.in
>new file mode 100644
>--- /dev/null
>+++ b/services/sync/locales/Makefile.in

Does this need to be its own Makefile, or could you just move the jar.mn up one level? (Presumably you could keep the .properties files in locales/ if you wanted, and just change the paths in the jar.mn).

Looks fine otherwise.
Comment 78 Ted Mielczarek [:ted.mielczarek] 2010-06-23 10:41:41 PDT
Comment on attachment 453265 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v6)

>diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in
>--- a/browser/installer/package-manifest.in
>+++ b/browser/installer/package-manifest.in
>@@ -213,16 +213,17 @@
> @BINPATH@/components/pref.xpt
> @BINPATH@/components/prefetch.xpt
> @BINPATH@/components/profile.xpt
> @BINPATH@/components/proxyObject.xpt
> @BINPATH@/components/rdf.xpt
> @BINPATH@/components/satchel.xpt
> @BINPATH@/components/saxparser.xpt
> @BINPATH@/components/sessionstore.xpt
>+@BINPATH@/components/services-crypto.xpt

#ifdef MOZ_SERVICES_SYNC, please (and the other bits)

>diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in
>--- a/browser/installer/removed-files.in
>+++ b/browser/installer/removed-files.in
>@@ -659,16 +659,17 @@ xpicleanup@BIN_SUFFIX@
>   components/profile.xpt
>   components/progressDlg.xpt
>   components/proxyObjInst.xpt
>   components/rdf.xpt
>   components/safebrowsing.xpt
>   components/satchel.xpt
>   components/saxparser.xpt
>   components/search.xpt
>+  components/services-crypto.xpt

Why is this being both added and removed???

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -8584,16 +8584,22 @@ if test "$MOZ_MORKREADER"; then
>   AC_DEFINE(MOZ_MORKREADER)
> fi
> 
> dnl Build Places if required
> if test "$MOZ_PLACES"; then
>   AC_DEFINE(MOZ_PLACES)
> fi
> 
>+dnl Build Sync Services if required
>+AC_SUBST(MOZ_SERVICES_SYNC)
>+if test "$MOZ_SERVICES_SYNC"; then

test -n "$MOZ_SERVICES_SYNC" is what we usually use (although I notice that the test above this isn't).

r=me with those fixes.
Comment 79 Axel Hecht [:Pike] 2010-06-23 10:50:10 PDT
(In reply to comment #77)
> (From update of attachment 453010 [details] [diff] [review])
> >diff --git a/services/sync/locales/Makefile.in b/services/sync/locales/Makefile.in
> >new file mode 100644
> >--- /dev/null
> >+++ b/services/sync/locales/Makefile.in
> 
> Does this need to be its own Makefile, or could you just move the jar.mn up one
> level? (Presumably you could keep the .properties files in locales/ if you
> wanted, and just change the paths in the jar.mn).
> 
> Looks fine otherwise.

I think we need to have that makefile for l10n repacks, or at least should have it to keep things consistent with how we do it for the other l10n modules.
Comment 80 Ed Lee :Mardak 2010-06-23 15:11:30 PDT
Created attachment 453521 [details] [diff] [review]
mozilla-central: hook 'services' into the build system (v7)

ifdef around package-manifest.in bits, removed removed-files, test -n

Leaving the makefiles unchanged from Axel's comments.
Comment 81 Ed Lee :Mardak 2010-06-23 15:37:40 PDT
http://hg.mozilla.org/mozilla-central/rev/3ff4b9aa29c7
Makefiles and build helpers for 'services'. 

http://hg.mozilla.org/mozilla-central/rev/227db4ad8cdf
Hook 'services' into the build system with MOZ_SERVICES_SYNC not-yet-set as a browser confvar. 

Full pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=227db4ad8cdf
Comment 82 Philipp von Weitershausen [:philikon] 2010-06-24 10:15:57 PDT
Comment on attachment 453383 [details] [diff] [review]
mozilla-central: hardcode constants

This patch is obsolete here since this bug is RESOLVED FIXED. See bug 574380 to follow up.
Comment 83 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2010-07-07 17:43:17 PDT
An update to the list of steps from comment #5.

1. hg clone .../fx-sync
2. hg clone .../mozilla-central
3. hg convert --filemap attachment fx-sync sync-for-mc
4. cd sync-for-mc
5. hg strip tip (this removes extraneous commit for tags)
6. (remove last line from .hg/shamap since that changeset was stripped)
7. cd ../mozilla-central
8. hg pull -f --branch=default ../sync-for-mc (branch=default makes sure only relevant changesets are brought in)
9. hg merge; hg commit
Comment 84 Axel Hecht [:Pike] 2010-07-15 14:32:13 PDT
We missed a piece of the l10n story, and thus the sync strings are still not exposed on the dashboard.

l10n repacks with l10n-merge on are fine, as they're picking up the strings from en-US, even though en-US doesn't even have those strings on yet. Not sure if that's worth fixing, depends a lot on the timeline of making this unconditional for fx. Is there a bug on that? Didn't find one naively.
Comment 85 Philipp von Weitershausen [:philikon] 2010-08-31 06:39:01 PDT
Checked in v6 of the filemap and a bash script to perform steps 1 through 6 from comment 83: http://hg.mozilla.org/services/fx-sync/rev/4eab6b53d2c5
Comment 86 Jens Hatlak (:InvisibleSmiley) 2010-09-07 07:04:31 PDT
Are there any plans (bug) to remove MOZ_SERVICES_SYNC? I wonder whether I should just assume MOZ_SERVICES_SYNC=1 for the SeaMonkey UI parts, which would spare me enabling preprocessing in several jar.mn files.
Comment 87 Mike Connor [:mconnor] 2010-09-07 08:12:51 PDT
At this time, there are no plans to remove that, as I don't expect all apps to build it.  It's up to you for the preprocessing, but I also don't know what you're optimizing for there.
Comment 88 Robert Kaiser 2010-09-07 09:23:46 PDT
What might be reasonable is erroring out on building FF (or SM, but that's our problem) when it's not built and getting rid of it in browser/ (i.e. in the UI), just for making things cleaner.
Comment 89 Mike Connor [:mconnor] 2010-09-07 09:53:38 PDT
That feels like shuffling deck chairs, at this point.  Definitely not something that's at all a priority.  SM is, of course, free to act as they see fit.

Note You need to log in before you can comment on or make changes to this bug.