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

Installer should warn about downgrades #5037

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

Installer should warn about downgrades #5037

nvaccessAuto opened this issue Apr 14, 2015 · 10 comments

Comments

@nvaccessAuto
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

Comment 6 by James Teh <jamie@... on 2015-07-15 07:17
In [c34eb3b]:

Merge branch 't5037' into next

Incubates #5037.

Changes:
Added labels: incubating

@nvaccessAuto
Copy link
Author

Comment 7 by James Teh <jamie@... on 2015-07-31 03:16
In [47bca33]:

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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.

@nvaccessAuto nvaccessAuto added this to the 2015.3 milestone Nov 10, 2015
jcsteh added a commit that referenced this issue Nov 23, 2015
…ly installed, you will now be warned that this is not recommended and that NVDA should be completely uninstalled before proceeding.

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

No branches or pull requests

2 participants