interpreter: add a new function add_project_dependencies()

[bonzini]

This function can be used to add fundamental dependencies such as libm, dependency('threads') or glib to all build products in one fell swoop. This can be useful whenever, due to a project's coding conventions, it is not really possible to compile any source file without including the dependency.

This function also makes it possible to add an include_directories() object to the project, removing the need to list both source and global directories. An example from QEMU is this:

  add_project_arguments('-isystem', meson.current_source_dir() / 'linux-headers',
                        '-isystem', 'linux-headers',
                        language: ['c', 'cpp'])

which might become:

   linux_headers = declare_dependency(include_directories: 'linux-headers').as_system('system')
   add_project_dependencies(linux_headers, language: ['c', 'cpp'])

[eli-schwartz]

This would also spare @rgommers the need to check if find_library('m') is found and then add -lm as project link args.

Seems reasonable enough, anyway.

[eli-schwartz]

add_project_arguments and suchlike cannot be used after build targets have been defined, IIRC.

[bonzini]

add_project_arguments and suchlike cannot be used after build targets have been defined, IIRC.

I think you could use override_dependency on a superproject and add_project_dependency() on a subproject, so I preferred to reuse the check on is_built().

[codecov]

Codecov Report

Merging #10206 (55a3c08) into master (bba588d) will decrease coverage by 18.18%.
The diff coverage is n/a.

❗ Current head 55a3c08 differs from pull request most recent head 8d437e2. Consider uploading reports for the commit 8d437e2 to get more accurate results

@@             Coverage Diff             @@
##           master   #10206       +/-   ##
===========================================
- Coverage   68.48%   50.30%   -18.19%     
===========================================
  Files         406      203      -203     
  Lines       87400    43590    -43810     
  Branches    19422     9664     -9758     
===========================================
- Hits        59854    21926    -37928     
+ Misses      23044    19688     -3356     
+ Partials     4502     1976     -2526     

Impacted Files	Coverage Δ
modules/qt5.py	0.00% <0.00%> (-100.00%)	⬇️
modules/modtest.py	0.00% <0.00%> (-100.00%)	⬇️
scripts/msgfmthelper.py	0.00% <0.00%> (-100.00%)	⬇️
modules/keyval.py	0.00% <0.00%> (-95.24%)	⬇️
scripts/vcstagger.py	0.00% <0.00%> (-91.67%)	⬇️
modules/sourceset.py	0.00% <0.00%> (-87.58%)	⬇️
scripts/depscan.py	0.00% <0.00%> (-83.92%)	⬇️
scripts/hotdochelper.py	0.00% <0.00%> (-81.49%)	⬇️
modules/i18n.py	0.00% <0.00%> (-80.16%)	⬇️
modules/unstable_wayland.py	0.00% <0.00%> (-79.67%)	⬇️
... and 321 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c0145c...8d437e2. Read the comment docs.

[eli-schwartz]

No, I mean this:

meson/mesonbuild/interpreter/interpreter.py

Lines 2670 to 2682 in 80192aa

	def _add_project_arguments(self, node: mparser.FunctionNode, argsdict: T.Dict[str, T.Dict[str, T.List[str]]],
	args: T.List[str], kwargs: 'kwargs.FuncAddProjectArgs') -> None:
	if self.subproject not in argsdict:
	argsdict[self.subproject] = {}
	self._add_arguments(node, argsdict[self.subproject],
	self.project_args_frozen, args, kwargs)
	def _add_arguments(self, node: mparser.FunctionNode, argsdict: T.Dict[str, T.List[str]],
	args_frozen: bool, args: T.List[str], kwargs: 'kwargs.FuncAddProjectArgs') -> None:
	if args_frozen:
	msg = f'Tried to use \'{node.func_name}\' after a build target has been declared.\n' \
	'This is not permitted. Please declare all arguments before your targets.'
	raise InvalidCode(msg)

It's forbidden to add global/project compile/link arguments if that would override the immutability of a previously declared build target. This isn't about the one you just created. Any build target at all.

[bonzini]

It's forbidden to add global/project compile/link arguments if that would override the immutability of a previously declared build target. This isn't about the one you just created. Any build target at all.

I understand, but I don't understand why it's relevant. I'm sure it is, but I am missing the link.

[eli-schwartz]

EDIT: I now see it shares implementation with add_project_arguments.

This official GitHub app is trash and makes diffs practically unreadable. Back to the website it is.

[dcbaker]

I've only looked at the implementation, not at whether this is a good idea. I'm leaving that up to @jpakkane

[rgommers]

This would also spare @rgommers the need to check if find_library('m') is found and then add -lm as project link args.

For context, that code can be found here and looks like:

# We need -lm for all C code (assuming it uses math functions, which is safe to
# assume for SciPy). For C++ it isn't needed, because libstdc++/libc++ is
# guaranteed to depend on it. For Fortran code, Meson already adds `-lm`.
m_dep = cc.find_library('m', required : false)
if m_dep.found()
  add_project_link_arguments('-lm', language : 'c')
endif

That is't too bad in the end, but figuring out the details and how every language treats mlib differently was a little nontrivial. So I guess it's the code comment rather than the add_project_link_arguments above that took the work here; if that logic could be built in I think that would make sense.

[tristan957]

Would be pretty interesting to see an add_project_include_directories() too.

[xclaesse]

Would be pretty interesting to see an add_project_include_directories() too.

Could be nice, but that could already be done easily with this PR:

incdir = include_directories('include')
dep = declare_dependency(include_directories: incdir)
add_project_dependency(dep)

Implementation looks good to me and I like the idea as well. LGTM.

[xclaesse]

add_project_arguments and suchlike cannot be used after build targets have been defined, IIRC.

I think you could use override_dependency on a superproject and add_project_dependency() on a subproject, so I preferred to reuse the check on is_built().

Using is_built() is stricter that relying on add_project_arguments() that checks we have no built targets in current project, because we also want to forbid using built targets from other subprojects. So I think we are all good on this.

[tristan957]

Would be pretty interesting to see an add_project_include_directories() too.

Could be nice, but that could already be done easily with this PR:

incdir = include_directories('include')
dep = declare_dependency(include_directories: incdir)
add_project_dependency(dep)

Implementation looks good to me and I like the idea as well. LGTM.

Yep good point. My motivation for making that comment was essentially that we now have global options for all the important kwargs in build_target(): add_project_link_args(), add_project_args(), add_project_dependencies(). So adding add_project_include_directories() would finish it out in my opinion.

[bonzini]

@dcbaker, does it look good now?

[bonzini]

Also ping @jpakkane.

[jpakkane]

There needs to be a test that you can't call this function after any build targets have been defined.

project(...)
executable(...)
add_project_dependencies(...) # <- Must error out.

This is in line with how other such functions work.

[eli-schwartz]

This does dispatch to self._add_project_arguments() so it should be enforcing that rule, although I guess in theory a refactoring could break that.

[bonzini]

This does dispatch to self._add_project_arguments() so it should be enforcing that rule

On top of that, it is also enforced by the fact that the dependency cannot be a "built" one. See also the new test cases/failing/124 override and add_project_dependency.

[eli-schwartz]

That doesn't really assert anything as the restriction jpakkane mentioned is about forbidding any dependencies at all, not built ones specifically.

[bonzini]

Ping.

[xclaesse]

@bonzini it conflicts now, but looks good to me.

[xclaesse]

LGTM when CI is green, thanks!