From d5052b78632150c83fa5ac20e9d09bbddbab8403 Mon Sep 17 00:00:00 2001 From: Daniel JB Clark Date: Thu, 8 Oct 2026 23:29:11 -0400 Subject: [PATCH] Fixed simulate diff mode aborting on repeated unversioned package installs DiffPkgOperations() compared the versions of two recorded installations of the same package without checking that both were set, which PkgVersionIsGreater() asserts. The diff report now keeps the record the way the manifest report does: a versioned record over an unversioned one, otherwise the higher version, otherwise the first one recorded. Ticket: none Changelog: Title Signed-off-by: Daniel JB Clark Co-Authored-By: Claude Fable 5.1 --- cf-agent/simulate_mode.c | 5 ++++- tests/unit/simulate_mode_test.c | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cf-agent/simulate_mode.c b/cf-agent/simulate_mode.c index b70274a9c13..bf5343bd449 100644 --- a/cf-agent/simulate_mode.c +++ b/cf-agent/simulate_mode.c @@ -827,7 +827,10 @@ bool DiffPkgOperations() MapRemove(removed, name_arch); PkgOperationRecord *prev_record = MapGet(installed, name_arch); - if ((prev_record == NULL) || PkgVersionIsGreater(pkg_ver, prev_record->pkg_ver)) + if ((prev_record == NULL) || + (NULL_OR_EMPTY(prev_record->pkg_ver) && !NULL_OR_EMPTY(pkg_ver)) || + (!NULL_OR_EMPTY(pkg_ver) && !NULL_OR_EMPTY(prev_record->pkg_ver) && + PkgVersionIsGreater(pkg_ver, prev_record->pkg_ver))) { char *msg = GetPkgOperationMsg(CHROOT_PKG_OPERATION_CODE_INSTALL, pkg_name, pkg_arch, pkg_ver); diff --git a/tests/unit/simulate_mode_test.c b/tests/unit/simulate_mode_test.c index 10b5513121a..87d71b4cc63 100644 --- a/tests/unit/simulate_mode_test.c +++ b/tests/unit/simulate_mode_test.c @@ -147,6 +147,40 @@ static void test_manifest_install_cancels_removal(void) unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); } +/* Two recorded installations of the same package without a version must be + * reported once, not abort on comparing the empty versions. */ +static void test_diff_repeated_unversioned_install(void) +{ + write_pkgs_ops("i,foo,,\r\n" + "i,foo,,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + const char *msg = "Package 'foo' would be installed"; + const char *first = strstr(output, msg); + assert_true(first != NULL); + assert_true(strstr(first + strlen(msg), "would be installed") == NULL); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* A versioned installation record is more specific than an unversioned one + * of the same package, so it is the one to report. */ +static void test_diff_unversioned_then_versioned_install(void) +{ + write_pkgs_ops("i,foo,,\r\n" + "i,foo,1.2.3,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(strstr(output, "Package 'foo [1.2.3]' would be installed") != NULL); + assert_true(strstr(output, "Package 'foo' would be installed") == NULL); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + int main() { PRINT_TEST_BANNER(); @@ -161,6 +195,8 @@ int main() unit_test(test_diff_removal_with_version), unit_test(test_diff_removal_with_other_version_keeps_install), unit_test(test_manifest_install_cancels_removal), + unit_test(test_diff_repeated_unversioned_install), + unit_test(test_diff_unversioned_then_versioned_install), }; int ret = run_tests(tests);