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

Store Windows 10 builds as strings, and map build IDs to them #11799

Closed
wants to merge 2 commits into from

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Oct 31, 2020

Link to issue number:

Fixes #11795. Alternative to #11796.

Summary of the issue:

As of 20H2, Microsoft now prefers xxHy notation (20H2) for Windows 10 versions instead of build IDs (2009).

Description of how this pull request fixes the issue:

Windows 10 version map keys are now in xxHy form. To maintain backward compatibility (and handle nonconformant build IDs like 2004), build IDs are logically mapped to their corresponding xxHy string.

Also, edited log.error call to use formatted string literals.

Testing performed:

Tested in source code version under 20H2 with the following conditions:

  • Passing in version parameter in integer and string.
  • Passing in an invalid int or str.

Known issues with pull request:

  • NVDA still uses build IDs in some places. For consistency, might we want to change these?

Change log entry:

== Changes for Developers ==

2009: 19042,
"15H1": 10240,
"15H2": 10586,
"16H2": 14393,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DHowett or @miniksa, are these correct (or at least sane) identifiers for the 2015 and 2016 versions of Windows 10?

As an aside, how do you comment on a range of lines using the keyboard?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope - see an earlier comment.

@codeofdusk
Copy link
Contributor Author

Cc @josephsl.

@josephsl
Copy link
Collaborator

Hi,

For 1507, 1511, and 1607: no, they are incorrect strings. The authoritative source for version strings is:
https://docs.microsoft.com/en-us/windows/release-information/

Thanks.

Copy link
Collaborator

@josephsl josephsl left a comment

Choose a reason for hiding this comment

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

Although consistency and compatibility are important, it is better to use publicly available documentation. As for build numbers (not exactly called build ID's), yes, we need to look for these and change them, perhaps in 2021.1 cycle.

2009: 19042,
"15H1": 10240,
"15H2": 10586,
"16H2": 14393,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope - see an earlier comment.

@codeofdusk
Copy link
Contributor Author

OK, so how should 15/16xx builds be handled? Choices:

  • Only refer to these builds by ID (1507, 1511, 1607). The API would then allow either a string or int from 2017 on.
    • This leads to no ambiguity, as from 2017 on there was a clear semi-annual system for releases.
    • However, this makes for a somewhat inconsistent API: three versions can only be referenced by ID. Might this actually make sense though, since there isn't a clear H1/H2 for them anyway?
  • Keep the current definitions of "H1" and "H2" for 2015/2016 builds.
    • This makes for a consistent API.
    • However, it might introduce additional confusion for 2015 builds, as July and November are both technically in the second half of 2015.
  • Remove the 15H1 and 15H2 keys, but keep 16H2.
    • This seems like a good tradeoff to me: we only have two builds that must be referenced by ID, and there isn't any ambiguity about what to consider H1 or H2 as with the 2015 builds.
  • Do something else I haven't thought of.

@josephsl
Copy link
Collaborator

Hi,

I think it would makes sense to go with what is publicly available instead of imposing our own version strings. After all, because NVDA's source code is public, people would understand certain things about Windows 10 through the screen reader source code, thus improving our credibility. At the same time, we have to be careful about presenting public information from another entity, and it makes more sense to show that NVDA is compliant with what is already out there instead of inventing our own wheel.

Thanks.

@feerrenrut
Copy link
Contributor

I don't understand the purpose of this approach. Unless the version string on older releases of Windows is being changed retroactively (IE Microsoft decides to change the name of '1909' to '19H2'), which I doubt they would do.

As I understand it, these version strings should be thought of only as opaque unique tokens from a programmatic point of view. They happen to encode information that humans can consume, but this extra information generally shouldn't be relied on by programs, there are other sources to use for that. The version string scheme can change at any point, eg how 20H2 did, the next version might be called 'Fred' for all we know, it would still be unique and could be tied to a build number. There may need to be a 22H1a and a 22H1b for instance. For strict ordering, the build number should be used instead.

To maintain backward compatibility

Can you describe what you mean by this?
Backwards compatibility for who? As consumers of what?

There also seems to be quite a bit of confused terminology going on, it would be really useful to define the terms you are using in the PR description so we can all first get on the same page.

@codeofdusk
Copy link
Contributor Author

Closing in favour of a different approach (currently being discussed in #11796).

@codeofdusk codeofdusk closed this Nov 20, 2020
feerrenrut pushed a commit that referenced this pull request Mar 25, 2021
Convenience methods and types have been added to the winVersion module for getting and comparing Windows versions. 
  - isWin10 function found in winVersion module has been removed.
  - class winVersion.WinVersion is a comparable and order-able type encapsulating Windows version information.
  - Function winVersion.getWinVer has been added to get a winVersion.WinVersion representing the currently running OS.
  - Convenience constants have been added for known Windows releases, see winVersion.WIN* constants.

Closes #11795
Closes #11837
Closes #11933
Replaces #11796
Replaces #11799
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

winVersion.isWin10: expand Windows 10 version dictionaries to cover versions of the form "YYHx"
3 participants