Skip to content
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.

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

Prevent apt from being confused about mullvad-vpn package version #7125

Conversation

MarkusPettersson98
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 commented Nov 4, 2024

Sometimes, apt thinks that some installs are downgrades due to the dev version hash being lower than the baseline version hash. This PR makes the test framework add --allow-downgrades to apt when installing the app. To make sure that we are running the expected app version, a runtime check has been added to the test_upgrade_app test.


This change is Reviewable

Copy link

linear bot commented Nov 4, 2024

@MarkusPettersson98 MarkusPettersson98 force-pushed the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch 4 times, most recently from 26df5ba to 53c17b1 Compare November 5, 2024 07:57
Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, 9 of 10 files at r2.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on @MarkusPettersson98)


test/test-runner/src/package.rs line 176 at r2 (raw file):

    cmd.args(["-o", "DPkg::Lock::Timeout=60"]);
    cmd.arg("-qy");
    // `apt` may sporadically consider installing a development build to be a downgrade from the baseline stable

We could be more specific about the bug behavior here just to document it somewhere, e.g. "apt may consider installing a development build to be a downgrade from the baseline stable if the major version is identical, in which case the ordering is incorrectly based on the git hash suffix"

@MarkusPettersson98 MarkusPettersson98 force-pushed the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch from 5297529 to 1db272f Compare November 5, 2024 08:26
Copy link
Contributor Author

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 7 of 15 files reviewed, 1 unresolved discussion (waiting on @Serock3)


test/test-runner/src/package.rs line 176 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

We could be more specific about the bug behavior here just to document it somewhere, e.g. "apt may consider installing a development build to be a downgrade from the baseline stable if the major version is identical, in which case the ordering is incorrectly based on the git hash suffix"

That's a good idea. Updated the comment, please let me know what you think 😊

@MarkusPettersson98 MarkusPettersson98 force-pushed the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch from 1db272f to aba615f Compare November 5, 2024 08:27
Serock3
Serock3 previously approved these changes Nov 7, 2024
Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, 7 of 7 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MarkusPettersson98)


test/test-manager/src/tests/install.rs line 119 at r5 (raw file):

            .app_package_filename
            .contains(&running_daemon_version),
        Error::DaemonVersion {

The Error::DaemonVersion variant is probably not needed, you could replace it with just a formatted error message.

@MarkusPettersson98 MarkusPettersson98 force-pushed the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch from aba615f to c9af76a Compare November 7, 2024 12:22
Copy link
Contributor Author

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 13 of 15 files reviewed, 1 unresolved discussion (waiting on @Serock3)


test/test-manager/src/tests/install.rs line 119 at r5 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

The Error::DaemonVersion variant is probably not needed, you could replace it with just a formatted error message.

You're right! Done

Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: 🌟

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@MarkusPettersson98 MarkusPettersson98 force-pushed the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch from c9af76a to e2d67b8 Compare November 8, 2024 12:09
@MarkusPettersson98 MarkusPettersson98 merged commit e914beb into main Nov 8, 2024
61 checks passed
@MarkusPettersson98 MarkusPettersson98 deleted the prevent-apt-and-dnf-from-being-confused-about-package-des-1412 branch November 8, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants