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

Change format of add/remove programs registration to improve winget support #13911

Merged
merged 11 commits into from
Aug 1, 2022

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Jul 15, 2022

Link to issue number:

Fixes #12469

Summary of the issue:

Winget is the new package manager in the most recent versions of Windows 10 and 11. It allows automatic updating via the command line. However, Winget threats Alpha versions of NVDA older than any released version. This means that, when an Alpha version for NVDA 2022.3 is installed, Winget will yet threat NVDA 2022.1 as an upgrade. See microsoft/winget-cli#1209 (comment) for more information about version parsing in Winget. In short, our versioning scheme for alpha versions (e.g. alpha-25875,d6a193ae) is considered pretty exotic.

I proposed three possible solutions in #12469 (comment). Option 3 is not possible with winget, see microsoft/winget-cli#1209 (comment). Therefore, this pr chooses option 1, but it extends on that (see below)

Description of user facing changes

In add/remove programs:

  • Old situation:
    • DisplayName: NVDA
    • DisplayVersion: alpha-25875,d6a193ae
  • New situation:
    • DisplayName: NVDA alpha-25875,d6a193ae
    • DisplayVersion: '2022.3.0.25875'

In about window:

  • Old situation: Version: alpha-25875,d6a193ae
  • New situation: Version: alpha-25875,d6a193ae (2022.3.0.25875)

Apart from fixing Winget this way, it will be much easier for end users to see on what year.major.minor branch of NVDA they are with their alpha version.
note that versions in other places are really not affected. Therefore this change is barely visible. Also note that having a version in the Displayname and the product version in the DisplayVersion for ARP is pretty common. This is used by Visual Studio for example.

Testing strategy:

Install a try build and observe that in Windows add/remove programs / apps and features, the pr identifier is moved to the display name whereas the version is constructed of year.major.minor.build.

Known issues with pull request:

  • For released versions, info is slightly doubling in the about window, e.g. Version: 2022.1 (2022.1.0.xxx). We could leave out the doubled parts, but that means we'll lose year.major.minor in Alpha again.

Change log entries:

Bug fixes

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@AppVeyorBot
Copy link

See test results for failed build of commit 3016b96849

@LeonarddeR LeonarddeR marked this pull request as ready for review July 18, 2022 17:37
@LeonarddeR LeonarddeR requested a review from a team as a code owner July 18, 2022 17:37
@LeonarddeR LeonarddeR requested a review from seanbudd July 18, 2022 17:37
@seanbudd seanbudd added the merge-early Merge Early in a developer cycle label Jul 18, 2022
source/installer.py Outdated Show resolved Hide resolved
source/installer.py Outdated Show resolved Hide resolved
LeonarddeR and others added 2 commits July 19, 2022 06:15
@AppVeyorBot
Copy link

See test results for failed build of commit bbf2b78072

@feerrenrut
Copy link
Contributor

To progress this, consideration should given to usages of the changed version numbers and what impact it will have.

In particular:

  • Is this version number sent to the (NV Access) update check server?
  • How will this affect usage statistics gathering?

source/installer.py Outdated Show resolved Hide resolved
source/installer.py Outdated Show resolved Hide resolved
@LeonarddeR
Copy link
Collaborator Author

@seanbudd your review comments didn't work because string formatting was used to fill the installDir. I changed this to a function instead that takes the install dir and also calculates the estimated size.

@LeonarddeR
Copy link
Collaborator Author

To progress this, consideration should given to usages of the changed version numbers and what impact it will have.

As noted above, this only has impact to add/remove programs.

  • Is this version number sent to the (NV Access) update check server?

No. versionInfo.version is not changed with this pr.

  • How will this affect usage statistics gathering?

It won't, as that relies on the update check mechanism.

@AppVeyorBot
Copy link

See test results for failed build of commit 3180e60398

@seanbudd seanbudd added this to the 2022.4 milestone Jul 25, 2022
@seanbudd seanbudd self-requested a review July 27, 2022 08:04
source/installer.py Outdated Show resolved Hide resolved
source/installer.py Outdated Show resolved Hide resolved
source/installer.py Outdated Show resolved Hide resolved
source/installer.py Outdated Show resolved Hide resolved
@seanbudd seanbudd merged commit 5aefa40 into nvaccess:master Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-early Merge Early in a developer cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Winget package manager treats NVDA Alpha as older than version 2020.4
4 participants