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

locate_tools: Search all VS installations for vcvarsall.bat #487

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

Javagedes
Copy link
Contributor

@Javagedes Javagedes commented Jan 10, 2024

Adds a new function, FindAllWithVsWhere which functions the same as FindWithVsWhere, but returns a list of all paths found, rather than the first path only.

Updates QueryVcVariables to use FindAllWithVsWhere, and loops through each path found, attempting to locate the vcvarsall.bat file. It will use the first one found; if none are found, it will print an error and raise an exception.

Testing

Installed multiple instances of VS tools and verified FindAllWithVsWhere produces paths for both. Removed vcvarsall.bat from one installation and verified that it was found in the second installation. Then removed vcvarsall.bat from both installations and verified the error message printed as expected.

Closes #486

Adds a new function, FindAllWithVsWhere which functions the same as
FindWithVsWhere, but returns a list of all paths found, rather than the
first path only.

Updates QueryVcVariables to use FindAllWithVsWhere, and loops through
each path found, attempting to locate the vcvarsall.bat file. It will
use the first one found; if none are found, it will print an error and
raise an exception.
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (dc3bc9a) 81.20% compared to head (76a6803) 81.18%.

Files Patch % Lines
edk2toollib/windows/locate_tools.py 78.57% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
- Coverage   81.20%   81.18%   -0.02%     
==========================================
  Files          57       57              
  Lines        7363     7372       +9     
==========================================
+ Hits         5979     5985       +6     
- Misses       1384     1387       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…und. FindAllWithVsWhere will return the list sorted from newest to oldest
Copy link
Member

@spbrogan spbrogan left a comment

Choose a reason for hiding this comment

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

Please make the changes related to sorting and guaranteeing the order. Overall looks good.

edk2toollib/windows/locate_tools.py Outdated Show resolved Hide resolved
Added `sort` command to the VsWhere command
Added typing hint for return of FindAllWithVsWhere
@Javagedes
Copy link
Contributor Author

Please make the changes related to sorting and guaranteeing the order. Overall looks good.

I've added the sort flag to the VsWhere command, which sorts from highest version (then latest install date) to lowest version

@Javagedes Javagedes merged commit 9c78d80 into tianocore:master Jan 11, 2024
10 of 12 checks passed
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.

[Bug]: WinRcPath can't find correct path when VS2019 build
4 participants