From 001681b418af914a36d98b04ccd1a33c02cc8ab2 Mon Sep 17 00:00:00 2001 From: Vinnie Magro Date: Thu, 3 Oct 2024 09:41:20 -0700 Subject: [PATCH] [antlir2][genrule_in_image] fix execution of buck-built binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../isolate_unshare_preexec/src/isolation.rs | 2 +- .../genrule_in_image/genrule_in_image.bzl | 2 - antlir/antlir2/genrule_in_image/src/main.rs | 15 +---- antlir/antlir2/genrule_in_image/tests/BUCK | 60 ++++++++++++++++++- antlir/antlir2/genrule_in_image/tests/par.py | 12 ++++ antlir/antlir2/genrule_in_image/tests/test.py | 10 ++++ 6 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 antlir/antlir2/genrule_in_image/tests/par.py diff --git a/antlir/antlir2/antlir2_isolate/isolate_unshare/isolate_unshare_preexec/src/isolation.rs b/antlir/antlir2/antlir2_isolate/isolate_unshare/isolate_unshare_preexec/src/isolation.rs index 061077ba891..5fd01634dd3 100644 --- a/antlir/antlir2/antlir2_isolate/isolate_unshare/isolate_unshare_preexec/src/isolation.rs +++ b/antlir/antlir2/antlir2_isolate/isolate_unshare/isolate_unshare_preexec/src/isolation.rs @@ -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}'"))?; diff --git a/antlir/antlir2/genrule_in_image/genrule_in_image.bzl b/antlir/antlir2/genrule_in_image/genrule_in_image.bzl index 031b274c12f..26928fa8037 100644 --- a/antlir/antlir2/genrule_in_image/genrule_in_image.bzl +++ b/antlir/antlir2/genrule_in_image/genrule_in_image.bzl @@ -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, ), @@ -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( diff --git a/antlir/antlir2/genrule_in_image/src/main.rs b/antlir/antlir2/genrule_in_image/src/main.rs index 75f017d8599..ec8b4c87c7d 100644 --- a/antlir/antlir2/genrule_in_image/src/main.rs +++ b/antlir/antlir2/genrule_in_image/src/main.rs @@ -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, @@ -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()?; @@ -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)?; diff --git a/antlir/antlir2/genrule_in_image/tests/BUCK b/antlir/antlir2/genrule_in_image/tests/BUCK index a9ecc701570..a3ae188a0a5 100644 --- a/antlir/antlir2/genrule_in_image/tests/BUCK +++ b/antlir/antlir2/genrule_in_image/tests/BUCK @@ -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") @@ -76,6 +76,62 @@ 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"], @@ -83,6 +139,8 @@ python_unittest( "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)", diff --git a/antlir/antlir2/genrule_in_image/tests/par.py b/antlir/antlir2/genrule_in_image/tests/par.py new file mode 100644 index 00000000000..aad2851d09f --- /dev/null +++ b/antlir/antlir2/genrule_in_image/tests/par.py @@ -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") diff --git a/antlir/antlir2/genrule_in_image/tests/test.py b/antlir/antlir2/genrule_in_image/tests/test.py index 98006507bed..124ce6b6ea9 100644 --- a/antlir/antlir2/genrule_in_image/tests/test.py +++ b/antlir/antlir2/genrule_in_image/tests/test.py @@ -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")