From f6eaf5590663a3da406bd2bf600bcbbbdeb49a44 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Sun, 26 Dec 2021 17:15:04 -0500 Subject: add_*_script: fix missing FeatureNew for non-string arguments In commit 2c0eaf5c4f4493146355eeb8521c17a3c2ef5acd support was added for install scripts to accept found programs, built executables, or custom targets. In commit c239ce31f55579cfe1e29b769a8bda97deca2166, this was extended to dist and postconf scripts too (although it was documented that those should not accept targets that are built by ninja). Despite the commit/PR claiming that all of these should always accept files and configured files, this was only true for arguments other than the first, until commit f808c955eab983b31feee130f0947c7cb254a94f. In amongst all this, FeatureNew checks were never registered for the first argument, only for additional arguments, until late in the game with the addition of FeatureNew checks for File objects. Fix this in part by moving the 3 different File checks into one, inside the function that processes the first script, and make that function check for FeatureNew on anything else too. --- mesonbuild/interpreter/mesonmain.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) (limited to 'mesonbuild/interpreter/mesonmain.py') diff --git a/mesonbuild/interpreter/mesonmain.py b/mesonbuild/interpreter/mesonmain.py index e973c5d..5a7fc74 100644 --- a/mesonbuild/interpreter/mesonmain.py +++ b/mesonbuild/interpreter/mesonmain.py @@ -79,13 +79,19 @@ class MesonMain(MesonInterpreterObject): }) def _find_source_script( - self, prog: T.Union[str, mesonlib.File, build.Executable, ExternalProgram], + self, name: str, prog: T.Union[str, mesonlib.File, build.Executable, ExternalProgram], args: T.List[str]) -> 'ExecutableSerialisation': largs: T.List[T.Union[str, build.Executable, ExternalProgram]] = [] + if isinstance(prog, (build.Executable, ExternalProgram)): + FeatureNew.single_use(f'Passing executable/found program object to script parameter of {name}', + '0.55.0', self.subproject, location=self.current_node) largs.append(prog) largs.extend(args) return self.interpreter.backend.get_executable_serialisation(largs) + elif isinstance(prog, mesonlib.File): + FeatureNew.single_use(f'Passing file object to script parameter of {name}', + '0.57.0', self.subproject, location=self.current_node) found = self.interpreter.find_program_impl([prog]) largs.append(found) largs.extend(args) @@ -147,12 +153,8 @@ class MesonMain(MesonInterpreterObject): args: T.Tuple[T.Union[str, mesonlib.File, build.Executable, ExternalProgram], T.List[T.Union[str, mesonlib.File, build.BuildTarget, build.CustomTarget, build.CustomTargetIndex, ExternalProgram]]], kwargs: 'AddInstallScriptKW') -> None: - if isinstance(args[0], mesonlib.File): - FeatureNew.single_use('Passing file object to script parameter of add_install_script', - '0.57.0', self.interpreter.subproject) - script_args = self._process_script_args('add_install_script', args[1], allow_built=True) - script = self._find_source_script(args[0], script_args) + script = self._find_source_script('add_install_script', args[0], script_args) script.skip_if_destdir = kwargs['skip_if_destdir'] script.tag = kwargs['install_tag'] self.build.install_scripts.append(script) @@ -168,11 +170,8 @@ class MesonMain(MesonInterpreterObject): args: T.Tuple[T.Union[str, mesonlib.File, ExternalProgram], T.List[T.Union[str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex]]], kwargs: 'TYPE_kwargs') -> None: - if isinstance(args[0], mesonlib.File): - FeatureNew.single_use('Passing file object to script parameter of add_postconf_script', - '0.57.0', self.interpreter.subproject) script_args = self._process_script_args('add_postconf_script', args[1], allow_built=True) - script = self._find_source_script(args[0], script_args) + script = self._find_source_script('add_postconf_script', args[0], script_args) self.build.postconf_scripts.append(script) @typed_pos_args( @@ -189,14 +188,11 @@ class MesonMain(MesonInterpreterObject): if args[1]: FeatureNew.single_use('Calling "add_dist_script" with multiple arguments', '0.49.0', self.interpreter.subproject) - if isinstance(args[0], mesonlib.File): - FeatureNew.single_use('Passing file object to script parameter of add_dist_script', - '0.57.0', self.interpreter.subproject) if self.interpreter.subproject != '': FeatureNew.single_use('Calling "add_dist_script" in a subproject', '0.58.0', self.interpreter.subproject) script_args = self._process_script_args('add_dist_script', args[1], allow_built=True) - script = self._find_source_script(args[0], script_args) + script = self._find_source_script('add_dist_script', args[0], script_args) self.build.dist_scripts.append(script) @noPosargs -- cgit v1.1 From 6e25548d06966a20bf7e5cc1fef184c4334c42df Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Sun, 26 Dec 2021 17:49:42 -0500 Subject: add_*_script: fix incorrect typed_pos_args allowing built targets In commit c239ce31f55579cfe1e29b769a8bda97deca2166 support was added to these functions to accept various non-string types. Despite the commit/PR documenting that only add_install_script is permitted to accept built files, the actual check parameter was set, for all three, to "True" (so the function was never invoked with False at all). This meant that actually attempting to use the allowed types would fail at postconf or dist, with python tracebacks in the former case and "Failed to run dist script" in the latter case. This was partially ameliorated in commit 6c5bfd4c241e94f5e0f4dea9ba7fb5d5090a4802 which added typed_pos_args, but unfortunately those typed_pos_args were all over the place. For postconf: - They banned external programs as additional args (which should be allowed) - They banned built executables (good) - They allowed custom targets as additional args (bad) For dist: - they allowed external programs (good) - they allowed built executables, but only as the first argument (bad, also ???) - they allowed custom targets, but only as additional arguments (bad, also ???) Fix this all to only allow the same argument types for both the script argument and the script-args arguments. That type is known at configure time and restricted to source files, configured files, and found programs. --- mesonbuild/interpreter/mesonmain.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'mesonbuild/interpreter/mesonmain.py') diff --git a/mesonbuild/interpreter/mesonmain.py b/mesonbuild/interpreter/mesonmain.py index 5a7fc74..819ff7f 100644 --- a/mesonbuild/interpreter/mesonmain.py +++ b/mesonbuild/interpreter/mesonmain.py @@ -162,28 +162,28 @@ class MesonMain(MesonInterpreterObject): @typed_pos_args( 'meson.add_postconf_script', (str, mesonlib.File, ExternalProgram), - varargs=(str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex) + varargs=(str, mesonlib.File, ExternalProgram) ) @noKwargs def add_postconf_script_method( self, args: T.Tuple[T.Union[str, mesonlib.File, ExternalProgram], - T.List[T.Union[str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex]]], + T.List[T.Union[str, mesonlib.File, ExternalProgram]]], kwargs: 'TYPE_kwargs') -> None: - script_args = self._process_script_args('add_postconf_script', args[1], allow_built=True) + script_args = self._process_script_args('add_postconf_script', args[1], allow_built=False) script = self._find_source_script('add_postconf_script', args[0], script_args) self.build.postconf_scripts.append(script) @typed_pos_args( 'meson.add_dist_script', - (str, mesonlib.File, build.Executable, ExternalProgram), - varargs=(str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex) + (str, mesonlib.File, ExternalProgram), + varargs=(str, mesonlib.File, ExternalProgram) ) @noKwargs def add_dist_script_method( self, - args: T.Tuple[T.Union[str, mesonlib.File, build.Executable, ExternalProgram], - T.List[T.Union[str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex]]], + args: T.Tuple[T.Union[str, mesonlib.File, ExternalProgram], + T.List[T.Union[str, mesonlib.File, ExternalProgram]]], kwargs: 'TYPE_kwargs') -> None: if args[1]: FeatureNew.single_use('Calling "add_dist_script" with multiple arguments', @@ -191,7 +191,7 @@ class MesonMain(MesonInterpreterObject): if self.interpreter.subproject != '': FeatureNew.single_use('Calling "add_dist_script" in a subproject', '0.58.0', self.interpreter.subproject) - script_args = self._process_script_args('add_dist_script', args[1], allow_built=True) + script_args = self._process_script_args('add_dist_script', args[1], allow_built=False) script = self._find_source_script('add_dist_script', args[0], script_args) self.build.dist_scripts.append(script) -- cgit v1.1 From cb6a6dbca4d117bd2b6fe7a516887fcd5342b4f2 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Sun, 26 Dec 2021 18:04:57 -0500 Subject: remove no longer needed validation routine We don't need to check when processing the script args, whether the correct types were passed. We check this upfront in typed_pos_args now. --- mesonbuild/interpreter/mesonmain.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'mesonbuild/interpreter/mesonmain.py') diff --git a/mesonbuild/interpreter/mesonmain.py b/mesonbuild/interpreter/mesonmain.py index 819ff7f..be1acc6 100644 --- a/mesonbuild/interpreter/mesonmain.py +++ b/mesonbuild/interpreter/mesonmain.py @@ -104,7 +104,7 @@ class MesonMain(MesonInterpreterObject): str, mesonlib.File, build.BuildTarget, build.CustomTarget, build.CustomTargetIndex, ExternalProgram, - ]], allow_built: bool = False) -> T.List[str]: + ]]) -> T.List[str]: script_args = [] # T.List[str] new = False for a in args: @@ -114,8 +114,6 @@ class MesonMain(MesonInterpreterObject): new = True script_args.append(a.rel_to_builddir(self.interpreter.environment.source_dir)) elif isinstance(a, (build.BuildTarget, build.CustomTarget, build.CustomTargetIndex)): - if not allow_built: - raise InterpreterException(f'Arguments to {name} cannot be built') new = True script_args.extend([os.path.join(a.get_subdir(), o) for o in a.get_outputs()]) @@ -153,7 +151,7 @@ class MesonMain(MesonInterpreterObject): args: T.Tuple[T.Union[str, mesonlib.File, build.Executable, ExternalProgram], T.List[T.Union[str, mesonlib.File, build.BuildTarget, build.CustomTarget, build.CustomTargetIndex, ExternalProgram]]], kwargs: 'AddInstallScriptKW') -> None: - script_args = self._process_script_args('add_install_script', args[1], allow_built=True) + script_args = self._process_script_args('add_install_script', args[1]) script = self._find_source_script('add_install_script', args[0], script_args) script.skip_if_destdir = kwargs['skip_if_destdir'] script.tag = kwargs['install_tag'] @@ -170,7 +168,7 @@ class MesonMain(MesonInterpreterObject): args: T.Tuple[T.Union[str, mesonlib.File, ExternalProgram], T.List[T.Union[str, mesonlib.File, ExternalProgram]]], kwargs: 'TYPE_kwargs') -> None: - script_args = self._process_script_args('add_postconf_script', args[1], allow_built=False) + script_args = self._process_script_args('add_postconf_script', args[1]) script = self._find_source_script('add_postconf_script', args[0], script_args) self.build.postconf_scripts.append(script) @@ -191,7 +189,7 @@ class MesonMain(MesonInterpreterObject): if self.interpreter.subproject != '': FeatureNew.single_use('Calling "add_dist_script" in a subproject', '0.58.0', self.interpreter.subproject) - script_args = self._process_script_args('add_dist_script', args[1], allow_built=False) + script_args = self._process_script_args('add_dist_script', args[1]) script = self._find_source_script('add_dist_script', args[0], script_args) self.build.dist_scripts.append(script) -- cgit v1.1