From a00433fdbc382eeb53529685f5552c29d323bef3 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Wed, 11 Apr 2018 19:51:23 +0530 Subject: [PATCH 1/3] configure_file: Add a new action 'copy' This will copy the file to the build directory without trying to read it or substitute values into it. Also do this optimization if the configuration_data() object passed to the `configuration:` kwarg is empty, and print a warning about it. See also: https://github.com/mesonbuild/meson/issues/1542 --- mesonbuild/interpreter.py | 40 +++++++++++++++--- run_unittests.py | 2 +- .../meson.build | 4 +- .../subdir/meson.build | 2 +- .../common/16 configure file/check_file.py | 10 ++++- .../16 configure file/invalid-utf8.bin.in | Bin 0 -> 10 bytes .../common/16 configure file/meson.build | 33 +++++++++++++-- .../27 library versions/subdir/meson.build | 2 +- .../src/meson.build | 2 +- .../failing/27 output subdir/meson.build | 2 +- .../unit/21 warning location/meson.build | 1 + 11 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 test cases/common/16 configure file/invalid-utf8.bin.in diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py index 49e9381a4..32a801992 100644 --- a/mesonbuild/interpreter.py +++ b/mesonbuild/interpreter.py @@ -1665,7 +1665,7 @@ permitted_kwargs = {'add_global_arguments': {'language'}, 'add_test_setup': {'exe_wrapper', 'gdb', 'timeout_multiplier', 'env'}, 'benchmark': {'args', 'env', 'should_fail', 'timeout', 'workdir', 'suite'}, 'build_target': known_build_target_kwargs, - 'configure_file': {'input', 'output', 'configuration', 'command', 'install_dir', 'capture', 'install', 'format'}, + 'configure_file': {'input', 'output', 'configuration', 'command', 'copy', 'install_dir', 'capture', 'install', 'format'}, 'custom_target': {'input', 'output', 'command', 'install', 'install_dir', 'build_always', 'capture', 'depends', 'depend_files', 'depfile', 'build_by_default'}, 'dependency': {'default_options', 'fallback', 'language', 'main', 'method', 'modules', 'optional_modules', 'native', 'required', 'static', 'version'}, 'declare_dependency': {'include_directories', 'link_with', 'sources', 'dependencies', 'compile_args', 'link_args', 'link_whole', 'version'}, @@ -3126,10 +3126,19 @@ root and issuing %s. raise InterpreterException("configure_file takes only keyword arguments.") if 'output' not in kwargs: raise InterpreterException('Required keyword argument "output" not defined.') - if 'configuration' in kwargs and 'command' in kwargs: - raise InterpreterException('Must not specify both "configuration" ' - 'and "command" keyword arguments since ' - 'they are mutually exclusive.') + actions = set(['configuration', 'command', 'copy']).intersection(kwargs.keys()) + if len(actions) == 0: + raise InterpreterException('Must specify an action with one of these ' + 'keyword arguments: \'configuration\', ' + '\'command\', or \'copy\'.') + elif len(actions) == 2: + raise InterpreterException('Must not specify both {!r} and {!r} ' + 'keyword arguments since they are ' + 'mutually exclusive.'.format(*actions)) + elif len(actions) == 3: + raise InterpreterException('Must specify one of {!r}, {!r}, and ' + '{!r} keyword arguments since they are ' + 'mutually exclusive.'.format(*actions)) if 'capture' in kwargs: if not isinstance(kwargs['capture'], bool): raise InterpreterException('"capture" keyword must be a boolean.') @@ -3177,6 +3186,20 @@ root and issuing %s. raise InterpreterException('Output file name must not contain a subdirectory.') (ofile_path, ofile_fname) = os.path.split(os.path.join(self.subdir, output)) ofile_abs = os.path.join(self.environment.build_dir, ofile_path, ofile_fname) + # Optimize copies by not doing substitution if there's nothing to + # substitute, and warn about this legacy hack + if 'configuration' in kwargs: + conf = kwargs['configuration'] + if not isinstance(conf, ConfigurationDataHolder): + raise InterpreterException('Argument "configuration" must be of type configuration_data') + if ifile_abs and not conf.keys(): + del kwargs['configuration'] + kwargs['copy'] = True + mlog.warning('Got an empty configuration_data() object: ' + 'optimizing copy automatically; if you want to ' + 'copy a file to the build dir, use the \'copy:\' ' + 'keyword argument added in 0.46.0', location=node) + # Perform the appropriate action if 'configuration' in kwargs: conf = kwargs['configuration'] if not isinstance(conf, ConfigurationDataHolder): @@ -3217,8 +3240,13 @@ root and issuing %s. if ifile_abs: shutil.copymode(ifile_abs, dst_tmp) mesonlib.replace_if_different(ofile_abs, dst_tmp) + elif 'copy' in kwargs: + os.makedirs(os.path.join(self.environment.build_dir, self.subdir), exist_ok=True) + shutil.copyfile(ifile_abs, ofile_abs) + shutil.copymode(ifile_abs, ofile_abs) else: - raise InterpreterException('Configure_file must have either "configuration" or "command".') + # Not reachable + raise AssertionError # If the input is a source file, add it to the list of files that we # need to reconfigure on when they change. FIXME: Do the same for # files() objects in the command: kwarg. diff --git a/run_unittests.py b/run_unittests.py index 3608d3e58..28d96a327 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -1827,7 +1827,7 @@ int main(int argc, char **argv) { r'meson.build:6: WARNING: a warning of some sort', r'sub' + os.path.sep + r'meson.build:4: WARNING: subdir warning', r'meson.build:7: WARNING: Module unstable-simd has no backwards or forwards compatibility and might not exist in future releases.', - r"meson.build:10: WARNING: The variable(s) 'MISSING' in the input file conf.in are not present in the given configuration data.", + r"meson.build:11: WARNING: The variable(s) 'MISSING' in the input file conf.in are not present in the given configuration data.", r'meson.build:1: WARNING: Passed invalid keyword argument "invalid".', ]: self.assertRegex(out, re.escape(expected)) diff --git a/test cases/common/114 multiple dir configure file/meson.build b/test cases/common/114 multiple dir configure file/meson.build index c76c6b496..a4615fae8 100644 --- a/test cases/common/114 multiple dir configure file/meson.build +++ b/test cases/common/114 multiple dir configure file/meson.build @@ -4,8 +4,8 @@ subdir('subdir') configure_file(input : 'subdir/someinput.in', output : 'outputhere', - configuration : configuration_data()) + copy: true) configure_file(input : cfile1, output : '@BASENAME@', - configuration : configuration_data()) + copy: true) diff --git a/test cases/common/114 multiple dir configure file/subdir/meson.build b/test cases/common/114 multiple dir configure file/subdir/meson.build index 9c72bf985..503df964a 100644 --- a/test cases/common/114 multiple dir configure file/subdir/meson.build +++ b/test cases/common/114 multiple dir configure file/subdir/meson.build @@ -1,7 +1,7 @@ configure_file(input : 'someinput.in', output : 'outputsubdir', install : false, - configuration : configuration_data()) + copy: true) py3 = import('python3').find_python() diff --git a/test cases/common/16 configure file/check_file.py b/test cases/common/16 configure file/check_file.py index 449b77a8f..6aa73e0a9 100644 --- a/test cases/common/16 configure file/check_file.py +++ b/test cases/common/16 configure file/check_file.py @@ -3,4 +3,12 @@ import os import sys -assert(os.path.exists(sys.argv[1])) +if len(sys.argv) == 2: + assert(os.path.exists(sys.argv[1])) +elif len(sys.argv) == 3: + f1 = open(sys.argv[1], 'rb').read() + f2 = open(sys.argv[2], 'rb').read() + if f1 != f2: + raise RuntimeError('{!r} != {!r}'.format(f1, f2)) +else: + raise AssertionError diff --git a/test cases/common/16 configure file/invalid-utf8.bin.in b/test cases/common/16 configure file/invalid-utf8.bin.in new file mode 100644 index 0000000000000000000000000000000000000000..98e9ed9a9d29682f5086f54614e856b7cb20924b GIT binary patch literal 10 OcmZQz0D=qUKmq^-_5nx$ literal 0 HcmV?d00001 diff --git a/test cases/common/16 configure file/meson.build b/test cases/common/16 configure file/meson.build index 5c3a1a593..76ec8c6e6 100644 --- a/test cases/common/16 configure file/meson.build +++ b/test cases/common/16 configure file/meson.build @@ -35,21 +35,27 @@ ofile = '@0@/config2.h'.format(meson.current_build_dir()) check_file = find_program('check_file.py') # Configure in source root with command and absolute paths -configure_file(input : 'dummy.dat', +outf = configure_file(input : 'dummy.dat', output : 'config2.h', command : [genprog, scriptfile, ifile, ofile], install_dir : 'share/appdir') -run_command(check_file, join_paths(meson.current_build_dir(), 'config2.h')) +ret = run_command(check_file, outf) +if ret.returncode() != 0 + error('Error running command: @0@\n@1@'.format(ret.stdout(), ret.stderr())) +endif # Same again as before, but an input file should not be required in # this case where we use a command/script to generate the output file. genscript2b = '@0@/generator-without-input-file.py'.format(meson.current_source_dir()) ofile2b = '@0@/config2b.h'.format(meson.current_build_dir()) -configure_file( +outf = configure_file( output : 'config2b.h', command : [genprog, genscript2b, ofile2b], install_dir : 'share/appdir') -run_command(check_file, join_paths(meson.current_build_dir(), 'config2b.h')) +ret = run_command(check_file, outf) +if ret.returncode() != 0 + error('Error running command: @0@\n@1@'.format(ret.stdout(), ret.stderr())) +endif found_script = find_program('generator.py') # More configure_file tests in here @@ -149,3 +155,22 @@ configure_file( configuration : conf7 ) test('test7', executable('prog7', 'prog7.c')) + +# Test empty configuration data object on invalid utf8 file +inf = 'invalid-utf8.bin.in' +outf = configure_file(input : inf, + output : 'invalid-utf8.bin', + configuration : configuration_data()) +ret = run_command(check_file, inf, outf) +if ret.returncode() != 0 + error('Error running command: @0@\n@1@'.format(ret.stdout(), ret.stderr())) +endif + +# Test copy of a binary file +outf = configure_file(input : inf, + output : 'somebinary.bin', + copy : true) +ret = run_command(check_file, inf, outf) +if ret.returncode() != 0 + error('Error running command: @0@\n@1@'.format(ret.stdout(), ret.stderr())) +endif diff --git a/test cases/common/27 library versions/subdir/meson.build b/test cases/common/27 library versions/subdir/meson.build index b51033d27..a83fdb5a9 100644 --- a/test cases/common/27 library versions/subdir/meson.build +++ b/test cases/common/27 library versions/subdir/meson.build @@ -3,6 +3,6 @@ # because there is no structure in the build dir. genlib = configure_file(input : '../lib.c', output : 'genlib.c', - configuration : configuration_data()) + copy: true) shared_library('genlib', genlib, install : false) diff --git a/test cases/common/76 configure file in custom target/src/meson.build b/test cases/common/76 configure file in custom target/src/meson.build index cbbce5cd9..e0ab9ebb2 100644 --- a/test cases/common/76 configure file in custom target/src/meson.build +++ b/test cases/common/76 configure file in custom target/src/meson.build @@ -12,7 +12,7 @@ endif compiler = configure_file(input : 'mycompiler.py', output : 'mycompiler2.py', - configuration : configuration_data()) + copy: true) custom_target('thing2', output : 'final2.dat', diff --git a/test cases/failing/27 output subdir/meson.build b/test cases/failing/27 output subdir/meson.build index 282d0b70a..4eb422ce4 100644 --- a/test cases/failing/27 output subdir/meson.build +++ b/test cases/failing/27 output subdir/meson.build @@ -2,4 +2,4 @@ project('outdir path', 'c') configure_file(input : 'foo.in', output : 'subdir/foo', - configuration : configuration_data()) + copy: true) diff --git a/test cases/unit/21 warning location/meson.build b/test cases/unit/21 warning location/meson.build index 15295a95c..52a93d18c 100644 --- a/test cases/unit/21 warning location/meson.build +++ b/test cases/unit/21 warning location/meson.build @@ -7,4 +7,5 @@ warning('a warning of some sort') import('unstable-simd') conf_data = configuration_data() +conf_data.set('NOTMISSING', 1) configure_file(input: 'conf.in' , output: 'conf', configuration: conf_data) From f5af0f9b5ab4b35a46ef2a700d4815d2aa4b8096 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Wed, 11 Apr 2018 20:27:25 +0530 Subject: [PATCH 2/3] configure_file: Don't use reserved keyword 'format' Might lead to weird bugs --- mesonbuild/interpreter.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py index 32a801992..25747c962 100644 --- a/mesonbuild/interpreter.py +++ b/mesonbuild/interpreter.py @@ -3146,13 +3146,13 @@ root and issuing %s. raise InterpreterException('"capture" keyword requires "command" keyword.') if 'format' in kwargs: - format = kwargs['format'] - if not isinstance(format, str): + fmt = kwargs['format'] + if not isinstance(fmt, str): raise InterpreterException('"format" keyword must be a string.') else: - format = 'meson' + fmt = 'meson' - if format not in ('meson', 'cmake', 'cmake@'): + if fmt not in ('meson', 'cmake', 'cmake@'): raise InterpreterException('"format" possible values are "meson", "cmake" or "cmake@".') # Validate input @@ -3208,7 +3208,7 @@ root and issuing %s. if inputfile is not None: os.makedirs(os.path.join(self.environment.build_dir, self.subdir), exist_ok=True) missing_variables = mesonlib.do_conf_file(ifile_abs, ofile_abs, - conf.held_object, format) + conf.held_object, fmt) if missing_variables: var_list = ", ".join(map(repr, sorted(missing_variables))) mlog.warning( From 4b9393e165c4521351f6fdfd8179997016bd84a8 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 22 May 2018 02:33:18 +0530 Subject: [PATCH 3/3] docs: Add manual entry for configure_file copy kwarg --- docs/markdown/Reference-manual.md | 8 +++++++- docs/markdown/snippets/configure_file_copy.md | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 docs/markdown/snippets/configure_file_copy.md diff --git a/docs/markdown/Reference-manual.md b/docs/markdown/Reference-manual.md index 2498b9899..d8750989b 100644 --- a/docs/markdown/Reference-manual.md +++ b/docs/markdown/Reference-manual.md @@ -166,7 +166,7 @@ finally use it in a call to `configure_file`. generated_file = configure_file(...) ``` -This function can run in two modes depending on the keyword arguments +This function can run in three modes depending on the keyword arguments passed to it. When a [`configuration_data()`](#configuration_data) object is passed @@ -179,6 +179,10 @@ When a list of strings is passed to the `command:` keyword argument, it takes any source or configured file as the `input:` and assumes that the `output:` is produced when the specified command is run. +Since *0.47.0*, when the `copy:` keyword argument is set to `true`, +this function will copy the file provided in `input:` to a file in the +build directory with the name `output:` in the current directory. + These are all the supported keyword arguments: - `capture` when this argument is set to true, Meson captures `stdout` @@ -187,6 +191,8 @@ These are all the supported keyword arguments: - `command` as explained above, if specified, Meson does not create the file itself but rather runs the specified command, which allows you to do fully custom file generation. +- `copy` *(added 0.47.0)* as explained above, if specified Meson only + copies the file from input to output. - `format` *(added 0.46.0)* the format of defines. It defaults to `meson`, and so substitutes `#mesondefine` statements and variables surrounded by `@` characters, you can also use `cmake` to replace `#cmakedefine` statements and variables with the `${variable}` syntax. Finally you can use diff --git a/docs/markdown/snippets/configure_file_copy.md b/docs/markdown/snippets/configure_file_copy.md new file mode 100644 index 000000000..fee04e435 --- /dev/null +++ b/docs/markdown/snippets/configure_file_copy.md @@ -0,0 +1,10 @@ +## New action 'copy' for configure_file() + +In addition to `configuration:` and `command:`, +[`configure_file()`](#Reference-manual.md#configure_file) now accepts a keyword +argument `copy:` which specifies a new action: copying the file specified with +the `input:` keyword argument to a file in the build directory with the name +specified with the `output:` keyword argument. + +These three keyword arguments are, as before, mutually exclusive. You can only +do one action at a time.