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

Update NFC sample driver for Windows Driver Compliance #851

Merged

Conversation

tenglu83
Copy link
Contributor

@tenglu83 tenglu83 commented Feb 4, 2023

Install driver to driver store.
Include reference to wudfrd.inf

Include reference to wudfrd.inf
@tenglu83
Copy link
Contributor Author

tenglu83 commented Feb 4, 2023

@jasonknichel Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@kahashimoto-ms
Copy link
Contributor

l Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@tenglu83 I have seen this before while editing INFs. I believe I was able to fix this by opening the INF in notepad and saving it with the encoding set to UTF-8.

Save the INF file in UTF-8 code.
@tenglu83
Copy link
Contributor Author

tenglu83 commented Feb 5, 2023

l Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@tenglu83 I have seen this before while editing INFs. I believe I was able to fix this by opening the INF in notepad and saving it with the encoding set to UTF-8.

Thanks for helping. I tried your method but it seems that the file diff still doesn't show.

Remove WUDFRD_ServiceInstall from INF
@tenglu83
Copy link
Contributor Author

tenglu83 commented Feb 5, 2023

l Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@tenglu83 I have seen this before while editing INFs. I believe I was able to fix this by opening the INF in notepad and saving it with the encoding set to UTF-8.

Thanks for helping. I tried your method but it seems that the file diff still doesn't show.

The diff starts to show on the next commits after I saved it as UTF-8. I think it worked. Unfortunately it cannot compare with the original file. Thanks again!

@kahashimoto-ms
Copy link
Contributor

l Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@tenglu83 I have seen this before while editing INFs. I believe I was able to fix this by opening the INF in notepad and saving it with the encoding set to UTF-8.

Thanks for helping. I tried your method but it seems that the file diff still doesn't show.

The diff starts to show on the next commits after I saved it as UTF-8. I think it worked. Unfortunately it cannot compare with the original file. Thanks again!

You're welcome. I'm glad that worked. I think the way you described it is the expected behavior.

@jasonknichel
Copy link
Collaborator

l Do you know why GitHub treats this INF file as binary file? I see the diff of INF change in some other PR can show on the web page, but this one doesn't.

@tenglu83 I have seen this before while editing INFs. I believe I was able to fix this by opening the INF in notepad and saving it with the encoding set to UTF-8.

Thanks for helping. I tried your method but it seems that the file diff still doesn't show.

The diff starts to show on the next commits after I saved it as UTF-8. I think it worked. Unfortunately it cannot compare with the original file. Thanks again!

You're welcome. I'm glad that worked. I think the way you described it is the expected behavior.

Please don't convert the file to UTF-8. If you do, we will undo that at a future time.

The PnP platform requires that INF files be ANSI or UTF-16. Git in general doesn't like UTF-16 and thinks it is a binary file and git prefers things to be ANSI or UTF-8. So the INF files that github will not diff (and says are binary files) are UTF-16 and the INF files that github will diff are either ANSI or UTF-8. Since the PNP platform requires that INF files be ANSI or UTF-16, at some point in the future, we will likely convert all INFs in this sample repo to be UTF-16 since that is preferred over ANSI since UTF-16 allows for more proper localization of the INF for those who choose to localize their INF.

We do not know of a way to get github to do a friendly diff of UTF-16 files and have reached out to some github folks to see if it is possible.

@jasonknichel
Copy link
Collaborator

Can you please remove these from the INF sample? They shouldn't be needed from Win8 and later, IIRC.

[MyDevice_Install.NT.CoInstallers]
AddReg=CoInstallers_AddReg

[CoInstallers_AddReg]
HKR,,CoInstallers32,0x00010000,"WUDFCoinstaller.dll"

Copy link
Collaborator

@jasonknichel jasonknichel left a comment

Choose a reason for hiding this comment

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

See comments about making sure you leave the INF file as UTF-16 and removing the coinstallers part.

@tenglu83
Copy link
Contributor Author

tenglu83 commented Feb 7, 2023

See comments about making sure you leave the INF file as UTF-16 and removing the coinstallers part.

@jasonknichel Shall I use UTF-16 BE or LE?

@tenglu83
Copy link
Contributor Author

tenglu83 commented Feb 7, 2023

See comments about making sure you leave the INF file as UTF-16 and removing the coinstallers part.

Addressed comments. Use UTF-16 LE coding.

@jasonknichel jasonknichel merged commit fe1031f into microsoft:develop Feb 8, 2023
@riverar
Copy link

riverar commented Feb 10, 2023

The PnP platform requires that INF files be ANSI or UTF-16.

This assertion doesn't appear to be true (anymore?). We have UTF-8 encoded driver INFs that are operating normally.

@jasonknichel
Copy link
Collaborator

The PnP platform requires that INF files be ANSI or UTF-16.

This assertion doesn't appear to be true (anymore?). We have UTF-8 encoded driver INFs that are operating normally.

@riverar - that assertion continues to be true. If you have UTF-8 INF files that 'work', you are probably getting lucky and only using characters that are present in ASCII. I believe that the UTF-8 representation of those characters is backwards compatible with ASCII/ANSI so if you only use ASCII characters in a UTF-8 file, from the INF parsing point of view, it would parse fine as an ASCII/ANSI file. But as soon as you used a UTF-8 character that wasn't backwards compatible with ASCII, it wouldn't get interpreted correctly.

This unfortunately isn't well documented in the INF documentation, but I did find that https://learn.microsoft.com/en-us/windows-hardware/drivers/install/general-guidelines-for-inf-files does say "You can create or modify an INF file by using any text editor in which you can control the insertion of line breaks. If your INF contains non-ASCII characters, save the file as a Unicode (UTF-16) file."

If you are using https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/infverif to verify the INF and you have saved the file as UTF-8 with a BOM (byte order mark), then infverif will tell you that UTF-8 is not supported:

infverif.exe .\test-utf-8.inf
ERROR test-utf-8.inf: INF has invalid or unknown file encoding, must be ANSI or UTF-16.

@riverar
Copy link

riverar commented Feb 10, 2023

@jasonknichel Makes sense, thanks. That means this documentation needs correction too https://learn.microsoft.com/windows-hardware/drivers/display/general-unicode-requirement.

@jasonknichel
Copy link
Collaborator

@jasonknichel Makes sense, thanks. That means this documentation needs correction too https://learn.microsoft.com/windows-hardware/drivers/display/general-unicode-requirement.

Thanks for a pointer to that page. That page might say an INF file should not be ANSI because the INF file being ANSI prevents proper localization of the strings in the INF to languages that cannot be represented in ANSI. So while the INF parsing platform may support the INF being ANSI, I can see why it would be actively discouraged. However, notepad has evolved since the time those screenshots were taken, so that page probably should be update to provide new screenshots and clarified to indicate that the INF files should be saved as "UTF-16 LE".

@tenglu83 tenglu83 deleted the nfc_driver_compliance_on_develop branch May 11, 2024 22:12
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.

None yet

4 participants