Installer should warn about downgrades #5037

Closed
nvaccessAuto opened this Issue Apr 14, 2015 · 10 comments

2 participants

@nvaccessAuto

Reported by jteh on 2015-04-14 06:57
(Spun off #308.)

While downgrading is generally discouraged, it does happen. When upgrading, we can (and should) unregister the previous install using the version being installed, as newer versions will always know about the past. However, when downgrading, this is bad because the version being installed (the downgrade) may fail to unregister something registered by a later version. This can cause severe issues in extreme cases; e.g. #308, where it could cause a security issue. Therefore, when downgrading, we should unregister the installation using the previous copy (the one we're about to overwrite).

Detecting downgrades is non-trivial due to snapshots. The best we can probably do is check the date of the executables.
Blocking #308

@nvaccessAuto

Comment 2 by leonarddr on 2015-04-14 07:08
What will this unregister process look like? Wouldn't it be the simplest to let the installer find the uninstaller of the currently installed version and execute that in silent mode beforehand?

@nvaccessAuto

Comment 3 by jteh on 2015-04-14 09:56
NVDA's installer is part of NVDA itself (i.e. Python). Basically, the NSIS_based uninstaller just calls NVDA's unregister install code (as we're planning to do) and then removes the entire program directory. We don't want to do this because it would remove systemConfig. Furthermore, because you can't remove files that are currently in use in Windows, this requires a reboot because (at the very least) it won't be able to remove the uninstaller.

In short, the full uninstaller would do a lot of pointless file removal and would probably cause breakage unless the user rebooted.

@nvaccessAuto

Comment 4 by jteh on 2015-06-23 04:27
After quite a bit of discussion, we've decided not to go with this approach. It is somewhat awkward, possibly not future-proof, and there are some edge cases that are difficult to understand, let alone counter; e.g. downgrading from a newer master snapshot which doesn't yet include this change to an older next snapshot which does include this change. Instead, we'll simply display a warning when downgrading, informing the user that we recommend complete uninstallation before installing an earlier version. We still want to give the user a chance to proceed anyway, as we sometimes want testers to test stuff in earlier snapshots (and we'll know whether a downgrade is safe in that case).

Specifics:

When the installer detects that the copy of NVDA being installed has an earlier timestamp than the existing copy (probably using the timestamps of nvda_slave.exe), it will display a dialog with the title "Warning" and the message:

You are attempting to install an earlier version of NVDA than the version currently installed. If you really wish to revert to an earlier version, you should first cancel this installation and completely uninstall NVDA before installing the earlier version.

There will be two buttons: "Proceed with installation (not recommended)" and "Cancel".
Changes:
Changed title from "When downgrading, installer should unregister install using previous copy" to "Installer should warn about downgrades"

@nvaccessAuto

Comment 6 by James Teh <jamie@... on 2015-07-15 07:17
In [c34eb3b]:
```CommitTicketReference repository="" revision="c34eb3b34d8b34c7ff016e983b0696024c52f0d4"
Merge branch 't5037' into next

Incubates #5037.

Changes:
Added labels: incubating
@nvaccessAuto

Comment 7 by James Teh <jamie@... on 2015-07-31 03:16
In [47bca33]:
```CommitTicketReference repository="" revision="47bca331e9c9af68dec58a8366b71b60c72cf8ca"
When attempting to install an earlier version of NVDA than is currently installed, you will now be warned that this is not recommended and that NVDA should be completely uninstalled before proceeding.

Fixes #5037.

Changes:
Removed labels: incubating
State: closed
@nvaccessAuto

Comment 8 by leonarddr on 2015-08-20 12:10
How does the installer detect whether there's a downgrade? Version number, build date? I went from next to rc1, which should be interpreted as a downgrade since next has features (such as Ease of Access support.

@nvaccessAuto

Comment 9 by jteh on 2015-08-20 23:04
Just modification date. There's no reliable way to do this across branches, since an older version of "next" is still probably "ahead" of a newer version of master. Also, aside from master and next, any branch could be considered "ahead" of any other branch depending on your perspective.

@nvaccessAuto

Comment 10 by jteh on 2015-08-20 23:05
Btw, downgrading from next to rc1 is currently a bad idea. It will cause major problems with secure desktops because the other branches don't have Ease of Access and thus don't know how to unregister it. This is precisely the reason for the downgrade warnings.

@nvaccessAuto

Comment 11 by leonarddr on 2015-08-21 07:03
At the time that I downgraded to RC1, the last next was from aug 13 or so, thus, the RC1 had a newer modification date and i didn't receive a downgrade warning eventhough I downgraded. I assume there's no convenient way to solve this, other than may be always triggering new master and next builds after an RC and release build.

@nvaccessAuto

Comment 12 by jteh on 2015-08-21 07:12
Afraid not. I also don't care as much. This is more for people running releases. We don't want people downgrading releases. Snapshot users are expected to be more aware of what they're doing and/or keep in touch with changes that might break them.

@jcsteh jcsteh was assigned by nvaccessAuto Nov 10, 2015
@nvaccessAuto nvaccessAuto added this to the 2015.3 milestone Nov 10, 2015
@jcsteh jcsteh added a commit that referenced this issue Nov 23, 2015
@jcsteh jcsteh When attempting to install an earlier version of NVDA than is current…
…ly installed, you will now be warned that this is not recommended and that NVDA should be completely uninstalled before proceeding.

Fixes #5037.
47bca33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment