From 2b6830f86e8e116b2ac8ebba85a709b86cf86196 Mon Sep 17 00:00:00 2001 From: Cullen Walsh Date: Sat, 21 Sep 2024 00:02:53 -0700 Subject: [PATCH] Revert "Refactor dep rewriting logic in github shims" Summary: X-link: https://github.com/facebook/buck2/pull/782 Broke oss buck2 CI. buck2 does some path remapping that was not tested against in the initial diff. The right way to handle this looks complex and will take some time to implement, so reverting the changes in the interim. Original commit changeset: 2cdf2fc1cbce Original Phabricator Diff: D62896839 Reviewed By: zpao Differential Revision: D63152891 fbshipit-source-id: 53aad436fdca15bece88b5ad0c12ea11390fb3f8 --- .buckconfig | 5 -- shim/shims.bzl | 181 +++++++++++++++---------------------------------- 2 files changed, 54 insertions(+), 132 deletions(-) diff --git a/.buckconfig b/.buckconfig index 1677e11d7ea..4cb2f3af884 100644 --- a/.buckconfig +++ b/.buckconfig @@ -13,14 +13,9 @@ fbsource = shim fbcode_macros = shim buck = none bazel_skylib = shim -folly = root [external_cells] prelude = bundled [parser] target_platform_detector_spec = target:root//...->prelude//platforms:default - -[oss] -original_cell = fbcode -mapped_dirs = folly diff --git a/shim/shims.bzl b/shim/shims.bzl index c8dc8c13e82..2c5b898e0c7 100644 --- a/shim/shims.bzl +++ b/shim/shims.bzl @@ -409,127 +409,64 @@ def _fix_resources(resources): fail("Unexpected type {} for resources".format(type(resources))) -def strip_third_party_rust_version(x: str) -> str: - # When upgrading libraries we either suffix them as `-old` or with a version, e.g. `-1-08` - # Strip those so we grab the right one in open source. - if x.endswith(":md-5"): # md-5 is the one exception - return x - xs = x.split("-") - for i in reversed(range(len(xs))): - s = xs[i] - if s == "old" or s.isdigit(): - xs.pop(i) - else: - break - return "-".join(xs) - -def _recell_dep(newcell: str): - return lambda path, _d: newcell + "//" + path - -def _strip_dir(newprefix: str): - def _strip(path: str, d: str) -> str: - path = path.removeprefix(d).removeprefix("/") - if newprefix.endswith("/"): - return newprefix + path - else: - return newprefix + "/" + path - - return _strip - -DEP_REWRITE_RULES = { - "fbcode": struct( - exact = { - "common/rust/shed/fbinit:fbinit": "shim//third-party/rust:fbinit", - "common/rust/shed/sorted_vector_map:sorted_vector_map": "shim//third-party/rust:sorted_vector_map", - "watchman/rust/watchman_client:watchman_client": "shim//third-party/rust:watchman_client", - }, - dirs = [ - ("buck2/facebook", lambda _path, _d: None), - ("buck2", _strip_dir("root//")), - ("common/ocaml/interop", _strip_dir("root//")), - ("third-party-buck/platform010/build/supercaml", _strip_dir("shim//third-party/ocaml")), - ("third-party-buck/platform010/build", _strip_dir("shim//third-party")), - ("folly", _recell_dep("folly")), - ], - ), - "fbsource": struct( - dirs = [ - ("third-party/rust", lambda path, _pre: "shim//" + strip_third_party_rust_version(path)), - ("third-party", _recell_dep("shim")), - ], - ), - "root": struct( - dirs = [ - ("folly", _recell_dep("folly")), - ], - ), - "third-party": struct( - dirs = [ - ("", _strip_dir("shim//third-party")), - ], - ), -} - -""" -Modify a dependency target to account for being executed inside an OSS build. -""" - -def _fix_dep( - x: str, - original_cell = None, - mapped_dirs = None) -> [ +def _fix_dep(x: str) -> [ None, str, ]: - if "//" not in x: - # This is a local target, aka ":foo". Don't touch + def remove_version(x: str) -> str: + # When upgrading libraries we either suffix them as `-old` or with a version, e.g. `-1-08` + # Strip those so we grab the right one in open source. + if x.endswith(":md-5"): # md-5 is the one exception + return x + xs = x.split("-") + for i in reversed(range(len(xs))): + s = xs[i] + if s == "old" or s.isdigit(): + xs.pop(i) + else: + break + return "-".join(xs) + + if x == "//common/rust/shed/fbinit:fbinit": + return "fbsource//third-party/rust:fbinit" + elif x == "//common/rust/shed/sorted_vector_map:sorted_vector_map": + return "fbsource//third-party/rust:sorted_vector_map" + elif x == "//watchman/rust/watchman_client:watchman_client": + return "fbsource//third-party/rust:watchman_client" + elif x.startswith("fbsource//third-party/rust:"): + return remove_version(x) + elif x.startswith(":"): return x - - (cell, path) = x.split("//", 1) - - if original_cell == None: - original_cell = read_config("oss", "original_cell", "fbcode") - - if mapped_dirs == None: - mapped_dirs = filter( - lambda d: d != "", - read_config("oss", "mapped_dirs", "").split(" "), - ) - - if cell == original_cell: - # Absolute target (aka "cell//foo:bar") where the cell matches the cell the OSS project - # came from. - for d in mapped_dirs: - if path == d or path.startswith(d + "/") or path.startswith(d + ":"): - # Not only same cell, but one of this OSS project's directories! - # Map it to the root cell - return "root//" + path - - if cell == "": - # Cell relative target (aka "//foo:bar") that may need to be mapped to a shim. - # Resolve the cell to properly apply rewrite rules - cell = native.get_cell_name() - if cell == "@": - cell = original_cell - - rules = DEP_REWRITE_RULES.get(cell) - - if rules == None: - # This cell does not have any associated rewrite rules + elif x.startswith("//buck2/facebook/"): + return None + elif x.startswith("//buck2/"): + return "root//" + x.removeprefix("//buck2/") + elif x.startswith("fbcode//common/ocaml/interop/"): + return "root//" + x.removeprefix("fbcode//common/ocaml/interop/") + elif x.startswith("fbcode//third-party-buck/platform010/build/supercaml"): + return "shim//third-party/ocaml" + x.removeprefix("fbcode//third-party-buck/platform010/build/supercaml") + elif x.startswith("fbcode//third-party-buck/platform010/build"): + return "shim//third-party" + x.removeprefix("fbcode//third-party-buck/platform010/build") + elif x.startswith("fbsource//third-party"): + return "shim//third-party" + x.removeprefix("fbsource//third-party") + elif x.startswith("third-party//"): + return "shim//third-party/" + x.removeprefix("third-party//") + elif x.startswith("//folly"): + oss_depends_on_folly = read_config("oss_depends_on", "folly", False) + if oss_depends_on_folly: + return "root//folly/" + x.removeprefix("//") + return "root//" + x.removeprefix("//") + elif x.startswith("root//folly"): return x - - exact = getattr(rules, "exact", {}).get(path) - if exact != None: - # This cell has a direct rewrite mapping - return exact - - for (d, f) in getattr(rules, "dirs", []): - if d == "" or path == d or path.startswith(d + "/") or path.startswith(d + ":"): - # The path matches the directory mapping, apply the rewrite rule - return f(path, d) - - # No rules applied, do not rewrite the target - return x + elif x.startswith("//fizz"): + return "root//" + x.removeprefix("//") + elif x.startswith("shim//"): + return x + elif x.startswith("prelude//"): + return x + else: + fail("Dependency is unaccounted for `{}`.\n".format(x) + + "Did you forget 'oss-disable'?") def _fix_dep_in_string(x: str) -> str: """Replace internal labels in string values such as env-vars.""" @@ -553,16 +490,6 @@ def _assert_eq(x, y): fail("Expected {} == {}".format(x, y)) def _test(): - _assert_eq(_fix_dep("//:foo", "fbcode", []), "//:foo") - _assert_eq(_fix_dep("fbcode//common/rust/shed/fbinit:fbinit"), "shim//third-party/rust:fbinit") - _assert_eq(_fix_dep("fbcode//buck2/foo:bar"), "root//foo:bar") - _assert_eq(_fix_dep("fbcode//abc/foo:bar", "fbcode", []), "fbcode//abc/foo:bar") - _assert_eq(_fix_dep("fbcode//folly:bar", "fbcode", []), "folly//folly:bar") - _assert_eq(_fix_dep("fbcode//folly/foo:bar", "fbcode", []), "folly//folly/foo:bar") - _assert_eq(_fix_dep("fbcode//folly:bar", "fbcode", ["folly"]), "root//folly:bar") - _assert_eq(_fix_dep("fbcode//folly/foo:bar", "fbcode", ["folly"]), "root//folly/foo:bar") - _assert_eq(_fix_dep("fbsource//third-party/rust:derive_more-1"), "shim//third-party/rust:derive_more") - _assert_eq(_fix_dep("fbsource//third-party/foo:bar"), "shim//third-party/foo:bar") - _assert_eq(_fix_dep("third-party//foo:bar"), "shim//third-party/foo:bar") + _assert_eq(_fix_dep("fbsource//third-party/rust:derive_more-1"), "fbsource//third-party/rust:derive_more") _test()