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

windows: Add "or later" to debug print of Windows version 8 (because we can't know) #1060

Closed
wants to merge 4 commits into from

Conversation

tormodvolden
Copy link
Contributor

@tormodvolden tormodvolden commented Feb 5, 2022

See also discussion in #1021.

Copy link
Member

@mcuee mcuee left a comment

Choose a reason for hiding this comment

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

The change looks to be an improvement over the current codes. I am fine with the changes.

@mcuee
Copy link
Member

mcuee commented Feb 5, 2022

Ref: https://docs.microsoft.com/en-us/windows/win32/sysinfo/operating-system-version

  • For applications that have been manifested for Windows 8.1 or Windows 10. Applications not manifested for Windows 8.1 or Windows 10 will return the Windows 8 OS version value (6.2). To manifest your applications for Windows 8.1 or Windows 10, refer to Targeting your application for Windows.

@tormodvolden
Copy link
Contributor Author

@mcuee Does it display "11" for you now?

(Never mind the appveyor stuff, I just tested it in the same CI run)

@mcuee
Copy link
Member

mcuee commented Feb 10, 2022

@mcuee Does it display "11" for you now?

Not yet. How do I add the manifest file? Without the manifest file it still shows "Windows 8 (or later)" for Windows 10 or 11.

C:\work\libusb\libusbtest\libusb\x64\Release\examples [winver ≡]> .\listdevs.exe
[timestamp] [threadID] facility level [function call] <message>
--------------------------------------------------------------------------------
[ 0.000187] [00005dc8] libusb: debug [libusb_init] created default context
[ 0.000351] [00005dc8] libusb: debug [libusb_init] libusb v1.0.25.11692
[ 0.000516] [00005dc8] libusb: debug [usbi_add_event_source] add HANDLE 00000000000000A8 events 0
[ 0.000635] [00005dc8] libusb: debug [usbi_io_init] using timer for timeouts
[ 0.000749] [00005dc8] libusb: debug [usbi_add_event_source] add HANDLE 00000000000000AC events 0
[ 0.000904] [00005dc8] libusb: debug [get_windows_version] Windows 8 (or later) 64-bit

@tormodvolden
Copy link
Contributor Author

I guess the manifest must be declared in the .vcxproj as is done in libwdi. But maybe you can take an application that already has a Windows 11 manifest, and run-time link with your new libusb.dll?

I have now updated the build version threshold to be 20000, like in libwdi. I also added a candidate for #1049 on top because then I can check in the appveyor artifacts that it worked correctly there, and we can test everything with one build.

@mcuee
Copy link
Member

mcuee commented Feb 11, 2022

Adding the manifest file seems to be simpler than what I think.
I just create a simple manifest file from the libwdi example and include it in the project (libusb-1.0.dll, listdevs and xusb). And now it can print the Windows versions.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
  <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1"> 
    <!-- If you don't have those in your app, Windows 10's GetVersionEx() -->
    <!-- DELIBERATELY reports the WRONG version... which brings us to     -->
    <!-- tonight's spelling bee: "How do you spell 'spineless morons'?"   -->
    <!-- The answer: "M-I-C-R-O-S-O-F-T; 'spineless morons'!"             -->
    <application>
      <!-- Windows 10 -->
      <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
      <!-- Windows 8.1 -->
      <supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
      <!-- Windows 8 -->
      <supportedOS Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}"/>
      <!-- Windows 7 -->
      <supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
      <!-- Windows Vista -->
      <supportedOS Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}"/> 
    </application> 
  </compatibility>
</assembly>

Windows 10 output:

C:\work\libusb\libusbtest\libusb\x64\Release\examples [winver ≡ +1 ~6 -0 !]> .\listdevs.exe
[timestamp] [threadID] facility level [function call] <message>
--------------------------------------------------------------------------------
[ 0.000322] [00001798] libusb: debug [libusb_init] created default context
[ 0.000477] [00001798] libusb: debug [libusb_init] libusb v1.0.25.11692
[ 0.000629] [00001798] libusb: debug [usbi_add_event_source] add HANDLE 0000000000000088 events 0
[ 0.000798] [00001798] libusb: debug [usbi_io_init] using timer for timeouts
[ 0.000953] [00001798] libusb: debug [usbi_add_event_source] add HANDLE 000000000000008C events 0
[ 0.001135] [00001798] libusb: debug [get_windows_version] Windows 10 64-bit

Windows 11 output:

PS C:\work\libusb\libusbtest\libusb\x64\Release\examples> .\listdevs.exe
[timestamp] [threadID] facility level [function call] <message>
--------------------------------------------------------------------------------
[ 0.000345] [00002240] libusb: debug [libusb_init] created default context
[ 0.000498] [00002240] libusb: debug [libusb_init] libusb v1.0.25.11692
[ 0.000554] [00002240] libusb: debug [usbi_add_event_source] add HANDLE 000000000000008C events 0
[ 0.000600] [00002240] libusb: debug [usbi_io_init] using timer for timeouts
[ 0.000626] [00002240] libusb: debug [usbi_add_event_source] add HANDLE 0000000000000090 events 0
[ 0.000690] [00002240] libusb: debug [get_windows_version] Windows 11 64-bit

@mcuee
Copy link
Member

mcuee commented Feb 11, 2022

I think the pull request is now good to be merged.

The only question now is whether we want to include the manifest file in the libusb-1.0.dll build or not. It is trivial for the VS build. It seems to be possible with the MinGW as well. This is not so important though.

Ref: https://stackoverflow.com/questions/33000158/embed-manifest-file-to-require-administrator-execution-level-with-mingw32

Ref: git has similar manifest file like libwdi.
https://github.com/git/git/blob/master/compat/win32/git.manifest

@tormodvolden
Copy link
Contributor Author

I think the manifest belongs to the application but it would be an option to add one to our examples/ builds.

@mcuee
Copy link
Member

mcuee commented Feb 16, 2022

I think the manifest belongs to the application but it would be an option to add one to our examples/ builds.

I agree with you. I think you can merge this pull request. Then we can consider the option to add the manifest to probably the xusb example in the future.

And guess "12" for whatever comes after. Although with macOS already at
12 this is not given :)

Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Pre-releases of Windows 11 had build numbers from 20000.

Thanks to Pete Batard for extensive explanation.

Fixes libusb#1021

Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Windows 10 and 11 is detected as Windows 8 unless there is an
application manifest. We simply cannot tell by appropriate means.

Note that we currently only use the detected version to check for the
minimal version (Vista), so it doesn't matter much.

Closes libusb#1021

Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
@tormodvolden
Copy link
Contributor Author

Merged.

@tormodvolden tormodvolden deleted the winver branch March 20, 2022 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants