Skip to content

Commit

Permalink
Only add direct deps to GETDEPS_CABAL_FLAGS
Browse files Browse the repository at this point in the history
Summary:
X-link: facebookincubator/zstrong#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
  • Loading branch information
Simon Marlow authored and facebook-github-bot committed Jul 2, 2024
1 parent 39650dd commit 9b5bfc0
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 20 deletions.
14 changes: 11 additions & 3 deletions build/fbcode_builder/getdeps/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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)


Expand Down
25 changes: 19 additions & 6 deletions build/fbcode_builder/getdeps/buildopts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
22 changes: 11 additions & 11 deletions build/fbcode_builder/getdeps/dyndeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions build/fbcode_builder/manifests/hsthrift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ gflags
glog
folly
fbthrift
wangle
fizz
boost

[build]
builder = make
Expand Down

0 comments on commit 9b5bfc0

Please sign in to comment.