Issue 6702768332996608: Issue 2432 - Auto-generate browser versions on requirements page (Closed)
|
Created:
April 29, 2015, 11:08 a.m. by Sebastian Noack Modified:
May 18, 2015, 11:14 a.m. CC:
saroyanm Visibility:
Public. |
Patch Set 1 : #
Total comments: 7
Patch Set 2 : Rewrite #Patch Set 3 : Use CMS_CACHE_DIR variable #Patch Set 4 : Got rid of CMS_CACHE_DIR variable in favor of Source.get_cache_dir() #
Total comments: 28
Patch Set 5 : Addressed comments #Patch Set 6 : Include minor version number for Thunderbird releases #Patch Set 7 : Addressed some more comments #
Total comments: 6
Patch Set 8 : Addressed comments #Patch Set 9 : Removed debug print #
Total comments: 2
Patch Set 10 : Removed build ID an try-block for SeaMonkey Aurora and Nightly #
MessagesTotal messages: 13
Note that this change is supposed to land after the website migration, to avoid conflicts with the migration scripts. Also this is still work in progress. For Chrome it is straight-forward, we can simply query its update URL. Opera uses a different update mechanism, but I figured out their netinstaller manifests, which we can use instead. However, for Yandex.Browser the only resource I could find was it's Russian Wikipedia article, which I scrap for now. Currently, this patch doesn't consider Mozilla products, yet. So far I didn't manage to send an update request that retrieves a non-empty update manifest for Firefox. But even then, their update URLs contain quite some information that will change over time, like exact OS, product and build versions. So Mozilla's update URL doesn't seem to help us here. Waldimir suggested to hard-code their release interval which indeed seems to be quite stable for Firefox, and generate the version numbers based on he current date. While this seems to work well for Firefox, it seems that Thunderbird and SeaMonkey have quite irregular release intervals. In the worst case, we could still rely on Wikipedia here. Another, probably better, option would be to rely on the versions the Adblock Plus build is marked to be compatible with.
http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... File globals/get_current_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:23: def get_opera_version(channel): Add this as inner function to get_opera_versions? Reason: code locality. http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:34: response = urllib2.urlopen(urllib2.Request('https://tools.google.com/service/update2', CHROME_UPDATE_XML)) From experience, this will fail regularly. Currently, this will crash page generation, not exactly an ideal solution. I think that we need to store the versions we get somewhere - if we fail next time we can fall back to the ones we got before. Question is: what would be a good location for this data? http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:44: return [min(versions) - 1] + versions I think that hardcoding this here is very suboptimal. This implies that we support the current Chrome version, the one before and all the unstable versions - without distinguishing between them. This should really be up to the requirements page. The return value can look like this: { 'oldstable': '36', 'stable': '37', 'unstable': ['38', '39', '40'] } http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:55: response = urllib2.urlopen('https://ru.wikipedia.org/wiki/%D0%AF%D0%BD%D0%B4%D0%B5%D0%BA%D1%81.%D0%91%D1%80%D0%B0%D1%83%D0%B7%D0%B5%D1%80') Please use https://api.browser.yandex.ru/update-info/browser/int/win-int.xml to get current Yandex Browser version. I guess we can ignore previous and beta versions here.
Here comes a new patch set, I ended up with an almost complete rewrite. 1. I removed the code scarping Wikipedia for Yandex versions. Wladimir figured out their update URLs, which we use now instead. 2. I also had another go at Mozilla. Together with Wlaimir we figured out their update URLs, which turned out being not too bad, and agreed to use them, here. 3. As suggested by Wladimir, instead a sorted list of version numbers I now return a dictionary, distinguishing between previous, current and unreleased versions in the template. 4. I introduced a macro to avoid unnecessary boilerplate in the template. 5. As suggest by Wladimr, I now record the version numbers, falling back to the cached versions, when requesting the versions fails for a particular browser. 6. Since the previously retrieved version numbers now stored, the new code uses that data to determine the previous version now. Note that this was required anyway, as there is no way to reliable determine the previous version for Yandex. Also this eliminates the need for special handling of the previous version for SeaMonkey and Thunderbird. http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... File globals/get_current_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:34: response = urllib2.urlopen(urllib2.Request('https://tools.google.com/service/update2', CHROME_UPDATE_XML)) On 2015/04/29 17:16:55, Wladimir Palant wrote: > From experience, this will fail regularly. Currently, this will crash page > generation, not exactly an ideal solution. > > I think that we need to store the versions we get somewhere - if we fail next > time we can fall back to the ones we got before. Question is: what would be a > good location for this data? Done. http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:44: return [min(versions) - 1] + versions On 2015/04/29 17:16:55, Wladimir Palant wrote: > I think that hardcoding this here is very suboptimal. This implies that we > support the current Chrome version, the one before and all the unstable versions > - without distinguishing between them. This should really be up to the > requirements page. The return value can look like this: > > { > 'oldstable': '36', > 'stable': '37', > 'unstable': ['38', '39', '40'] > } Done. http://codereview.adblockplus.org/6702768332996608/diff/6231550869897216/glob... globals/get_current_browser_versions.py:55: response = urllib2.urlopen('https://ru.wikipedia.org/wiki/%D0%AF%D0%BD%D0%B4%D0%B5%D0%BA%D1%81.%D0%91%D1%80%D0%B0%D1%83%D0%B7%D0%B5%D1%80') On 2015/04/29 17:16:55, Wladimir Palant wrote: > Please use https://api.browser.yandex.ru/update-info/browser/int/win-int.xml to > get current Yandex Browser version. I guess we can ignore previous and beta > versions here. Done.
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... File .hgignore (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... .hgignore:6: ^cache/ Better: ^cache$ http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:33: return doc.getElementsByTagName('update')[0] Unfortunately, you forgot my comment on code locality on the previous changeset. With this patch it got worse - related code isn't grouped together. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:50: update = get_mozilla_update('aus2-community', 'SeaMonkey', '2.32', build, channel) Build ID is tied to the version number - if you are passing in build ID then version number should be passed in as well. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta', '20150101215737')] What about Aurora and Nightly? Note that these update channels don't seem to care about build IDs. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:144: cache = {} Honestly, that's an extremely complicated way of doing this. My understanding is that the idea was the following: cachedir = source.get_cache_dir() try: os.makedirs(cachedir) except OSError: pass filename = os.path.join(cachedir, 'browsers.json') try: file = open(filename, 'r+') except OSError: file = open(filename, 'w') with file: cache = json.load(file) if file.mode == 'r+' else {} No messing with file descriptors, no retries, no attempts to distinguish JSON reading from a newly created file... http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:151: print >>sys.stderr, "Warning: Failed to get %s versions, falling back to cached versions" % browser Use logging.warning? http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:165: versions['previous'] = cached_previous This algorithm won't work for Thunderbird. Here the releases go like this: 31.6.0 => 38.0 => 38.1.0 => 38.2.0. Maybe we should just include the second version number part for Thunderbird stable meaning that we'll list 38.1 and 38.2 as supported once 38.2 comes out. Also a nit: if cached_current != current: ... elif 'previous' in cached_versions: versions['previous'] = cached_versions['previous'] It must hurt to do two lookups instead of one but the current code really makes the logic harder to follow ;) http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:174: del unreleased[i] I doubt that this will work correctly if you need to remove more than one element. Either way, this seems to be way simpler: versions['unreleased'] = sorted(set(versions['unreleased']) - {current, previous}) http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:177: file.seek(0) Truncate file?
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... File .hgignore (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... .hgignore:6: ^cache/ On 2015/04/30 15:34:47, Wladimir Palant wrote: > Better: > > ^cache$ Fair enough, but why? The only difference is that ^cache/ only matches directories, while ^cache$ matches any kind of file, right? http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:33: return doc.getElementsByTagName('update')[0] On 2015/04/30 15:34:47, Wladimir Palant wrote: > Unfortunately, you forgot my comment on code locality on the previous changeset. > With this patch it got worse - related code isn't grouped together. Here you have your code locality. ;P http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:50: update = get_mozilla_update('aus2-community', 'SeaMonkey', '2.32', build, channel) On 2015/04/30 15:34:47, Wladimir Palant wrote: > Build ID is tied to the version number - if you are passing in build ID then > version number should be passed in as well. Well, I just tried to don't repeat myself. But it doesn't seem to be a big deal here. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta', '20150101215737')] On 2015/04/30 15:34:47, Wladimir Palant wrote: > What about Aurora and Nightly? Note that these update channels don't seem to > care about build IDs. They do not exist. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:144: cache = {} On 2015/04/30 15:34:47, Wladimir Palant wrote: > Honestly, that's an extremely complicated way of doing this. My understanding is > that the idea was the following: > > cachedir = source.get_cache_dir() > try: > os.makedirs(cachedir) > except OSError: > pass > > filename = os.path.join(cachedir, 'browsers.json') > try: > file = open(filename, 'r+') > except OSError: > file = open(filename, 'w') > > with file: > cache = json.load(file) if file.mode == 'r+' else {} > > No messing with file descriptors, no retries, no attempts to distinguish JSON > reading from a newly created file... Yes your approach is slightly simpler, but IMO not great. If you want to open a file for reading and writing, creating it if it doesn't exist, using O_RDWR|O_CREAT is exactly how you are supposed to do it. No unnecessary round-trips to the kernel, no race conditions, and not even more complex than the alternative: os.fdopen(os.open(filename, os.O_RDWR | os.O_CREAT), 'w+') vs. try: file = open(filename, 'r+') except OSError: file = open(filename, 'w') It can be argued whether attempting to create the directory before the file, or catching ENOENT when opening the file is prefereable. I prefer the latter, we already used that pattern a couple of times in our code and the complexity added here is neglectable. But how is a bogus check for the file mode based on a previous branch, to detect an empty file any better than checking the position of the file after a failed attempt of reading it? Not to mention that I'm not a fan of the logic switching the modes in the first place. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:151: print >>sys.stderr, "Warning: Failed to get %s versions, falling back to cached versions" % browser On 2015/04/30 15:34:47, Wladimir Palant wrote: > Use logging.warning? I almost used the logging module. But then I looked into the CMS itself, because I want to generate consistent output. And this is apparently the way we log warning in the CMS. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:165: versions['previous'] = cached_previous On 2015/04/30 15:34:47, Wladimir Palant wrote: > This algorithm won't work for Thunderbird. Here the releases go like this: > 31.6.0 => 38.0 => 38.1.0 => 38.2.0. Maybe we should just include the second > version number part for Thunderbird stable meaning that we'll list 38.1 and 38.2 > as supported once 38.2 comes out. Alternatively, we could stop listening the previous major version, when beta is two major versions ahead of stable? This should match what we currently list on the requirements page. I don't mind. > Also a nit: > > if cached_current != current: > ... > elif 'previous' in cached_versions: > versions['previous'] = cached_versions['previous'] > > It must hurt to do two lookups instead of one but the current code really makes > the logic harder to follow ;) Yes it hurts, and makes the code here inconsistent. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:174: del unreleased[i] On 2015/04/30 15:34:47, Wladimir Palant wrote: > I doubt that this will work correctly if you need to remove more than one > element. It does work correctly for any number of elements, because I iterate from the end to the beginning of the list. Hence if we remove an item the next item is still at current index - 1. > Either way, this seems to be way simpler: > > versions['unreleased'] = sorted(set(versions['unreleased']) - {current, > previous}) Without a fairly complex sort function, this will potentially mess up the order, e.g: sorted(['2.2', '2.10']) -> ['2.10', '2.2'] Using a sorted set, this code would rather look like: unreleased = set(versions['unreleased']) unreleased -= {current} previous = versions.get('previous') if previous: unreleased -= {previous} versions['unreleased'] = sorted(unreleased, key=lambda ver: [ int(num) if num else s for num, s in re.findall(r'(\d+)|([a-z]+)', ver, re.I) ]) However, it can be simplified if we assume that that we only have strings consisting of dots and digits, but I'm not sure whether this is a good idea: unreleased = set(versions['unreleased']) unreleased -= {current, versions.get('previous')} versions['unreleased'] = sorted(unreleased, key=lambda ver: map(int, ver.split('.')) http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:177: file.seek(0) On 2015/04/30 15:34:47, Wladimir Palant wrote: > Truncate file? Done.
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:165: versions['previous'] = cached_previous Never mind. I found a way to include the minor version for Thunderbird stable releases, I'm quite happy with.
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:151: print >>sys.stderr, "Warning: Failed to get %s versions, falling back to cached versions" % browser On 2015/04/30 20:26:20, Sebastian Noack wrote: > On 2015/04/30 15:34:47, Wladimir Palant wrote: > > Use logging.warning? > > I almost used the logging module. But then I looked into the CMS itself, because > I want to generate consistent output. And this is apparently the way we log > warning in the CMS. Done, after making the cms use the logging module as well. While on it, I also added another warning when the previous version can't be determined. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:165: versions['previous'] = cached_previous On 2015/04/30 15:34:47, Wladimir Palant wrote: > Also a nit: > > if cached_current != current: > ... > elif 'previous' in cached_versions: > versions['previous'] = cached_versions['previous'] > > It must hurt to do two lookups instead of one but the current code really makes > the logic harder to follow ;) I made this and some other checks obsolete, by always setting "previous" (to None if it can't be determined). http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:174: del unreleased[i] On 2015/04/30 20:26:20, Sebastian Noack wrote: > On 2015/04/30 15:34:47, Wladimir Palant wrote: > > I doubt that this will work correctly if you need to remove more than one > > element. > > It does work correctly for any number of elements, because I iterate from the > end to the beginning of the list. Hence if we remove an item the next item is > still at current index - 1. > > > Either way, this seems to be way simpler: > > > > versions['unreleased'] = sorted(set(versions['unreleased']) - {current, > > previous}) > > Without a fairly complex sort function, this will potentially mess up the order, > e.g: sorted(['2.2', '2.10']) -> ['2.10', '2.2'] > > Using a sorted set, this code would rather look like: > > unreleased = set(versions['unreleased']) > unreleased -= {current} > previous = versions.get('previous') > if previous: > unreleased -= {previous} > versions['unreleased'] = sorted(unreleased, key=lambda ver: [ > int(num) if num else s for num, s > in re.findall(r'(\d+)|([a-z]+)', ver, re.I) > ]) > > However, it can be simplified if we assume that that we only have strings > consisting of dots and digits, but I'm not sure whether this is a good idea: > > unreleased = set(versions['unreleased']) > unreleased -= {current, versions.get('previous')} > versions['unreleased'] = sorted(unreleased, > key=lambda ver: map(int, ver.split('.')) Never mind, I am using a set now. As this will not only simplify the logic, but also make sure that there are no duplicates among the unreleased versions.
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... File .hgignore (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/.hgi... .hgignore:6: ^cache/ On 2015/04/30 20:26:20, Sebastian Noack wrote: > Fair enough, but why? The only difference is that ^cache/ only matches > directories, while ^cache$ matches any kind of file, right? Yes, and we use the latter consistently for all our repositories - along with /cache/ as the Git translation. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:33: return doc.getElementsByTagName('update')[0] On 2015/04/30 20:26:20, Sebastian Noack wrote: > Here you have your code locality. ;P Thank you, much better. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta', '20150101215737')] On 2015/04/30 20:26:20, Sebastian Noack wrote: > They do not exist. That would be news to me. https://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/ https://aus2-community.mozilla.org/update/3/SeaMonkey/2.13.1/20120909051705/L... https://aus2-community.mozilla.org/update/3/SeaMonkey/2.13.1/20120909051705/L... http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:55: BROWSERS['firefox'] = lambda: get_mozilla_versions('Firefox', '37.0') Style nit: Please don't align columns. This might look nice at first but will make changing code unnecessarily messy. http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:139: def get_browser_versions(context, browser): Just realized: Thunderbird isn't a browser :). But I think that naming this get_app_versions will make it harder to understand what this global does. http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:178: # Remove duplicated from unreleased versions. Occasionally, Typo: duplicates
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta', '20150101215737')] On 2015/05/13 13:17:23, Wladimir Palant wrote: > On 2015/04/30 20:26:20, Sebastian Noack wrote: > > They do not exist. > > That would be news to me. > > https://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/ > https://aus2-community.mozilla.org/update/3/SeaMonkey/2.13.1/20120909051705/L... > https://aus2-community.mozilla.org/update/3/SeaMonkey/2.13.1/20120909051705/L... You seem to be right. But for reference, Aurora builds for Windows and nightlies for all platforms are currently broken. I considered that case in the code. However currently, this gets 2.33 both as stable and beta (seems to be right according to Wikipedia) and 2.36 as Aurora. But I have no idea where the gap is coming from. Even if beta is supposed to be 2.34, shouldn't Aurora be not higher than 2.35? IMO, the whole situation with SeaMonkey versions is currently extremly confusing, to users reading our requirements page, as well for developers debugging this code. So maybe we should just list the current (and previous) release version of SeaMonkey, adding a note below, along the lines of: "The Adblock Plus development builds should work as well with SeaMonkey Beta, Aurora and Nightly. However, SeaMonkey pre-releases aren't available as the time of writing due to Bug 1086553." http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:55: BROWSERS['firefox'] = lambda: get_mozilla_versions('Firefox', '37.0') On 2015/05/13 13:17:23, Wladimir Palant wrote: > Style nit: Please don't align columns. This might look nice at first but will > make changing code unnecessarily messy. Done. http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:139: def get_browser_versions(context, browser): On 2015/05/13 13:17:23, Wladimir Palant wrote: > Just realized: Thunderbird isn't a browser :). But I think that naming this > get_app_versions will make it harder to understand what this global does. Yeah, I already realized myself, and thought about renaming it again. IMO, nothing wrong with get_app_versions(), but fair enough. http://codereview.adblockplus.org/6702768332996608/diff/5723151296102400/glob... globals/get_browser_versions.py:178: # Remove duplicated from unreleased versions. Occasionally, On 2015/05/13 13:17:23, Wladimir Palant wrote: > Typo: duplicates Done.
Up to you if you want to address the nit below. LGTM if you don't. http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob... globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta', '20150101215737')] On 2015/05/13 18:35:11, Sebastian Noack wrote: > However currently, this gets 2.33 both as stable and beta (seems to be right > according to Wikipedia) and 2.36 as Aurora. But I have no idea where the gap is > coming from. Even if beta is supposed to be 2.34, shouldn't Aurora be not higher > than 2.35? Firefox 37 is the current stable version - that would correspond to SeaMonkey 2.34 (it's always "2.{ff_version - 3}"). Aurora being SeaMonkey 2.36 makes sense then. The strange part is rather that SeaMonkey 2.34 isn't released yet and that their beta channel is outdated. According to their Wiki (https://wiki.mozilla.org/SeaMonkey:Home_Page) the 2.34 release has been canceled due to issues with their build infrastructure (https://bugzilla.mozilla.org/show_bug.cgi?id=1114876). The same issue seems to be responsible for missing beta builds (https://groups.google.com/forum/#!topic/mozilla.support.seamonkey/d95E6knhHTM). > IMO, the whole situation with SeaMonkey versions is currently > extremly confusing, to users reading our requirements page, as well for > developers debugging this code. > > So maybe we should just list the current (and previous) release version of > SeaMonkey, adding a note below, along the lines of: > > "The Adblock Plus development builds should work as well with SeaMonkey Beta, > Aurora and Nightly. However, SeaMonkey pre-releases aren't available as the time > of writing due to Bug 1086553." It should be fine if we only list the versions that one can update to. I don't see a big point for this message. http://codereview.adblockplus.org/6702768332996608/diff/5721036024709120/glob... File globals/get_browser_versions.py (right): http://codereview.adblockplus.org/6702768332996608/diff/5721036024709120/glob... globals/get_browser_versions.py:81: version = get_seamonkey_version('2.13.1', '20120909051705', channel, platform='Linux_x86-gcc3') Nit: use '2.32', '-' here? Aurora and Nightly channels didn't require a build number back when they were working.
Before I'm going to push this, can you please provision
/var/cache/web.adblockplus.org/browsers.json with the content below, that the
previous versions can be determined on first run. Also you have to make sure
that the file is writable by the user running the page generation.
{
"firefox": {"current": "36"},
"seamonkey": {"current": "2.32"},
"thunderbird": {"current": "31.5"},
"chrome": {"current": "41"},
"opera": {"current": "28"},
"yandex": {"current": "14.12"}
}
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob...
File globals/get_browser_versions.py (right):
http://codereview.adblockplus.org/6702768332996608/diff/5653164804014080/glob...
globals/get_browser_versions.py:98: 'unreleased': [get_seamonkey_version('beta',
'20150101215737')]
Nighties are back. Now, we get following versions:
2.33 (Release and Beta)
2.36 (Aurora)
2.38 (Nightly)
Does that sound right?
http://codereview.adblockplus.org/6702768332996608/diff/5721036024709120/glob...
File globals/get_browser_versions.py (right):
http://codereview.adblockplus.org/6702768332996608/diff/5721036024709120/glob...
globals/get_browser_versions.py:81: version = get_seamonkey_version('2.13.1',
'20120909051705', channel, platform='Linux_x86-gcc3')
On 2015/05/14 19:53:19, Wladimir Palant wrote:
> Nit: use '2.32', '-' here? Aurora and Nightly channels didn't require a build
> number back when they were working.
Done. Note that while nightlies weren't working, specifying less than 14
characters as build ID, returned an outdated version, while specifying at least
14 characters got you an empty update manifest, which was handled by the
try-block here.
On 2015/05/15 10:24:54, Sebastian Noack wrote:
> "firefox": {"current": "36"},
Sorry, since FF 38, was released yesterday, this is supposed to be 37.
LGTM 
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
