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

mixer build delta-packs may generate too few deltas in some scenarios #782

Open
phmccarty opened this issue Sep 1, 2022 · 3 comments
Open
Labels

Comments

@phmccarty
Copy link
Contributor

Consider this test case:

#!/bin/bash

# mixver: 10
mixer init --clear-version 36940
mixer config set Mixer.LOG "$(cat mixversion).log"
mixer bundle add curl
sudo mixer build bundles
sudo mixer build update

# mixver: 20
mixer versions update
mixer config set Mixer.LOG "$(cat mixversion).log"
mixer bundle add telemetrics
sudo mixer build bundles
sudo mixer build update

# mixver: 30
mixer versions update
mixer config set Mixer.LOG "$(cat mixversion).log"
mixer bundle remove telemetrics
sudo mixer build bundles
sudo mixer build update

# mixver: 40
mixer versions update --upstream-version=36980
mixer config set Mixer.LOG "$(cat mixversion).log"
sudo mixer build bundles
sudo mixer build update
sudo mixer build delta-packs --from 30 --delta-workers 25

After creating this update stream, one would expect the delta pack for curl from version 10 (pack-curl-from-10.tar) to contain a delta for libcurl.so.4.8.0, but it does not.

Here is how libcurl.so.4.8.0 changed across mix versions:

$ grep 'libcurl\.so\.4\.8\.0' update/www/*/Manifest.*
update/www/10/Manifest.curl:F...        cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        10      /usr/lib64/libcurl.so.4.8.0
update/www/10/Manifest.full:F...        cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        10      /usr/lib64/libcurl.so.4.8.0
update/www/10/Manifest.os-core-webproxy:F...    cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        10      /usr/lib64/libcurl.so.4.8.0
update/www/20/Manifest.full:F...        cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        20      /usr/lib64/libcurl.so.4.8.0
update/www/20/Manifest.telemetrics:F... cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        20      /usr/lib64/libcurl.so.4.8.0
update/www/30/Manifest.full:F...        cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e        20      /usr/lib64/libcurl.so.4.8.0
update/www/40/Manifest.curl:F...        0558084bbf9f8146d8ba9a1bfe5e10ebb3dfd05abdde1fd89a7604a0e1f58ad4        40      /usr/lib64/libcurl.so.4.8.0
update/www/40/Manifest.full:F...        0558084bbf9f8146d8ba9a1bfe5e10ebb3dfd05abdde1fd89a7604a0e1f58ad4        40      /usr/lib64/libcurl.so.4.8.0
update/www/40/Manifest.os-core-webproxy:F...    0558084bbf9f8146d8ba9a1bfe5e10ebb3dfd05abdde1fd89a7604a0e1f58ad4        40      /usr/lib64/libcurl.so.4.8.0

When mixer build delta-packs is run, there is a delta generated for the library, but its name is 20-40-cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e-0558084bbf9f8146d8ba9a1bfe5e10ebb3dfd05abdde1fd89a7604a0e1f58ad4, because mixer consults Manifest.full for versions 30 and 40 when determining which deltas to create.

For the delta to be valid for the curl and os-core-webproxy bundles when updating to version 40, its name would have to be 10-40-cdf3620d7c5fc0711221586cb0de43c97d8caa71a601f0501dc30b8908279b7e-0558084bbf9f8146d8ba9a1bfe5e10ebb3dfd05abdde1fd89a7604a0e1f58ad4... Note the naming difference is the prefix (10-40 versus 20-40).

In this particular case, the generated delta (with 20-40 prefix) will not be included in any delta pack, due to how telemetrics was added in version 20 and removed in version 30, but this problem is a general one: It may be encountered whenever content is added/removed to an arbitrary number of bundles at different versions, with content not changing at the package level.

@phmccarty phmccarty added the bug label Sep 1, 2022
@phmccarty
Copy link
Contributor Author

One idea to fix this would be to teach mixer not to rely on the "last changed" values listed Manifest.full for the "from" version when creating deltas. Instead, it could parse every relevant bundle manifest for delta candidates, since those file entries have accurate "last changed" values. (Note that the "last changed" values in a Manifest.full are the maximum of all values from the bundle manifests, so the values are informative but not actionable.)

IIRC, this is the approach swupd-server used for delta creation...

@fenrus75
Copy link
Contributor

fenrus75 commented Sep 1, 2022 via email

@phmccarty
Copy link
Contributor Author

plan B would be to remove the version from the hash filename completely... in the sha1 world we were worried about collisions potentially but in sha256 this should be safe

I like this idea a bit more than plan A (from my last comment), though the barrier to entry is higher, since swupd-client would need to support the change as well. And then there is the question of breaking or preserving backwards compatibility...

fenrus75 added a commit that referenced this issue Sep 1, 2022
in #782 a funky issue is found where
we do not use the right versions for delta files in some cases.

A full fix is complicated, but the client IGNORES the versions! so we can just hardcode
the versions to anything (10 and 20 here) and it'll suddenly resolve itself
fenrus75 added a commit that referenced this issue Sep 2, 2022
in #782 a funky issue is found where
we do not use the right versions for delta files in some cases.

A full fix is complicated, but the client IGNORES the versions! so we can just hardcode
the versions to anything (10 and 20 here) and it'll suddenly resolve itself
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants