Hi, This version of the patch is looking good to me. I noticed a few intltool-related things you can remove from .gitignore: /intltool-*, /po/.intltool-merge-cache, /m4/intltool.m4.
On Tue, Aug 29, 2017 at 1:55 PM Aleksander Morgado <[email protected]> wrote: > Hey Daiki, > > On Tue, Aug 29, 2017 at 4:20 PM, Daiki Ueno <[email protected]> wrote: > > Hello, > > > > (Sending this off-list, as the downloadable archive is not updated yet: > > > https://lists.freedesktop.org/archives/modemmanager-devel/2017-August.txt.gz > ) > > > > Adding the mailing list in CC of this reply. > > > Here are quick comments: > > > >> data/polkit-policy-i18n.its | 6 ++ > >> data/polkit-policy-i18n.loc | 4 ++ > > > > To use custom ITS scripts in your package, you need to put them under > > "its/" directory and adjust GETTEXTDATADIR pointing to the parent > > directory. In this particular case, you might want to put them under: > > > > data/its/polkit-policy-i18n.its > > data/its/polkit-policy-i18n.loc > > > > and set GETTEXTDATADIR around the xgettext and msgfmt calls: > > > > GETTEXTDATADIR=$(top_srcdir)/data > > > > Is this mandatory? It looks like adding them under data/ directly > worked ok, without any explicit GETTEXTDATADIR. > > > By the way, have you seen the upstream ITS scripts? > > > > https://cgit.freedesktop.org/polkit/tree/data/polkit.its > > https://cgit.freedesktop.org/polkit/tree/data/polkit.loc > > > > Oh, I didn't see them, nice to see they're basically the same as what > I did :) What's the way to go with this? Should I import those files > into the MM sources, or reference somehow the ones installed in > /usr/share/gettext/its/, or none of those required? > I would suggest referencing the ones installed in the system, if that's possible. It may not be possible if you need to generate the translated policy files even when configured --without-polkit. > >> --- a/configure.ac > >> +++ b/configure.ac > >> @@ -82,15 +82,15 @@ > dnl----------------------------------------------------------------------------- > >> dnl i18n > >> dnl > >> > >> -IT_PROG_INTLTOOL([0.40.0]) > >> - > >> AM_GNU_GETTEXT([external]) > >> -AM_GNU_GETTEXT_VERSION([0.19.3]) > >> +AM_GNU_GETTEXT_VERSION([0.19.8]) > >> > >> GETTEXT_PACKAGE=ModemManager > >> AC_SUBST(GETTEXT_PACKAGE) > >> AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE,"$GETTEXT_PACKAGE", [Gettext > package]) > >> > >> +AC_PATH_PROG([MSGFMT], [msgfmt]) > > > > This should be already set by AM_GNU_GETTEXT. > > > > Ok, will remove it > > >> --- a/data/Makefile.am > >> +++ b/data/Makefile.am > >> @@ -61,37 +61,36 @@ diagrams = \ > >> > >> > >> # Polkit > >> -polkit_policy_in_in_files = org.freedesktop.ModemManager1.policy.in.in > >> if WITH_POLKIT > >> +org.freedesktop.ModemManager1.policy: > org.freedesktop.ModemManager1.policy.in > >> + $(AM_V_GEN) $(MSGFMT) --xml -d $(top_srcdir)/po/ -o $@ --template > $< > > [...] > >> DISTCLEANFILES = \ > >> + org.freedesktop.ModemManager1.policy \ > > > > I see that .policy.in.in is expanded twice: first by AC_CONFIG_FILES, > > and then by msgfmt. That would require msgfmt installed on user's > > system. > > > > It's probably reasonable to assume msgfmt is available everywhere these > > days, but if you ever want to avoid the dependency, you could: > > > > - first expand translations with msgfmt and add resulting .policy.in to > > EXTRA_DIST > > - secondly, expand variables in .policy.in with sed at 'make' time, > > instead of AC_CONFIG_FILES > > > > I had that order of generation in the v2 patch of the file actually, > so may just switch back to it. > > Thanks for checking this! > > -- > Aleksander > https://aleksander.es >
_______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
