Patches on other channels

[JessicaTegner]

Hi.
Something that has been bugging me, and I can't seem to find an answer for is, why do we only have the possibility to create patches on the "stable" channel.
Why are patches not available for the beta and alpha channels?

[JessicaTegner]

Ok. So after further testing, it seems that patches do indeed get produced for other channels, however the documentation says otherwise.

From docs/commands.md

The build command wraps pyinstaller to create the final executable. All options are passed to pyinstaller. Once built the executable is archived, in a pyupdater compatible format, and placed in the pyu-data/new directory. If you supply a version number with an alpha or beta tag, when processed this binary will be placed on the respective release channel. Note that patches will only be created for the stable channel.

from docs/usage-cli-advanced.md

PyUpdater supports 3 release channels. Release channels are specified when providing a version number to the --app-version flag. Patches are only created for the stable channel. Examples below.

And possibly more places.

This issue could be a good one for new contributors, since we only need the documentation updated, to reflect the behavior in the code (which is, that patches gets produced on all release channels, instead of just the "stable").

[dennisvang]

It would also be very helpful if the documentation contained a separate section on release channels, with a bit more background on the use of release channels in general, and examples describing basic usage.

[dennisvang]

@NicklasTegner It turns out this is not just a documentation issue:

Although patches are indeed created for pre-release channels (alpha, beta), they are never actually applied by the client.

This can be seen in client.updates.LibUpdate._download(), which only calls _patch_update() if channel == "stable" (also in 3.1.1).

Here's the relevant part:

def _download(self):
    if self.name is not None:
        ...
            patch_success = False
            if self.channel == "stable":
                patch_success = self._patch_update()
            ...
            if patch_success:  # pragma: no cover
                ...
            else:
                ...
                update_success = self._full_update()
                ...
    ...

So I guess this issue should be re-opened.

Maybe it is as simple as removing the if self.channel == "stable" condition...

[dennisvang]

Some additional observations:

• patch names include a platform specifier, but they do not include a channel specifier, e.g. Acme-mac-61
  (UPDATE: Sorry, I was wrong here, as you can see in the Patch source. It's just the patch names in the test data which are missing a channel specifier.)
• patch number does not take into account platform (or channel) at all

This is illustrated, for example, by the "py_repo_config" object in .pyupdater/config.pyu, which keeps only one patch count, regardless of the platforms and number of release channels available.

Also see core.package_handler.patch.Patch._check_make_patch().

Maybe this is all intentional, but I think it is quite confusing.

To me it would make more sense to use consistent filenames for the full package archives and the patches (e.g. only a different extension), and separate the patches by platform (and possibly channel), just like it is done for full packages.

Any thoughts?

[JessicaTegner]

@dennisvang in response to your 2 latest comments.
This seems like a huge oversite. As in, why would you only create patches for the stable channel. Seems like a random limitation put in, for no apparent reason. Maybe @JMSwag has some more insite on this.
It probably wouldn't just be removing the if statement, since you would have to also update how the filenames were constructed, as you pointed out yourself.
In addition, when/if you do that, you would also need a compat layer of some sort, that could convert "old" repos to new ones (where the filenames have changed)

[dennisvang]

@NicklasTegner I think there's also a bit of confusion on my part, because it is not quite clear to me what is the intention of the "release channels" in pyupdater. The strict option in Client.update_check() suggests to me that the channels are either treated as separate, isolated release paths, or disregarded completely. However, the single patch counter suggests that there is a single, linear release path (as I would expect)...

Perhaps @JMSwag could elaborate?

Here's how I see it:

• Pre-release packages should not be treated differently from final release (stable) packages in any way: The pre-release segment ("a", "b", "alpha", ...) in the version string only serves to enable filtering during the check for available updates.
• If we support a single release path (per platform), without branches, then having a single "patch counter" (per platform) makes sense. I assume this is why the current implementation has only one counter for the patch number.

Here's an example of what I have in mind when I talk about a "linear" release path, per platform:

1.0 ⇒ patch0 ⇒ 1.1a0 ⇒ patch1 ⇒ 1.1a1 ⇒ patch2 ⇒ 1.1b0 ⇒ patch3 ⇒ 1.1 ⇒ patch4 ⇒ 1.2a0

Now, suppose our client is at 1.0 (i.e. a "stable" release).

If the client checks the "stable" channel for updates, it will find 1.1 available, and will download patch0 through patch3 and apply them.

If the client checks the "beta" channel for updates, it will try to find the latest version, which can be either "beta" or "stable", whichever is higher. In this case, the latest version is "stable" 1.1, so the client will also download patch0 through patch3.

If the client checks the "alpha" channel, it will find 1.2a0, so it will download patch0 through patch4.

If the client is currently at e.g. 1.1a1 and it checks the "beta" channel for updates, it will find 1.1 and it will download patch2 and patch3.

@JMSwag Does this make sense? Is this how it is supposed to work?

[JessicaTegner]

@dennisvang that sounds to me, like how it's supposed to work

[JMSwag]

@dennisvang @NicklasTegner

Agreed. The suggested approach is how it should be.

[dennisvang]

@JMSwag @NicklasTegner

Allright, thanks for the quick feedback. :)

I think the current implementation already works mostly along these lines, but it is not quite there yet, for example:

• get_highest_version()
• LibUpdate._download()
• Patcher._get_required_patches()

If you don't mind, I can start working on this right away.

[JMSwag]

Please, by all means ❤️

[dennisvang]

Great. I think I'll start by creating some tests based on my example above.

[dennisvang]

@JMSwag @NicklasTegner

I think the client-side is coming along reasonably well, as you can see in (draft) PR #310.

I'll start working on the patch creation side now (package_handler, etc.).