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

[Suggestion] Relax compatability check to Micro Version (third field) #322

Open
nephros opened this issue Oct 28, 2022 · 9 comments
Open
Labels
feature request feature requests, suggestions, ideas etc.

Comments

@nephros
Copy link
Contributor

nephros commented Oct 28, 2022

DESCRIPTION

The patch compatibility check has been a nuisance for both patch developers and users for a long time.

The majority of patches apply over a wide range of SFOS versions - still developers must mark every release as compatible, and/or users must enable "Developer Mode" to be able to use patches not or not yet marked compatible.
This is acerbated by the fact that Web Catalog traditionally lacks behind in offering compatible versions to select to developers.

I propose relaxing the version checking logic to make Patchmanager "accept" (make installable) patches marked at least compatible with minor releases (so, a patch marked compatible with 4.4.0.38 will apply on any 4.4.0 release).

ADDITIONAL INFORMATION

I believe this is safe to do, as there is a "final check" in that patches that are actually incompatible will in all likelyhood not apply at all rather than apply but not work correctly.

@nephros nephros added the feature request feature requests, suggestions, ideas etc. label Oct 28, 2022
@nephros
Copy link
Contributor Author

nephros commented Oct 28, 2022

Implementation idea:

"Developer Mode" currently disables (or rather relaxes) the version check, but it also does more, such as enabling the message about old patch.json formats in the Patch Details page.

We should split "Version Check" from "Developer Mode", and offer either as a config option.

The "Version Check" option might default to "relaxed" checking, with users opting in to strict checking (which IMO is preferable wrt usability to having it the other way around).

@nephros nephros changed the title [Suggestion] Relax compatability checking to Major Version [Suggestion] Relax compatability checking to Minor Version Oct 28, 2022
@nephros nephros added this to the 3.2.3 milestone Oct 28, 2022
nephros pushed a commit to nephros/patchmanager that referenced this issue Oct 28, 2022
nephros pushed a commit to nephros/patchmanager that referenced this issue Oct 28, 2022
@b100dian
Copy link
Contributor

b100dian commented Nov 3, 2022

In general I think it's a good suggestion to have less restrictions in lieu of requiring patch authors to update in such cases.
However I just stumbled upon a patch that:

So I think we should consider some user feedback on auto-upgrading. And I am torn if we should auto-allow patches with different minor versions (maybe show them in another color than red, like yellow?)

@Olf0
Copy link
Contributor

Olf0 commented Nov 7, 2022

Thank you @nephros for addressing this issue Patches from the Web Catalog have since its inception.

Background:

The majority of patches apply over a wide range of SFOS versions - still developers must mark every release as compatible, and/or users must enable "Developer Mode" to be able to use patches not or not yet marked compatible.

This feature was deliberately designed this way by Andrey for the Web Catalog, out of the bad experience with RPM Patches, whose spec files allowed them to be installed on any SailfishOS release newer than <some version> (as a result of dependencies specified in the spec file), used with Patchmanager 2 may result in the GUI failing to start after a successful SailfishOS upgrade. I always perceived to be forced to enumerate all point releases supported by a Web Catalog Patch as overkill, because the Web Catalog was introduced by Patchmanager 3, which also resolved the OS upgrade issues PM 2 exhibited.

Basic idea:

I propose relaxing the version checking logic to make Patchmanager "accept" (make installable) patches marked at least compatible with minor releases (so, a patch marked compatible with 4.4.0.38 will apply on any 4.4.0 release).

I suggest not to alter the current interface, but to extend it in a backward compatible way, because …

  • a Patch author may really need to specify the point version of a SailfishOS release; this change would make it impossible to do so.
  • it is not obvious to Patch authors that the version string a.b.c.d now matches any (ERE) ^a\.b\.c\.[0-9]+$.

Hence IMO the syntax and meaning (semantics) of the current interface shall be retained and extended to be able to also express a.b.c.[0-9]+ explicitly.

  • Ideas for a syntax: Either a.b.c (e.g., 4.4.0; omitting the point version field to mean "any") or a.b.c.x (e.g., 4.4.0.x or 4.4.0.?, 4.4.0.* or any string matching the ERE ^[0-9]+\.[0-9]+\.[0-9]+\.[xyz?*]$; explicitly denoting the arbitrary point version field). While a.b.c is nicely terse, I personally prefer a.b.c.x, because always having to denote all four fields is less error prone, IMO (especially in "mixed" lists, e.g., 4.3.0.15, 4.4.0.x)
  • Although I currently do not believe that allowing for 4.4.x.x or even 4.x.x.x is needed, all variants of this scheme would enable this, if deemed necessary.

Implementation idea:

"Developer Mode" currently disables (or rather relaxes) the version check, but it also does more, such as enabling the message about old patch.json formats in the Patch Details page.
We should split "Version Check" from "Developer Mode", and offer either as a config option.

Absolutely agreed!
I.e., offering both options, a separate "SailfishOS version check" (dis- and enabling exactly the same relaxed version checks the former "Developer mode" en- and disabled) and a "Developer mode" for all other functions activated by the former "Developer mode".

The "Version Check" option might default to "relaxed" checking, with users opting in to strict checking (which IMO is preferable wrt usability to having it the other way around).

Here I do not concur: I still believe (as Andrey) that generally Patch authors should test against a new SailfishOS release and explicitly mark all versions of their Patches as compatible (or not; and the default must be "incompatible" for this scheme to work well). There even might be Patch authors who want to follow the strict rules Andrey envisioned by testing and marking every point release. Defaulting to "SailfishOS version check = off" would make all these efforts futile, hence IMO "SailfishOS version check" should continue to default to "on" for now.

I also believe we shall not alter too much in one go: Hence I suggest to first allow for expressing "compatible to any a.b.c.x release", then spread the news (maybe as a new FSO "topic") and see if Patch authors pick this up in the upcoming 6 months.
If switching the default for the "SailfishOS version check" option to "off" is still deemed necessary then, flicking this switch is a trivial PR.

nephros pushed a commit to nephros/patchmanager that referenced this issue Jan 7, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Jan 7, 2023
@Olf0 Olf0 changed the title [Suggestion] Relax compatability checking to Minor Version [Suggestion] Relax compatability check to Micro Version (third field) Jan 9, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Jan 11, 2023
Even though this is outdated, something similar needs to be implemented to address [issue \sailfishos-patches#332](sailfishos-patches#322).
@Olf0
Copy link
Contributor

Olf0 commented Jan 28, 2023

Ah, this is the aspect which was deliberately left out of issue #333 & PR #334.

@nephros
Copy link
Contributor Author

nephros commented Feb 5, 2023

Another consideration related to this:

Up until now, at least to my mind, the "relax/disable" compat check has always been about making patches developed for an older SFOS version apply to a newer one.
Which is likely the main use case for that functionality.

Now with 4.4/4.5, I come across the situation that 4.5 compatible patches will actually not apply on previous versions, such as 4.4.

So I guess any "relaxed" vs "strict" checking should be handled three ways:

  • exact version
  • relax checking for lower versions
  • relax checking for newer versions

where the latter two should can either be switched on separately, or both at the same time.

@Olf0
Copy link
Contributor

Olf0 commented Feb 5, 2023

Technically I concur with your assessment, which creates a requirement for 5 (!) settings of this switch. I also tried to describe the five cases exhaustingly and to choose concise wording:

The installed SailfishOS release version must match at least one entry in the compatibility list of a Patch dependent on this setting:

  1. Exact: All four version fields must match.
  2. Allow for a different point release: The first three version fields must match (i.e., including the "micro" version).
  3. Allow for newer releases: The first three version fields must be equal or less (i.e., excluding the "point" version).
  4. Allow for older releases: The first three version fields must be equal or greater (i.e., excluding the "point" version).
  5. Disable version check: Anything goes.

I separated cases 3 and 4 in order to make this a list to choose a single item from, because I failed to imagine a easily understandable user interface with radio buttons for these five cases.

But (this is a classic), is this a case of developers fleeing into creating user configurable options, because they do not know how to handle this better? I am inclined to say "Yes", because the average user will have no idea, which SailfishOS releases have the property of version 4.5.0 that Patches for older SFOS versions are prone to fail.

OTOH, I currently assume that we or Patchmanager as a technical entity will never have sufficient information at hand to make a proper decision, because a trivial Patch will still likely work on many SFOS releases (including 4.5.0) and incompatibilities arise very dependent on which software component(s) a Patch alters; the "category" chosen by a Patch developer may be used as a hint, but is a quite fuzzy one. So this may be a case where we as developers of PM have no real alternative but to put the burden to make a proper choice on the users.

Ultimately this setting should not be a global setting, but one for each Patch installed with a selectable global default. Technically this seems to be the best solution, but in terms of usability I am doubting that.

Which makes my stream of thoughts circling back to an earlier assessment, that the global options 1, 2 and 5 above might be sufficient.

Still pondering, what do you think?

@nephros
Copy link
Contributor Author

nephros commented Feb 5, 2023

Ultimately this setting should not be a global setting, but one for each Patch installed with a selectable global default.

Ah, interesting indeed.

So a global option to "empower" a user to force installing an "incompatible" (for whatever reason) patch, and a "force apply this" option for each patch.

I like this. I like this a lot!

For one, the PM daemon currently does not care about any compatibility, it just has a list of patches the user set to "apply this". So there would be no changes in the core of PM necessary to enable this.

All "policy" (i.e. which patches are handed to the deamon to apply) is handled in the PM GUI so we're free to hand the user as much rope to hang themselves with as we like (and make the length of rope configurable to the user if needed).

Ui-wise, yes, switching from "let me ignore any version for any patch " to "empower/emburden me to make decisions myself" mode, where each patch must be manually "force-applied" may be annoying to some.

But OTOH it simplifies everything.
Both for the implementation (PM doesn't need to guess what users want) as well as concept-wise (the user does not need to know what PM means by compatible).
We just have "what the patch developer sais" vs "what the user can do".

Brilliant, brilliant, brilliant.

I am aware I'm basically just repeating what you said above ;)

@Olf0
Copy link
Contributor

Olf0 commented Feb 5, 2023

I am aware I'm basically just repeating what you said above ;)

Not in every detail, you took the idea one step further and simplified it. 😄

But it is good to know that such "policy" requires only changes in the PM UI component(s), not the daemon.

Let us continue to refine the concept in the upcoming week, I have a timeout for today.

@Olf0
Copy link
Contributor

Olf0 commented Feb 7, 2023

Preface

In continued to think about a scheme, which is …

  1. easy to use for simple users
  2. mighty for power-users
  3. simple to implement

Point 1 above requires a global setting for all Patches. Point 2 is well served by a setting for each Patch. For Point 3 it is crucial to find a scheme in which the global setting and the one for a Patch cannot contradict; among other aspects, this implies that the scheme must be independent of the order in which the settings are set, because the user may set the global and the per Patch setting in any order, as often as he / she wants to.

I also simplified the entries and their descriptions slightly.

Settings

Global setting

The installed SailfishOS release version must match at least one entry in the compatibility list of a Patch dependent on this setting. Note that you can override this setting for each Patch individually in its settings.

  • Exact: All four version fields must match.
  • Ignore point release: The first three version fields must match.
  • Disable version check: "Anything goes."

Per Patch setting

The installed SailfishOS release version must match at least one entry in the compatibility list of this Patch dependent on this setting, with which the setting for all Patches in Patchmanager's settings can be overridden.

  • Use setting for all Patches: See Patchmanager's settings.
  • Exact: All four version fields must match.
  • Ignore point release: The first three version fields must match.
  • Disable version check: "Anything goes."

Handling of these two settings

Both settings default to their first entry. Then everything is up to the user.

I am afraid that users will sooner or later ask for a "Reset to default" for these two settings (or all settings). It is also likely that the ability to reset these will lower support efforts, because some users tend to play with settings, forget what they did and then complain that things are not working as expected.

@Olf0 Olf0 modified the milestone: 3.2.3 Feb 7, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Jul 25, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Jul 25, 2023
nephros pushed a commit to nephros/patchmanager that referenced this issue Jul 25, 2023
Even though this is outdated, something similar needs to be implemented to address [issue \sailfishos-patches#332](sailfishos-patches#322).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request feature requests, suggestions, ideas etc.
Projects
None yet
Development

No branches or pull requests

3 participants