Merge pull request #1878 from sparkle-project/sparkle-version-element

Favor sparkle:version and sparkle:shortVersionString elements over enclosure attributes
sparkle-project · Jun 27, 2021 · ad4ec8e · ad4ec8e
2 parents 6639fea + 8997bad
commit ad4ec8e
Show file tree

Hide file tree

Showing 11 changed files with 68 additions and 32 deletions.
diff --git a/Resources/SampleAppcast.xml b/Resources/SampleAppcast.xml
@@ -6,35 +6,39 @@
         <language>en</language>
         <item>
             <title>Version 2.0 (2 bugs fixed; 3 new features)</title>
+            <link>http://sparkle-project.org</link>
+            <sparkle:version>2.0</sparkle:version>
             <sparkle:releaseNotesLink>
                 http://you.com/app/2.0.html
             </sparkle:releaseNotesLink>
-            <link>http://sparkle-project.org</link>
             <pubDate>Wed, 09 Jan 2006 19:20:11 +0000</pubDate>
-            <enclosure url="http://you.com/app/Your%20Great%20App%202.0.zip" sparkle:version="2.0" length="1623481" type="application/octet-stream" sparkle:edSignature="WVyVJpOx+a5+vNWJVY79TRjFKveNk+VhGJf2iti4CZtJsJewIUGvh/1AKKEAFbH1qUwx+vro1ECuzOsMmumoBA==" />
+            <enclosure url="http://you.com/app/Your%20Great%20App%202.0.zip" length="1623481" type="application/octet-stream" sparkle:edSignature="WVyVJpOx+a5+vNWJVY79TRjFKveNk+VhGJf2iti4CZtJsJewIUGvh/1AKKEAFbH1qUwx+vro1ECuzOsMmumoBA==" />
             <sparkle:minimumSystemVersion>10.11</sparkle:minimumSystemVersion>
         </item>
 
         <item>
             <title>Version 1.5 (8 bugs fixed; 2 new features)</title>
+            <link>http://sparkle-project.org</link>
+            <sparkle:version>1.5</sparkle:version>
             <sparkle:releaseNotesLink>
                 http://you.com/app/1.5.html
             </sparkle:releaseNotesLink>
-            <link>http://sparkle-project.org</link>
             <pubDate>Wed, 01 Jan 2006 12:20:11 +0000</pubDate>
-            <enclosure url="http://you.com/app/Your%20Great%20App%201.5.zip" sparkle:version="1.5" length="1472893" type="application/octet-stream" sparkle:edSignature="pNFd7KbcQSu+Mq7UYrbQXTPq82luht2ACXm/r2utp1u/Uv/5hWqctdT2jwQgMejW7DRoeV/hVr6J4VdZYdwWDw==" />
+            <enclosure url="http://you.com/app/Your%20Great%20App%201.5.zip" length="1472893" type="application/octet-stream" sparkle:edSignature="pNFd7KbcQSu+Mq7UYrbQXTPq82luht2ACXm/r2utp1u/Uv/5hWqctdT2jwQgMejW7DRoeV/hVr6J4VdZYdwWDw==" />
             <sparkle:minimumSystemVersion>10.11</sparkle:minimumSystemVersion>
         </item>
 
         <!-- Now here's an example of a version with a weird internal version number (like an SVN revision) but a human-readable external one. -->
         <item>
             <title>Version 1.4 (5 bugs fixed; 2 new features)</title>
+            <link>http://sparkle-project.org</link>
             <sparkle:releaseNotesLink>
                 http://you.com/app/1.4.html
             </sparkle:releaseNotesLink>
-            <link>http://sparkle-project.org</link>
+            <sparkle:version>241</sparkle:version>
+            <sparkle:shortVersionString>1.4</sparkle:shortVersionString>
             <pubDate>Wed, 25 Dec 2005 12:20:11 +0000</pubDate>
-            <enclosure url="http://you.com/app/Your%20Great%20App%201.4.zip" sparkle:version="241" sparkle:shortVersionString="1.4" sparkle:edSignature="Ody3D/ybSMH4T+P/oNj3LN4F0SA8RJGLEr1TI4UemrBAiJ9aEcDnYV3u58P75AbcFjI13jPYmHDUHXMSTFQbDw==" length="1472349" type="application/octet-stream" />
+            <enclosure url="http://you.com/app/Your%20Great%20App%201.4.zip" sparkle:edSignature="Ody3D/ybSMH4T+P/oNj3LN4F0SA8RJGLEr1TI4UemrBAiJ9aEcDnYV3u58P75AbcFjI13jPYmHDUHXMSTFQbDw==" length="1472349" type="application/octet-stream" />
             <sparkle:minimumSystemVersion>10.11</sparkle:minimumSystemVersion>
         </item>
     </channel>

diff --git a/Sparkle/SUAppcastItem.m b/Sparkle/SUAppcastItem.m
@@ -312,23 +312,28 @@ - (nullable instancetype)initWithDictionary:(NSDictionary *)dict relativeToURL:(
 {
     self = [super init];
     if (self) {
+        _title = [(NSString *)[dict objectForKey:SURSSElementTitle] copy];
+
         NSDictionary *enclosure = [dict objectForKey:SURSSElementEnclosure];
 
         // Try to find a version string.
-        // Finding the new version number from the RSS feed is a little bit hacky. There are two ways:
+        // Finding the new version number from the RSS feed is a little bit hacky. There are a few ways:
         // 1. A "sparkle:version" attribute on the enclosure tag, an extension from the RSS spec.
-        // 2. If there isn't a version attribute, Sparkle will parse the path in the enclosure, expecting
+        // 2. If there isn't a version attribute, see if there is a version element (this is now the recommended path).
+        // 3. If there isn't a version element, Sparkle will parse the path in the enclosure, expecting
         //    that it will look like this: http://something.com/YourApp_0.5.zip. It'll read whatever's between the last
         //    underscore and the last period as the version number. So name your packages like this: APPNAME_VERSION.extension.
         //    The big caveat with this is that you can't have underscores in your version strings, as that'll confuse Sparkle.
         //    Feel free to change the separator string to a hyphen or something more suited to your needs if you like.
         NSString *newVersion = [enclosure objectForKey:SUAppcastAttributeVersion];
         if (newVersion == nil) {
-            newVersion = [dict objectForKey:SUAppcastAttributeVersion]; // Get version from the item, in case it's a download-less item (i.e. paid upgrade).
+            // Get version from the item
+            newVersion = [dict objectForKey:SUAppcastElementVersion];
         }
-        if (newVersion == nil) // no sparkle:version attribute anywhere?
+        if (newVersion == nil)
         {
-            SULog(SULogLevelError, @"warning: <%@> for URL '%@' is missing %@ attribute. Version comparison may be unreliable. Please always specify %@", SURSSElementEnclosure, [enclosure objectForKey:SURSSAttributeURL], SUAppcastAttributeVersion, SUAppcastAttributeVersion);
+            // No sparkle:version element/attribute anywhere?
+            SULog(SULogLevelError, @"warning: Item '%@' is missing '<%@>' element. Version comparison may be unreliable. Please always specify %@", _title, SUAppcastElementVersion, SUAppcastElementVersion);
 
             // Separate the url by underscores and take the last component, as that'll be closest to the end,
             // then we remove the extension. Hopefully, this will be the version.
@@ -340,13 +345,12 @@ - (nullable instancetype)initWithDictionary:(NSDictionary *)dict relativeToURL:(
 
         if (!newVersion) {
             if (error) {
-                *error = [NSString stringWithFormat:@"Feed item lacks %@ attribute, and version couldn't be deduced from file name (would have used last component of a file name like AppName_1.3.4.zip)", SUAppcastAttributeVersion];
+                *error = [NSString stringWithFormat:@"Feed item lacks %@ element, and version couldn't be deduced from file name (would have used last component of a file name like AppName_1.3.4.zip)", SUAppcastElementVersion];
             }
             return nil;
         }
 
         _propertiesDictionary = [[NSDictionary alloc] initWithDictionary:dict];
-        _title = [(NSString *)[dict objectForKey:SURSSElementTitle] copy];
         _dateString = [(NSString *)[dict objectForKey:SURSSElementPubDate] copy];
         _itemDescription = [(NSString *)[dict objectForKey:SURSSElementDescription] copy];
 

diff --git a/Sparkle/SUConstants.h b/Sparkle/SUConstants.h
@@ -77,6 +77,8 @@ extern NSString *const SUAppcastAttributeVersion;
 extern NSString *const SUAppcastAttributeOsType;
 extern NSString *const SUAppcastAttributeInstallationType;
 
+extern NSString *const SUAppcastElementVersion;
+extern NSString *const SUAppcastElementShortVersionString;
 extern NSString *const SUAppcastElementCriticalUpdate;
 extern NSString *const SUAppcastElementDeltas;
 extern NSString *const SUAppcastElementMinimumAutoupdateVersion;

diff --git a/Sparkle/SUConstants.m b/Sparkle/SUConstants.m
@@ -73,6 +73,8 @@
 NSString *const SUAppcastAttributeOsType = @"sparkle:os";
 NSString *const SUAppcastAttributeInstallationType = @"sparkle:installationType";
 
+NSString *const SUAppcastElementVersion = SUAppcastAttributeVersion;
+NSString *const SUAppcastElementShortVersionString = SUAppcastAttributeShortVersionString;
 NSString *const SUAppcastElementCriticalUpdate = @"sparkle:criticalUpdate";
 NSString *const SUAppcastElementDeltas = @"sparkle:deltas";
 NSString *const SUAppcastElementMinimumAutoupdateVersion = @"sparkle:minimumAutoupdateVersion";

diff --git a/TestApplication/sparkletestcast.xml b/TestApplication/sparkletestcast.xml
@@ -6,6 +6,7 @@
     <language>en</language>
       <item>
         <title>Version 2.0</title>
+        <sparkle:version>2.0</sparkle:version>
         <description>
           <![CDATA[
             <ul>
@@ -17,7 +18,7 @@
           ]]>
         </description>
         <pubDate>Sat, 26 Jul 2014 15:20:11 +0000</pubDate>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="2.0" length="$INSERT_ARCHIVE_LENGTH" type="application/octet-stream" sparkle:edSignature="$INSERT_EDDSA_SIGNATURE" />
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="$INSERT_ARCHIVE_LENGTH" type="application/octet-stream" sparkle:edSignature="$INSERT_EDDSA_SIGNATURE" />
       </item>
   </channel>
 </rss>
diff --git a/Tests/Resources/testappcast.xml b/Tests/Resources/testappcast.xml
@@ -6,26 +6,26 @@
         <title>Version 2.0</title>
         <description>desc</description>
         <pubDate>Sat, 26 Jul 2014 15:20:11 +0000</pubDate>
+        <!-- Test sparkle:version in enclosure -->
         <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="2.0" />
         <sparkle:criticalUpdate sparkle:version="1.5" />
     </item>
     <!-- The best chosen release -->
     <item>
         <title>Version 3.0</title>
+        <sparkle:version>3.0</sparkle:version>
         <sparkle:phasedRolloutInterval>86400</sparkle:phasedRolloutInterval>
         <!-- Test legacy critical update tag -->
         <sparkle:tags><sparkle:criticalUpdate /></sparkle:tags>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="3.0" length="1346234" />
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
         <sparkle:deltas>
             <enclosure url="http://localhost:1337/3.0_from_2.0.patch"
-            sparkle:version="3.0"
             sparkle:deltaFrom="2.0"
             length="1235"
             type="application/octet-stream"
             sparkle:edSignature="..." />
 
             <enclosure url="http://localhost:1337/3.0_from_1.0.patch"
-            sparkle:version="3.0"
             sparkle:deltaFrom="1.0"
             length="1485"
             type="application/octet-stream"
@@ -35,14 +35,16 @@
     <!-- A release testing minimumSystemVersion -->
     <item>
         <title>Version 4.0</title>
+        <sparkle:version>4.0</sparkle:version>
         <pubDate>Sat, 26 Jul 2014 15:20:13 +0000</pubDate>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="4.0" length="1346234" />
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
         <sparkle:minimumSystemVersion>17.0.0</sparkle:minimumSystemVersion>
     </item>
     <!-- A joke release testing maximumSystemVersion -->
     <item>
         <title>Version 5.0</title>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="5.0" length="1346234" />
+        <sparkle:version>5.0</sparkle:version>
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
         <sparkle:maximumSystemVersion>2.0.0</sparkle:maximumSystemVersion>
     </item>
   </channel>

diff --git a/Tests/Resources/testappcast_info_updates.xml b/Tests/Resources/testappcast_info_updates.xml
@@ -23,19 +23,18 @@
     <item>
         <title>Version 4.0</title>
         <pubDate>Sat, 26 Jul 2014 15:20:13 +0000</pubDate>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="4.0" length="1346234" />
+        <sparkle:version>4.0</sparkle:version>
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
         <sparkle:informationalUpdate></sparkle:informationalUpdate>
         <link>http://sparkle-project.org</link>
         <sparkle:deltas>
-            <enclosure url="http://localhost:1337/3.0_from_2.0.patch"
-            sparkle:version="3.0"
+            <enclosure url="http://localhost:1337/4.0_from_2.0.patch"
             sparkle:deltaFrom="2.0"
             length="1235"
             type="application/octet-stream"
             sparkle:edSignature="..." />
 
-            <enclosure url="http://localhost:1337/3.0_from_1.0.patch"
-            sparkle:version="3.0"
+            <enclosure url="http://localhost:1337/4.0_from_1.0.patch"
             sparkle:deltaFrom="1.0"
             length="1485"
             type="application/octet-stream"

diff --git a/Tests/Resources/testappcast_minimumAutoupdateVersion.xml b/Tests/Resources/testappcast_minimumAutoupdateVersion.xml
@@ -4,12 +4,14 @@
     <title>For unit test only</title>
     <item>
         <title>Version 3.0</title>
+        <sparkle:version>3.0</sparkle:version>
         <sparkle:minimumAutoupdateVersion>2.0</sparkle:minimumAutoupdateVersion>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="3.0" length="1346234" />
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
     </item>
     <item>
         <title>Version 2.0</title>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="2.0" length="1346234" />
+        <sparkle:version>2.0</sparkle:version>
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" length="1346234" />
     </item>
   </channel>
 </rss>
diff --git a/Tests/Resources/testlocalizedreleasenotesappcast.xml b/Tests/Resources/testlocalizedreleasenotesappcast.xml
@@ -5,8 +5,9 @@
     <!-- A release testing localized release notes -->
     <item>
         <title>Version 6.0</title>
+        <sparkle:version>6.0</sparkle:version>
         <pubDate>Sat, 26 Jul 2019 15:20:13 +0000</pubDate>
-        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" sparkle:version="6.0" />
+        <enclosure url="http://localhost:1337/Sparkle_Test_App.zip" />
         <sparkle:releaseNotesLink>https://sparkle-project.org/#works</sparkle:releaseNotesLink>
         <sparkle:releaseNotesLink xml:lang='en'>
             https://sparkle-project.org/#localized_notes_link_works

diff --git a/Tests/SUAppcastTest.swift b/Tests/SUAppcastTest.swift
@@ -30,13 +30,15 @@ class SUAppcastTest: XCTestCase {
             XCTAssertEqual("desc", items[0].itemDescription)
             XCTAssertEqual("Sat, 26 Jul 2014 15:20:11 +0000", items[0].dateString)
             XCTAssertTrue(items[0].isCriticalUpdate)
+            XCTAssertEqual(items[0].versionString, "2.0")
 
             // This is the best release matching our system version
             XCTAssertEqual("Version 3.0", items[1].title)
             XCTAssertNil(items[1].itemDescription)
             XCTAssertNil(items[1].dateString)
             XCTAssertTrue(items[1].isCriticalUpdate)
             XCTAssertEqual(items[1].phasedRolloutInterval, 86400)
+            XCTAssertEqual(items[1].versionString, "3.0")
 
             XCTAssertEqual("Version 4.0", items[2].title)
             XCTAssertNil(items[2].itemDescription)
@@ -58,6 +60,7 @@ class SUAppcastTest: XCTestCase {
 
             XCTAssertEqual(bestAppcastItem, items[1])
             XCTAssertEqual(deltaItem!.fileURL!.lastPathComponent, "3.0_from_1.0.patch")
+            XCTAssertEqual(deltaItem!.versionString, "3.0")
 
             // Test latest delta update item available
             var latestDeltaItem: SUAppcastItem?

diff --git a/generate_appcast/FeedXML.swift b/generate_appcast/FeedXML.swift
@@ -71,7 +71,13 @@ func writeAppcast(appcastDestPath: URL, updates: [ArchiveItem]) throws {
     var numItems = 0
     for update in updates {
         var item: XMLElement
-        let existingItems = try channel.nodes(forXPath: "item[enclosure[@sparkle:version=\"\(update.version)\"]]")
+
+        var existingItems = try channel.nodes(forXPath: "item[enclosure[@\(SUAppcastAttributeVersion)=\"\(update.version)\"]]")
+        if existingItems.count == 0 {
+            // Fall back to see if any items are using the element version variant
+            existingItems = try channel.nodes(forXPath: "item[\(SUAppcastElementVersion)=\"\(update.version)\"]")
+        }
+
         let createNewItem = existingItems.count == 0
 
         // Update all old items, but aim for less than 5 in new feeds
@@ -93,6 +99,20 @@ func writeAppcast(appcastDestPath: URL, updates: [ArchiveItem]) throws {
         if nil == findElement(name: "pubDate", parent: item) {
             item.addChild(XMLElement.element(withName: "pubDate", stringValue: update.pubDate) as! XMLElement)
         }
+
+        var versionElement = findElement(name: SUAppcastElementVersion, parent: item)
+        if nil == versionElement {
+            versionElement = XMLElement.element(withName: SUAppcastElementVersion, uri: sparkleNS) as? XMLElement
+            item.addChild(versionElement!)
+        }
+        versionElement?.setChildren([text(update.version)])
+
+        var shortVersionElement = findElement(name: SUAppcastElementShortVersionString, parent: item)
+        if nil == shortVersionElement {
+            shortVersionElement = XMLElement.element(withName: SUAppcastElementShortVersionString, uri: sparkleNS) as? XMLElement
+            item.addChild(shortVersionElement!)
+        }
+        shortVersionElement?.setChildren([text(update.shortVersion)])
 
         if let html = update.releaseNotesHTML {
             let descElement = findOrCreateElement(name: "description", parent: item)
@@ -157,8 +177,6 @@ func writeAppcast(appcastDestPath: URL, updates: [ArchiveItem]) throws {
         }
         var attributes = [
             XMLNode.attribute(withName: "url", stringValue: archiveURL) as! XMLNode,
-            XMLNode.attribute(withName: SUAppcastAttributeVersion, uri: sparkleNS, stringValue: update.version) as! XMLNode,
-            XMLNode.attribute(withName: SUAppcastAttributeShortVersionString, uri: sparkleNS, stringValue: update.shortVersion) as! XMLNode,
             XMLNode.attribute(withName: "length", stringValue: String(update.fileSize)) as! XMLNode,
             XMLNode.attribute(withName: "type", stringValue: update.mimeType) as! XMLNode,
         ]
@@ -181,8 +199,6 @@ func writeAppcast(appcastDestPath: URL, updates: [ArchiveItem]) throws {
             for delta in update.deltas {
                 var attributes = [
                     XMLNode.attribute(withName: "url", stringValue: URL(string: delta.archivePath.lastPathComponent.addingPercentEncoding(withAllowedCharacters: CharacterSet.urlPathAllowed)!, relativeTo: update.archiveURL)!.absoluteString) as! XMLNode,
-                    XMLNode.attribute(withName: SUAppcastAttributeVersion, uri: sparkleNS, stringValue: update.version) as! XMLNode,
-                    XMLNode.attribute(withName: SUAppcastAttributeShortVersionString, uri: sparkleNS, stringValue: update.shortVersion) as! XMLNode,
                     XMLNode.attribute(withName: SUAppcastAttributeDeltaFrom, uri: sparkleNS, stringValue: delta.fromVersion) as! XMLNode,
                     XMLNode.attribute(withName: "length", stringValue: String(delta.fileSize)) as! XMLNode,
                     XMLNode.attribute(withName: "type", stringValue: "application/octet-stream") as! XMLNode,