Skip to content

Commit

Permalink
[antlir2][rpms] do a better job of recording feature details
Browse files Browse the repository at this point in the history
Summary:
Before properly cleaning up the way rpm features get merged in the depgraph,
make sure we are tracing the target graph through in a way that's easier to
read later by recording the original feature label on each `rpm` feature.

This also records the antlir2 layer label and resolved transaction in the dnf
history table, so it's possible to see why certain things were installed.

Test Plan:
```
❯ buck2 test fbcode//antlir/antlir2/test_images/rpms:
Buck UI: https://www.internalfb.com/buck2/fa5cba49-298f-4f1c-996c-72ae200a06a4
Test UI: https://www.internalfb.com/intern/testinfra/testrun/3940649860928808
Network: Up: 11 MiB  Down: 0 B  (reSessionID-9041bb35-34ca-400d-9e54-1a021cf6798a)
Jobs completed: 117. Time elapsed: 53.7s.
Cache hits: 0%. Commands: 72 (cached: 0, remote: 0, local: 72)
Tests finished: Pass 19. Fail 0. Fatal 0. Skip 0. Build failure 0

❯ buck2 build --show-output fbcode//antlir/antlir2/test_images/rpms:simple--layer--features
Buck UI: https://www.internalfb.com/buck2/848ab645-8d98-4015-a363-7daf34bca7d9
Network: Up: 0 B  Down: 0 B
Jobs completed: 4. Time elapsed: 0.1s.
BUILD SUCCEEDED
fbcode//antlir/antlir2/test_images/rpms:simple--layer--features buck-out/v2/gen/fbcode/513e0f216bd9b87a/antlir/antlir2/test_images/rpms/__simple--layer--features__/features.json

vmagro@devvm11640.ftw0 in fbsource
❯ jq < buck-out/v2/gen/fbcode/513e0f216bd9b87a/antlir/antlir2/test_images/rpms/__simple--layer--features__/features.json
[
  {
    "feature_type": "rpm",
    "label": "fbcode//antlir/antlir2/test_images/rpms:simple--layer--features",
    "data": {
      "items": [
        {
          "action": "install",
          "rpm": {
            "subject": "foo-2",
            "src": null,
            "subjects_src": null
          },
          "feature_label": "fbcode//antlir/antlir2/test_images/rpms:simple--layer--features"
        },
        {
          "action": "install",
          "rpm": {
            "subject": "foobarbaz",
            "src": null,
            "subjects_src": null
          },
          "feature_label": "fbcode//antlir/antlir2/test_images/rpms:simple--layer--features"
        }
      ]
    },
    "run_info": [
      "buck-out/v2/gen/fbcode/0b8ca35c5e5df329/antlir/antlir2/features/__rpm__/shared/rpm"
    ]
  },
```

Reviewed By: sergeyfd

Differential Revision: D47635567

fbshipit-source-id: ec6ac1d5f75bec951e5734ff14e14b1f771d4529
  • Loading branch information
vmagro authored and facebook-github-bot committed Jul 21, 2023
1 parent 9eeec53 commit 324ddcf
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 2 deletions.
1 change: 1 addition & 0 deletions antlir/antlir2/bzl/feature/feature.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ def _impl(ctx: "context") -> ["provider"]:
toolchains = Toolchains(
cxx = ctx.attrs.cxx_toolchain,
),
label = ctx.label,
)

analysis = _analyze_feature[inline["feature_type"]](**analyze_kwargs)
Expand Down
1 change: 1 addition & 0 deletions antlir/antlir2/bzl/feature/feature_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ AnalyzeFeatureContext = record(
unique_action_identifier = str.type,
actions = "actions",
toolchains = Toolchains.type,
label = "label",
)

def data_only_feature_analysis_fn(
Expand Down
5 changes: 5 additions & 0 deletions antlir/antlir2/bzl/feature/rpms.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def rpms_install(
"action": "install",
"subjects": subjects,
},
analyze_uses_context = True,
)

def rpms_remove_if_exists(*, rpms: [[[str.type, "selector"]], "selector"]) -> ParseTimeFeature.type:
Expand All @@ -82,6 +83,7 @@ def rpms_remove_if_exists(*, rpms: [[[str.type, "selector"]], "selector"]) -> Pa
"action": "remove_if_exists",
"subjects": rpms,
},
analyze_uses_context = True,
)

action_enum = enum("install", "remove_if_exists")
Expand All @@ -95,13 +97,15 @@ rpm_source_record = record(
rpm_item_record = record(
action = action_enum.type,
rpm = rpm_source_record.type,
feature_label = "target_label",
)

rpms_record = record(
items = [rpm_item_record.type],
)

def rpms_analyze(
ctx: "AnalyzeFeatureContext",
action: str.type,
subjects: [str.type],
srcs: {str.type: "artifact"} = {},
Expand Down Expand Up @@ -130,6 +134,7 @@ def rpms_analyze(
rpm_item_record(
action = action_enum(action),
rpm = rpm,
feature_label = ctx.label.raw_target(),
)
for rpm in rpms
],
Expand Down
12 changes: 10 additions & 2 deletions antlir/antlir2/dnf/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,13 @@ def driver(spec) -> None:
out.write("\n")
sys.exit(1)

# setting base.args will record a comment in the history db
base.args = [
"antlir2",
spec["layer_label"],
json.dumps(spec["resolved_transaction"], sort_keys=True),
]

# dnf go brrr
base.do_transaction(
TransactionProgress(
Expand Down Expand Up @@ -416,10 +423,11 @@ def driver(spec) -> None:
sys.exit(1)

for pkg, reason in set_reasons:
base.history.set_reason(pkg, reason)
if REASON_FROM_STRING[pkg.reason] != reason:
base.history.set_reason(pkg, reason)
# commit that change to the db
rpmdb_version = base.history.last().end_rpmdb_version
base.history.beg(rpmdb_version, [], [])
base.history.beg(rpmdb_version, [], [], "antlir2: correct installed reasons")
base.history.end(rpmdb_version)


Expand Down
1 change: 1 addition & 0 deletions antlir/antlir2/features/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ feature_impl(
name = "rpm",
deps = [
"serde_json",
"//antlir/buck/buck_label:buck_label",
],
)

Expand Down
5 changes: 5 additions & 0 deletions antlir/antlir2/features/rpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use antlir2_features::types::BuckOutSource;
use anyhow::Context;
use anyhow::Error;
use anyhow::Result;
use buck_label::Label;
use serde::de::Error as _;
use serde::Deserialize;
use serde::Serialize;
Expand Down Expand Up @@ -93,6 +94,7 @@ impl<'a, 'de: 'a> Deserialize<'de> for Source<'a> {
pub struct RpmItem<'a> {
pub action: Action,
pub rpm: Source<'a>,
pub feature_label: Label<'a>,
}

#[derive(
Expand Down Expand Up @@ -199,6 +201,7 @@ struct DriverSpec<'a> {
excluded_rpms: Option<&'a BTreeSet<String>>,
resolved_transaction: Option<DnfTransaction>,
ignore_postin_script_error: bool,
layer_label: Label<'a>,
}

#[derive(Debug, Copy, Clone, Serialize)]
Expand Down Expand Up @@ -302,6 +305,7 @@ fn run_dnf_driver(
.map(|subject| RpmItem {
action: item.action,
rpm: Source::Subject(subject.to_owned().into()),
feature_label: item.feature_label.clone(),
})
.collect()),
_ => Ok(vec![item]),
Expand All @@ -320,6 +324,7 @@ fn run_dnf_driver(
excluded_rpms: Some(ctx.dnf().excluded_rpms()),
resolved_transaction,
ignore_postin_script_error: internal_only_options.ignore_postin_script_error,
layer_label: ctx.label().clone(),
})
.context("while serializing dnf-driver input")?;

Expand Down
7 changes: 7 additions & 0 deletions antlir/antlir2/test_images/rpms/BUCK
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("//antlir/antlir2/bzl/feature:defs.bzl", "feature")
load("//antlir/antlir2/testing:image_test.bzl", "image_sh_test")
load("//antlir/bzl:build_defs.bzl", "buck_genrule", "python_binary")
load("//antlir/rpm/dnf2buck:repo.bzl", "repo_set")
load(":defs.bzl", "expected_t", "test_rpms")
Expand Down Expand Up @@ -316,3 +317,9 @@ test_rpms(
flavor = "//antlir/antlir2/test_images:test-image-flavor",
parent_layer = protected_installed,
)

image_sh_test(
name = "test-history",
layer = simple,
test = "test-history.sh",
)
29 changes: 29 additions & 0 deletions antlir/antlir2/test_images/rpms/test-history.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash
# 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.

set -e

# 'foo' should only be touched in transaction 1
if [ "$(dnf history list foo | tail -n +3 | awk '{ print $1 }')" != "1" ]; then
echo "foo was altered outside of transaction 1!"
dnf history list foo
exit 1
fi

tx1comment="$(dnf history info 1)"
if [[ "$tx1comment" != *"//antlir/antlir2/test_images/rpms:simple--layer"* ]]; then
echo "comment did not have label"
echo "$tx1comment"
exit 1
fi

# 'foobar' should have been touched in transaction 1 and 2 because the reason
# needed to be changed
if [ "$(dnf history list foobar | tail -n +3 | awk '{ print $1 }')" != $'2\n1' ]; then
echo "foobar should have been changed in tx 1 and 2"
dnf history list foobar
exit 1
fi

0 comments on commit 324ddcf

Please sign in to comment.