search

YunoHost-Apps / Experimental_helpers

6 stars 12 forks source link

Add check_app_version_changed helper to allow preventing unnecessary upstream app upgrade #17

Closed JimboJoe closed 6 years ago

JimboJoe commented 6 years ago

This helper should be used to avoid an upgrade of the upstream version when it's not needed (but yet allowing to upgrade other parts of the YunoHost application (e.g. nginx conf).

Currently used in Discourse package.

maniackcrudelis commented 6 years ago

We already have a helper to handle that https://github.com/YunoHost-Apps/Experimental_helpers/blob/master/ynh_abort_if_up_to_date/ynh_abort_if_up_to_date_2

Is this one different ?

JimboJoe commented 6 years ago

Yes, it's different in the fact that the upgrade script can make other upgrade actions (change the nginx or other configuration files), while not touching the upstream application itself if not needed. The mentioned script aborts the upgrade all the way.

maniackcrudelis commented 6 years ago

OK But unless I missed something, the real difference is here there no exit 0. I think that it would be better to keep only one helper, and merge this one with the existing one.

JimboJoe commented 6 years ago

Well, the two scripts don't compare the same things: this one compares the "upstream part" (before ~ynh) of the package versions, whereas the other one compares the global version (and could eventually be managed in the core).

maniackcrudelis commented 6 years ago

The existing helper check the whole version number, with ~ynh, but without checking the 2 parts separately.

But, as I understand, if the whole version number don't change, we don't have to update anything. But, if one or the other part of the version number is different, then we could update a part of the app.

Then, wouldn't be interesting to update the existing helper to be more clever and be able to check the 2 parts of the version number and give an exit code (like this helper) according to what should be done? Or simply exit if there's nothing to do.

JimboJoe commented 6 years ago

Then a principle could be: Set a check_app_version_changed helper that returns:

Does that sound legit?

maniackcrudelis commented 6 years ago

Yes, that sounds good to me

JimboJoe commented 6 years ago

Here is an attempt to merge the two helpers (unit-tested). What do you think?

maniackcrudelis commented 6 years ago

That seems good. Maybe just change the return values, UPSTREAM_APP and YNH_APP by something clearer about the use case of each of them. Because these values are going to be used in the upgrade scripts. Maybe something like UPGRADE_APP and UPGRADE_PACKAGE.