-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New find_xgettext method to i18n module #13900
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,3 +74,70 @@ for normal keywords. In addition it accepts these keywords: | |
* `mo_targets` *required*: mo file generation targets as returned by `i18n.gettext()`. | ||
|
||
*Added 0.62.0* | ||
|
||
|
||
### i18n.find_xgettext() | ||
|
||
``` meson | ||
i18n.find_xgettext(args: [...], required: true) | ||
``` | ||
|
||
Find the `xgettext` program, and return a `XgettextProgram` object. | ||
`args` are a list of arguments to provide to `xgettext`. | ||
If `required` is `true` and `xgettext` cannot be found, it will result in | ||
an error. | ||
|
||
This function is to be used when the `gettext` function workflow it not suitable | ||
for your project. For example, it can be used to produce separate `.pot` files | ||
for each executable. | ||
|
||
*Added 1.7.0* | ||
|
||
#### `XgettextProgram` methods | ||
|
||
`XgettextProgram` is an [[@external_program]]. Therefore, it has the same methods, | ||
and can be use in any places requiring an `ExternalProgram`. Furthermore, it has | ||
the following method: | ||
|
||
##### `extract()` | ||
|
||
Positional arguments are the following: | ||
|
||
- name `str`: the name of the resulting pot file. | ||
- sources `list[str|File|build_tgt|custom_tgt]`: | ||
source files or targets. May be a list of `string`, `File`, [[@build_tgt]], | ||
or [[@custom_tgt]] returned from other calls to this function. | ||
|
||
Keyword arguments are the following: | ||
|
||
- merge `bool`: | ||
if `true`, will merge the resulting pot file with extracted pot files | ||
related to dependencies of the given source targets. For instance, | ||
if you build an executable, then you may want to merge the executable | ||
translations with the translations from the dependent libraries. | ||
- install `bool`: if `true`, will add the resulting pot file to install targets. | ||
- install_tag `str`: install tag to use for the install target. | ||
- install_dir `str`: directory where to install the resulting pot file. | ||
- alias `list[build_tgt]`: | ||
a list of build targets using the same resulting pot file. For | ||
instance, if you build both a static and a shared library from the | ||
same sources, you may specify the static library target as an alias, | ||
to use the generated pot file when that static library is used as a | ||
dependency of another target. | ||
Comment on lines
+121
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really understand what this is doing. But it seems backwards somehow. |
||
|
||
The `extract()` method returns a [[@custom_tgt]]. | ||
|
||
|
||
Usually, you want to pass one build target as sources, and the list of header files | ||
for that target. If the number of source files would result in a command line that | ||
is too long, the list of source files is written to a file at config time, to be | ||
used as input for the `xgettext` program. | ||
|
||
The `merge: true` argument is to be given to targets that will actually read | ||
the resulting `.mo` file. Each time you call the `extract()` method, it maps the | ||
source targets to the resulting pot file. When `merge: true` is given, all | ||
generated pot files from dependencies of the source targets are included to | ||
generate the final pot file. Therefore, adding a dependency to source target | ||
will automatically add the translations of that dependency to the needed | ||
translations for that source target. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
## i18n module find_xgettext | ||
|
||
There is a new `find_xgettext` function in `i18n` module that acts as a | ||
wrapper around `xgettext`. It allows to extract strings to translate from | ||
source files. | ||
|
||
This function is convenient, because: | ||
- It can find the sources files from a build target; | ||
- It will use an intermediate file when the number of source files is too | ||
big to be handled directly from the command line; | ||
- It is able to get strings to translate from the dependencies of the given | ||
targets. | ||
Comment on lines
+1
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ultimately the goal here is just to avoid the i18n.gettext() assumption that POTFILES is the source of truth for xgettext, right? It feels like this function adds complexity which doesn't directly serve that. In particular, it seems odd to have a function which finds the program, and then have that yield an object with a single method. We could instead directly provide an |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
from __future__ import annotations | ||
|
||
from os import path | ||
from pathlib import Path | ||
import itertools | ||
import shlex | ||
import typing as T | ||
|
||
|
@@ -13,8 +15,10 @@ | |
from ..options import OptionKey | ||
from .. import mlog | ||
from ..interpreter.type_checking import CT_BUILD_BY_DEFAULT, CT_INPUT_KW, INSTALL_TAG_KW, OUTPUT_KW, INSTALL_DIR_KW, INSTALL_KW, NoneType, in_set_validator | ||
from ..interpreterbase import FeatureNew, InvalidArguments | ||
from ..interpreterbase.decorators import ContainerTypeInfo, KwargInfo, noPosargs, typed_kwargs, typed_pos_args | ||
from ..interpreter.interpreterobjects import _ExternalProgramHolder | ||
from ..interpreterbase import FeatureNew | ||
from ..interpreterbase.exceptions import InterpreterException, InvalidArguments | ||
from ..interpreterbase.decorators import ContainerTypeInfo, KwargInfo, noPosargs, noKwargs, typed_kwargs, typed_pos_args | ||
from ..programs import ExternalProgram | ||
from ..scripts.gettext import read_linguas | ||
|
||
|
@@ -24,7 +28,7 @@ | |
from . import ModuleState | ||
from ..build import Target | ||
from ..interpreter import Interpreter | ||
from ..interpreterbase import TYPE_var | ||
from ..interpreterbase import TYPE_var, TYPE_kwargs | ||
|
||
class MergeFile(TypedDict): | ||
|
||
|
@@ -65,6 +69,21 @@ | |
its_files: T.List[str] | ||
mo_targets: T.List[T.Union[build.BuildTarget, build.CustomTarget, build.CustomTargetIndex]] | ||
|
||
class XgettextProgramT(TypedDict): | ||
|
||
args: T.List[str] | ||
required: bool | ||
|
||
class XgettextProgramExtract(TypedDict): | ||
|
||
merge: bool | ||
install: bool | ||
install_dir: T.Optional[str] | ||
install_tag: T.Optional[str] | ||
alias: T.List[T.Union[build.BuildTarget, build.BothLibraries]] | ||
|
||
SourcesType = T.Union[str, mesonlib.File, build.BuildTarget, build.BothLibraries, build.CustomTarget] | ||
|
||
|
||
_ARGS: KwargInfo[T.List[str]] = KwargInfo( | ||
'args', | ||
|
@@ -115,6 +134,161 @@ | |
} | ||
|
||
|
||
class XgettextProgram(ExternalProgram): | ||
Check failure Code scanning / CodeQL Missing call to `__init__` during object initialization Error
Class XgettextProgram may not be initialized properly as
method ExternalProgram.__init__ Error loading related location Loading __init__ method Error loading related location Loading |
||
|
||
def __init__(self, xgettext: ExternalProgram, args: T.List[str]): | ||
self.__dict__.update(xgettext.__dict__) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love this, but I understand why you did this. |
||
self.command.extend(args) | ||
|
||
self.pot_files: T.Dict[str, build.CustomTarget] = {} | ||
|
||
def extract(self, | ||
name: str, | ||
sources: T.List[SourcesType], | ||
merge: bool, | ||
install: bool, | ||
install_dir: T.Optional[str], | ||
install_tag: T.Optional[str], | ||
alias: T.List[T.Union[build.BuildTarget, build.BothLibraries]], | ||
interpreter: Interpreter) -> build.CustomTarget: | ||
|
||
if not name.endswith('.pot'): | ||
name += '.pot' | ||
|
||
source_files = self._get_source_files(sources, interpreter) | ||
|
||
command = self.command.copy() | ||
command.append(f'--directory={interpreter.environment.get_source_dir()}') | ||
command.append(f'--directory={interpreter.environment.get_build_dir()}') | ||
command.append('--output=@OUTPUT@') | ||
|
||
depends = list(self._get_depends(sources)) if merge else [] | ||
rsp_file = self._get_rsp_file(name, source_files, depends, command, interpreter) | ||
inputs: T.List[T.Union[mesonlib.File, build.CustomTarget]] | ||
if rsp_file: | ||
inputs = [rsp_file] | ||
depend_files = list(source_files) | ||
command.append('--files-from=@INPUT@') | ||
else: | ||
inputs = list(source_files) + depends | ||
depends = None | ||
depend_files = None | ||
command.append('@INPUT@') | ||
|
||
ct = build.CustomTarget( | ||
'', | ||
interpreter.subdir, | ||
interpreter.subproject, | ||
interpreter.environment, | ||
command, | ||
inputs, | ||
[name], | ||
depend_files = depend_files, | ||
extra_depends = depends, | ||
install = install, | ||
install_dir = [install_dir] if install_dir else None, | ||
install_tag = [install_tag] if install_tag else None, | ||
description = 'Extracting translations to {}', | ||
) | ||
|
||
for source_id in self._get_source_id(itertools.chain(sources, alias)): | ||
self.pot_files[source_id] = ct | ||
self.pot_files[ct.get_id()] = ct | ||
|
||
interpreter.add_target(ct.name, ct) | ||
return ct | ||
|
||
def _get_source_files(self, sources: T.Iterable[SourcesType], interpreter: Interpreter) -> T.Set[mesonlib.File]: | ||
source_files = set() | ||
for source in sources: | ||
if isinstance(source, mesonlib.File): | ||
source_files.add(source) | ||
elif isinstance(source, str): | ||
mesonlib.check_direntry_issues(source) | ||
source_files.add(mesonlib.File.from_source_file(interpreter.source_root, interpreter.subdir, source)) | ||
elif isinstance(source, build.BuildTarget): | ||
source_files.update(source.get_sources()) | ||
elif isinstance(source, build.BothLibraries): | ||
source_files.update(source.get('shared').get_sources()) | ||
return source_files | ||
|
||
def _get_depends(self, sources: T.Iterable[SourcesType]) -> T.Set[build.CustomTarget]: | ||
depends = set() | ||
for source in sources: | ||
if isinstance(source, build.BuildTarget): | ||
for source_id in self._get_source_id(source.get_dependencies()): | ||
if source_id in self.pot_files: | ||
depends.add(self.pot_files[source_id]) | ||
elif isinstance(source, build.CustomTarget): | ||
# Dependency on another extracted pot file | ||
source_id = source.get_id() | ||
if source_id in self.pot_files: | ||
depends.add(self.pot_files[source_id]) | ||
return depends | ||
|
||
def _get_rsp_file(self, | ||
name: str, | ||
source_files: T.Iterable[mesonlib.File], | ||
depends: T.Iterable[build.CustomTarget], | ||
arguments: T.List[str], | ||
interpreter: Interpreter) -> T.Optional[mesonlib.File]: | ||
source_list = '\n'.join(source.relative_name() for source in source_files) | ||
for dep in depends: | ||
source_list += '\n' + path.join(dep.subdir, dep.get_filename()) | ||
|
||
estimated_cmdline_length = len(source_list) + len(arguments) + sum(len(arg) for arg in arguments) + 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this code is correct, but it looks really odd because of the way your count the spaces in arguments, I think: # count the space between each argument as well
estimated_cmdline_length = len(source_list) + sum(len(arg) + 1 for arg in arguments) + 1 would be more clear |
||
if estimated_cmdline_length < 8000: | ||
# Maximum command line length on Windows is 8191 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have a helper function to calculate an rsp threshold in the ninja backend, which includes an environment variable to tune whether we use an rsp file or not. I wonder if it would be appropriate to use that function here (maybe moved to |
||
# Limit on other OS is higher, but a too long command line wouldn't | ||
# be practical in any ways. | ||
return None | ||
|
||
rsp_file = Path(interpreter.environment.build_dir, interpreter.subdir, name+'.rsp') | ||
rsp_file.write_text(source_list, encoding='utf-8') | ||
|
||
return mesonlib.File.from_built_file(interpreter.subdir, rsp_file.name) | ||
|
||
def _get_source_id(self, sources: T.Iterable[T.Union[SourcesType, build.CustomTargetIndex]]) -> T.Iterable[str]: | ||
for source in sources: | ||
if isinstance(source, build.Target): | ||
yield source.get_id() | ||
elif isinstance(source, build.BothLibraries): | ||
yield source.get('static').get_id() | ||
yield source.get('shared').get_id() | ||
|
||
|
||
class XgettextProgramHolder(_ExternalProgramHolder[XgettextProgram]): | ||
|
||
def __init__(self, xgettext_program: XgettextProgram, interpreter: Interpreter) -> None: | ||
super().__init__(xgettext_program, interpreter) | ||
self.methods.update({ | ||
'extract': self.extract_method, | ||
}) | ||
|
||
@typed_pos_args('XgettextProgram.extract', str, varargs=(str, mesonlib.File, build.BuildTarget, build.BothLibraries, build.CustomTarget), min_varargs=1) | ||
@typed_kwargs( | ||
'XgettextProgram.extract', | ||
KwargInfo('merge', bool, default=False), | ||
KwargInfo('alias', ContainerTypeInfo(list, (build.BuildTarget, build.BothLibraries)), listify=True, default=[]), | ||
INSTALL_KW, | ||
INSTALL_DIR_KW, | ||
INSTALL_TAG_KW, | ||
) | ||
def extract_method(self, args: T.Tuple[str, T.List[SourcesType]], kwargs: XgettextProgramExtract) -> build.CustomTarget: | ||
if kwargs['install'] and not kwargs['install_dir']: | ||
raise InvalidArguments('XgettextProgram.extract: "install_dir" keyword argument must be set when "install" is true.') | ||
|
||
if not self.held_object.found(): | ||
raise InterpreterException('XgettextProgram.extract: "xgettext" command not found.') | ||
|
||
return self.held_object.extract(*args, **kwargs, interpreter=self.interpreter) | ||
|
||
@noPosargs | ||
@noKwargs | ||
def found_method(self, args: TYPE_var, kwargs: TYPE_kwargs) -> bool: | ||
return self.held_object.found() | ||
|
||
|
||
class I18nModule(ExtensionModule): | ||
|
||
INFO = ModuleInfo('i18n') | ||
|
@@ -125,6 +299,7 @@ | |
'merge_file': self.merge_file, | ||
'gettext': self.gettext, | ||
'itstool_join': self.itstool_join, | ||
'find_xgettext': self.find_xgettext, | ||
}) | ||
self.tools: T.Dict[str, T.Optional[T.Union[ExternalProgram, build.Executable]]] = { | ||
'itstool': None, | ||
|
@@ -398,6 +573,23 @@ | |
|
||
return ModuleReturnValue(ct, [ct]) | ||
|
||
@FeatureNew('i18n.find_xgettext', '1.7.0') | ||
@noPosargs | ||
@typed_kwargs( | ||
'i18n.find_xgettext', | ||
_ARGS, | ||
KwargInfo('required', bool, default=True), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use the shared option that also handles UserFeatureOptions? Or add the disabler argument. There may be something I'm missing here. |
||
) | ||
def find_xgettext(self, state: ModuleState, args: TYPE_var, kwargs: XgettextProgramT) -> XgettextProgram: | ||
tool = 'xgettext' | ||
if self.tools[tool] is None or not self.tools[tool].found(): | ||
self.tools[tool] = state.find_program(tool, required=kwargs['required'], for_machine=mesonlib.MachineChoice.BUILD) | ||
|
||
xgettext = T.cast(ExternalProgram, self.tools[tool]) | ||
return XgettextProgram(xgettext, kwargs['args']) | ||
|
||
|
||
def initialize(interp: 'Interpreter') -> I18nModule: | ||
return I18nModule(interp) | ||
mod = I18nModule(interp) | ||
mod.interpreter.append_holder_map(XgettextProgram, XgettextProgramHolder) | ||
return mod |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
project( | ||
'gettext extractor', | ||
'c', | ||
default_options: {'default_library': 'static'}, | ||
meson_version: '1.7.0', | ||
) | ||
|
||
i18n = import('i18n') | ||
xgettext = i18n.find_xgettext( | ||
args: ['-ktr', '--add-comments=TRANSLATOR:', '--from-code=UTF-8'], | ||
required: false, | ||
) | ||
|
||
if not xgettext.found() | ||
error('MESON_SKIP_TEST xgettext command not found') | ||
endif | ||
|
||
subdir('src') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
#include "lib1.h" | ||
|
||
#include <stdio.h> | ||
|
||
#define tr(STRING) (STRING) | ||
|
||
void say_something(void) | ||
{ | ||
printf("%s\n", tr("Something!")); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#ifndef LIB1_H | ||
#define LIB1_H | ||
|
||
void say_something(void); | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
lib1 = library('mylib1', 'lib1.c') | ||
lib1_pot = xgettext.extract('lib1', lib1) | ||
lib1_includes = include_directories('.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should be called "recursive" instead of "merge".