search

OpenPrinting / system-config-printer

Graphical user interface for CUPS administration
GNU General Public License v2.0
156 stars 87 forks source link

build: Migrate from Intltool to Gettext #243

Closed inigomartinez closed 2 years ago

inigomartinez commented 2 years ago

Recent versions of Gettext are able to translate several formats without using Intltool tools.

The build system has been migrated from Intltool to Gettext.

zdohnal commented 2 years ago

Hi Inigo,

thank you for the PR!

I have a several notes and questions regarding the PR, I'll post them after review.

zdohnal commented 2 years ago

Inigo,

yesterday I briefly checked the manual for intltool -> gettext migration 1 2 - did you follow the steps there?

There are some changes mentioned regarding:

  1. po/Makevars - some updates to options, some new options...
  2. Makefile.am - SUBDIRS having '. po', having desktop file in CLEANFILES, having desktop and appdata xml in po/POTFILES.in

Is there a reason why the changes weren't useful?

I'm not questioning the proposed solution, I just want to understand why it differs from the manual...

inigomartinez commented 2 years ago

yesterday I briefly checked the manual for intltool -> gettext migration 1 2 - did you follow the steps there?

Yes, mainly, because I also checked some of the different gettext migrations I worked in the past, though it is true that not all of them have the same directory structure.

1. po/Makevars - some updates to options, some new options...

Actually they are not necessary because only system-config-printer.desktop needed some custom treatment due to the inclusion of the X-GNOME-FullName key. As far as I know, X-GNOME keys have been deprecated for some time but I have also included it for translation.

2. Makefile.am - SUBDIRS having '. po', having desktop file in CLEANFILES, having desktop and appdata xml in po/POTFILES.in

I have changed the SUBDIRS variable to follow the proposed order. I have also added the desktop files to the CLEANFILES variable.

I'm not questioning the proposed solution, I just want to understand why it differs from the manual...

No problem. I overlooked the SUBDIRS step because it wasn't needed in other packages, and I missed the CLEANFILES update. They are fixed now.

zdohnal commented 2 years ago 
1. po/Makevars - some updates to options, some new options...

Actually they are not necessary because only system-config-printer.desktop needed some custom treatment due to the inclusion of the X-GNOME-FullName key.

I was mainly interested in the --from-code=UTF-8 in XGETTEXT_OPTIONS which is mentioned as needed but currently isn't in Makevars - is it unnecessary to have it there? If so, why?

As far as I know, X-GNOME keys have been deprecated for some time but I have also included it for translation.

Thanks for the info! Then IMO we can get rid of it after we merge the PR.

2. Makefile.am - SUBDIRS having '. po', having desktop file in CLEANFILES, having desktop and appdata xml in po/POTFILES.in

I have changed the SUBDIRS variable to follow the proposed order. I have also added the desktop files to the CLEANFILES variable.

IMO we should update po/POTFILES.in as well, to reflect we want to translate the created desktop and appdata files, not their .in scripts. Does it make sense?

No problem. I overlooked the SUBDIRS step because it wasn't needed in other packages, and I missed the CLEANFILES update. They are fixed now.

Thank you for the update!

inigomartinez commented 2 years ago

I was mainly interested in the --from-code=UTF-8 in XGETTEXT_OPTIONS which is mentioned as needed but currently isn't in Makevars - is it unnecessary to have it there? If so, why?

I have also added it, along with the recommended flags, to be consistent with the guide and the rest of the GNOME packages but it is not actually necessary because the files to be translated are pure ASCII files and don't include any UTF-8 characters (yet?), only the generated files do have them.

IMO we should update po/POTFILES.in as well, to reflect we want to translate the created desktop and appdata files, not their .in scripts. Does it make sense?

No, actually the in files, where the strings to be translated are, is the one that must be present there. The generated desktop and appdata files has the strings to be translated along with the translated strings, which must not be translated.

zdohnal commented 2 years ago

I was mainly interested in the --from-code=UTF-8 in XGETTEXT_OPTIONS which is mentioned as needed but currently isn't in Makevars - is it unnecessary to have it there? If so, why?

I have also added it, along with the recommended flags, to be consistent with the guide and the rest of the GNOME packages but it is not actually necessary because the files to be translated are pure ASCII files and don't include any UTF-8 characters (yet?), only the generated files do have them.

Ok, noted.

IMO we should update po/POTFILES.in as well, to reflect we want to translate the created desktop and appdata files, not their .in scripts. Does it make sense?

No, actually the in files, where the strings to be translated are, is the one that must be present there. The generated desktop and appdata files has the strings to be translated along with the translated strings, which must not be translated.

Aha, ok - I've just followed the guide and it says the generated files should be there, so I thought it is an error in PR.

I started to see a warning message when compilation goes into po folder:

Makefile:210: warning: ignoring prerequisites on suffix rule definition

Can you investigate and possibly fix it?

The last mentioned options from Makevars are PO_DEPENDS_ON_POT and DIST_DEPENDS_ON_UPDATE_PO - are those set to 'no' by default?

inigomartinez commented 2 years ago

I started to see a warning message when compilation goes into po folder:

Makefile:210: warning: ignoring prerequisites on suffix rule definition

Can you investigate and possibly fix it?

Seems that it is something triggered by files generated by the autopoint script. I suppose that with newer versions updated files that don't trigger that warning will be generated overwriting old files..

Regarding the warning, here you can find more information.

The last mentioned options from Makevars are PO_DEPENDS_ON_POT and DIST_DEPENDS_ON_UPDATE_PO - are those set to 'no' by default?

Seems that is set to yes but it's not fully clear. Actually this is not necessary until domain is changed, and these two options could be added if domain change happens. However, I'm also including these options now.

zdohnal commented 2 years ago

Hmm, something still updates Makefile.in.in and Makefile.in with the incorrect targets, I need to figure out what to set for autotools to make it work. But since the warning isn't critical (because it probably didn't work even before the PR), I'll merge the PR.

I need to update the other automated files as well.