Skip to content

Commit

Permalink
[antlir2][remove] add must_be_empty flag
Browse files Browse the repository at this point in the history
Summary:
In some cases it may be desirable to delete an empty directory but fail if it
has any contents.

Test Plan:
This is impossible to unittest without T166070409

```
❯ hg diff
 diff --git a/fbcode/antlir/antlir2/test_images/remove/TARGETS b/fbcode/antlir/antlir2/test_images/remove/TARGETS
 --- a/fbcode/antlir/antlir2/test_images/remove/TARGETS
+++ b/fbcode/antlir/antlir2/test_images/remove/TARGETS
@@ -31,6 +31,17 @@
     parent_layer = ":base",
 )

+image.layer(
+    name = "remove-empty-dir",
+    features = [
+        feature.remove(
+            must_be_empty = True,
+            path = "/b",
+        ),
+    ],
+    parent_layer = ":base",
+)
+
 image_diff_test(
     name = "remove-test",
     diff = "remove.toml",

❯ buck2 build fbcode//antlir/antlir2/test_images/remove:remove-empty-dir

2023-10-19T14:21:52.466415Z ERROR compile:remove{self=Remove { path: "/b", must_exist: true, must_be_empty: true }}: remove: error=Directory not empty (os error 39)
2023-10-19T14:21:52.466666Z ERROR compile: antlir2::cmd::compile: error=Directory not empty (os error 39)
2023-10-19T14:21:52.470541Z ERROR map: antlir2::cmd::map: error="isolated command failed: exit status: 1"
stderr:
Compile(
    IO(
        Os {
            code: 39,
            kind: DirectoryNotEmpty,
            message: "Directory not empty",
        },
    ),
)
Directory not empty (os error 39)
```

Reviewed By: justintrudell

Differential Revision: D50456566

fbshipit-source-id: 7123779f182853819a7176f81bfbb473f2917425
  • Loading branch information
vmagro authored and facebook-github-bot committed Oct 23, 2023
1 parent cd8e65b commit 23ce88e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
5 changes: 4 additions & 1 deletion antlir/antlir2/bzl/feature/remove.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ load(":feature_info.bzl", "ParseTimeFeature", "data_only_feature_analysis_fn")
def remove(
*,
path: str,
must_exist: bool = True) -> ParseTimeFeature:
must_exist: bool = True,
must_be_empty: bool = False) -> ParseTimeFeature:
"""
Recursively remove a file or directory
Expand All @@ -24,6 +25,7 @@ def remove(
feature_type = "remove",
plugin = antlir2_dep("features:remove"),
kwargs = {
"must_be_empty": must_be_empty,
"must_exist": must_exist,
"path": path,
},
Expand All @@ -32,6 +34,7 @@ def remove(
remove_record = record(
path = str,
must_exist = bool,
must_be_empty = bool,
)

remove_analyze = data_only_feature_analysis_fn(
Expand Down
7 changes: 6 additions & 1 deletion antlir/antlir2/features/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub type Feature = Remove;
pub struct Remove {
pub path: PathInLayer,
pub must_exist: bool,
pub must_be_empty: bool,
}

impl antlir2_depgraph::requires_provides::RequiresProvides for Remove {
Expand Down Expand Up @@ -51,7 +52,11 @@ impl antlir2_compile::CompileFeature for Remove {
trace!("'{}' did not exist", self.path.display());
Ok(())
} else if e.kind() == std::io::ErrorKind::IsADirectory {
std::fs::remove_dir_all(&path).map_err(antlir2_compile::Error::from)
if self.must_be_empty {
std::fs::remove_dir(&path).map_err(antlir2_compile::Error::from)
} else {
std::fs::remove_dir_all(&path).map_err(antlir2_compile::Error::from)
}
} else {
Err(e.into())
}
Expand Down

0 comments on commit 23ce88e

Please sign in to comment.