Add chrome-% for Firefox

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Build Config
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

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

It is needed for multi-locale Firefox desktop builds.
(Assignee)

Updated

a year ago
Blocks: 1358824
(Assignee)

Comment 1

a year ago
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
```
Flags: needinfo?(bugspam.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?
Flags: needinfo?(bugspam.Callek)
(Assignee)

Comment 3

a year ago
:coop, can you mentor me through this or point out the right person to help me?
Flags: needinfo?(coop)
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.
CCing Mike, since he's the one who's been working on centralizing search for both mobile and desktop.
(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.
Flags: needinfo?(coop)
Comment hidden (mozreview-request)
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?
Component: General → Build Config
Product: Localization Infrastructure and Tools → Firefox
(Assignee)

Comment 9

a year ago
> 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.
Attachment #8865651 - Flags: review?(l10n)
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.
What is chrome-% exactly?
(Assignee)

Updated

a year ago
Flags: needinfo?(gandalf)
(Assignee)

Comment 12

a year ago
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.
Flags: needinfo?(gandalf)
Attachment #8865651 - Flags: review?(ted)
(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.
(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.
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.
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.
(Assignee)

Updated

a year ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED

Comment 17

a year ago
mozreview-review
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.
Attachment #8865651 - Flags: review?(ted) → review+

Comment 18

a year ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4da81020780
Add chrome-% for Firefox Desktop. r=ted
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e4da81020780
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.