1362496 - Add chrome-% for Firefox

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

We currently don't have `make chrome-%` command for Firefox, just for toolkit and Fennec.

It is needed for multi-locale Firefox desktop builds.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Need some guidance here.

I started by trying to add:

+chrome-%:
+       @$(MAKE) chrome AB_CD=$*
+


to `browser/locales/Makefile.in` but that produces an error:

```
processing /Users/zbraniecki/src/larch/browser/locales/jar.mn
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/action/jar_maker.py", line 17, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/action/jar_maker.py", line 13, in main
    return mozbuild.jar.main(args)
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/jar.py", line 597, in main
    jm.makeJar(infile, options.d)
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/jar.py", line 333, in makeJar
    self.processJarSection(info, jardir)
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/jar.py", line 388, in processJarSection
    self._processEntryLine(e, outHelper, jf)
  File "/Users/zbraniecki/src/larch/python/mozbuild/mozbuild/jar.py", line 449, in _processEntryLine
    ', '.join(src_base)))
RuntimeError: File ".deps/generated_pl/list.json" not found in /Users/zbraniecki/src/larch/browser/locales, /Users/zbraniecki/src/larch/obj-x86_64-apple-darwin16.5.0/browser/locales
```

Comment 2

[Justin Wood (:Callek)]

I'm not really all that familiar with this searchplugin stuff... but:

https://dxr.mozilla.org/mozilla-central/source/mobile/locales/Makefile.in#79
is pretty different than:
https://dxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#87

Though the jar.mn directives are pretty similar. Maybe someone of the build peers (coop's team) could assist?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

:coop, can you mentor me through this or point out the right person to help me?

Comment 4

[Axel Hecht [:Pike]]

Yeah, that's not going to work.

I suggest to do what libs-% does, without the jazz to put things up for langpacks.

mobile only does mobile because the jar.mn packages some toolkit files selectively. On desktop, we want the full monty, and recurse into all the directories that libs-% does.

Comment 5

[Francesco Lodolo [:flod]]

CCing Mike, since he's the one who's been working on centralizing search for both mobile and desktop.

Comment 6

[Chris Cooper [:coop] (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> :coop, can you mentor me through this or point out the right person to help
> me?

Axel's approach seems sound, and mkaply seems to have touched most of the code around there.

If you still need a build peer to assist, I can try to loop in Ted.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1362496 - Add chrome-% for Firefox Desktop.

In order to be able to produce multi-locale Firefox Desktop, we need make chrome-% to work
for browser/locales the same way as it works for mobile/android/locales.

Review commit: https://reviewboard.mozilla.org/r/137266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137266/

Comment 8

[Axel Hecht [:Pike]]

Moving over to Firefox Build, as that's what this is about.

I'm not fond of the patch, as there's a ton of repitition, which rubs me backwards. I didn't find anything non-aweful now.

I wonder if it's bad to pass XPI_ROOT_ID for all subtargets in libs-% ? If not, we could define the list of subdirs once, and then loop over that?

I'm surprised to not see searchplugins in chrome-%, too?

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

> I'm surprised to not see searchplugins in chrome-%, too?

I'll add that.

Also, this and bug 1362617 give me a whole ab-CD directory and ab-CD.manifest file in ./obj/dist/bin, but I still don't get it packaged I think.

Comment 10

[Axel Hecht [:Pike]]

Zibi, I think you need to find a build peer to review this, the actual implementation will depend a ton on opinions on how to make make make stuff.

Comment 11

[Mike Kaply [:mkaply]]

What is chrome-% exactly?

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8865651 [details]
Bug 1362496 - Add chrome-% for Firefox Desktop.

Ted, can you review this? Axel said that he'd love to see us not duplicating the entry list, between libs-% and chrome-%, but I don't know if that's possible.

Comment 13

[Axel Hecht [:Pike]]

(In reply to Mike Kaply [:mkaply] from comment #11)
> What is chrome-% exactly?

It's the make target we use on Fx Android to stage additional localized chrome into a multi-lingual package, so that the package step can bundle it all up. It's working on slightly different paths and options than the libs-% target.

Comment 14

[Nick Alexander :nalexander [he/him]]

(In reply to Axel Hecht [:Pike] from comment #13)
> (In reply to Mike Kaply [:mkaply] from comment #11)
> > What is chrome-% exactly?
> 
> It's the make target we use on Fx Android to stage additional localized
> chrome into a multi-lingual package, so that the package step can bundle it
> all up. It's working on slightly different paths and options than the libs-%
> target.

Could we drive towards _removing_ this from Android rather than adding it to all platforms?  This stuff is incredibly difficult to reason about; I'd much rather Desktop lead us to the brave new world than cult-the-cult-of-the-cult from Android.

Comment 15

[Axel Hecht [:Pike]]

I think getting the basics shared is a good thing, and then we can talk about getting the packaging step sanitized once Callek has all the things over in taskcluster, and we only have one spot that calls l10n, and rides the train, and can be tested on try.

Note, I do think that the way that we duplicate stuff in libs-%  and chrome-% is not sane, which is why I didn't give an r+ on that patch. I didn't have a constructive counter-proposal, though, which is what I'm hoping Ted to have.

Comment 16

[Mike Kaply [:mkaply]]

I'll be honest - my work here was pretty much guess work in trying to get things to work the way they needed. 

I actually think we're doing too much work on desktop (preprocessing the search files). That work is unnecessary now.

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 8865651 [details]
Bug 1362496 - Add chrome-% for Firefox Desktop.

https://reviewboard.mozilla.org/r/137266/#review158502

I think this is OK. Ideally we wouldn't repeat ourselves, but I think trying to generalize this in a make function would just wind up making it uglier.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4da81020780
Add chrome-% for Firefox Desktop. r=ted

Comment 19

[(not currently active) Ted Mielczarek]

We definitely want to make things better, and our current plan is to start moving on that once Callek gets his l10n automation in the right state so we're not making his life harder.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4da81020780