475113 - make profile info easier to localize

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Robert Kaiser]

Bug 461979 - make profile info easier to localize

We should port as much of this as we can to suite/ but we need some good idea what to do about the bookmarks file.

Comment 1

[Robert Kaiser]

Attachment: move most awkward-for-localizers stuff out of L10n

Here is a patch that should move most awkward stuff out of the area localizers need to touch but still leaves a number of points where they can customize the localized builds.

Axel, please review this from the POV of L10n and build system usage, I hope it's somewhat sane what I'm doing here.

Neil, the files coming out of this process for default en-US builds are the same with the exception of changed bookmarks (not sure if your OK is enough or if we should raise those changes with the Council or so) and another exception is that the one left-over search category description went into a .dtd.

The patch needs thorough testing with an actual L10n, but I have already tested that the include stuff works with files I intend to use in the de L10n (e.g. adding an additional sidebar panel with stuff from the SeaMonkey SeaMonkey site).

Comment 2

[Robert Kaiser]

As those are all new files on the L10n side and l10n-central isn't actively used for SeaMonkey anyway right now, I added files there for testing, see http://hg.mozilla.org/l10n-central/de/rev/daa89ff60bc3 - I tested a new profile in a build with the patch and those files, and everything checks out as it should.

Comment 3

[[e-mail]]

Comment on attachment 358615 [details] [diff] [review]
move most awkward-for-localizers stuff out of L10n

>+%/defaults/profile/bookmarks.html: bookmarks.inc generic/profile/bookmarks.html.in
>+	$(SYSINSTALL) -D $(dir $@)
>+	$(RM) ./more-bookmarks.inc bookmarks.html.in
Why the ./ (if you needed it, each file would need it)

>+	cp $(srcdir)/generic/profile/bookmarks.html.in .
>+	cp $(LOCALE_SRCDIR)/profile/more-bookmarks.inc .
Would it be easier to use $^ ? (What are the vpaths for anyway?)

>+	$(PYTHON) $(MOZILLA_SRCDIR)/config/Preprocessor.py \
>+	  -I $< \
I think an explicit #include in bookmarks.html.in would be more obvious.

>+	cp $(srcdir)/generic/profile/panels.rdf.in .
>+	cp $< .
$< only gives you the first file... $^ would be better?

>+	grep "<RDF:Description about=" panels.inc | \
>+	  $(PERL) -pe 's/.*<RDF:Description about="(.*)">.*/        <RDF:li resource="\1"\/>/g;' \
>+	  > panels-urn.inc
If you can't do it all in Perl then you're doing something wrong ;-)
As I see it you have two choices:
> sed -n 's/.*<RDF:Description about="\(.*\)">.*/        <RDF:li resource="\1"\/>/p'
> $(PERL) -ne 'print if s/.*<RDF:Description about="(.*)">.*/        <RDF:li resource="\1"\/>/g;'

>+libs:: $(FINAL_TARGET)/defaults/profile/bookmarks.html ;
>+libs:: $(FINAL_TARGET)/defaults/profile/search.rdf ;
>+libs:: $(FINAL_TARGET)/defaults/profile/panels.rdf ;
Are those semicolons correct?

>+# LOCALIZATION NOTE: The 'en-US' strings in somee URLs will be replaced with
Typo: some(e)

>+# LOCALIZATION NOTE (seamonkey_l10n):
>+# insert full bookmark line for localized SeaMonkey page (personal toolbar)
>+# e.g. #define seamonkey_l10n <DT><A HREF="http://www.seamonkey.tlh/">SeaMonkey tlhIngan</a>
I take it this example is from your Klingon langpack? It might be an idea to use a working example ;-)

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> (From update of attachment 358615 [details] [diff] [review])
> >+%/defaults/profile/bookmarks.html: bookmarks.inc generic/profile/bookmarks.html.in
> >+	$(SYSINSTALL) -D $(dir $@)
> >+	$(RM) ./more-bookmarks.inc bookmarks.html.in
> Why the ./ (if you needed it, each file would need it)

Without it, it came around to delete the file in $(LOCALE_SRCDIR)/profile/ which is not what we want, I guess...
This is a bit whacky though, and I think we probably should place the copied files in a subdir and clean up that subdir.

> >+	cp $(srcdir)/generic/profile/bookmarks.html.in .
> >+	cp $(LOCALE_SRCDIR)/profile/more-bookmarks.inc .
> Would it be easier to use $^ ? (What are the vpaths for anyway?)

The vpaths are for making locale-merge work. How that works is pretty undocumented though, I hope Axel can tell if I'm doing the right thing for it.

> >+	$(PYTHON) $(MOZILLA_SRCDIR)/config/Preprocessor.py \
> >+	  -I $< \
> I think an explicit #include in bookmarks.html.in would be more obvious.

In that case, I need to copy this file file as well, as includes are relative to the file it's included in and therefore cannot cross the srcdir/objdir "barrier".
 
> If you can't do it all in Perl then you're doing something wrong ;-)

No, in that case I just know too little what I'm doing ;-)

> As I see it you have two choices:

Thanks for those, I guess I'll go with the sed one (given it works out) to not make us depend on perl more.

> >+libs:: $(FINAL_TARGET)/defaults/profile/bookmarks.html ;
> >+libs:: $(FINAL_TARGET)/defaults/profile/search.rdf ;
> >+libs:: $(FINAL_TARGET)/defaults/profile/panels.rdf ;
> Are those semicolons correct?

I copied them from what Axel did for Firefox but I don't know why they are there.

> >+# LOCALIZATION NOTE (seamonkey_l10n):
> >+# insert full bookmark line for localized SeaMonkey page (personal toolbar)
> >+# e.g. #define seamonkey_l10n <DT><A HREF="http://www.seamonkey.tlh/">SeaMonkey tlhIngan</a>
> I take it this example is from your Klingon langpack? It might be an idea to
> use a working example ;-)

I intentionally used something as an example that isn't in any real langpack we have and that clearly doesn't exist...

Comment 5

[[e-mail]]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 358615 [details] [diff] [review])
> > >+%/defaults/profile/bookmarks.html: bookmarks.inc generic/profile/bookmarks.html.in
> > >+	$(SYSINSTALL) -D $(dir $@)
> > >+	$(RM) ./more-bookmarks.inc bookmarks.html.in
> > Why the ./ (if you needed it, each file would need it)
> Without it, it came around to delete the file in $(LOCALE_SRCDIR)/profile/
> which is not what we want, I guess...
Weird, I didn't expect that.

> > >+	$(PYTHON) $(MOZILLA_SRCDIR)/config/Preprocessor.py \
> > >+	  -I $< \
> > I think an explicit #include in bookmarks.html.in would be more obvious.
> In that case, I need to copy this file file as well, as includes are relative
> to the file it's included in and therefore cannot cross the srcdir/objdir
> "barrier".
Well, you're already copying everything else, so it would look more consistent.

Comment 6

[[e-mail]]

Comment on attachment 358615 [details] [diff] [review]
move most awkward-for-localizers stuff out of L10n

OK, so the idea is sound, but do anything you can to clean up the Makefile.in!

Comment 7

[Robert Kaiser]

Attachment: address Neil's comments, make things cleaner

This patch addresses Neil's comments and makes things cleaner and more reliable by putting all the copied/included files in a profile/ subdir.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 359619 [details] [diff] [review]
address Neil's comments, make things cleaner

Chatted with KaiRo, I've gotten better since I did the Firefox part.

In particular for individual files, going with $(firstword $(wildcard ) ) should be better than vpath, see http://mxr.mozilla.org/mobile-browser/source/locales/Makefile.in for examples.

Comment 9

[Robert Kaiser]

Attachment: use firstword instead of vpath

This patch uses firstword instead of vpath, as Axel requested, hopefully making the path even cleaner :)

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 371855 [details] [diff] [review]
use firstword instead of vpath

r=me, too. Sorry for the lag.

Comment 11

[Robert Kaiser]

Pushed as http://hg.mozilla.org/comm-central/rev/82c4914ad8ea

Comment 12

[Robert Kaiser]

Attachment: Also adjust filter.py

This small followup is needed so that checks don't fail on locales actually using the more-bookmarks.inc and panels.inc files as intended.

Neil, can you at least rs this one? It doesn't influence the actual building process, just the output of compare-locales, which localizers use for checking if everything is alright (and which drives http://l10n.mozilla.org/dashboard/ output).

Comment 13

[[e-mail]]

Comment on attachment 375334 [details] [diff] [review]
Also adjust filter.py

rs=me

Comment 14

[Robert Kaiser]

filter.py fixup pushed as http://hg.mozilla.org/comm-central/rev/84d75e481b54

Comment 15

[Robert Kaiser]

Axel, the filter.py fixup makes compare-locales not generate a "missing string" not, but it still comes up with an "Unparsed content" error in dashboard... Is there a way to make compare-locales ignore those two files completely?

Comment 16

[Axel Hecht [:Pike]]

This slipped my review, I have a grammar in compare-locales that makes sure that doing something like

PreProcessor.py -I defines.inc install.rdf.in > install.rdf

is more or less safe, by erroring on random content that's not a blank line. Not really  fool proof, but better than nothing, catching stuff like

define FOO "This is broken due to the missing leading #"

The way to not run into this is to rename the included files that don't follow this scheme to have a different extension. Not nice, but I think the best compromise.

Comment 17

[Robert Kaiser]

Attachment: Really fix compare-locales choking by renaming away from .inc

OK, so renaming the files that allow arbitrary content away from .inc should fix the choking of compare-locales. With moving to .extra, the additional bookmarks file can also drop the more- prefix.

Comment 18

[Robert Kaiser]

Pushed additional fix as http://hg.mozilla.org/comm-central/rev/c710b0fa3b57

Comment 19

[Robert Kaiser]

Missed file removes in last checkin, corrected with http://hg.mozilla.org/comm-central/rev/30b786110f58

Comment 20

[[e-mail]]

Attachment: I don't like $(SYSINSTALL) -D

The only other usage of this strange style is in browser/locales/Makefile.in which is presumably where you copied it from, but everywhere else seems to prefer $(NSINSTALL) which works just as well (or better, in my case!)

Comment 21

[[e-mail]]

Comment on attachment 377967 [details] [diff] [review]
I don't like $(SYSINSTALL) -D

Pushed changeset eefe315765e2 to comm-central.