~lattis/muon

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
6 2

[PATCH v2] implement automatic array conversion of even more args

Details
Message ID
<20210909030541.1168849-1-eschwartz@archlinux.org>
DKIM signature
pass
Download raw message
Patch: +8 -8
While we are here, take the opportunity to actually check the type of
the contents, which formerly was just "obj_array containing obj_any".

I have left alone:
- link_with, since it needs to handle both libraries and custom_target
- include_directories, as it does custom type coercion
- dependencies, since it needs to handle dependencies,
  external_libraries, and declare_dependencies, and using obj_dependency
  at least causes external library failure
---
 src/functions/default.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/functions/default.c b/src/functions/default.c
index 18219d5..61a4a36 100644
--- a/src/functions/default.c
+++ b/src/functions/default.c
@@ -87,7 +87,7 @@ func_project(struct workspace *wk, uint32_t _, uint32_t args_node, uint32_t *obj
		kw_version
	};
	struct args_kw akw[] = {
		[kw_default_options] = { "default_options", obj_array },
		[kw_default_options] = { "default_options", ARG_TYPE_ARRAY_OF | obj_string },
		[kw_license] = { "license" },
		[kw_meson_version] = { "meson_version", obj_string },
		[kw_subproject_dir] = { "subproject_dir", obj_string },
@@ -324,8 +324,8 @@ func_declare_dependency(struct workspace *wk, uint32_t _, uint32_t args_node, ui
		kw_include_directories,
	};
	struct args_kw akw[] = {
		[kw_link_with] = { "link_with", obj_array },
		[kw_include_directories] = { "include_directories", obj_any },
		[kw_link_with] = { "link_with", ARG_TYPE_ARRAY_OF | obj_any },
		[kw_include_directories] = { "include_directories", ARG_TYPE_ARRAY_OF | obj_any },
		0
	};

@@ -382,7 +382,7 @@ tgt_common(struct workspace *wk, uint32_t args_node, uint32_t *obj, enum tgt_typ
	struct args_kw akw[] = {
		[kw_sources] = { "sources", ARG_TYPE_ARRAY_OF | obj_any },
		[kw_include_directories] = { "include_directories", ARG_TYPE_ARRAY_OF | obj_any },
		[kw_dependencies] = { "dependencies", obj_array },
		[kw_dependencies] = { "dependencies", ARG_TYPE_ARRAY_OF | obj_any },
		[kw_install] = { "install", obj_bool },
		[kw_install_dir] = { "install_dir", obj_string },
		[kw_link_with] = { "link_with", ARG_TYPE_ARRAY_OF | obj_any },
@@ -391,9 +391,9 @@ tgt_common(struct workspace *wk, uint32_t args_node, uint32_t *obj, enum tgt_typ
		[kw_extra_files] = { "extra_files", obj_any }, // ignored
		[kw_target_type] = { "target_type", obj_string },
		/* lang args */
		[kw_c_args] = { "c_args", obj_array },
		[kw_cpp_args] = { "cpp_args", obj_array },
		[kw_objc_args] = { "objc_args", obj_array },
		[kw_c_args] = { "c_args", ARG_TYPE_ARRAY_OF | obj_string },
		[kw_cpp_args] = { "cpp_args", ARG_TYPE_ARRAY_OF | obj_string },
		[kw_objc_args] = { "objc_args", ARG_TYPE_ARRAY_OF | obj_string },
		[kw_link_args] = { "link_args", ARG_TYPE_ARRAY_OF | obj_string },
		0
	};
@@ -621,7 +621,7 @@ func_subproject(struct workspace *wk, uint32_t rcvr, uint32_t args_node, uint32_
		kw_default_options,
	};
	struct args_kw akw[] = {
		[kw_default_options] = { "default_options", obj_array },
		[kw_default_options] = { "default_options", ARG_TYPE_ARRAY_OF | obj_string },
		0
	};

-- 
2.33.0
Details
Message ID
<20210909203719.7yffd5mkir7mgwjq@hostomei>
In-Reply-To
<20210909030541.1168849-1-eschwartz@archlinux.org> (view parent)
DKIM signature
pass
Download raw message
Thank you, this looks good to me.  I really appreciate you taking the
time to test this on some real projects.  As far as list flattening
goes, I have been addressing it so far with ad-hoc application of the
obj_array_foreach_flat, which iterates over the array as if it were
flattened.  Maybe ARG_TYPE_ARRAY_OF should imply that the array gets
flattened also.  We could even flatten all arrays passed as arguments,
but I'm not sure about that imo.
Details
Message ID
<970c33ba-ca66-f8e3-6d82-95bee09fb849@archlinux.org>
In-Reply-To
<20210909203719.7yffd5mkir7mgwjq@hostomei> (view parent)
DKIM signature
pass
Download raw message
On 9/9/21 4:37 PM, Stone Tickle wrote:
> Thank you, this looks good to me.  I really appreciate you taking the
> time to test this on some real projects.  As far as list flattening
> goes, I have been addressing it so far with ad-hoc application of the
> obj_array_foreach_flat, which iterates over the array as if it were
> flattened.  Maybe ARG_TYPE_ARRAY_OF should imply that the array gets
> flattened also.  We could even flatten all arrays passed as arguments,
> but I'm not sure about that imo.


From IRC:


06:37 PM <@dcbaker> Meson flattens arrays for all inputs unless those
inputs are specifically marked no flatten
06:37 PM <@dcbaker> I think the only functions that are no-flatten are
the message family


06:39 PM <@dcbaker> I've pushed *very* hard to flatten everything unelss
they'res a really, really good reason not to, because semantically meson
DSL treats `'a' == ['a'] == [[[[['a']]]]]` as true
06:40 PM <@dcbaker> I think I'd push back against that if we were
designing meson today, but keeping it consistant is important to me
06:41 PM <eschwartz> muon is technically being designed today :D


06:45 PM <@dcbaker> IFAIU *that* is why Meson does the list flattening,
so that you don't end up with garbage like `c_args: ['-DFOO'] + a + b +
['-DGARBAGE']`, you get `c_args: ['-DFOO', a, b, '-DGARBAGE']`


Incidentally, the automatic array coercion is implemented (for the
modern actually-does-type-checking arg passing style) here:
https://github.com/mesonbuild/meson/blob/9e61b4fe99fad66c9dc7484ab62c34a75dec5b60/mesonbuild/interpreterbase/decorators.py#L298-L300


tl;dr

At time of use, meson has a general habit wherever arrays are accepted,
of automatically coercing things into arrays, then flattening them
essentially as a matter of principle.

Either one might or might not be done for any given function, but the
latter is essentially universal.


-- 
Eli Schwartz
Bug Wrangler and Trusted User
Details
Message ID
<20210911215630.jj76opc7k45jyws3@hostomei>
In-Reply-To
<970c33ba-ca66-f8e3-6d82-95bee09fb849@archlinux.org> (view parent)
DKIM signature
pass
Download raw message
> tl;dr
>
> At time of use, meson has a general habit wherever arrays are accepted,
> of automatically coercing things into arrays, then flattening them
> essentially as a matter of principle.
>
> Either one might or might not be done for any given function, but the
> latter is essentially universal.

Thank you for the context!  I think that I would prefer to not have any
magical coercion of arrays, but I don't think it is so bad that we
should cause builds to fail.  I have done what I suggested and made
ARG_TYPE_ARRAY_OF flatten its argument as well.  I suppose it is the
same as meson's listify() now, except that it can typecheck all elements
of the list.  I also think that if we are going to do one, we should do
both, so at least it is predictable.  Oh, and most functions that take
an obj_array should probably take ARG_TYPE_ARRAY_OF | whatever anyway
since that gives us extra typechecking.

Stone
Details
Message ID
<7d20ba3e-e3e1-2062-3790-9c593e632950@archlinux.org>
In-Reply-To
<20210909030541.1168849-1-eschwartz@archlinux.org> (view parent)
DKIM signature
pass
Download raw message
On 9/8/21 23:05, Eli Schwartz wrote:
> While we are here, take the opportunity to actually check the type of
> the contents, which formerly was just "obj_array containing obj_any".
> 
> I have left alone:
> [...]
> - dependencies, since it needs to handle dependencies,
>   external_libraries, and declare_dependencies, and using obj_dependency
>   at least causes external library failure

In commit 8f9511aeab7eeaa22c6381263185461af15c00b9

"ensure deps is an array of dependencies"

external libraries found via get_compiler('c').find_library('foo') are
no longer accepted, which violates my intention from this commit.


$ for i in */; do (cd $i; muon setup builddir || read); done
[...]
info detected compiler gcc 11.1.0 (cc)
info configuring 'chipmunk', version: 6.2.2
info '-Wno-unused-parameter' supported: YES
info found library 'm' at '/usr/lib/libm.so'
/home/eschwartz/git/mesonbuild/wrapdb/subprojects/Chipmunk2D-Chipmunk-6.2.2/meson.build:20:46:
error: expected type dependency, got external_library
 20 | ], include_directories : inc, dependencies : m_dep)
                                                   ^
/home/eschwartz/git/mesonbuild/wrapdb/subprojects/Chipmunk2D-Chipmunk-6.2.2/meson.build:10:7:
error: in builtin function library
 10 | lib = library('chipmunk', ['src/chipmunk.c', 'src/cpArbiter.c',
'src/cpArray.c', 'src/cpBB.c',
            ^



-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
Details
Message ID
<20210915211639.3qter3p2myt5mluo@hostomei>
DKIM signature
pass
Download raw message
Oops!  Very sorry about that.  The reason I changed it back is because
only obj_dependency is handled in the output generator.  Since the
output generator was assuming everything was an obj_dependency, there
were some pretty strange errors.  I suppose the proper fix for this is
to handle obj_external_library in output.c.

By the way, would there be any difference between link_with and
dependency in this case?

Stone
Details
Message ID
<20210915220830.64pqracozvjwcc4o@hostomei>
In-Reply-To
<7d20ba3e-e3e1-2062-3790-9c593e632950@archlinux.org> (view parent)
DKIM signature
pass
Download raw message
I just pushed a fix, which at least works for Chipmunk2D.
Reply to thread Export thread (mbox)