hg move /mobile to /mobile/xul

RESOLVED FIXED

Status

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Dependency tree / graph

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
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.

Updated

7 years ago
Blocks: 701816

Updated

7 years ago
Depends on: 701864

Comment 2

7 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

7 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

7 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

7 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

7 years ago
Created attachment 573987 [details] [diff] [review]
patch v.1

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

7 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

7 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 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+
Oh, and remember to remove the NetworkGeolocationProvider.js changes
(Assignee)

Comment 11

7 years ago
yup.  ignore the changes to dom/system/NetworkGeolocationProvider.js.  I will remove that locally.
(Assignee)

Comment 12

7 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

7 years ago
ah, got it.  there are some mozconfig that needed to be changed.
(Assignee)

Comment 15

7 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
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).
(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.
would it make sense to have a mobile/common directory for anything that is shared between xul and native?
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
(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

7 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

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

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

Updated

7 years ago
Blocks: 702302

Comment 25

7 years ago
With comment 24 addressed, that'd also be an r=me.

Comment 26

7 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

7 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

7 years ago
Created attachment 574988 [details] [diff] [review]
Patch to land on mozilla-central
Attachment #573987 - Attachment is obsolete: true
Attachment #574988 - Flags: superreview+
Attachment #574988 - Flags: review+
(Assignee)

Comment 29

7 years ago
Created attachment 575026 [details] [diff] [review]
Patch to land on birch

straight forward move, and build files fixup.

Comment 30

7 years ago
Created attachment 575059 [details] [diff] [review]
update mobile/xul/config/mozconfigs android branding

This will land on top of attachment 574988 [details] [diff] [review].
Attachment #575059 - Flags: review?(mark.finkle)
Attachment #575059 - Flags: review?(mark.finkle) → review+

Comment 31

7 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

7 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

7 years ago
Created attachment 575092 [details] [diff] [review]
in-tree vs buildbot-configs mozconfig

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

7 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

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

Comment 36

7 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
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Blocks: 705137
You need to log in before you can comment on or make changes to this bug.