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

Add option to have multiple updates in a single appcast that can be selectively chosen #51

Closed
andymatuschak opened this issue Oct 23, 2011 · 38 comments · Fixed by #1879
Closed

Comments

@andymatuschak
Copy link
Contributor

**livings124 (livings124)* reported (on Launchpad) on' 2009-01-12:*

I'm not sure if "Answers" or here is the best place to add feature
requests, so I'm guessing it's here.

It would be useful to have multiple updates listed in a single appcast
file, with some sort of key separating them. This would be similar to
how it works now with minimum version numbers.

For an example, I could have 2 updates, one with no key and one with the
key "beta". There would be some sort of method (delegate maybe?) to
specify all the valid keys (as an array most likely). If no keys are
specified, the update with the "beta" key would be ignored and only the
other would be considered. If one of the keys specified in the code is
"beta", however, both would be considered, and the one with the higher
version number would be used. This would simplify things greatly for
users that want to support optional updates for betas, etc.; it's easier
to maintain and feels less hacky than having multiple feeds that have to
be synchronized.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-01-11:*

That would simplify things, but I'm trying not to complicate the schema
or add too many features unnecessarily. I'm trying to keep things
simple, sorry.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

If I were to write a patch for this, would that help get it in. I've
already looked through the code - it would be very trivial to add and
requires only 2 more methods (accessor and mutator, so basically one).
The ability to tag appcast items would greatly enhance and simplify
Sparkle's functionality - I could think of more examples where this
would be very useful.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-01-12:*

Yeah, that'd definitely help, though not for 1.5—I'm trying to minimize
change for convergence now.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

Alright, I'll get you the patch. Btw, is there any eta on when 1.5 final
is going to be out? It seems pretty solid to me (I'm using it in a
released product without any negative feedback regarding updating).

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-01-12:*

Hopefully I'll get a chance to test and release within a few weeks. Time
is tight here at school, but I'm not going to change much more.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

Attached is a patch to add "tags".

In the enclosure, add "sparkle:tag="value"

Using SUUpdater, you can specify a set of tags that you want to include
when checking for updates. Any appcast item that has either no tag or
one of the tags specified in your SUUpdater object will be considered.

Any chance at all this could make it to 1.5? It will save me having to
build myself. :)

Cheers,
Mitch

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-01-12:*

Sorry, too big a chance for 1.5 this close to release, but thanks a lot
for the patch!

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

For consistency with sparkle:minimumSystemVersion, dict should probably
be used instead of enclosure.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

Updated patch moving sparkle:tag outside of the enclosure.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-12:*

I shouldn't do this when I'm this tired. Updated patch again without
typo.

@andymatuschak
Copy link
Contributor Author

**Hofman (cmhofman)* wrote on 2009-01-13:*

Andy is definitely right to reject this for 1.5. In fact, I see
compelling reasons to to reject this even altogether:

  • When a user accepts several tags, which item will be used? It will now choose an arbitrary one. I think this can only work if there's also some kind of popup to choose between a set of applicable updates, as in another RFE.
  • It is not backward compatible. Note that (older) Sparkle versions without this feature don't know how to deal with appcasts using this feature (and they do get them!). And you cannot have the app updated before it checks ;-) I really think this should be a showstopper, as I see no easy fix.
  • I don't see much of a simplification here. It just replaces one potentially confusing part (using different feedURLs) with another (using a confusing "tag", which doesn't have an obvious meaning). Beware of feature bloat, as it leads to confusion!

These issues (and perhaps more) should be carefully considered before
applying this.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-01-13:*

  • When a user accepts several tags, which item will be used? It will now
    choose an arbitrary one. I think this can only work if there's also some
    kind of popup to choose between a set of applicable updates, as in
    another RFE.

I don't understand this comment. You can already put multiple items in
an appcast, and it will chose the newest one which meets the system
requirements. The tag would behave the same way. IE: I have 3 listed
(from newest to oldest). The newest requires 10.5 and has beta tag. The
second requires 10.5 and has no tag. The third has no tags or
requirements. On 10.4 the third is selected. On 10.5 with no tags set in
the appcast, the second is selected. On 10.5 with the beta tag enabled
in code, the first is selected.

  • It is not backward compatible. Note that (older) Sparkle versions
    without this feature don't know how to deal with appcasts using this
    feature (and they do get them!). And you cannot have the app updated
    before it checks ;-) I really think this should be a showstopper, as I
    see no easy fix.

The same can be said about system minimum version for older Sparkle
versions. IMHO, that is a larger problem than the tag system, as that
can result in installing software that might not even launch as opposed
to something like beta software. If anything, this is an argument to put
it in 1.5, as you might as well only have to deal with this issue with
one release, but I digress. This feature is opt-in, so developers can
handle is however they want.

  • I don't see much of a simplification here. It just replaces one
    potentially confusing part (using different feedURLs) with another
    (using a confusing "tag", which doesn't have an obvious meaning). Beware
    of feature bloat, as it leads to confusion!

Having to maintain a single appcast is a huge simplification. When you
release beta software, add a new item with a beta tag; when it's
released just remove that item. This is a lot easier to maintain than
having a separate beta feed that sometimes points to beta software,
sometimes points to the same as released. Over time with multiple feeds,
you might end up with a lot of separate appcast to maintain forever
(different types of betas); with tags you can just remove the item and
you're still good. 3 lines of documentation would fully explain the tag
functionality, and it's opt-in. I agree with avoiding feature bloat
fully, but a single, multipurpose tag is not at all bloaty - in fact, it
might avoid in the future adding more specific built-in features
regarding filtering; it's generic enough to be used for many different
uses. Plus regarding confusion, this will not be user-facing, only
developer-facing.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-14:*

I have used a patched version of this in Transmission for the last few
releases without problems - it appears to be ready for primetime.
Considering that this is the type of feature where the advantage will
require it to be implemented as early as possible, can I suggest this go
into 1.5 (even if it's not documented, having this in the code base will
be advantageous when it's actually used in future versions, as discussed
previously). I wasn't going to push for this to be in 1.5, but since
1.5 has been lingering in RC mode for 9 months perhaps things have
changed.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-14:*

7 months, still a long time ;)

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-04-14:*

I've put some thought into this problem, and I'm not sure this is the
best way to do it in the end. There are some tags for which it makes
sense to attach semantic meaning, like "paid," which is another big
problem for a lot of Sparkle's clients (though not Transmission, of
course!)

Llike: you want people using 3.0b3 to get notified of their free update
to 3.0 (to thank them for testing), but you don't want people using 2.5
to get that. Futhermore, what if you want to issue a critical security
update to 2.5 (call it 2.5.1) that they should get, but 3.0 shouldn't
see that update?

I think you need a graph, basically. I was thinking posets with labels.
Labels could be "sticky" or not, potentially applying to incident edges
coming from a vertex with that label. This would be useful for the
"paid" label.

So between these hesitations and my continued promise that 1.5 is
feature-frozen, I think this needs to wait.

It's maybe not a bad idea to get this kind of thing out there, though.
And you can! Just branch Sparkle and apply your changes; it'll appear in
Sparkle's main launchpad page for other people to merge in if they like.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-14:*

Interesting idea with the posets - but that seems like it might get
confusing/complicated really quick for developers. With the tag concept
from this patch and some creativity, I think your example can be
handled.

Yes, I understand that 1.5 is feature frozen and respect that, but I was
considering Hofman's point of backwards compatibility - the faster the
change is in the code, the easier things are for developers that want to
use this functionality. Btw, is there an eta for 1.5?

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-04-15:*

You're definitely right about the backwards compatibility point. I'll
think about whether this simpler concept might work for "paid" kinds of
tags. Posets won't be too bad if there's UI for it. It's just "what
versions should this version trump?" And straight version number
comparison could be optionally implicit.

As for an ETA on 1.5 final, I'm not in a huge rush since it's pretty
stable as is. If nothing else, I'll have a little more time to work on
it this summer when I'm not doing coursework all the time.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-15:*

Sure tags would work for paid apps - just use a "paid" tag on all items
that require payment. Those tagged items will only be considered by
Sparkle if the app has set to use the "paid" tag somewhere in code. The
code could be expanded to have items in the appcast have multiple tags
if that's a concern, but the current implementation is generic and
simple enough to be useful in all cases I can come up with.

And as I said the only reason I'm pushing on the eta of 1.5 is because
of the feature-freeze and backwards compatibility concerns. So many apps
are currently using the RC that it seems the current trunk could be
released and anything being worked on now could be 1.51/1.6. But of
course, the release schedule is completely up to you.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-04-15:*

Hm. Consider the case of Inquisitor, where the upgrade from 1.0 -> 2.0
cost money, but 3.0 was released for free. There are multiple "paths"
from 1.0 to 3.0; one of them's free, and one of them costs money!

If the suggested implementation is "just check whatever's latest," that
does solve this problem, but it fails for the following case:

2.0 is a paid upgrade from 1.0, so the developer adds the "paid" tag.
Then 2.1 is released. If the developer puts the "paid" tag on 2.1 to
restrict 1.0 users again, users of 2.0 won't get the free upgrade! I
guess one alternative is to use uinque tags for each branch point as
well. But meh...

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-15:*

Yup, the solution would be a "paid2.x" tag (or something similar). Of
course this is a bit complex, but it is a complex situation. Honestly I
can't think of a simpler solution to this.

In your example, if the appcast supported multiple tags per appcast
item, there could be items in a single appcast for 1.1 (no tag), 2.1
("paid2.x" tag), and 2.5b ("paid2.x" tag and "beta" tag). If the user is
running 1.0 and hasn't paid, the "paid2.x" tag won't be set so it'll
grab 1.1 (regardless if the "beta" tag is set in the app). If the 1.0
user has paid, but doesn't have the "beta" tag enabled, it'll grab 2.1.
And so forth for 2.5b2. Now if the user is running 2.0, the "paid2.x"
tag will always be set, so it will grab 2.1 or 2.5b depending on the
"beta" flag.

IMHO this is highly extensible and solves the problem of multiple tags
without being too complicated, and only requires the addition of
supporting multiple tags in an appcast item (plus the currently posted
patch).

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-04-15:*

I see. I could definitely see adding preliminary support for this to 1.5, letting all the logic be client-side through the appropriate delegate method:

  • (SUAppcastItem *)bestValidUpdateInAppcast:(SUAppcast *)appcast forUpdater:(SUUpdater *)bundle;

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-04-15:*

What is wrong with the (very) simple way that my patch solves it? I
don't really see the need for the delegate method - much like how it
currently decides the best item for different OS requirements, it picks
the item with the highest version number where the tag requirement is
met.

Unless you mean not adding the new tag field, in which case how would
the code know which item meets the appropriate criteria?

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2009-04-28:*

This works, it just feels kinda kludgy somehow. I'll give it more
thought when I start working on the next version. Thanks again for the
patch!

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-08-02:*

Is there ever going to be a release. The longer this waits, the more of
a pain it will be for developers. What specifically is holding up 1.5?

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2009-12-21:*

bump

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2010-01-14:*

Now that the word on the street is that you're skipping 1.5 and going to
1.6, can this be looked at again. I have been using this in Transmission
for about a year now and it's worked really well.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2010-01-14:*

Oh, no, word on the street has gotten out!

I still don't like the tag solution. I'll either figure out why or add
that delegate method.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2010-01-14:*

I still don't understand the delegate option - without a tag there
wouldn't be enough criteria to make a valid choice. I'm open to other
thoughts and ideas, as long as the solution results in multiple items in
a single appcast.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2010-01-14:*

Yeah, so, the delegate would have to be passed the complete appcast item
dictionary, including all custom keys defined.

The problem I'm having here is that if I do some kind of tagging /
branching system, I'd like it to support the paid case, too. You can't
just use this technique with a "paid" key to mark paid updates: you'd
need to be able to release new updates to users running both 1.x and 2.x
if you had, say, a security vulnerability. There's actually a poset
involved here.

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2010-01-14:*

Fair enough, a delegate system combined with the added tags of the
current patch would make sense in that case. Perhaps the delegate method
would give an array sorted in order of highest version (removing those
that don't match OS requirements), and from there, the delegate method
would return the item it chooses; if the delegate, the first item will
simply be the one used (matching current behavior).

@andymatuschak
Copy link
Contributor Author

**livings124 (livings124)* wrote on 2010-08-06:*

Any chance of this going in anytime soon? The "tag" functionality is
generic and flexible enough to support
https://answers.launchpad.net/sparkle/+question/119398 with a single
appcast as well.

@andymatuschak
Copy link
Contributor Author

**Andy Matuschak (andymatuschak)* wrote on 2010-08-09:*

No, sorry. I want to fix this the right way, and I think we need posets.

@andymatuschak
Copy link
Contributor Author

I'm not sure that any solution we've considered would have a sufficiently persuasive value-to-complexity ratio over simply using multiple appcasts to be worth the effort.

@livings124
Copy link

I still contend that the original solution I posted is much, much less complex than multiple appcasts (and by a large factor too). I've been using this patch for years without issue. Multiple appcasts are flawed in that you have to code into the app which one to look at, and makes maintenance of multiple appcasts extremely annoying.

@andymatuschak
Copy link
Contributor Author

You're absolutely right: it's definitely less complex and definitely better. But it's also not itself a great solution, and we can't easily remove API like this once added. I don't think it's prudent to add API which works but which we don't like very much.

@livings124
Copy link

If you want to make it simpler, you can instead use pre-defined tags such as "beta", etc. There needs to be a better solution than manually managing multiple-appcasts.

@andymatuschak
Copy link
Contributor Author

Yes, I think that might be a more amenable route. I’ve started to go down that road in the quiet-automatic-updates branch with a criticalUpdate tag.

@zorgiepoo
Copy link
Member

This is a long old issue.. But I like the original proposal here where an appcast item can provide a set of tags (or "channels" because Sparkle has prematurely decided to use "tags" for something else now). By default, non-default channels would be ignored.

The posets and paid case I feel can be solved independently by #329 for specifying a version point where an update became a paid or major one. I don't think this feature should be related to offering paid upgrades, but rather should be for beta or nightly, or even demo channels.

The -bestValidUpdateInAppcast: delegate method has been almost forever buggy and slightly painful to support. It's likely difficult to use, and likely misused. And most importantly it requires developers implementing it before shipping an update they've already shipped, and is less friendly to external updaters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants