Skip to content

Commit

Permalink
Strip rust-toolchain.toml from current directory of build script runs
Browse files Browse the repository at this point in the history
Summary:
See the explanation on `create_cwd`.

This fixes the incompatibility with proc-macro2 1.0.57 observed in D45864167.

Alternatives considered:

- `http_archive` has an `excludes` mechanism which could work here, but it is hideous because it is based on regex, unlike everything else in BUCK which use globs. `excludes = ["^proc-macro2-1\\.0\\.57/rust-toolchain(\\.toml)?$"]`. The approach in this diff avoids adding any noise into the reindeer-generated targets.

- I considered implementing a workaround in cargo_buildscript.bzl instead of in buildscript_run.py, basically by making a `filegroup` that overlays `rust-toolchain/.go_away_rustup` + `rust-toolchain.toml/.go_away_rustup` on top of the crate's sources. Having rust-toolchain.toml be a directory causes rustup's file read of that path to fail, which it treats the same as if the path does not exist. I prefer the more straightforward workaround in this diff.

Reviewed By: themarwhal

Differential Revision: D45942826

fbshipit-source-id: 730f2e0568645825f76f4f4cfec600a2ebd4a444
  • Loading branch information
David Tolnay authored and facebook-github-bot committed May 17, 2023
1 parent eaf3323 commit 9546535
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 8 deletions.
15 changes: 10 additions & 5 deletions prelude/rust/cargo_buildscript.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,27 @@ load("@prelude//decls/toolchains_common.bzl", "toolchains_common")
def _cargo_buildscript_impl(ctx: "context") -> ["provider"]:
toolchain_info = ctx.attrs._rust_toolchain[RustToolchainInfo]

manifest_dir = ctx.attrs.manifest_dir[DefaultInfo].default_outputs[0]
cwd = ctx.actions.declare_output("cwd", dir = True)
out_dir = ctx.actions.declare_output("OUT_DIR", dir = True)
rustc_flags = ctx.actions.declare_output("rustc_flags")

cmd = cmd_args(
ctx.attrs.runner[RunInfo],
cmd_args("--buildscript=", ctx.attrs.buildscript[RunInfo], delimiter = ""),
cmd_args("--rustc-cfg=", ctx.attrs.rustc_cfg[DefaultInfo].default_outputs[0], delimiter = ""),
cmd_args("--manifest-dir=", ctx.attrs.manifest_dir[DefaultInfo].default_outputs[0], delimiter = ""),
cmd_args("--create-cwd=", cwd.as_output(), delimiter = ""),
cmd_args("--outfile=", rustc_flags.as_output(), delimiter = ""),
)

# See https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts

env = {}
env["CARGO"] = "/bin/false"
env["CARGO_MANIFEST_DIR"] = manifest_dir
env["CARGO_PKG_NAME"] = ctx.attrs.package_name
env["CARGO_PKG_VERSION"] = ctx.attrs.version
env["OUT_DIR"] = out_dir.as_output()
env["RUSTC"] = cmd_args(toolchain_info.compiler).relative_to(manifest_dir)
env["RUSTC"] = cmd_args(toolchain_info.compiler).relative_to(cwd)
env["RUSTC_LINKER"] = "/bin/false"
env["RUST_BACKTRACE"] = "1"
env["TARGET"] = toolchain_info.rustc_target_triple
Expand All @@ -62,7 +63,7 @@ def _cargo_buildscript_impl(ctx: "context") -> ["provider"]:
# Environment variables specified in the target's attributes get priority
# over all the above.
for k, v in ctx.attrs.env.items():
env[k] = cmd_args(v).relative_to(manifest_dir)
env[k] = cmd_args(v).relative_to(cwd)

ctx.actions.run(
cmd,
Expand Down Expand Up @@ -119,7 +120,11 @@ def buildscript_run(
path.removeprefix(prefix_with_trailing_slash): path
for path in glob(
include = ["{}/**".format(prefix)],
exclude = ["{}/METADATA.bzl".format(prefix)],
exclude = [
"{}/METADATA.bzl".format(prefix),
"{}/rust-toolchain".format(prefix),
"{}/rust-toolchain.toml".format(prefix),
],
)
},
copy = False,
Expand Down
66 changes: 63 additions & 3 deletions prelude/rust/tools/buildscript_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,62 @@ def cfg_env(rustc_cfg: Path) -> Dict[str, str]:
return cfgs


def create_cwd(path: Path, manifest_dir: Path) -> Path:
"""Create a directory with most of the same contents as manifest_dir, but
excluding Rustup's rust-toolchain.toml configuration file.
Keeping rust-toolchain.toml goes wrong in the situation that all of the
following happen:
1. toolchains//:rust uses compiler = "rustc", like the
system_rust_toolchain.
2. The rustc in $PATH is rustup's rustc shim.
3. A third-party dependency has both a rust-toolchain.toml and a build.rs
that runs "rustc" or env::var_os("RUSTC"), such as to inspect `rustc
--version` or to compile autocfg-style probe code.
Cargo defines that build scripts run using the package's manifest directory
as the current directory, so the rustc subprocess spawned from build.rs
would also run in that manifest directory. But other rustc invocations
performed by Buck run from the repo root.
Rustup only looks at one rust-toolchain.toml file, using the nearest one
present in any parent directory. The file can set `channel` to control which
installed version of rustc to run.
It is bad if it's possible for the rustc run by a build script vs rustc run
by the rest of the build to be different toolchains. In order to configure
their crate appropriately, build scripts rely on using the same rustc that
their crate will be later compiled by.
This problem doesn't happen during Cargo-based builds because rustup
installs both a cargo shim and a rustc shim. When you run a rustup-managed
Cargo, one of the first things it does is define a RUSTUP_TOOLCHAIN
environment variable pointing to the rustup channel id of the currently
selected cargo. Subsequent invocations of the rustup cargo shim or rustc
shim with this variable in the environment no longer pay attention to any
rust-toolchain.toml file.
We cannot follow the same approach because there is no API in rustup for
finding out a suitable RUSTUP_TOOLCHAIN value consistent with which
toolchain "rustc" currently refers to, and even if there were, it isn't
guaranteed that "rustc" refers to a rustup-managed toolchain in the first
place.
"""

path.mkdir(exist_ok=True)

for dir_entry in manifest_dir.iterdir():
if dir_entry.name not in ["rust-toolchain", "rust-toolchain.toml"]:
link = path.joinpath(dir_entry.name)
link.unlink(missing_ok=True)
link.symlink_to(os.path.relpath(dir_entry, path))

return path


def run_buildscript(buildscript: str, env: Dict[str, str], cwd: str) -> str:
try:
proc = subprocess.run(
Expand All @@ -65,13 +121,17 @@ def run_buildscript(buildscript: str, env: Dict[str, str], cwd: str) -> str:
class Args(NamedTuple):
buildscript: str
rustc_cfg: Path
manifest_dir: Path
create_cwd: Path
outfile: IO[str]


def arg_parse() -> Args:
parser = argparse.ArgumentParser(description="Run Rust build script")
parser.add_argument("--buildscript", type=str, required=True)
parser.add_argument("--rustc-cfg", type=Path, required=True)
parser.add_argument("--manifest-dir", type=Path, required=True)
parser.add_argument("--create-cwd", type=Path, required=True)
parser.add_argument("--outfile", type=argparse.FileType("w"), required=True)

return Args(**vars(parser.parse_args()))
Expand All @@ -86,11 +146,11 @@ def main() -> None: # noqa: C901
os.makedirs(out_dir, exist_ok=True)
env["OUT_DIR"] = os.path.abspath(out_dir)

manifest_dir = os.getenv("CARGO_MANIFEST_DIR")
env["CARGO_MANIFEST_DIR"] = os.path.abspath(manifest_dir)
cwd = create_cwd(args.create_cwd, args.manifest_dir)
env["CARGO_MANIFEST_DIR"] = os.path.abspath(cwd)

env = dict(os.environ, **env)
script_output = run_buildscript(args.buildscript, env, cwd=manifest_dir)
script_output = run_buildscript(args.buildscript, env=env, cwd=cwd)

cargo_rustc_cfg_pattern = re.compile("^cargo:rustc-cfg=(.*)")
flags = ""
Expand Down

0 comments on commit 9546535

Please sign in to comment.