Skip to content

Fix-up of PR 12943 - Avoid os.path.commonPath` when comparing paths of the binaries as it does not work for files on different drives#13009

Merged
seanbudd merged 1 commit into
nvaccess:masterfrom
lukaszgo1:I13008
Oct 31, 2021

Conversation

@lukaszgo1

@lukaszgo1 lukaszgo1 commented Oct 30, 2021

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #13008

Summary of the issue:

PR #12943 improved code which retrieves version information for a given executable files placed in system32 under 64-bit versions of Windows. To do so it is necessary to check if path of the given binary starts with path of system32 directory for the current system. The recommended way to compare path's in a case insensitive way was os.pathcommonPath`, however it turns out it does not work when path's are from different drives.
In case of reporter of #13008 they had Office installed in a custom location probably on non system disk.

Description of how this pull request fixes the issue:

This PR simplifies the code to use standard str.casefold for comparisons.

Testing strategy:

  • Made sure that product name and product version can be retrieved for a binary placed on non system disk - previously it resulted in an exception.
  • Got confirmation from reporter of word problems 2013  #13008 that they problem is no longer reproducible.

Known issues with pull request:

None known

Change log entries:

None needed - regression not in a release.

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

@lukaszgo1 lukaszgo1 marked this pull request as ready for review October 31, 2021 10:24
@lukaszgo1 lukaszgo1 requested a review from a team as a code owner October 31, 2021 10:24
@lukaszgo1

Copy link
Copy Markdown
Contributor Author

cc @seanbudd Would you be able to review this?

@seanbudd seanbudd changed the title Fix-up of PR 12943 - Avoid os.path.commonPath` when comparing path's of the binaries as it does not work for files on different drives Fix-up of PR 12943 - Avoid os.path.commonPath` when comparing paths of the binaries as it does not work for files on different drives Oct 31, 2021
@seanbudd seanbudd merged commit 205df17 into nvaccess:master Oct 31, 2021
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Oct 31, 2021
@lukaszgo1 lukaszgo1 deleted the I13008 branch November 1, 2021 09:04
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.

word problems 2013

3 participants