Skip to content

New addon api versioning approach #9151

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.

Sign up for GitHub

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

Merged
feerrenrut merged 37 commits into from
Jan 31, 2019
Merged

New addon api versioning approach #9151

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
0300289
Move version string regex add tests
feerrenrut Dec 15, 2018
2e939a0
Replace compatibility checks
feerrenrut Dec 15, 2018
d2f8c53
Keep API versions as tuples
feerrenrut Dec 21, 2018
a3723f6
Dont show incompat addons dialog on startup
feerrenrut Jan 7, 2019
ff117c9
fix logic error: enable show incompat addons dialog button
feerrenrut Jan 10, 2019
7c15c4d
Remove need to manually call destroy on IncompatibleAddonsDialog
feerrenrut Jan 10, 2019
07de7d1
Fix freeze on exit when destroy not called manually for AutoWidthColu…
feerrenrut Jan 10, 2019
79dff12
Ensure confirmation checkbox has focus for all installer warnings
feerrenrut Jan 10, 2019
3fb5087
Set a new, more accurate label
feerrenrut Jan 10, 2019
8270385
Fix wording of button
feerrenrut Jan 10, 2019
6096af2
Re-order params and make them not optional.
feerrenrut Jan 10, 2019
b0e79f2
Add an "addon information" button to the incompatible addons list dialog
feerrenrut Jan 10, 2019
ef10db5
Removes unnecessary code
feerrenrut Jan 10, 2019
9d7eb43
Validate manifest condition: minRequiredVersion <= lastTested
feerrenrut Jan 10, 2019
b8f22de
update the user guide
feerrenrut Jan 10, 2019
beb0343
remove unnecessary check for manifest errors
feerrenrut Jan 11, 2019
03bcf42
Raise error for failed audioducking unless its access denied
feerrenrut Jan 11, 2019
222276a
improve consitency of messages for incompatible addons
feerrenrut Jan 11, 2019
d07981e
Further consistency of wording in GUI
feerrenrut Jan 13, 2019
896b2be
Review actions for #9151
feerrenrut Jan 14, 2019
c3ba83c
Extract version formatting into functions
feerrenrut Jan 14, 2019
b75bbcc
use `_setStateToNone(state)` to initialise state dict
feerrenrut Jan 14, 2019
0de603e
Fix userguide wording.
feerrenrut Jan 14, 2019
69d8b0a
Audio ducking failed warning
feerrenrut Jan 15, 2019
9ba7b77
Addon install warning / error dialog sounds
feerrenrut Jan 15, 2019
c9da315
Use context manager for writing to file.
feerrenrut Jan 15, 2019
42d6eb4
Stop incompatible addons from mutating the disabled state of addons.
feerrenrut Jan 15, 2019
f18136c
fix translator comment errors
feerrenrut Jan 16, 2019
250a46d
Review actions #9151
feerrenrut Jan 16, 2019
30de90f
Try to reduce confusion in the addToPackagePath method
feerrenrut Jan 17, 2019
5e77fcb
Improve comment in `addToPackagePath`
feerrenrut Jan 18, 2019
d62d356
Remove unused function
feerrenrut Jan 29, 2019
a6b45f0
Tidy addon installation
feerrenrut Jan 29, 2019
db76178
Clarified formatting of version strings.
feerrenrut Jan 30, 2019
89281cc
Renamed _showAddonUntestedDialog
feerrenrut Jan 30, 2019
47640ff
Updated developer guide.
feerrenrut Jan 31, 2019
0883ef8
Update changes file for #9151
feerrenrut Jan 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Extract version formatting into functions 
Review actions for #9151
  • Loading branch information
@feerrenrut
feerrenrut committed Jan 14, 2019
commit c3ba83c56c980cfb6c3c37f1315d1c729a78062e
23 changes: 23 additions & 0 deletions source/addonAPIVersion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import buildVersion
import re
from logHandler import log

"""
This module contains add-on API version information for this build of NVDA. This file provides information on
Expand All @@ -27,9 +28,31 @@
#: @type: RegexObject
ADDON_API_VERSION_REGEX = re.compile(r"^(0|\d{4})\.(\d)\.(\d)$")


def getAPIVersionTupleFromString(version):
"""Converts a string containing an NVDA version to a tuple of the form (versionYear, versionMajor, versionMinor)"""
match = ADDON_API_VERSION_REGEX.match(version)
if not match:
raise ValueError(version)
return tuple(int(i) if i is not None else 0 for i in match.groups())


def formatAsString(versionTuple):
"""Converts an API version tuple as a string for displaying in the GUI
Examples:
- (2018, 1, 1) becomes "2018.1.1"
- (2018, 1, 0) becomes "2018.1"
- (0, 0, 0) becomes "0.0"
"""
# Translators: shown when an addon API version string is unknown
default = _("unknown")
if not versionTuple:
return default
try:
year, major, minor = versionTuple
if minor is 0:
return "{y}.{M}".format(y=year, M=major)
return "{y}.{M}.{m}".format(y=year, M=major, m=minor)
except:
log.debug("Error formatting versionTuple: {}".format(repr(versionTuple)), exc_info=True)
return default
7 changes: 6 additions & 1 deletion source/buildVersion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ def _updateVersionFromVCS():


def formatDevVersionString():
return "%s.%s.%sdev"%(version_year,version_major,version_minor)
return "{y}.{M}.{m}dev".format(y=version_year, M=version_major, m=version_minor)

def formatVersionString(withBuildPart = False):
if withBuildPart:
return "{y}.{M}.{m}.{b}".format(y=version_year, M=version_major, m=version_minor, b=version_build)
return "{y}.{M}.{m}".format(y=version_year, M=version_major, m=version_minor)

@LeonarddeR LeonarddeR Jan 18, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realised that this will still print versions like "2019.1.0" when used in the gui, in the incompatible add-ons dialog for example. There is also some overlap with addonAPIVersion.formatAsString

@feerrenrut feerrenrut Jan 21, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it's important to consider the difference between version strings and (essentially) release name. In the release name, you definitely don't won't any extra unnecessary detail. In the version string you want it to be explicit. We don't want any doubt about with someone is having trouble with 2019.1.0 or 2019.1.1. In the incompatible add-ons dialog, which is essentially a diagnostics screen now, I would argue that presenting a slightly more technical version of the information is appropriate. I was thinking the same thing for the "about add-on" dialog.

In terms of overlap, there is some overlap. Though it's fairly minor. I think it's a good idea to keep the concepts of api version and nvda separate.

@LeonarddeR LeonarddeR Jan 21, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea to keep the concepts of api version and nvda separate.

This makes sense, as long as we explicitly state "NVDA api version" (not NVDA version) when mentioning the version number in the incompatible dialogs.

@feerrenrut feerrenrut Jan 23, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this over the last few days, I'm questioning this a little. addonAPIVersion.CURRENT will always equal the version number, addonAPIVersion.BACK_COMPAT_TO will always be equal to a some past addonAPIVersion.CURRENT version. For end users it's probably easiest for these to be referred to as NVDA versions, otherwise we risk confusion along the lines of "well what version of NVDA do I have to install to get THAT API version", but most likely they will assume they are equal anyway. For add-on authors, its important to realise that its the API changes that they need to care about, rather than the NVDA versions, this is a subtle difference. I'll look at implementing changing this so that one is implemented in terms of the other. There is also the distinction of end user (E.G. in the GUI) facing version strings, and developer facing ones (exe properties, log files)


def getCurrentVersionTuple():
return version_year, version_major, version_minor
Expand Down
39 changes: 10 additions & 29 deletions source/gui/addonGui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,6 @@ def __init__(self, parent, title, message,
self.SetSizer(mainSizer)
self.Center(wx.BOTH | wx.CENTER_ON_SCREEN)


def _getAPIVersionString(versionTuple=None):
# Translators: shown when a version is unknown in the about add-on dialog
default = _("unknown")
if not versionTuple:
return default
try:
year, major, minor = versionTuple
return "{year}.{major}.{minor}".format(year=year, major=major, minor=minor)
except:
log.debug("Error formatting versionTuple: {}".format(repr(versionTuple)), exc_info=True)
return default

def _showAddonInfo(addon):
manifest = addon.manifest
# Translators: message shown in the Addon Information dialog.
Expand All @@ -131,12 +118,12 @@ def _showAddonInfo(addon):
if url:
# Translators: the url part of the About Add-on information
message.append(_("URL: {url}").format(url=url))
minimumNVDAVersion = _getAPIVersionString(addon.minimumNVDAVersion)
minimumNVDAVersion = addonAPIVersion.formatAsString(addon.minimumNVDAVersion)
message.append(
# Translators: the minimum NVDA version part of the About Add-on information
_("Minimum required NVDA version: {}").format(minimumNVDAVersion)
)
lastTestedNVDAVersion = _getAPIVersionString(addon.lastTestedNVDAVersion)
lastTestedNVDAVersion = addonAPIVersion.formatAsString(addon.lastTestedNVDAVersion)
message.append(
# Translators: the last NVDA version tested part of the About Add-on information
_("Last NVDA version tested: {}").format(lastTestedNVDAVersion)
Expand Down Expand Up @@ -293,13 +280,8 @@ def _showAddonRequiresNVDAUpdateDialog(self, bundle):
).format(
summary=bundle.manifest['summary'],
version=bundle.manifest['version'],
minimumNVDAVersion="{}.{}.{}".format(*bundle.minimumNVDAVersion),
NVDAVersion="%d.%d.%d (%s)" % (
buildVersion.version_year,
buildVersion.version_major,
buildVersion.version_minor,
buildVersion.version
)
minimumNVDAVersion=addonAPIVersion.formatAsString(bundle.minimumNVDAVersion),
NVDAVersion=buildVersion.formatVersionString()
)
ConfirmAddonInstallDialog(
parent=self,
Expand All @@ -317,7 +299,7 @@ def _showAddonUntestedDialog(self, bundle):
" An updated version of this add-on is required,"
" the minimum add-on API supported by this version of NVDA is {backCompatToAPIVersion}"
).format(
backCompatToAPIVersion="{}.{}.{}".format(*addonAPIVersion.BACK_COMPAT_TO),
backCompatToAPIVersion=addonAPIVersion.formatAsString(addonAPIVersion.BACK_COMPAT_TO),
**bundle.manifest
)
return ConfirmAddonInstallDialog(
Expand Down Expand Up @@ -649,7 +631,7 @@ def __init__(
"The following add-ons are incompatible with NVDA version {}."
" These add-ons can not be enabled."
" Please contact the add-on author for further assistance."
).format("%s.%s.%s" % APIVersion)
).format(buildVersion.formatVersionString())
AddonSelectionIntroLabel=wx.StaticText(self, label=introText)
AddonSelectionIntroLabel.Wrap(self.scaleSize(maxControlWidth))
sHelper.addItem(AddonSelectionIntroLabel)
Expand Down Expand Up @@ -699,9 +681,8 @@ def _getIncompatReason(self, addon):
# with Year.Major.Minor. EG 2019.1.0
Comment thread
feerrenrut marked this conversation as resolved.
Outdated
return _(
"An updated version of NVDA is required."
"NVDA version {}.{}.{} or later".format(
*addon.minimumNVDAVersion
))
"NVDA version {} or later".format(addonAPIVersion.formatAsString(addon.minimumNVDAVersion))
)
elif not addonVersionCheck.isAddonTested(
addon,
backwardsCompatToVersion=self._APIBackwardsCompatToVersion
Expand All @@ -710,8 +691,8 @@ def _getIncompatReason(self, addon):
# with Year.Major.Minor. EG 2019.1.0
Comment thread
feerrenrut marked this conversation as resolved.
Outdated
return _(
"An updated version of this add-on is required."
"The minimum supported API version is now {}.{}.{}".format(
*self._APIBackwardsCompatToVersion
"The minimum supported API version is now {}".format(
addonAPIVersion.formatAsString(self._APIBackwardsCompatToVersion)
)
)

Expand Down
6 changes: 3 additions & 3 deletions source/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def getRecursiveDataFiles(dest,source,excludes=()):
"dest_base":"nvda_noUIAccess",
"uac_info": ("asInvoker", False),
"icon_resources":[(1,"images/nvda.ico")],
"version":"%s.%s.%s.%s"%(version_year,version_major,version_minor,version_build),
"version":formatVersionString(withBuildPart=True),
"description":"NVDA application",
"product_version":version,
"copyright":copyright,
Expand All @@ -181,7 +181,7 @@ def getRecursiveDataFiles(dest,source,excludes=()):
{
"script": "nvda_slave.pyw",
"icon_resources": [(1,"images/nvda.ico")],
"version":"%s.%s.%s.%s"%(version_year,version_major,version_minor,version_build),
"version":formatVersionString(withBuildPart=True),
"description": name,
"product_version": version,
"copyright": copyright,
Expand All @@ -192,7 +192,7 @@ def getRecursiveDataFiles(dest,source,excludes=()):
# uiAccess will be enabled at runtime if appropriate.
"uac_info": ("asInvoker", False),
"icon_resources": [(1,"images/nvda.ico")],
"version":"%s.%s.%s.%s"%(version_year,version_major,version_minor,version_build),
"version":formatVersionString(withBuildPart=True),
"description": "NVDA Ease of Access proxy",
"product_version": version,
"copyright": copyright,
Expand Down
30 changes: 27 additions & 3 deletions tests/unit/test_addonVersionCheck.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
from addonHandler import AddonBase
from addonHandler import addonVersionCheck

def versionString(version):
return "{}.{}.{}".format(*version)

latestVersionTuple = (2018, 2, 0)
nextVersionTuple = (2018, 3, 0)
previousVersionTuple = (2018, 1, 0)
Expand Down Expand Up @@ -144,3 +141,30 @@ def test_getAPIVersionTupleFromString_devPrepended_raises(self):
and some extra prepended results in an error being raised
"""
self.assertRaises(ValueError, addonAPIVersion.getAPIVersionTupleFromString, "dev2019.1.0")


class TestFormatAsString(unittest.TestCase):

def test_formatAsString_full(self):
res = addonAPIVersion.formatAsString((2019, 1, 1))
self.assertEqual("2019.1.1", res)

def test_formatAsString_missingMinor(self):
res = addonAPIVersion.formatAsString((2019, 1, 0))
self.assertEqual("2019.1", res)

def test_formatAsString_zeros(self):
res = addonAPIVersion.formatAsString((0, 0, 0))
self.assertEqual("0.0", res)

def test_formatAsString_none(self):
res = addonAPIVersion.formatAsString(None)
self.assertEqual("unknown", res)

def test_formatAsString_tupleTooSmall(self):
res = addonAPIVersion.formatAsString((2019, 1))
self.assertEqual("unknown", res)

def test_formatAsString_tupleTooLong(self):
res = addonAPIVersion.formatAsString((2019, 1, 1, 1))
self.assertEqual("unknown", res)