Skip to content

Fixed simulate diff mode aborting on repeated unversioned package installs #6424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
djbclark wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed simulate diff mode aborting on repeated unversioned package installs #6424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion cf-agent/simulate_mode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,31 @@ static inline char *GetPkgOperationMsg(ChrootPkgOperationCode op, const char *pk
return msg;
}

static bool PkgRecordShouldBeReplaced(const PkgOperationRecord *prev_record,
const char *new_ver)
{
if (prev_record == NULL)
{
/* Nothing recorded yet */
return true;
}

if (NULL_OR_EMPTY(new_ver))
{
/* An unversioned record never replaces an existing one */
return false;
}

if (NULL_OR_EMPTY(prev_record->pkg_ver))
{
/* A versioned record is more specific than an unversioned one */
return true;
}

/* Both are versioned, keep the higher version */
return PkgVersionIsGreater(new_ver, prev_record->pkg_ver);
}

bool DiffPkgOperations()
{
const char *pkgs_ops_csv_file = ToChangesChroot(CHROOT_PKGS_OPS_FILE);
Expand Down Expand Up @@ -893,7 +918,7 @@ 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 (PkgRecordShouldBeReplaced(prev_record, pkg_ver))
{
char *msg = GetPkgOperationMsg(CHROOT_PKG_OPERATION_CODE_INSTALL,
pkg_name, pkg_arch, pkg_ver);
Expand Down
114 changes: 109 additions & 5 deletions tests/unit/simulate_mode_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ static void test_diff_install_cancels_removal(void)
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, "would be removed") == NULL);
assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed"));
assert_false(StringContains(output, "would be removed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}
Expand All @@ -98,7 +98,7 @@ static void test_diff_version_and_architecture(void)
char output[4096];
assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output)));

assert_true(strstr(output, "Package 'foo-x86_64 [1.2.3]' would be installed") != NULL);
assert_true(StringContains(output, "Package 'foo-x86_64 [1.2.3]' would be installed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}
Expand Down Expand Up @@ -141,8 +141,106 @@ static void test_manifest_install_cancels_removal(void)
char output[4096];
assert_true(call_with_captured_stdout(&ManifestPkgOperations, output, sizeof(output)));

assert_true(strstr(output, "Package 'foo [1.2.3]' would be present") != NULL);
assert_true(strstr(output, "would be absent") == NULL);
assert_true(StringContains(output, "Package 'foo [1.2.3]' would be present"));
assert_false(StringContains(output, "would be absent"));

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)));

assert_true(StringContains(output, "Package 'foo' would be installed"));
/* Reported once: the report has no other line. */
assert_int_equal(StringCountTokens(output, strlen(output), "\n"), 1);

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(StringContains(output, "Package 'foo [1.2.3]' would be installed"));
assert_false(StringContains(output, "Package 'foo' would be installed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}

/* An unversioned installation record must never replace an existing
* versioned installation record of the same package. */
static void test_diff_versioned_then_unversioned_install(void)
{
write_pkgs_ops("i,foo,1.2.3,\r\n"
"i,foo,,\r\n");

char output[4096];
assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output)));

assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed"));
assert_false(StringContains(output, "Package 'foo' would be installed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}

/* When two installations are recorded with different versions and the first
* has the higher version, the first record is kept. */
static void test_diff_install_higher_version_first_keeps_first(void)
{
write_pkgs_ops("i,foo,2.0.0,\r\n"
"i,foo,1.0.0,\r\n");

char output[4096];
assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output)));

assert_true(StringContains(output, "Package 'foo [2.0.0]' would be installed"));
assert_false(StringContains(output, "Package 'foo [1.0.0]' would be installed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}

/* When two installations are recorded with different versions and the last
* has the higher version, the last record replaces the first. */
static void test_diff_install_higher_version_last_keeps_last(void)
{
write_pkgs_ops("i,foo,1.0.0,\r\n"
"i,foo,2.0.0,\r\n");

char output[4096];
assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output)));

assert_true(StringContains(output, "Package 'foo [2.0.0]' would be installed"));
assert_false(StringContains(output, "Package 'foo [1.0.0]' would be installed"));

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}

/* Two recorded installations of the same package with the same version
* must be reported once. */
static void test_diff_repeated_versioned_install(void)
{
write_pkgs_ops("i,foo,1.2.3,\r\n"
"i,foo,1.2.3,\r\n");

char output[4096];
assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output)));

assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed"));
/* Reported once: the report has no other line. */
assert_int_equal(StringCountTokens(output, strlen(output), "\n"), 1);

unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE));
}
Expand All @@ -161,6 +259,12 @@ 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),
unit_test(test_diff_versioned_then_unversioned_install),
unit_test(test_diff_install_higher_version_first_keeps_first),
unit_test(test_diff_install_higher_version_last_keeps_last),
unit_test(test_diff_repeated_versioned_install),
};

int ret = run_tests(tests);
Expand Down
Loading