From 9b5bfc0f32b05d6faf6d49f23965d7c2bcfc5879 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Tue, 2 Jul 2024 09:32:47 -0700 Subject: [PATCH] Only add direct deps to GETDEPS_CABAL_FLAGS Summary: X-link: https://github.com/facebookincubator/zstrong/pull/899 You shouldn't be able to depend on a library unless it is in your direct dependencies, also this shortens the massive GETDEPS_CABAL_FLAGS to something more sensible. Reviewed By: chadaustin Differential Revision: D58244928 fbshipit-source-id: 3e93f26ef197252cd723a65c1752dad53b5327b6 --- build/fbcode_builder/getdeps/builder.py | 14 ++++++++++--- build/fbcode_builder/getdeps/buildopts.py | 25 +++++++++++++++++------ build/fbcode_builder/getdeps/dyndeps.py | 22 ++++++++++---------- build/fbcode_builder/manifests/hsthrift | 3 +++ 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/build/fbcode_builder/getdeps/builder.py b/build/fbcode_builder/getdeps/builder.py index 794ec4ce..5a5ea230 100644 --- a/build/fbcode_builder/getdeps/builder.py +++ b/build/fbcode_builder/getdeps/builder.py @@ -179,7 +179,9 @@ def build(self, reconfigure: bool) -> None: # needs to be updated to include all of the directories containing the runtime # library dependencies in order to run the binaries. script_path = self.get_dev_run_script_path() - dep_munger = create_dyn_dep_munger(self.build_opts, self.install_dirs) + dep_munger = create_dyn_dep_munger( + self.build_opts, self._compute_env(), self.install_dirs + ) dep_dirs = self.get_dev_run_extra_path_dirs(dep_munger) # pyre-fixme[16]: Optional type has no attribute `emit_dev_run_script`. dep_munger.emit_dev_run_script(script_path, dep_dirs) @@ -227,7 +229,11 @@ def _compute_env(self): # CMAKE_PREFIX_PATH is only respected when passed through the # environment, so we construct an appropriate path to pass down return self.build_opts.compute_env_for_install_dirs( - self.install_dirs, env=self.env, manifest=self.manifest + self.loader, + self.dep_manifests, + self.ctx, + env=self.env, + manifest=self.manifest, ) def get_dev_run_script_path(self): @@ -237,7 +243,9 @@ def get_dev_run_script_path(self): def get_dev_run_extra_path_dirs(self, dep_munger=None): assert self.build_opts.is_windows() if dep_munger is None: - dep_munger = create_dyn_dep_munger(self.build_opts, self.install_dirs) + dep_munger = create_dyn_dep_munger( + self.build_opts, self._compute_env(), self.install_dirs + ) return dep_munger.compute_dependency_paths(self.build_dir) diff --git a/build/fbcode_builder/getdeps/buildopts.py b/build/fbcode_builder/getdeps/buildopts.py index 6bda130e..b315c175 100644 --- a/build/fbcode_builder/getdeps/buildopts.py +++ b/build/fbcode_builder/getdeps/buildopts.py @@ -221,7 +221,7 @@ def get_context_generator(self, host_tuple=None): ) def compute_env_for_install_dirs( - self, install_dirs, env=None, manifest=None + self, loader, dep_manifests, ctx, env=None, manifest=None ): # noqa: C901 if env is not None: env = env.copy() @@ -300,8 +300,16 @@ def compute_env_for_install_dirs( env["FBSOURCE_DATE"] = hash_data.date # reverse as we are prepending to the PATHs - for d in reversed(install_dirs): - self.add_prefix_to_env(d, env, append=False) + for m in reversed(dep_manifests): + is_direct_dep = ( + manifest is not None and m.name in manifest.get_dependencies(ctx) + ) + self.add_prefix_to_env( + loader.get_project_install_dir(m), + env, + append=False, + is_direct_dep=is_direct_dep, + ) # Linux is always system openssl system_openssl = self.is_linux() @@ -336,7 +344,12 @@ def add_homebrew_package_to_env(self, package, env) -> bool: return False def add_prefix_to_env( - self, d, env, append: bool = True, add_library_path: bool = False + self, + d, + env, + append: bool = True, + add_library_path: bool = False, + is_direct_dep: bool = False, ) -> bool: # noqa: C901 bindir = os.path.join(d, "bin") found = False @@ -374,7 +387,7 @@ def add_prefix_to_env( add_flag(env, "CPPFLAGS", f"-I{includedir}", append=append) # For non-pkgconfig projects Cabal has no way to find the includes or # libraries, so we provide a set of extra Cabal flags in the env - if not has_pkgconfig: + if not has_pkgconfig and is_direct_dep: add_flag( env, "GETDEPS_CABAL_FLAGS", @@ -419,7 +432,7 @@ def add_prefix_to_env( add_flag(env, "LDFLAGS", f"-L{libdir}", append=append) if add_library_path: add_path_entry(env, "LIBRARY_PATH", libdir, append=append) - if not has_pkgconfig: + if not has_pkgconfig and is_direct_dep: add_flag( env, "GETDEPS_CABAL_FLAGS", diff --git a/build/fbcode_builder/getdeps/dyndeps.py b/build/fbcode_builder/getdeps/dyndeps.py index 0dd9a455..25e15cd3 100644 --- a/build/fbcode_builder/getdeps/dyndeps.py +++ b/build/fbcode_builder/getdeps/dyndeps.py @@ -26,9 +26,9 @@ def copyfile(src, dest) -> None: class DepBase(object): - def __init__(self, buildopts, install_dirs, strip) -> None: + def __init__(self, buildopts, env, install_dirs, strip) -> None: self.buildopts = buildopts - self.env = buildopts.compute_env_for_install_dirs(install_dirs) + self.env = env self.install_dirs = install_dirs self.strip = strip @@ -168,8 +168,8 @@ def check_call_verbose(self, args: List[str]) -> None: class WinDeps(DepBase): - def __init__(self, buildopts, install_dirs, strip) -> None: - super(WinDeps, self).__init__(buildopts, install_dirs, strip) + def __init__(self, buildopts, env, install_dirs, strip) -> None: + super(WinDeps, self).__init__(buildopts, env, install_dirs, strip) self.dumpbin = self.find_dumpbin() def find_dumpbin(self) -> str: @@ -334,8 +334,8 @@ def _get_dev_run_script_contents(self, path_dirs) -> str: class ElfDeps(DepBase): - def __init__(self, buildopts, install_dirs, strip) -> None: - super(ElfDeps, self).__init__(buildopts, install_dirs, strip) + def __init__(self, buildopts, env, install_dirs, strip) -> None: + super(ElfDeps, self).__init__(buildopts, env, install_dirs, strip) # We need patchelf to rewrite deps, so ensure that it is built... args = [sys.executable, sys.argv[0]] @@ -448,14 +448,14 @@ def rewrite_dep(self, objfile, depname, old_dep, new_dep, final_lib_dir) -> None def create_dyn_dep_munger( - buildopts, install_dirs, strip: bool = False + buildopts, env, install_dirs, strip: bool = False ) -> Optional[DepBase]: if buildopts.is_linux(): - return ElfDeps(buildopts, install_dirs, strip) + return ElfDeps(buildopts, env, install_dirs, strip) if buildopts.is_darwin(): - return MachDeps(buildopts, install_dirs, strip) + return MachDeps(buildopts, env, install_dirs, strip) if buildopts.is_windows(): - return WinDeps(buildopts, install_dirs, strip) + return WinDeps(buildopts, env, install_dirs, strip) if buildopts.is_freebsd(): - return ElfDeps(buildopts, install_dirs, strip) + return ElfDeps(buildopts, env, install_dirs, strip) return None diff --git a/build/fbcode_builder/manifests/hsthrift b/build/fbcode_builder/manifests/hsthrift index 7dd42ca1..8a958820 100644 --- a/build/fbcode_builder/manifests/hsthrift +++ b/build/fbcode_builder/manifests/hsthrift @@ -16,6 +16,9 @@ gflags glog folly fbthrift +wangle +fizz +boost [build] builder = make