Closed
Bug 701833
Opened 13 years ago
Closed 13 years ago
hg move /mobile to /mobile/xul
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dougt, Assigned: dougt)
References
Details
Attachments
(4 files, 1 obsolete file)
205.64 KB,
patch
|
dougt
:
review+
dougt
:
superreview+
|
Details | Diff | Splinter Review |
216.72 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
1.47 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
524 bytes,
patch
|
Details | Diff | Splinter Review |
This bug is to move our xul based browser into a subdirectory of mobile. The plan is to:
hg move mobile/* into mobile/xul/
Also, we will make --enable-application=mobile to be converted into mobile/xul. This is so that there doesn't need to be any change to build infrastructure.
Comment 1•13 years ago
|
||
let's do it
Comment 2•13 years ago
|
||
Does this have an impact on which directories go into the l10n builds? I.e., is the intention to move mobile/locales as well?
Comment 3•13 years ago
|
||
I don't know, but my first guess would be that there would be a mobile/xul/locales and a mobile/locales.
Comment 4•13 years ago
|
||
I'd prefer to have a single localization entry point, fwiw.
It'd be OK to have #ifdef's in mobile/locales/jar.mn though, if we want to switch particular files off and on. There shouldn't be #ifdef's in the actual files, though.
Assignee | ||
Comment 5•13 years ago
|
||
axel, i am going to need some help on the l10n bits of this patch. I am going to get it "building", then you can advice (provide a patch if your heart desired) to fix this up.
For what its worth, i do not think we should share strings between mobile/xul and mobile/android. I could see us having mobile/windowsphone which might do something completely different for l10n. Do you worry about that?
Assignee | ||
Comment 6•13 years ago
|
||
ted, could you review the build/config stuff, specifically us mapping --enable-application=mobile => mobile/xul
mfinkle, your sr, sir, since it's your project.
Built locally on Android and worked. I pushed to try; waiting results.
Assignee: nobody → doug.turner
Attachment #573987 -
Flags: superreview?(mark.finkle)
Attachment #573987 -
Flags: review?(ted.mielczarek)
Comment 7•13 years ago
|
||
Soooooo, this really depends on what you want to ship/build/automate.
The decision we have to make at this point really boils down to:
Is Firefox mobile on the different platforms one product, with a single set of localization coverage? Or are the product expectations for Firefox on each OS/UX combo supposed to be different?
Also, are there features that should be parity within one localization across those variants? I can picture search being one, at least.
Not sure if a bug is the right forum to figure these out?
Comment 8•13 years ago
|
||
(In reply to Doug Turner (:dougt) from comment #5)
> I could see us having mobile/windowsphone
> which might do something completely different for l10n. Do you worry about
> that?
Yes. You guys will always be able to simply outrun us in terms of "new platform, do something completely different". We'll need to find a baseline of what's in l10n, and what's a localized Firefox product, and going from there, you can pick platforms. We should stay Mozilla and not be Opera, IMHO.
Comment 9•13 years ago
|
||
Comment on attachment 573987 [details] [diff] [review]
patch v.1
sr=me
the long term plan would be to remove the mobile -> mobile/xul hacks too, but we can wait for native to become our default release. won't be too long.
Attachment #573987 -
Flags: superreview?(mark.finkle) → superreview+
Comment 10•13 years ago
|
||
Oh, and remember to remove the NetworkGeolocationProvider.js changes
Assignee | ||
Comment 11•13 years ago
|
||
yup. ignore the changes to dom/system/NetworkGeolocationProvider.js. I will remove that locally.
Assignee | ||
Comment 12•13 years ago
|
||
it builds fine locally, but not on try. Any ideas?
https://tbpl.mozilla.org/php/getParsedLog.php?id=7362425&tree=Try&full=1
Assignee | ||
Comment 13•13 years ago
|
||
ah, got it. there are some mozconfig that needed to be changed.
Assignee | ||
Comment 14•13 years ago
|
||
Assignee | ||
Comment 15•13 years ago
|
||
so the burn is because releng has different mozconfigs that we don't control:
http://hg.mozilla.org/build/buildbot-configs/raw-file/production/mozilla2/linux-android/try/nightly/mozconfig
Comment 16•13 years ago
|
||
Do the changes from this bug mean that localizers will need to work both in /mobile/android *and* /mobile/xul in order to get the native build localized? For instance, I believe that the native build will still need the netError overrides which would be in /mobile/xul (and not tollkit).
Comment 17•13 years ago
|
||
(In reply to Staś Małolepszy :stas from comment #16)
> Do the changes from this bug mean that localizers will need to work both in
> /mobile/android *and* /mobile/xul in order to get the native build
> localized? For instance, I believe that the native build will still need
> the netError overrides which would be in /mobile/xul (and not tollkit).
Not exactly. Localizers will need to work in /mobile/xul to localize the XUL version of Fennec. Localizers will need to work in the /mobile/native (I don't like /mobile/android) to localize the native version of Fennec. There will not be files shared between them. I am pushing for /mobile/native/android for the android specific files (Java etc) and /mobile/native/core for the Gecko wrapper files (XUL, JS, XPCOM components, etc). Both of these sub folders will have localizable files in them.
Comment 18•13 years ago
|
||
would it make sense to have a mobile/common directory for anything that is shared between xul and native?
Comment 19•13 years ago
|
||
cc-ing RelEng.
1) Are there any corresponding changes needed in our continuous integration / release automation?
2) If so, should we track those changes in this bug also, or file a separate linked bug?
OS: Mac OS X → All
Comment 20•13 years ago
|
||
(In reply to John O'Duinn [:joduinn] from comment #19)
> cc-ing RelEng.
>
> 1) Are there any corresponding changes needed in our continuous integration
> / release automation?
>
> 2) If so, should we track those changes in this bug also, or file a separate
> linked bug?
Ah. Already being tracked in bug#701864. Never mind me. (Thanks nthomas!)
Comment 21•13 years ago
|
||
Doug, I'm not sure where you're at with this, but could you hold off on landing?
I need to discuss the solution with the team. This will most likely affect 8.0.1 negatively, so we'll want to land after the chemspill, not before. And it's quite possible our solution will require a quiet time or downtime to land to avoid burning.
Could you let me know what this blocks, as well?
Assignee | ||
Comment 22•13 years ago
|
||
RelEng:
i would like to land this as soon as possible. The only bit that I know that needs to change is the stuff we discussed on IRC -- the mozconfig's you are using for building xul fennec need to also be bumped (s/mobile/mobile\/xul/g)
Please let me know by tuesday what would block this from happening on Wednesday morning.
Brad,
> Would it make sense to have a mobile/common directory for anything that is shared between xul and native?
Yes, i think so. At this point, however, we have so much dead code in the native fennec, i would like to move those pieces later.
Comment 23•13 years ago
|
||
Comment on attachment 573987 [details] [diff] [review]
patch v.1
Review of attachment 573987 [details] [diff] [review]:
-----------------------------------------------------------------
::: Makefile.in
@@ +165,5 @@
>
> buildsymbols:
> ifdef MOZ_CRASHREPORTER
> ifdef USE_ELF_HACK
> + ifeq (mobile,$(MOZ_BUILD_APP))
Don't indent the makefile conditionals.
@@ +166,5 @@
> buildsymbols:
> ifdef MOZ_CRASHREPORTER
> ifdef USE_ELF_HACK
> + ifeq (mobile,$(MOZ_BUILD_APP))
> + $(MAKE) -C mobile/xul/installer elfhack
Only one tab here (and two lines below).
::: configure.in
@@ +4753,5 @@
> else
> + # default mobile to be mobile/xul
> + if test "$MOZ_BUILD_APP" = "mobile" ; then
> + MOZ_BUILD_APP=mobile/xul
> + fi
I don't really want this to live here forever. Do you think we'll wind up reshuffling things so that --enable-application=mobile builds the native ui in the near future? We should just be able to hg mv mobile/native/confvars.sh etc up a level at that point.
::: dom/system/NetworkGeolocationProvider.js
@@ +57,5 @@
> dump(aMsg);
> }
> }
>
> +LOG("AHHHAHAHHAAHH!!!");
Funny, but make sure you take this out of the patch.
Attachment #573987 -
Flags: review?(ted.mielczarek) → review+
Comment 24•13 years ago
|
||
Comment on attachment 573987 [details] [diff] [review]
patch v.1
We shouldn't do the l10n side this way:
This patch effectively creates two applications to localize, and that raises a ton of questions. Answering those now will just slow us down, I think.
I propose:
Keep all l10n strings inside mobile/locales/en-US.
Don't move:
crashreporter, chrome/overrides/*, chrome/region.properties, defines.inc, mobile-l10n.js, profile, searchplugins/*, updater/*
All of these *strongly* smell like shared files.
Remove:
installer/setup.ini, that was forgotten in bug 649078.
All the remaining files in chrome can probably move to chrome/xul or so.
The new android_strings.dtd for the native UI can land as you see fit, just en-US/native/android_string.dtd or so sounds good to me.
Attachment #573987 -
Flags: review-
Comment 25•13 years ago
|
||
With comment 24 addressed, that'd also be an r=me.
Comment 26•13 years ago
|
||
<Pike> I really care about the l10n strings for all those to be in one mobile/locales/en-US
<Pike> dougt: given the amount of files moving, it probably doesn't matter much if none of them stay where they used to be
if you can add a script that as all the file moves inside en-US, we could use that to move the l10n files inside l10n's ab-CD/mobile, too.
Assignee | ||
Comment 27•13 years ago
|
||
The patch we intent to land will have no shared files between the XUL version of Fennec and the native UI rewrite. As time moves on, we may promote files up into /mobile/shared/*. We also need to do alot of work to trim down what files we ship in the native UI.
Pike and I disagreed on the directory structure. I asked how we can make this easier for Pike (can we do the renames in hg for him), but there wasn't anything clear that he knew about.
Pike, if there are concrete things we can to do help shipping two products for you, please file bugs or lets move this to the newsgroup. Shipping both the native ui and the xul ui for the short term is what we are doing.
Assignee | ||
Comment 28•13 years ago
|
||
Attachment #573987 -
Attachment is obsolete: true
Attachment #574988 -
Flags: superreview+
Attachment #574988 -
Flags: review+
Assignee | ||
Comment 29•13 years ago
|
||
straight forward move, and build files fixup.
Comment 30•13 years ago
|
||
This will land on top of attachment 574988 [details] [diff] [review].
Attachment #575059 -
Flags: review?(mark.finkle)
Updated•13 years ago
|
Attachment #575059 -
Flags: review?(mark.finkle) → review+
Comment 31•13 years ago
|
||
[20:24] <aki> hitting an elfhack bug in android native |make buildsymbols|
[20:24] <aki> i suppose this can be resolved on birch
[20:24] <aki> http://pastebin.mozilla.org/1384816
Comment 32•13 years ago
|
||
This is also breaking on my mozilla-central android-xul builds during "make buildsymbols":
make -C mobile/xul/installer elfhack
make[1]: Entering directory `/builds/slave/m-cen-andrd-xul-ntly/build/obj-firefox/mobile/xul/installer'
===
=== If you get failures below, please file a bug describing the error
=== and your environment (compiler and linker versions), and use
=== --disable-elf-hack until this is fixed.
===
cd ../../../dist/bin; find . -name "*.so" | xargs ../../../build/unix/elfhack/elfhack
xargs: ../../../build/unix/elfhack/elfhack: No such file or directory
make[1]: *** [elfhack] Error 127
make[1]: Leaving directory `/builds/slave/m-cen-andrd-xul-ntly/build/obj-firefox/mobile/xul/installer'
make: *** [buildsymbols] Error 2
I'm going to try "ac_add_options --disable-elf-hack" and see if I can get further.
Comment 33•13 years ago
|
||
output of
diff user-mc/mobile/xul/config/mozconfigs/android/nightly buildbot-configs/mozilla2/android-xul/mozilla-central/nightly/mozconfig
where user-mc is my user repo of mozilla-central, with the above patches applied.
Comment 34•13 years ago
|
||
I was able to get past buildsymbols by adding "ac_add_options --disable-elf-hack" to the mozconfig.
However, after that it breaks during multilocale:
22:39:33 INFO - #####
22:39:33 INFO - ##### Running add-locales step.
22:39:33 INFO - #####
22:39:33 INFO - rmtree: /builds/slave/m-cen-andrd-xul-ntly/./build/obj-firefox/merged
22:39:33 INFO - mkdir: /builds/slave/m-cen-andrd-xul-ntly/./build/obj-firefox/merged
22:39:33 INFO - Running command: python /builds/slave/m-cen-andrd-xul-ntly/./compare-locales/scripts/compare-locales -m /builds/slave/m-cen-andrd-xul-ntly/./build/obj-firefox/merged l10n.ini /builds/slave/m-cen-andrd-xul-ntly/./l10n-central cs in /builds/slave/m-cen-andrd-xul-ntly/./build/mobile/xul/locales
22:39:34 ERROR - Traceback (most recent call last):
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/./compare-locales/scripts/compare-locales", line 71, in <module>
22:39:34 INFO - app = EnumerateApp(inipath, l10nbase, locales)
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 323, in __init__
22:39:34 INFO - self.setupConfigParser(inipath)
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 335, in setupConfigParser
22:39:34 INFO - self.config.loadConfigs()
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 113, in loadConfigs
22:39:34 INFO - self.onLoadConfig(urlopen(self.inipath))
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 128, in onLoadConfig
22:39:34 INFO - self.addChild(title, path, cp)
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 161, in addChild
22:39:34 INFO - cp.loadConfigs()
22:39:34 INFO - File "/builds/slave/m-cen-andrd-xul-ntly/compare-locales/lib/Mozilla/Paths.py", line 113, in loadConfigs
22:39:34 INFO - self.onLoadConfig(urlopen(self.inipath))
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 121, in urlopen
22:39:34 INFO - return _opener.open(url, data)
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 374, in open
22:39:34 INFO - response = self._open(req, data)
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 392, in _open
22:39:34 INFO - '_open', req)
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 353, in _call_chain
22:39:34 INFO - result = func(*args)
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 1196, in file_open
22:39:34 INFO - return self.open_local_file(req)
22:39:34 INFO - File "/tools/python-2.5.1/lib/python2.5/urllib2.py", line 1216, in open_local_file
22:39:34 INFO - stats = os.stat(localfile)
22:39:34 INFO - OSError: [Errno 2] No such file or directory: '/builds/slave/m-cen-andrd-xul-ntly/build/mobile/services/sync/locales/l10n.ini'
22:39:34 ERROR - Return code: 1
22:39:34 FATAL - Halting on failure while running python /builds/slave/m-cen-andrd-xul-ntly/./compare-locales/scripts/compare-locales -m /builds/slave/m-cen-andrd-xul-ntly/./build/obj-firefox/merged l10n.ini /builds/slave/m-cen-andrd-xul-ntly/./l10n-central cs
22:39:34 FATAL - Exiting 1
Comment 35•13 years ago
|
||
Tried changing the enable-application line to mobile/xul; didn't help.
We need to make a call here.
I'm leaning towards adding ac_add_options --disable-elf-hack to both android mozconfigs and fixing l10n on m-c and birch as a followup.
We may want to consider disabling mobile l10n, including multilocale, on mozilla-central until this is fixed, otherwise we'll have red android nightlies due to multilocale.
Not particularly happy about this, but this was a large change with very minimal testing time.
Updated•13 years ago
|
Attachment #575026 -
Flags: review+
Assignee | ||
Comment 36•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/6b42ff06457a
elf hack followup:
https://bugzilla.mozilla.org/show_bug.cgi?id=703305
multilocale followup:
https://bugzilla.mozilla.org/show_bug.cgi?id=703306
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•