search

sailfishos-chum / sailfishos-chum-gui

GUI application for utilising the SailfishOS:Chum community repository
https://openrepos.net/content/olf/sailfishoschum-gui-installer
MIT License
13 stars 17 forks source link

Overhaul spec file #131

Closed Olf0 closed 1 year ago

Olf0 commented 1 year ago
piggz commented 1 year ago

lgtm

Olf0 commented 1 year ago

BTW, as @piggz correctly denoted, the old MR #124 should be reviewed (and maybe merged), first, because a single change in MR #131 is already in MR #124 (now I know why I had then gut feeling, that I already did this, once).

P.S.: Please mind the other open MRs for sailfishos-chum/main and sailfishos-chum-gui.

Olf0 commented 1 year ago

A maybe dumb question, because I may stick to a rule I do not fully comprehend and which may not be applicable here: "devel packages should be build-depended upon via pkgconfig(<package>)"

So is BuildRequires: PackageKit-Qt5-devel in line 27 correct or should it better be BuildRequires: pkgconfig(PackageKit-Qt5)?

I know a .pc file has to exist for a devel-package to be supported by pkgconf / pkgconfig, but have no spontaneous idea where to look (reading documentation may alleviate that, some day :wink:). Though I suppose that there is one, because both pkcon -p search name pkgconf | grep -E '^Available |^Installed ' | tr -s ' ' | cut -f 2 -d ' ' | grep -F PackageKit-Qt5 and pkcon -p search name pkgconfig | grep -E '^Available |^Installed ' | tr -s ' ' | cut -f 2 -d ' ' | grep -F PackageKit-Qt5 yield, e.g.: PackageKit-Qt5-devel-0.9.6+git-1.3.1.jolla.armv7hl

P.S.: If I get a "go", I will also address this and / or the next comment in this MR.

Olf0 commented 1 year ago

Oh, and I also wondered, if the two %if 0%{?sailfishos_version} && 0%{?sailfishos_version} < 40[01]00 would be easier to comprehend if rewritten as %if %{defined sailfishos_version} && 0%{?sailfishos_version} < 40[01]00 because that what I think they are supposed to mean, IIUC.

IMO both are equivalent, but I had to look at it for a while to understand that this is (supposedly) supposed to mean: IF sailfishos_version is defined AND sailfishos_version < 4.0.?.00`, THEN …

Reference, e.g.: https://rpm-software-management.github.io/rpm/manual/spec.html#conditionals

P.S.: If I get a "go", I will also address this and / or the comment before this one in this MR.

nephros commented 1 year ago

A maybe dumb question, because I may stick to a rule I do not fully comprehend and which may not be applicable here: "devel packages should be build-depended upon via pkgconfig(<package>)"

I guess that is the recommended way, but it's not a strict rule, it depends on the build system of the dependee. That may not be using pkgconfig, or prefer another way of finding the information such as cmake or qmake. (E.g. PackageKit-Qt5-devel has packagekitqt5-config.cmake but does not "Provide" it)

So is BuildRequires: PackageKit-Qt5-devel in line 27 correct or should it better be BuildRequires: pkgconfig(PackageKit-Qt5)?

~/tmp $ pkcon download . PackageKit-Qt5-devel
~/tmp $ rpm -qPp PackageKit-Qt5-devel-0.9.6+git-1.5.2.jolla.aarch64.rpm
PackageKit-Qt5-devel = 0.9.6+git-1.5.2.jolla
PackageKit-Qt5-devel(aarch-64) = 0.9.6+git-1.5.2.jolla
pkgconfig(packagekitqt5) = 0.9.7

Therefore, a possible correct dep is pkgconfig(packagekitqt5) for this package. But as Chum-GUI is CMake-based, the more generic -devel dep is fine, and would work even on systems where the -devel package does not include the .pc file. (Even though cmake can and often does use pkgconfig in the background.)

Olf0 commented 1 year ago

Thanks a lot @nephros for this concise explanation.

nephros commented 2 weeks ago:

I guess that is the recommended way, but it's not a strict rule, […] […], a possible correct dep is pkgconfig(packagekitqt5) for this package.

As pkgconfig is always deployed on SailfishOS and Jolla heavily uses it (this recommendation / rule I remembered is actually from a sailor), there seems to be no reason not to express this dependency as BuildRequires: pkgconfig(packagekitqt5). OTOH I gather, that the current way of specifying this dependency is also 100% correct.

Olf0 commented 1 year ago

So ultimately I intend to defer these three changes to later, in order to get the safe and easy changes in, first:

Olf0 commented 1 year ago

Therefore, a possible correct dep is pkgconfig(packagekitqt5) for this package. But as Chum-GUI is CMake-based, the more generic -devel dep is fine, …

Unfortunately this turned out not to be true, see issue https://github.com/sailfishos-chum/sailfishos-chum-gui/issues/150#issuecomment-1426162902.

nephros commented 1 year ago

Therefore, a possible correct dep is pkgconfig(packagekitqt5) for this package. But as Chum-GUI is CMake-based, the more generic -devel dep is fine, …

Unfortunately this turned out not to be true, see issue #150 (comment).

Well. There's the issue of build deps specification, and actual dependencies of a binary.

It is I still correct for the .spec for either version of SFOS to depend on pkgconfig(packagekitqt5). However this is a build time dependency, and the resulting binaries may runtime depend on any number of libraries - which is detected by the rpmbuild process and added to the metadata of the built RPM file.

So in this case the "an upgrade must uninstall the Chum GUI package" situation could not have been avoided except if there had been a 4.5 compatible build environment before 4.5EA was published. No amount of .spec magic could work around this.

(What can work around the de-installation is having a package resolvable which does not have the library soname problem - i.e. a specially-built version which links against the "future" version of the library.)

(Btw: I wrote you an email about these dependency issues of this and another package you maintain, together with the solution (purpose-built binaries) very shortly after 4.5EA was announced, but it seems not to have arrived in time or at the correct place.)

Olf0 commented 1 year ago

Well. There's the issue of build deps specification, and actual dependencies of a binary.

Oops, ACK. So wrong consideration on my side, but still took a correct action. Sorry, it is Friday night after an eventful working week. My brain works at less than two thirds of its usual capacity. Still I wanted to have these issues resolved now, hence … "tunnel view" (… and the light at its end are just the headlights of an oncoming train).

(Btw: I wrote you an email about these dependency issues of this and another package you maintain, together with the solution (purpose-built binaries) very shortly after 4.5EA was announced, but it seems not to have arrived in time or at the correct place.)

Oops again, that was last Friday, which was worse than today, because the SFOS 4.5.0 EA release was published at noon that day. Thanks for the email and the hint today, I now read it. Ultimately I did the right thing last Friday night: Rebuild it against the SFOS 4.5.0.16 DoD repos at the SFOS-OBS.

But aaargl, just realised that the GitHub CI run builds will not install on SFOS 4.5.0 for all packages, which have a build time dependency on libsolv or PackageKit-Qt5-devel, because … Coderus did his job, but I did not update the CI workflow configuration files, yet.

Thank you very much!