Skip to content

Commit

Permalink
Adds new 'build' and 'source' bool args for include_directories() func.
Browse files Browse the repository at this point in the history
Motivation: Frequent, unnecessary (and maybe even problematic) doubling
up of both build and src-relative include search paths as discussed here:
#11919

'build' specifies whether the include dirs should be treated as build-
relative and 'src', whether source-relative.  They both default to true
to keep previous behaviour when unspecified.

Added new 'test cases/common/267 include_directories use types' test (should
these live under 'common'?).  Added documentation and new feature snippet.

Respect the new bools in vs2010backend (and adding more type annotations
to help readability).
  • Loading branch information
GertyP committed Sep 21, 2023
1 parent 7440767 commit 22d96cf
Show file tree
Hide file tree
Showing 15 changed files with 272 additions and 39 deletions.
18 changes: 18 additions & 0 deletions docs/markdown/snippets/inc_dir_use_type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Added a new `build` and 'source' args to `include_directories(...)`

The previous behaviour of `include_directories(...)` is that it ends up adding
both source-relative and build-relative include search paths to the compile
options or, if using absolute paths, then simply duplicating the same absolute
include search path. Even if the user wants _either_ a build-relative _or_
src-relative path only, they're forced to use both, which could cause problems
as well as just adding unnecessary compile options.

New `build` and `source` bool arguments are added to `include_directories(...)`.
If unspecified, both `build` and `source` default to True.
It is invalid for both to be False.
It is invalid to use absolute paths with _only_ 'build' set to True, since a
user asking for build-relative absolute include directories is meaningless or
at least suggests a misunderstanding.

Absolute include search paths are allowed if `source` is `true`. If `build`
is also `true`, any absolute paths will only be added once.
41 changes: 34 additions & 7 deletions docs/yaml/functions/include_directories.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ name: include_directories
returns: inc
description: |
Returns an opaque object which contains the directories (relative to
the current directory) given in the positional arguments. The result
can then be passed to the `include_directories:` keyword argument when
building executables or libraries. You can use the returned object in
any subdirectory you want, Meson will make the paths work
automatically.
the current build and/or source directory) given in the positional
arguments. The result can then be passed to the `include_directories:`
keyword argument when building executables or libraries. You can use
the returned object in any subdirectory you want, Meson will make the
paths work automatically.
Note that this function call itself does not add the directories into
the search path, since there is no global search path. For something
Expand All @@ -16,8 +16,18 @@ description: |
[[executable]], which adds current source and build
directories to include path.
Each directory given is converted to two include paths: one that is
relative to the source root and one relative to the build root.
Each given directory is converted to a build-root-relative include
search path (if `build` is `true`) and/or a source-root-relative
path (if `source` is `true`).
The default values for `build` and `source` are `true`. It is
invalid for both to be `false`.
It is invalid to use absolute paths with _only_ 'build' `true`,
since asking for build-relative-only absolute include directories is
meaningless or at least a user misunderstanding. However, absolute
search paths are allowed if `source` is `true`. If `build` is also
`true`, any absolute paths will only be added once.
example: |
For example, with the following source tree layout in
Expand Down Expand Up @@ -70,3 +80,20 @@ kwargs:
they will be used with the `-isystem` compiler argument rather than
`-I` on compilers that support this flag (in practice everything
except Visual Studio).
build:
type: bool
default: true
since: 1.3.0
description: |
Specifies whether the resultant include search directories should be
treated as build-relative include search directories.
source:
type: bool
default: true
since: 1.3.0
description: |
Specifies whether the resultant include search directories should be
treated as source-relative include search directories, while also
allowing absolute directory paths.
56 changes: 43 additions & 13 deletions mesonbuild/backend/ninjabackend.py
Original file line number Diff line number Diff line change
Expand Up @@ -2839,27 +2839,59 @@ def generate_llvm_ir_compile(self, target, src):
self.add_build(element)
return (rel_obj, rel_src)

# Returns a list of include dir arg strings.
# If 'd' is an absolute path, then there's just one include dir arg.
# If 'build_relative' and 'source_relative' then the list contains the
# src-relative dir and the build-relative dir... IN THIS ORDER -
# [build-relative, src-relative]
# This is because -
#
# - We know that this function is used with CompilerArgs.__iadd_ (+=)
# which has very particular behaviour.
#
# - With include dirs, CompilerArgs '+=' reverses the input sequence and
# prepends the reverse of that again to its internal commands list, so -
# commands += [a]
# commands += [b]
# gives [b,a,...], which is different from -
# commands += [a,b]
# which gives [a,b,...]
#
# Because of this, returning any build-relative path before a src-relative
# path means the build-relative path will come earlier in the final command
# args and so take precedence, which is what we want.
#
# Because this is internal, it's reasonable to assume all 'd' paths have
# checked we don't use absolute paths with ONLY 'build_relative' paths.
@lru_cache(maxsize=None)
def generate_inc_dir(self, compiler: 'Compiler', d: str, basedir: str, is_system: bool) -> \
T.Tuple['ImmutableListProtocol[str]', 'ImmutableListProtocol[str]']:
def _generate_inc_dir(self, compiler: 'Compiler', d: str, basedir: str,
is_system: bool, build_relative: bool,
source_relative: bool) -> \
T.List['ImmutableListProtocol[str]']:
# Avoid superfluous '/.' at the end of paths when d is '.'
if d not in ('', '.'):
expdir = os.path.normpath(os.path.join(basedir, d))
else:
expdir = basedir
srctreedir = os.path.normpath(os.path.join(self.build_to_src, expdir))
sargs = compiler.get_include_args(srctreedir, is_system)

inc_dirs: T.List[T.List[str]] = []

is_builddir_suitable = build_relative and not os.path.isabs(d)

# There may be include dirs where a build directory has not been
# created for some source dir. For example if someone does this:
#
# inc = include_directories('foo/bar/baz')
#
# But never subdir()s into the actual dir.
if os.path.isdir(os.path.join(self.environment.get_build_dir(), expdir)):
bargs = compiler.get_include_args(expdir, is_system)
else:
bargs = []
return (sargs, bargs)
if is_builddir_suitable and os.path.isdir(os.path.join(self.environment.get_build_dir(), expdir)):
inc_dirs += compiler.get_include_args(expdir, is_system)

if source_relative:
srctreedir = os.path.normpath(os.path.join(self.build_to_src, expdir))
inc_dirs += compiler.get_include_args(srctreedir, is_system)

return inc_dirs

def _generate_single_compile(self, target: build.BuildTarget, compiler: 'Compiler',
is_generated: bool = False) -> 'CompilerArgs':
Expand Down Expand Up @@ -2909,10 +2941,8 @@ def _generate_single_compile_target_args(self, target: build.BuildTarget, compil
# -Ipath will add to begin of array. And without reverse
# flags will be added in reversed order.
for d in reversed(i.get_incdirs()):
# Add source subdir first so that the build subdir overrides it
(compile_obj, includeargs) = self.generate_inc_dir(compiler, d, basedir, i.is_system)
commands += compile_obj
commands += includeargs
commands += self._generate_inc_dir(compiler, d, basedir, i.is_system,
i.build_relative, i.source_relative)
for d in i.get_extra_build_dirs():
commands += compiler.get_include_args(d, i.is_system)
# Add per-target compile args, f.ex, `c_args : ['-DFOO']`. We set these
Expand Down
38 changes: 26 additions & 12 deletions mesonbuild/backend/vs2010backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ def split_link_args(args):
other.append(arg)
return lpaths, libs, other

def _get_cl_compiler(self, target):
def _get_cl_compiler(self, target) -> compilers.Compiler:
for lang, c in target.compilers.items():
if lang in {'c', 'cpp'}:
return c
Expand All @@ -1022,19 +1022,25 @@ def _prettyprint_vcxproj_xml(self, tree: ET.ElementTree, ofname: str) -> None:
replace_if_different(ofname, ofname_tmp)

# Returns: (target_args,file_args), (target_defines,file_defines), (target_inc_dirs,file_inc_dirs)
def get_args_defines_and_inc_dirs(self, target, compiler, generated_files_include_dirs, proj_to_src_root, proj_to_src_dir, build_args):
def get_args_defines_and_inc_dirs(self, target: build.BuildTarget, compiler: compilers.Compiler,
generated_files_include_dirs: T.List[str], proj_to_src_root: str,
proj_to_src_dir: str, build_args: T.List[str]) -> \
T.Tuple[
T.Tuple[T.List[str], T.Dict[str, CompilerArgs]],
T.Tuple[T.List[str], T.Dict[str, list]],
T.Tuple[T.List[str], T.Dict[str, list]]]:
# Arguments, include dirs, defines for all files in the current target
target_args = []
target_defines = []
target_inc_dirs = []
target_args: T.List[str] = []
target_defines: T.List[str] = []
target_inc_dirs: T.List[str] = []
# Arguments, include dirs, defines passed to individual files in
# a target; perhaps because the args are language-specific
#
# file_args is also later split out into defines and include_dirs in
# case someone passed those in there
file_args: T.Dict[str, CompilerArgs] = {l: c.compiler_args() for l, c in target.compilers.items()}
file_defines = {l: [] for l in target.compilers}
file_inc_dirs = {l: [] for l in target.compilers}
file_defines: T.Dict[str, list] = {l: [] for l in target.compilers}
file_inc_dirs: T.Dict[str, list] = {l: [] for l in target.compilers}
# The order in which these compile args are added must match
# generate_single_compile() and generate_basic_compiler_args()
for l, comp in target.compilers.items():
Expand Down Expand Up @@ -1081,13 +1087,21 @@ def get_args_defines_and_inc_dirs(self, target, compiler, generated_files_includ
# reversed is used to keep order of includes
for i in reversed(d.get_incdirs()):
curdir = os.path.join(d.get_curdir(), i)
try:
if d.source_relative:
# Add source subdir first so that the build subdir overrides it
args.append('-I' + os.path.join(proj_to_src_root, curdir)) # src dir
args.append('-I' + self.relpath(curdir, target.subdir)) # build dir
except ValueError:
# Include is on different drive
args.append('-I' + os.path.normpath(curdir))

# We assert that we don't have any absolute paths with ONLY build-relative
# include dirs but since we allow abs paths in conjunction with source
# and/or build-relative include dirs, any abs paths will already be added
# via the 'source_relative' step above, so don't duplicate -
if d.build_relative and not os.path.isabs(i):
try:
args.append('-I' + self.relpath(curdir, target.subdir)) # build dir
except ValueError: # From 'relpath'...
# ... if curdir and target.subdir are different drives
args.append('-I' + os.path.normpath(curdir))

for i in d.get_extra_build_dirs():
curdir = os.path.join(d.get_curdir(), i)
args.append('-I' + self.relpath(curdir, target.subdir)) # build dir
Expand Down
29 changes: 26 additions & 3 deletions mesonbuild/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,24 @@ class IncludeDirs(HoldableObject):
# Interpreter has validated that all given directories
# actually exist.
extra_build_dirs: T.List[str] = field(default_factory=list)
build_relative: bool = True
source_relative: bool = True

def __repr__(self) -> str:
r = '<{} {}/{}>'
return r.format(self.__class__.__name__, self.curdir, self.incdirs)

def __post_init__(self):
assert self.build_relative or self.source_relative, \
'IncludeDirs must enable build_relative, or source_relative, or both'

# It's meaningless to explicitly specify build-relative include dirs but
# with absolute paths. This is in addition to the same validation on
# user-supplied include dirs, since we also instantiate some internal
# IncludeDirs elsewhere, which we can prevent breaking this expectation.
assert self.source_relative or not any(os.path.isabs(d) for d in self.incdirs), \
'build-relative-only IncludeDirs with absolute paths is not expected.'

def get_curdir(self) -> str:
return self.curdir

Expand All @@ -399,10 +412,20 @@ def to_string_list(self, sourcedir: str, builddir: str) -> T.List[str]:
be added if this is unset
:returns: A list of strings (without compiler argument)
"""

strlist: T.List[str] = []
for idir in self.incdirs:
strlist.append(os.path.join(sourcedir, self.curdir, idir))
strlist.append(os.path.join(builddir, self.curdir, idir))
if self.source_relative:
strlist.extend(os.path.join(sourcedir, self.curdir, idir) for idir in self.incdirs)

if self.build_relative:
if self.source_relative:
# Only append relative incdirs paths to the builddir, since any absolute paths
# will already have been appended above.
strlist.extend(os.path.join(builddir, self.curdir, idir) for idir in self.incdirs if not os.path.isabs(idir))
else:
# We can assume they're relative paths that can all be appended
strlist.extend(os.path.join(builddir, self.curdir, idir) for idir in self.incdirs)

return strlist

@dataclass(eq=False)
Expand Down
27 changes: 23 additions & 4 deletions mesonbuild/interpreter/interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2781,12 +2781,18 @@ def extract_incdirs(self, kwargs, key: str = 'include_directories') -> T.List[bu
return result

@typed_pos_args('include_directories', varargs=str)
@typed_kwargs('include_directories', KwargInfo('is_system', bool, default=False))
@typed_kwargs(
'include_directories',
KwargInfo('is_system', bool, default=False),
KwargInfo('build', bool, default=True, since='1.3.0'),
KwargInfo('source', bool, default=True, since='1.3.0'),
)
def func_include_directories(self, node: mparser.BaseNode, args: T.Tuple[T.List[str]],
kwargs: 'kwtypes.FuncIncludeDirectories') -> build.IncludeDirs:
return self.build_incdir_object(args[0], kwargs['is_system'])
return self.build_incdir_object(args[0], kwargs['is_system'], kwargs['build'], kwargs['source'])

def build_incdir_object(self, incdir_strings: T.List[str], is_system: bool = False) -> build.IncludeDirs:
def build_incdir_object(self, incdir_strings: T.List[str], is_system: bool = False,
build_relative: bool = True, source_relative: bool = True) -> build.IncludeDirs:
if not isinstance(is_system, bool):
raise InvalidArguments('Is_system must be boolean.')
src_root = self.environment.get_source_dir()
Expand Down Expand Up @@ -2850,7 +2856,20 @@ def build_incdir_object(self, incdir_strings: T.List[str], is_system: bool = Fal
absdir_build = os.path.join(absbase_build, a)
if not os.path.isdir(absdir_src) and not os.path.isdir(absdir_build):
raise InvalidArguments(f'Include dir {a} does not exist.')
i = build.IncludeDirs(self.subdir, incdir_strings, is_system)

if not (build_relative or source_relative):
raise InvalidArguments('include_directories must use \'build\'-relative, '
'\'source\'-relative, or both.')

if build_relative and not source_relative:
for d in incdir_strings:
if os.path.isabs(d):
raise InvalidArguments(
f'Absolute paths ({d}) in include_directories() with only '
'\'build\'-relative use is not expected.')

i = build.IncludeDirs(self.subdir, incdir_strings, is_system,
build_relative=build_relative, source_relative=source_relative)
return i

@typed_pos_args('add_test_setup', str)
Expand Down
3 changes: 3 additions & 0 deletions mesonbuild/interpreter/kwargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ class FuncImportModule(ExtractRequired):
class FuncIncludeDirectories(TypedDict):

is_system: bool
build: bool
source: bool


class FuncAddLanguages(ExtractRequired):

Expand Down
49 changes: 49 additions & 0 deletions test cases/common/267 include_directories relative/meson.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
project('include dirs type tests', 'cpp', meson_version: '>=1.3.0' )

fs = import('fs')

# Copy src header, with a modification, to build dir so we can test finding it though
# 'src-relative', 'build-relative', and 'src-and-build-relative' use types.
configure_file( input: 'whereareyoufindingme.h',
output: 'whereareyoufindingme.h',
configuration: {'SRC_DIR': 'build dir'}
)

# Test that src-relative uses the unadulerated src header
exe_src = executable(
'prog_src',
'src/main.cpp',
implicit_include_directories: false,
include_directories: include_directories('.', build: false, source: true))
test('src-relative', exe_src, args: ['Hello from @SRC_DIR@'])

fs = import('fs')

# Test that build-relative uses the tweaked header, copied to the build dir.
exe_build = executable(
'prog_build',
'src/main.cpp',
implicit_include_directories: false,
include_directories: include_directories('.', build: true, source: false))
test('build-relative', exe_build, args: ['Hello from build dir'])

# Check 'src-and-build-relative' by including one header that's only found under the src and one that only found under the build dir.
subdir('moreheaders')
exe_both = executable(
'prog_both',
'src/main2.cpp',
implicit_include_directories: false,
include_directories: include_directories('moreheaders', build: true, source: true),
dependencies: build_only_h_dep,
)
test('both', exe_both, args: ['Src-only','Build-only'])

# Finally check the same but through ensuring that unspecified 'build' and 'source' defaults to both true
exe_both_default = executable(
'prog_both_default',
'src/main2.cpp',
implicit_include_directories: false,
include_directories: include_directories('moreheaders'),
dependencies: build_only_h_dep,
)
test('both default', exe_both_default, args: ['Src-only','Build-only'])
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#define BUILD_ONLY_MSG "Build-only"
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# It's a shame configure_file(..., copy: true) has been deprecated.
# It was simpler for this use-case and would do the copying at setup/configure-time rather than at build-time,
# which better fits our usage.

fs = import('fs')
build_only_h = fs.copyfile('build_only._h','build_only.h')
build_only_h_dep = declare_dependency(sources: build_only_h)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#define SRC_ONLY_MSG "Src-only"
Loading

0 comments on commit 22d96cf

Please sign in to comment.