SynoCommunity / spkrepo

Synology Package Repository
http://spkrepo.readthedocs.org
MIT License
156 stars 26 forks source link

Supporting `os_max_ver` or hide non dsm7 packages from dsm7 users #63

Closed publicarray closed 8 months ago

publicarray commented 3 years ago

Conversation started here: https://github.com/SynoCommunity/spksrc/issues/4322#issuecomment-766393619

hgy59 commented 3 years ago

As there will be packages that cannot be ported to DSM7 it is a must that spkrepo does not return packages with firmware revision < 40000 for package queries with revision >= 40000. Otherwise there will be packages displayed in DSM7 package center that are not installable. A common solution would implement os_max_ver in the repository and regard the build-number in os_max_ver when searching for packages matching the query of devices. This will need data model migration and redesign of the queries.

A fast solution would be to hard code the limit of build number >= 40000 for DSM7 as queries have to be adjusted only.

But there are kernel dependent packages under development that will have different packages for DSM 6.2.1/6.2.2/6.2.3/6.2.4 and here the common approach will be to set os_max_ver to os_min_ver to nail packages to a specific version. I doubt that this is supported by the current spkrepo implementation too.

mreid-tt commented 1 year ago

Hey team, this issue has come up again now with us building for DSM 7.1 (https://github.com/SynoCommunity/spksrc/pull/5567#issuecomment-1398803128). Some users still running DSM 7.0 are being presented with Invalid file format errors when downloading updates from the repository like this:

dsm70-package-error

This leaves these users confused as to why this is happening and there is no clear communication on what they should do. I've done some comparison with two VirtualDSM servers running DSM 7.0 and DSM 7.1 to get to a reasonable understanding of this.

First I took a look at the installed packages on each using the command find /var/packages/ -name INFO -exec cat {} \; | grep -E 'os_|package=|maintainer=|^version='. I looked at a Synology package SMBService and found the following:

DSM 7.0 (update pending):

package="SMBService"
version="4.10.18-0316"
maintainer="Synology Inc."
os_min_ver="7.0-42156"

DSM 7.1:

package="SMBService"
version="4.10.18-0548"
maintainer="Synology Inc."
os_min_ver="7.1-42781"

The first thing I noted was that the package didn't use os_max_ver even though there was an available upgrade on the DSM 7.0 unit. After upgrade the package details were as follows:

DSM 7.0 (update installed):

package="SMBService"
version="4.10.18-0330"
maintainer="Synology Inc."
os_min_ver="7.0-42218-4"

So why didn't the unit attempt to upgrade to the available "4.10.18-0548" which the DSM 7.1 saw as avialable? I then dug around and found the caches of what is available for download in JSON files at /run/synopkg/pkglist/.

These were of two types, 'synoserver' and 'otherserver' with the latter being SynoCommunity. Their respective high-level structures were as follows:

synoserver.raw.enu:

synoserver raw-structure-json

otherserver.raw.enu:

otherserver raw-structure-json

I then took a look at the nzbdrone package in the 'otherserver' data between DSM 7.0 and DSM 7.1 and they were mostly identical with the exception of this line:

DSM 7.0:

"link":"https://packages.synocommunity.com/nzbdrone/22/nzbdrone.v22.f15047%5Bapollolake-avoton-braswell-broadwell-broadwellnk-broadwellntbap-bromolow-cedarview-denverton-dockerx64-geminilake-grantley-purley-kvmx64-v1000-x86-x86_64%5D.spk?arch=kvmx64&build=42218",

DSM 7.1:

"link":"https://packages.synocommunity.com/nzbdrone/22/nzbdrone.v22.f42661%5Bapollolake-avoton-braswell-broadwell-broadwellnk-broadwellnkv2-broadwellntbap-bromolow-cedarview-denverton-epyc7002-geminilake-grantley-kvmx64-purley-r1000-v1000%5D.spk?arch=kvmx64&build=42962",

All other information including the "version":"20230217-22" was identical. Now this was in stark contrast to the SMBService package on 'synoserver'. The differences there were as follows (excluding "changelog", "thumbnail" and "thumbnail_retina"):

DSM 7.0:

"download_count":6815771,
"link":"https://global.synologydownload.com/download/Package/spk/SMBService/4.10.18-0330/SMBService-x86_64-4.10.18-0330.spk",
"md5":"a0046efb7edb518058c1f43c963d9afa",
"size":10792595,
"version":"4.10.18-0330"

DSM 7.1:

"download_count":6815668,
"link":"https://global.synologydownload.com/download/Package/spk/SMBService/4.10.18-0548/SMBService-x86_64-4.10.18-0548.spk",
"md5":"6dc8d92f77f726f6903b80b6083ccf93",
"size":11034383,
"version":"4.10.18-0548"

So what can we conclude from this? My guess is that Synology is maintaining in their repository builds for 7.0 and 7.1 separately and based on the DSM client requesting the JSON index it will serve to them the newest version compatible with their arch as well as their DSM version. At the moment we only seem to be serving differentiated data based on arch compatibility only. I am hoping that we can update the back-end to include this logic.

In the long run we can have richer data from our back end included in the JSON file such as "category", "download_count", "md5", "recent_download_count" and "size". We could even expand the Makefile variables to allow for building of multiple versions within the same major release (i.e. DSM 7.0 and 7.1).

Welcome to feedback on my examination. A zip file containing the source files used for this comparison is attached for further analysis.

package-server-data.zip

EDIT: Corrected conclusion below...

mreid-tt commented 1 year ago

So I had a further look at this and I have come to some updated conclusions. Recall this comparison of the JSON data files?

I then took a look at the nzbdrone package in the 'otherserver' data between DSM 7.0 and DSM 7.1 and they were mostly identical with the exception of this line:

DSM 7.0:

"link":"https://packages.synocommunity.com/nzbdrone/22/nzbdrone.v22.f15047%5Bapollolake-avoton-braswell-broadwell-broadwellnk-broadwellntbap-bromolow-cedarview-denverton-dockerx64-geminilake-grantley-purley-kvmx64-v1000-x86-x86_64%5D.spk?arch=kvmx64&build=42218",

DSM 7.1:

"link":"https://packages.synocommunity.com/nzbdrone/22/nzbdrone.v22.f42661%5Bapollolake-avoton-braswell-broadwell-broadwellnk-broadwellnkv2-broadwellntbap-bromolow-cedarview-denverton-epyc7002-geminilake-grantley-kvmx64-purley-r1000-v1000%5D.spk?arch=kvmx64&build=42962",

Well it turns out that our back end does in fact consider the DSM version of the client it is serving to. The difference between these files when I compared the embedded INFO files (excluding "arch" and "checksum") were:

DSM 7.0:

os_min_ver="6.1-15047"

DSM 7.1:

os_min_ver="7.1-42661"

Looking at a comparison of the overall package with diff we get:

% diff -rq nzbdrone.v22.f15047 nzbdrone.v22.f42661 
Files nzbdrone.v22.f15047/INFO and nzbdrone.v22.f42661/INFO differ
Files nzbdrone.v22.f15047/PACKAGE_ICON.PNG and nzbdrone.v22.f42661/PACKAGE_ICON.PNG differ
Only in nzbdrone.v22.f42661/WIZARD_UIFILES: uninstall_uifile
Files nzbdrone.v22.f15047/conf/privilege and nzbdrone.v22.f42661/conf/privilege differ
Files nzbdrone.v22.f15047/package.tgz and nzbdrone.v22.f42661/package.tgz differ
Files nzbdrone.v22.f15047/scripts/installer and nzbdrone.v22.f42661/scripts/installer differ
Files nzbdrone.v22.f15047/scripts/service-setup and nzbdrone.v22.f42661/scripts/service-setup differ
Files nzbdrone.v22.f15047/syno_signature.asc and nzbdrone.v22.f42661/syno_signature.asc differ

And comparing the contents of the package.tgz we get:

% diff -rq package.v22.f15047 package.v22.f42661 
Files package.v22.f15047/bin/mediainfo and package.v22.f42661/bin/mediainfo differ
Files package.v22.f15047/bin/openssl and package.v22.f42661/bin/openssl differ
Files package.v22.f15047/bin/sqlite3 and package.v22.f42661/bin/sqlite3 differ
Files package.v22.f15047/lib/engines-1.1/afalg.so and package.v22.f42661/lib/engines-1.1/afalg.so differ
Files package.v22.f15047/lib/engines-1.1/capi.so and package.v22.f42661/lib/engines-1.1/capi.so differ
Files package.v22.f15047/lib/engines-1.1/padlock.so and package.v22.f42661/lib/engines-1.1/padlock.so differ
Files package.v22.f15047/lib/libcares.so and package.v22.f42661/lib/libcares.so differ
Files package.v22.f15047/lib/libcares.so.2 and package.v22.f42661/lib/libcares.so.2 differ
Files package.v22.f15047/lib/libcares.so.2.5.1 and package.v22.f42661/lib/libcares.so.2.5.1 differ
Files package.v22.f15047/lib/libcrypto.so and package.v22.f42661/lib/libcrypto.so differ
Files package.v22.f15047/lib/libcrypto.so.1.1 and package.v22.f42661/lib/libcrypto.so.1.1 differ
Files package.v22.f15047/lib/libcurl.so and package.v22.f42661/lib/libcurl.so differ
Files package.v22.f15047/lib/libcurl.so.4 and package.v22.f42661/lib/libcurl.so.4 differ
Files package.v22.f15047/lib/libcurl.so.4.8.0 and package.v22.f42661/lib/libcurl.so.4.8.0 differ
Files package.v22.f15047/lib/libmediainfo.so and package.v22.f42661/lib/libmediainfo.so differ
Files package.v22.f15047/lib/libmediainfo.so.0 and package.v22.f42661/lib/libmediainfo.so.0 differ
Files package.v22.f15047/lib/libmediainfo.so.0.0.0 and package.v22.f42661/lib/libmediainfo.so.0.0.0 differ
Files package.v22.f15047/lib/libsqlite3.so and package.v22.f42661/lib/libsqlite3.so differ
Files package.v22.f15047/lib/libsqlite3.so.0 and package.v22.f42661/lib/libsqlite3.so.0 differ
Files package.v22.f15047/lib/libsqlite3.so.0.8.6 and package.v22.f42661/lib/libsqlite3.so.0.8.6 differ
Files package.v22.f15047/lib/libssl.so and package.v22.f42661/lib/libssl.so differ
Files package.v22.f15047/lib/libssl.so.1.1 and package.v22.f42661/lib/libssl.so.1.1 differ
Files package.v22.f15047/lib/libz.so and package.v22.f42661/lib/libz.so differ
Files package.v22.f15047/lib/libz.so.1 and package.v22.f42661/lib/libz.so.1 differ
Files package.v22.f15047/lib/libz.so.1.2.13 and package.v22.f42661/lib/libz.so.1.2.13 differ
Files package.v22.f15047/lib/libzen.so and package.v22.f42661/lib/libzen.so differ
Files package.v22.f15047/lib/libzen.so.0 and package.v22.f42661/lib/libzen.so.0 differ
Files package.v22.f15047/lib/libzen.so.0.0.0 and package.v22.f42661/lib/libzen.so.0.0.0 differ
Files package.v22.f15047/share/Sonarr/package_info and package.v22.f42661/share/Sonarr/package_info differ

Then the reason for these disparities became clear... our back end was serving up these packages to the respective DSM versions:

DSM 7.0:

PackageVersion=x64-6.1_20230217-22

DSM 7.1:

PackageVersion=x64-7.1_20230217-22

Now this is very different than when we compare the SMBService package. Excluding "create_time", we get:

DSM 7.0:

version="4.10.18-0330"
os_min_ver="7.0-42218-4"
extractsize="39672"
toolkit_version="42220"

DSM 7.1:

version="4.10.18-0548"
os_min_ver="7.1-42781"
extractsize="40508"
toolkit_version="42934"

So it seems that the main reason that the updates presented to DSM 7.0 users fail to install is because they are being sent DSM 6.1 packages which have very different package files. This would be so because the server is looking for something for a DSM 7.0 user and assumes that the package with DSM 6.1 as the minimum OS would do.

This circles back to the subject of this issue. If we implement os_max_ver as a workaround for missing 7.0 builds then users would in theory be presented with the last available build before we moved to builds for 7.1. Personally, this can exclude many users from up to date packages (especially since DSM 7.0 is still supported).

I would prefer we instead (in the spksrc) compile for 7.0 in addition to 7.1 and the back end should serve up to the correct version to our users. We should not need to do anything different on our back-end. Assuming I'm correct, how difficult would this be to do?

As before, my source data is attached as reference... package-data.zip

hgy59 commented 1 year ago

The reason behind this is, that the repository always serves the highest available package version. This was OK for DSM 5.x vs DSM 6.x - where 5.x packages can be installed on DSM 6 - but is broken for DSM 6 vs DSM 7. I suppose there is just a small code change required in the spkrepo code, but I never digged into this.

hgy59 commented 1 year ago

@mreid-tt thank you for contributing to this repository and investigating.

hope you find time to solve this one, as it is a very annoying issue.

currently the repo delivers a package with a lower version for any higher DSM version. i.e. if we have a package published for DSM 5.2 only, it will be shown for installation in the Package Center of any DSM 6.x and DSM 7.x Version. In DSM 7 we get the known error "invalid file format..." but in DSM 6 it is possible to install such a package. (and I fear that a package published for SRM 1.2 only, would be shown on Models with DSM 5, DSM 6 and DSM 7)

I propose to additionally disable the delivery of DSM 5 packages for DSM 6, as we have different installer code for DSM 5 and DSM 6.

The final target to fix this issue will be, to update the version evaluation, so that after finding the highest DSM Version of a package (that is less or equal the version requested), an additional verification ensures that the DSM Main Version of the package version found matches the DSM Main Version requested.

mreid-tt commented 1 year ago

hope you find time to solve this one, as it is a very annoying issue.

I'll see if I can take a look at it. I am thinking that perhaps we can alter this function if I understand it correctly:

https://github.com/SynoCommunity/spkrepo/blob/e1561931c33d952f506dde215b6cf5d409707d47/spkrepo/views/nas.py#L64

I'm considering adding a compatibility table to be looked up based on the DSM version being queried. So let's say if you are DSM 6.x >= 6.1 it will serve you the DSM 6.1 version but if you are on DSM 7.0 and all we have is DSM 6.1 and DSM 7.1 versions it will serve neither. Haven't quite worked out all the logic yet because I think it can get a bit complex so a compatibility table that can be updated periodically seems like a likely solution.

mreid-tt commented 11 months ago

@hgy59, I was thinking on this some more and I was wondering if we should revisit your original suggestion of os_max_ver but in the spksrc itself. I see a function here:

mk/spksrc.tc-flags.mk

ifeq ($(strip $(TC_OS_MIN_VER)),)
TC_OS_MIN_VER = $(word 1,$(subst ., ,$(TC_VERS))).$(word 2,$(subst ., ,$(TC_VERS)))-$(TC_BUILD)
endif

Perhaps we can just make a simple function like this:

DSM_5_MAX_VERS = 5.9-6999
DSM_6_MAX_VERS = 6.9-39999

ifeq ($(strip $(TC_OS_MAX_VER)),)
TC_OS_MAJOR_VER = $(word 1,$(subst ., ,$(TC_VERS)))
ifeq ($(TC_OS_MAJOR_VER),5)
TC_OS_MAX_VER = $(DSM_5_MAX_VERS)
else ifeq ($(TC_OS_MAJOR_VER),6)
TC_OS_MAX_VER = $(DSM_6_MAX_VERS)
endif
endif

And then with that modify the rest of the build process to include an os_max_ver if the TC_OS_MAX_VER is set. What do you think?

hgy59 commented 11 months ago

IMHO we do not need any os_max_ver handling.

This is an issue of the spkrepo that, can be fixed without any max version handling.

I hope this will be easy to implement in spkrepo...