Skip to content

Commit

Permalink
[antlir2][genrule_in_image] fix execution of buck-built binaries
Browse files Browse the repository at this point in the history
Summary:
In dev mode, buck-built binaries are absolute symlinks to their location in the
repo. This requires mounting in the repo at the same location it exists on the
host, which requires creating that mountpoint, which requires that the root
always be ephemeral.

Test Plan:
```
❯ buck2 test fbcode//antlir/antlir2/genrule_in_image/tests:
Buck UI: https://www.internalfb.com/buck2/5a99d6bc-0303-49ba-a96f-4340a7649b71
Test UI: https://www.internalfb.com/intern/testinfra/testrun/8444249535665592
Network: Up: 12KiB  Down: 16KiB  (reSessionID-62e17593-51bc-40d8-88b1-0b6182ce916c)
Jobs completed: 39. Time elapsed: 8.4s.
Cache hits: 0%. Commands: 10 (cached: 0, remote: 0, local: 10). Fallback: 1/10
Tests finished: Pass 10. Fail 0. Fatal 0. Skip 0. Build failure 0
```

Reviewed By: naveedgol

Differential Revision: D63803843

fbshipit-source-id: f019579689b41cff8f46eb3783cabbdd3e83e8d9
  • Loading branch information
vmagro authored and facebook-github-bot committed Oct 3, 2024
1 parent 2397ada commit 001681b
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ pub(crate) fn setup_isolation(isol: &IsolationContext) -> Result<()> {
.symlink_contents("/proc/self/fd", "fd")
.context("while creating /dev/fd symlink")?;

for devname in ["null", "random", "urandom"] {
for devname in ["fuse", "null", "random", "urandom"] {
let dev = tmpfs
.create(devname)
.with_context(|| format!("while creating device file '{devname}'"))?;
Expand Down
2 changes: 0 additions & 2 deletions antlir/antlir2/genrule_in_image/genrule_in_image.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ def _impl(ctx: AnalysisContext) -> list[Provider] | Promise:
cmd_args(ctx.attrs._working_format, format = "--working-format={}"),
cmd_args(out.as_output(), format = "--out={}"),
"--dir" if out_is_dir else cmd_args(),
"--ephemeral" if ctx.attrs.ephemeral_root else cmd_args(),
"--",
ctx.attrs.bash,
),
Expand Down Expand Up @@ -97,7 +96,6 @@ _genrule_in_image = rule(
attrs = {
"bash": attrs.arg(),
"default_out": attrs.option(attrs.string(), default = None),
"ephemeral_root": attrs.bool(default = False),
# TODO: exec_layer should probably be the default (which would make this
# behave more like a standard genrule)
"exec_layer": attrs.option(
Expand Down
15 changes: 3 additions & 12 deletions antlir/antlir2/genrule_in_image/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ struct Args {
layer: PathBuf,
#[clap(long)]
rootless: bool,
#[clap(long)]
ephemeral: bool,
#[clap(value_enum, long)]
/// On-disk format of the layer storage
working_format: WorkingFormat,
Expand Down Expand Up @@ -84,7 +82,7 @@ fn main() -> Result<()> {
.as_ref()
.map_or(args.layer.as_path(), |o| o.mountpoint()),
);
builder.ephemeral(args.ephemeral);
builder.ephemeral(true);
#[cfg(facebook)]
builder.platform(["/usr/local/fbcode", "/mnt/gvfs"]);
let cwd = std::env::current_dir()?;
Expand All @@ -93,17 +91,10 @@ fn main() -> Result<()> {
Path::new("/__genrule_in_image__/working_directory"),
cwd.as_path(),
))
// TODO(T185979228) we really should make all submounts recursively
// readonly, but that's hard and for now we should at least make sure
// that buck-out is readonly
.inputs((
Path::new("/__genrule_in_image__/working_directory/buck-out"),
cwd.join("buck-out"),
))
.inputs((cwd.as_path(), cwd.as_path()))
.working_directory(Path::new("/__genrule_in_image__/working_directory"))
.tmpfs(Path::new("/tmp"))
// TODO(vmagro): make this a devtmpfs after resolving permissions issues
.tmpfs(Path::new("/dev"));
.devtmpfs(Path::new("/dev"));

if args.out.dir {
std::fs::create_dir_all(&args.out.out)?;
Expand Down
60 changes: 59 additions & 1 deletion antlir/antlir2/genrule_in_image/tests/BUCK
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
load("//antlir/antlir2/bzl/feature:defs.bzl", "feature")
load("//antlir/antlir2/bzl/image:defs.bzl", "image")
load("//antlir/antlir2/genrule_in_image:genrule_in_image.bzl", "genrule_in_image")
load("//antlir/bzl:build_defs.bzl", "python_unittest")
load("//antlir/bzl:build_defs.bzl", "python_binary", "python_unittest")

oncall("antlir")

Expand Down Expand Up @@ -76,13 +76,71 @@ genrule_in_image(
layer = ":layer",
)

genrule_in_image(
name = "cannot-write-to-buck-out",
out = "f",
bash = """
if echo foo > buck-out/foo ;
then
echo "write should not have worked"
exit 1
fi
""",
layer = ":layer",
)

python_binary(
name = "par",
srcs = ["par.py"],
main_function = "antlir.antlir2.genrule_in_image.tests.par.main",
)

image.layer(
name = "with-xarexec",
features = [
# @oss-disable
],
parent_layer = ":layer",
)

genrule_in_image(
name = "run-exe-par",
out = "f",
bash = """
$(exe :par) $OUT
""",
layer = ":with-xarexec",
)

image.layer(
name = "with-par",
features = [
feature.install(
src = ":par",
dst = "/par",
),
],
parent_layer = ":with-xarexec",
)

genrule_in_image(
name = "run-installed-par",
out = "f",
bash = """
/par $OUT
""",
layer = ":with-par",
)

python_unittest(
name = "test",
srcs = ["test.py"],
env = {
"BUCK_SCRATCH_PATH": "$(location :buck-scratch-path)",
"DEFAULT_OUT": "$(location :default-out)",
"DOT_DIR": "$(location :dot-dir)",
"EXE_PAR": "$(location :run-exe-par)",
"INSTALLED_PAR": "$(location :run-installed-par)",
"NAMED_DIR": "$(location :named-dir)",
"NAMED_OUTS": "$(location :named-outs)",
"SINGLE_FILE": "$(location :single-file)",
Expand Down
12 changes: 12 additions & 0 deletions antlir/antlir2/genrule_in_image/tests/par.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

import sys


def main():
out = sys.argv[1]
with open(out, "w") as f:
f.write("From par\n")
10 changes: 10 additions & 0 deletions antlir/antlir2/genrule_in_image/tests/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,13 @@ def test_buck_scratch_path(self) -> None:
f = Path(os.getenv("BUCK_SCRATCH_PATH"))
self.assertTrue(f.exists())
self.assertEqual(f.read_text(), "/__genrule_in_image__/buck_scratch_path\n")

def test_exe_par(self) -> None:
f = Path(os.getenv("EXE_PAR"))
self.assertTrue(f.exists())
self.assertEqual(f.read_text(), "From par\n")

def test_installed_par(self) -> None:
f = Path(os.getenv("INSTALLED_PAR"))
self.assertTrue(f.exists())
self.assertEqual(f.read_text(), "From par\n")

0 comments on commit 001681b

Please sign in to comment.