From c151988b397e15d67d83047a2e33d3df28353987 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Wed, 27 Oct 2021 19:56:09 -0400 Subject: [PATCH] intro-install_plan: fix destinations for build_targets with custom install_dir There are a couple issues that combine to make the current handling a bit confusing. - we call it "install_dir_name" but it is only ever the class default - CustomTarget always has it set to None, and then we check if it is None then create a different variable with a safe fallback. The if is useless -- it cannot fail, but if it did we'd get an undefined variable error when we tried to use `dir_name` Remove the special handling for CustomTarget. Instead, just always accept None as a possible value of outdir_name when constructing install data, and, if it is None, fall back to {prefix}/outdir regardless of what type it used to be. --- mesonbuild/backend/backends.py | 24 +++++++++---------- mesonbuild/build.py | 5 ++-- .../unit/98 install all targets/meson.build | 4 ++++ unittests/allplatformstests.py | 8 +++++++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py index 0ccb6cd86..ff55aade5 100644 --- a/mesonbuild/backend/backends.py +++ b/mesonbuild/backend/backends.py @@ -134,7 +134,7 @@ class InstallData: class TargetInstallData: fname: str outdir: str - outdir_name: InitVar[str] + outdir_name: InitVar[T.Optional[str]] strip: bool install_name_mappings: T.Mapping[str, str] rpath_dirs_to_remove: T.Set[bytes] @@ -146,7 +146,9 @@ class TargetInstallData: tag: T.Optional[str] = None can_strip: bool = False - def __post_init__(self, outdir_name: str) -> None: + def __post_init__(self, outdir_name: T.Optional[str]) -> None: + if outdir_name is None: + outdir_name = os.path.join('{prefix}', self.outdir) self.out_name = os.path.join(outdir_name, os.path.basename(self.fname)) @dataclass(eq=False) @@ -1534,7 +1536,7 @@ class Backend: for t in self.build.get_targets().values(): if not t.should_install(): continue - outdirs, install_dir_name, custom_install_dir = t.get_install_dir() + outdirs, default_install_dir_name, custom_install_dir = t.get_install_dir() # Sanity-check the outputs and install_dirs num_outdirs, num_out = len(outdirs), len(t.get_outputs()) if num_outdirs != 1 and num_outdirs != num_out: @@ -1570,7 +1572,7 @@ class Backend: tag = t.install_tag[0] or ('devel' if isinstance(t, build.StaticLibrary) else 'runtime') mappings = t.get_link_deps_mapping(d.prefix) i = TargetInstallData(self.get_target_filename(t), first_outdir, - install_dir_name, + default_install_dir_name, should_strip, mappings, t.rpath_dirs_to_remove, t.install_rpath, install_mode, t.subproject, tag=tag, can_strip=can_strip) @@ -1595,7 +1597,7 @@ class Backend: implib_install_dir = self.environment.get_import_lib_dir() # Install the import library; may not exist for shared modules i = TargetInstallData(self.get_target_filename_for_linking(t), - implib_install_dir, install_dir_name, + implib_install_dir, default_install_dir_name, False, {}, set(), '', install_mode, t.subproject, optional=isinstance(t, build.SharedModule), tag='devel') @@ -1604,7 +1606,7 @@ class Backend: if not should_strip and t.get_debug_filename(): debug_file = os.path.join(self.get_target_dir(t), t.get_debug_filename()) i = TargetInstallData(debug_file, first_outdir, - install_dir_name, + default_install_dir_name, False, {}, set(), '', install_mode, t.subproject, optional=True, tag='devel') @@ -1616,7 +1618,7 @@ class Backend: if outdir is False: continue f = os.path.join(self.get_target_dir(t), output) - i = TargetInstallData(f, outdir, install_dir_name, False, {}, set(), None, + i = TargetInstallData(f, outdir, default_install_dir_name, False, {}, set(), None, install_mode, t.subproject, tag=tag) d.targets.append(i) @@ -1635,9 +1637,7 @@ class Backend: if first_outdir is not False: for output, tag in zip(t.get_outputs(), t.install_tag): f = os.path.join(self.get_target_dir(t), output) - if not install_dir_name: - dir_name = os.path.join('{prefix}', first_outdir) - i = TargetInstallData(f, first_outdir, dir_name, + i = TargetInstallData(f, first_outdir, default_install_dir_name, False, {}, set(), None, install_mode, t.subproject, optional=not t.build_by_default, tag=tag) @@ -1648,9 +1648,7 @@ class Backend: if outdir is False: continue f = os.path.join(self.get_target_dir(t), output) - if not install_dir_name: - dir_name = os.path.join('{prefix}', outdir) - i = TargetInstallData(f, outdir, dir_name, + i = TargetInstallData(f, outdir, default_install_dir_name, False, {}, set(), None, install_mode, t.subproject, optional=not t.build_by_default, tag=tag) diff --git a/mesonbuild/build.py b/mesonbuild/build.py index 245f6121c..3de5ef46a 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -624,13 +624,14 @@ class Target(HoldableObject): def get_install_dir(self) -> T.Tuple[T.List[T.Union[str, Literal[False]]], str, Literal[False]]: # Find the installation directory. - default_install_dir, install_dir_name = self.get_default_install_dir() + default_install_dir, default_install_dir_name = self.get_default_install_dir() outdirs = self.get_custom_install_dir() if outdirs and outdirs[0] != default_install_dir and outdirs[0] is not True: # Either the value is set to a non-default value, or is set to # False (which means we want this specific output out of many # outputs to not be installed). custom_install_dir = True + default_install_dir_name = None else: custom_install_dir = False # if outdirs is empty we need to set to something, otherwise we set @@ -640,7 +641,7 @@ class Target(HoldableObject): else: outdirs = [default_install_dir] - return outdirs, install_dir_name, custom_install_dir + return outdirs, default_install_dir_name, custom_install_dir def get_basename(self) -> str: return self.name diff --git a/test cases/unit/98 install all targets/meson.build b/test cases/unit/98 install all targets/meson.build index 3d131e66d..c90632aa6 100644 --- a/test cases/unit/98 install all targets/meson.build +++ b/test cases/unit/98 install all targets/meson.build @@ -36,6 +36,10 @@ static_library('static', 'lib.c', executable('app', 'main.c', install: true, ) +executable('app-otherdir', 'main.c', + install: true, + install_dir: 'otherbin', +) shared_library('shared', 'lib.c', install: true, ) diff --git a/unittests/allplatformstests.py b/unittests/allplatformstests.py index 71e919e12..6e2f22dab 100644 --- a/unittests/allplatformstests.py +++ b/unittests/allplatformstests.py @@ -3961,6 +3961,8 @@ class AllPlatformTests(BasePlatformTests): Path(installpath, 'usr/lib/bothcustom.lib'), Path(installpath, 'usr/lib/shared.lib'), Path(installpath, 'usr/lib/versioned_shared.lib'), + Path(installpath, 'usr/otherbin'), + Path(installpath, 'usr/otherbin/app-otherdir.pdb'), } elif is_windows() or is_cygwin(): expected_devel |= { @@ -3978,6 +3980,8 @@ class AllPlatformTests(BasePlatformTests): expected_runtime = expected_common | { Path(installpath, 'usr/bin'), Path(installpath, 'usr/bin/' + exe_name('app')), + Path(installpath, 'usr/otherbin'), + Path(installpath, 'usr/otherbin/' + exe_name('app-otherdir')), Path(installpath, 'usr/bin/' + exe_name('app2')), Path(installpath, 'usr/' + shared_lib_name('shared')), Path(installpath, 'usr/' + shared_lib_name('both')), @@ -4086,6 +4090,10 @@ class AllPlatformTests(BasePlatformTests): 'destination': '{bindir}/' + exe_name('app'), 'tag': 'runtime', }, + f'{self.builddir}/' + exe_name('app-otherdir'): { + 'destination': '{prefix}/otherbin/' + exe_name('app-otherdir'), + 'tag': 'runtime', + }, f'{self.builddir}/subdir/' + exe_name('app2'): { 'destination': '{bindir}/' + exe_name('app2'), 'tag': 'runtime',